augur
augur copied to clipboard
filter: Update functional test coverage
Description of proposed changes
Improves coverage of existing functional tests, motivated by work in #854. See individual commits.
Related issue(s)
N/A
Testing
See tests.
@huddlej
In this PR, I'm worried that modifying the sequence index directly and copying the metadata TSV to CSV will decouple the original source data from the test outputs.
Valid, I made these changes naively with the single goal of increasing code coverage. I hadn't thought about decoupling.
Regarding the metadata CSV input, this feels like more of a test of
io.read_metadata
thanaugur filter
. Is there CSV-specific filter logic in the sqlite branch that needs testing there? Or can we drop the metadata CSV test here?
I agree that this is more a test of io.read_metadata
and should be moved. Mainly since we support CSV input, I wanted to test that this functionality also makes its way to the sqlite engine. It's currently not using io.read_metadata
, which is another problem I'll need to solve first. But overall, more coverage in filter
and supporting modules like io
makes me more confident that the rewrite has functional parity with the existing pandas engine.
Maybe we can have a separate conversation about how to manage data in functional tests? Even if the answer is to just document the plan I had in my head, that would be better than nothing... :)
Good idea, maybe a README alongside data files will suffice. I'll start a PR based on your 2nd paragraph above: #952
My thinking was that the sequences and metadata and their downstream processed files in
tests/functional/filter/
should be logically consistent such that you could always regenerate the downstream files from these starting points.
+1 for maintaining this property, which is adjacent to snapshot/"known good" testing.