Remove Snakemake tests (and dependency) #1302
Conversation
Codecov ReportPatch has no changes to coverable lines. 📢 Thoughts on this report? Let us know!. |
|
@codecov comment above does not tell the full story. No coverable lines were changed directly, but there are definitely coverage drops... I think the coverage drop in export v1 is fine given it's now deprecated. Not sure about the other ones, I'll have to take a deeper look. Thoughts from anyone? |
@victorlin I wouldn't let the numbers codecov reports stop this from being merged - remember it's supposed to be a tool to help us, not prevent progress. As I understand it, the lines flagged here were only tested in the sense that if a fatal error was encountered the (snakemake-based) test would fail, otherwise everything would pass and we'd be none the wiser. |
There are a few data files that are referenced by pytests. Move those to tests/data. These could be converted to Cram tests, though that would require some work. Also, note that the TB build removed here is not the same as the TB build running in pathogen-repo-ci. I did a quick spot-check and noticed, at the very least, the latter does not run `augur reconstruct-sequences` while the former does. That's one less point of test coverage. Maybe it is covered by other pathogen-repo-ci, but those aren't factored into the code coverage report.
Previous commits removed usage of Snakemake in this repo.
victorlin
force-pushed
the
victorlin/update-testing
branch
from
78a9741 to
4f0c374
Compare
|
Thanks @jameshadfield, I'll merge this now! |
Description of proposed changes
Main motivation is to remove Snakemake from the list of dev dependencies. This will likely cause a drop in test coverage, which can be seen by the codecov coverage report (as a comment on this PR once CI finishes running). However, note that pathogen-repo-ci is not currently factored into those coverage reports.
Related issue(s)
Checklist
[ ] If making user-facing changes, add a message in CHANGES.md summarizing the changes in this PR