Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Start of issue 718, file not existing clean up #772

Merged
merged 14 commits into from
Mar 16, 2015
6 changes: 6 additions & 0 deletions ChangeLog
Original file line number Diff line number Diff line change
@@ -1,3 +1,9 @@
2015-03-16 Jessica Mizzi <mizzijes@msu.edu>

* khmer/kfile.py: Added file not existing error for system exit
* tests/{test_scripts,test_functions}.py: Added tests for
check_file_status for file existence and force option

2015-03-15 Kevin Murray <spam@kdmurray.id.au> & Titus Brown <titus@idyll.org>

* tests/test_counting_hash.py: Skip get_raw_tables test if python doesn't
Expand Down
14 changes: 13 additions & 1 deletion khmer/kfile.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,22 @@ def check_file_status(file_path, force):
AND if the file is NOT a fifo/block/named pipe then a warning is printed
and sys.exit(1) is called
"""
mode = None

if file_path is '-':
return
mode = os.stat(file_path).st_mode
try:
mode = os.stat(file_path).st_mode
except OSError:
print >>sys.stderr, "ERROR: Input file %s does not exist" % \
file_path

if not force:
print >>sys.stderr, "Exiting"
sys.exit(1)
else:
return

# block devices will be nonzero
if S_ISBLK(mode) or S_ISFIFO(mode):
return
Expand Down
19 changes: 19 additions & 0 deletions tests/test_functions.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
import collections
from khmer.utils import (check_is_pair, broken_paired_reader, check_is_left,
check_is_right)
from khmer.kfile import check_file_status


def test_forward_hash():
Expand Down Expand Up @@ -115,6 +116,24 @@ def test_extract_hashbits_info():
print >>sys.stderr, '...failed to remove {fn}'.format(fn)


def test_check_file_status_kfile():
fn = utils.get_temp_filename('thisfiledoesnotexist')
check_file_status_exited = False
try:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

generate a guaranteed-not-to-exist filename with utils.get_temp_filename('thisfiledoesnotexist')

check_file_status(fn, False)
except SystemExit:
check_file_status_exited = True
assert check_file_status_exited


def test_check_file_status_kfile_force():
fn = utils.get_temp_filename('thisfiledoesnotexist')
try:
check_file_status(fn, True)
except OSError as e:
assert False


FakeFQRead = collections.namedtuple('Read', ['name', 'quality', 'sequence'])
FakeFastaRead = collections.namedtuple('Read', ['name', 'sequence'])

Expand Down
12 changes: 12 additions & 0 deletions tests/test_scripts.py
Original file line number Diff line number Diff line change
Expand Up @@ -2814,3 +2814,15 @@ def test_roundtrip_casava_format_2():
r = open(infile).read()
r2 = open(outfile).read()
assert r == r2, (r, r2)


def test_existance_failure():
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You didn't run a spellchecker :)

expected_output = 'ERROR: Input file'

args = [utils.get_temp_filename('thisfiledoesnotexistatall')]

status, out, err = utils.runscript(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test should specifically test the changes you made to kfile.py - if you remove those changes, will this test still succeed? It doesn't look like it to me.

So, I think you should actually write two tests -- one test for a script like extract-paired-reads, that tests the error message and exit status. And one test in test_script_arguments.py that runs the code in kfile directly, and verifies that behavior. Note that sys.exit raises SystemExit, so you can look for that in the test.

(Let me know if you'd like me to expand on all of this!)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ctb Will the test in test_scripts.py work to as the first test you described? If I'm understanding you, there only needs to be another test in test_script_arguments.py to test the code in kfile. I would like some expansion on the test_script_arguments.py test if possible.

Also - how do I a run a spell checker? Thanks!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for running a spell checker - what editor are you using? it'll depend on that. or you can just copy/paste the code into Word...

yes, I misread the test in test_scripts - it's good now!

the test in test_script_arguments should be of the specific kfile function, as opposed to running a script that runs the kfile function. so, it should call kfile.check_file_status specifically, and verify that it raises a SystemExit exception when the file doesn't exist; and maybe another test to check that if you specify force, it doesn't exit; etc. Basically you should test all of the if/else paths in the code. Does that make more sense?

'extract-paired-reads.py', args, fail_ok=True)
assert status == 1

assert expected_output in err