fix: tests no longer create files in working directory or test data dirs#853
Conversation
There was a problem hiding this comment.
Pull request overview
This pull request updates the test suite to avoid writing generated output files into the repository working directory or committed test-data directories, primarily by redirecting output into pytest-managed temporary directories. It also adds small CLI affordances to support directing outputs to temp locations for script-based tests.
Changes:
- Redirect test-generated outputs to
tmp_path(via passing tmp-rooted filenames and/ormonkeypatch.chdir(tmp_path)) and remove now-unneeded manual cleanup calls. - Update graphics tests to run from
tmp_pathby symlinking required data files into the temp directory before execution. - Add explicit output controls for CLI utilities used by tests (
synteny liftover --outfile,hic bam2mat --outdir) and refresh pre-commit hook versions.
Reviewed changes
Copilot reviewed 26 out of 34 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tests/utils/test_cbook.py | Writes depends-test inputs/outputs under tmp_path instead of CWD. |
| tests/graphics/test_tree.py | Uses monkeypatch.chdir(tmp_path) to keep generated images out of the repo. |
| tests/graphics/test_synteny.py | Symlinks graphics data into tmp_path and runs from there. |
| tests/graphics/test_landscape.py | Symlinks graphics data into tmp_path and runs from there. |
| tests/graphics/test_karyotype.py | Symlinks graphics data into tmp_path and runs from there. |
| tests/graphics/test_grabseeds.py | Runs from tmp_path so PDFs/JSON land in temp. |
| tests/graphics/test_glyph.py | Runs from tmp_path to avoid writing into the repo. |
| tests/formats/test_sizes.py | Writes FASTA under tmp_path (avoids CWD pollution). |
| tests/formats/test_obo.py | Runs from tmp_path so downloaded OBO file lands in temp. |
| tests/formats/test_bed.py | Avoids chdir; passes absolute input path to summary(). |
| tests/config.py | Moves test_scripts.log to system temp; runs script tests inside a temp working dir with try/finally restore. |
| tests/compara/synteny.py/tests.yml | Routes synteny test outputs to __TMP__ and uses --outfile. |
| tests/compara/synfind.py/tests.yml | Routes synfind test outputs to __TMP__. |
| tests/assembly/test_allmaps.py | Runs allmaps tests from tmp_path and avoids writing to inputs dir. |
| tests/assembly/hic.py/tests.yml | Uses new --outdir=__TMP__ so bam2mat outputs land in temp. |
| tests/assembly/hic.py/inputs/test.resolution_500000.json | Adds reference JSON fixture for bam2mat tests. |
| tests/assembly/allmaps.py/inputs/scaffolds.fasta.gz.sizes | Adds sizes fixture to avoid generating it during tests. |
| tests/assembly/allmaps.py/inputs/JM-2.unplaced.fasta | Adds/updates allmaps test fixture input. |
| tests/assembly/allmaps.py/inputs/JM-2.unplaced.agp | Adds/updates allmaps test fixture input. |
| tests/assembly/allmaps.py/inputs/JM-2.summary.txt | Adds summary fixture output for allmaps-related testing. |
| tests/assembly/allmaps.py/inputs/JM-2.lifted.bed | Adds lifted BED fixture output for allmaps-related testing. |
| tests/assembly/allmaps.py/inputs/JM-2.fasta.sizes | Adds FASTA sizes fixture output for allmaps-related testing. |
| tests/assembly/allmaps.py/inputs/JM-2.chr.agp | Adds AGP fixture output for allmaps-related testing. |
| tests/assembly/allmaps.py/inputs/JM-2.agp | Adds AGP fixture output for allmaps-related testing. |
| tests/apps/test_fetch.py | Runs cookie-writing logic from tmp_path to avoid polluting repo CWD. |
| tests/apps/test_base.py | Runs file-writing tests from tmp_path to avoid polluting repo CWD. |
| src/jcvi/compara/synteny.py | Adds --outfile support to liftover() output naming. |
| src/jcvi/assembly/hic.py | Adds --outdir support to bam2mat() so outputs can be redirected. |
| .pre-commit-config.yaml | Bumps pre-commit hook revisions. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| p = OptionParser(liftover.__doc__) | ||
| p.set_stripnames() | ||
| p.set_outfile() | ||
|
|
There was a problem hiding this comment.
Fixed in the latest commit by changing p.set_outfile() to p.set_outfile(outfile=None), so opts.outfile is None by default and the .lifted.anchors fallback filename is used correctly when no --outfile flag is provided.
d96e11f to
ddc899f
Compare
|
@tanghaibao ready for review |
Pytest tests were writing output files into the repo working directory and
tests/graphics/data/— particularly graphics tests that produced PDFs with no post-test cleanup — requiring manual removal before commits.Changes
Redirect file creation to
tmp_pathtest_cbook.py::test_depends,test_sizes.py::test_sizes: passtmp_path-rooted paths instead of bare filenamestest_obo.py::test_oboreader,test_allmaps.py,test_base.py(test_cleanup,test_need_update,test_download,test_getpath),test_fetch.py::test_get_cookies: addmonkeypatch.chdir(tmp_path)so all output lands in pytest's auto-cleaned temp dir; remove now-redundant manualcleanup()callsGraphics tests: symlink data →
tmp_path, redirect outputTests that
chdirintotests/graphics/data/and produce PDFs there now symlink the data directory contents intotmp_pathandmonkeypatch.chdirthere instead. Output files (PDFs, intermediate images) are written totmp_pathand cleaned up automatically.Affected:
test_karyotype.py,test_landscape.py,test_synteny.py,test_grabseeds.py,test_tree.pytests/config.pyLOG = open("test_scripts.log", "a")was executed at module import time, creating the file in CWD. Changed to write totempfile.gettempdir().tests/formats/test_bed.pyRemoved
os.chdirentirely; passes the absolute path to the bed file directly tosummary().