Add tests in tests/test_sourmash.py for the function load_pathlist_from_file

[keyabarve]

#1423 partly fixes issue #1369; this issue is for the remaining bit of #1369.

Please add some Python unit tests in tests/test_sourmash.py for the load_pathlist_from_file function, including empty file list, badly formatted ones, ones with duplicates.

[ctb]

Copied from #1423 comment -

in terms of actual tests, please see this example for the kind of thing you want to do - create a list of files that's broken in various ways, and then use the actual function load_pathlist_from_file to try loading and assert that what happens is correct (an error, or something else). I would suggest just getting started with something and then I can help suggest more tests to add! The key point is that the tests should cover every explicit and implicit branch (if/else, try/except) in the function.

[keyabarve]

@ctb Is there a specific block where I need to add these tests in tests/test_sourmash.py?

[ctb]

no hard and fast rules - generally I try to group tests together in the file by what they're testing, but as long as the names are sensible, it doesn't matter.

[keyabarve]

@ctb @luizirber I'm not sure about how to get started on writing the tests. Could you provide some tips?

[ctb]

hi @keyabarve - I'm thinking about a blog post on testing, but in the meantime...

let's say the goal is to write a test to ensure that what happens when the user provides an empty pathlist file makes sense. we don't really have any firm expectations for what this would look like, but a good rule is that probably you don't want to have the user see an exception being raised (so you want a nice error message!), and you also want to avoid errors or exceptions that are incidental (i.e. the exception being raised is not obviously related to the contents of the pathlist file.)

first, note that on the command line if you do something like

# make an empty file
touch empty.txt 
# try loading it as a pathlist
sourmash search tests/test-data/47.fa.sig empty.txt

you get an ugly error that violates all of my rules above. so clearly some work here is needed! (I suspected as much when I created this issue... the pathlist code is simple and ugly!)

I'm pretty used to writing tests for py.test now, but I still sometimes write some standalone Python code first to explore behavior. try creating a .py file, pathy.py, with the following code:

from sourmash import sourmash_args

sourmash_args.load_pathlist_from_file('empty.txt')

this should generate an error (make sure empty.txt exists and is empty).

(This is basically just a reminder that you can always just poke and prod at this in Python or via the command line.)

So, once you've gotten that code running and failing, your next task is to put this into a standalone test format that py.test can run. I almost always start a new test by copying an old one, which is appropriate in this case! Check out test_multi_index_load_from_pathlist_2, which

• runs in a temporary directory that is created before the test and removed after it (see the @utils.in_tempdir decorator at the beginning)
• gets a list of files
• checks the list to make sure it's the expected length
• turns that list into a pathlist file in the temporary directory
• and calls MultiIndex.load_from_pathlist(...) on the created pathlist file
• while telling py.test that we expect this call to raise a ValueError.

so to get started on this issue,

• copy that test to a new test function (make sure to change the name to something unique that starts with test_!)
• change the pathlist file creation to just create an empty file
• run sourmash_args.load_pathlist_from_file(...) on that empty file
• tell pytest to expect the exception you expect

For bonus points, go look at the load_pathlist_from_file(...) code in src/sourmash/sourmash_args.py and "fix" it so that it raises the proper exception type with a good error message, instead of the exception you're currently getting.

HTH!