From 22119caa85db0e6a6be546055895d5cba7b89f35 Mon Sep 17 00:00:00 2001 From: Wei Li Date: Tue, 4 Aug 2026 15:35:40 -0400 Subject: [PATCH 1/3] Make the test suite use this repo's compiled helpers The suite shells out to the bundled C++ helpers by bare name (RRA, mageckGSEA), so it exercises whichever build comes first on PATH. That is the right binary after `pip install .`, which compiles them via BuildWithHelpers and installs them alongside the package -- but not during ordinary local development. An editable install never runs that build step, and a MAGeCK v1 package in the same environment ships commands with the *same names*. The result is that the suite silently tests v1's binaries. Locally that surfaced as four mageckGSEA failures asserting fixes that are present in this checkout: the absent-gene and degenerate-pathway tests got v1's answers, and the -H test failed on a usage error because v1 has no --skip_header flag at all. Nothing in the output points at the binary being the wrong one. Add a conftest that puts rra/bin and gsea/bin first on PATH when the helpers have been built there, so the tests always describe this checkout. When they have not been built it changes nothing and the installed binaries are used, so CI behaviour is unaffected. Co-Authored-By: Claude Opus 5 (1M context) --- tests/conftest.py | 38 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) create mode 100644 tests/conftest.py diff --git a/tests/conftest.py b/tests/conftest.py new file mode 100644 index 0000000..ceff8a5 --- /dev/null +++ b/tests/conftest.py @@ -0,0 +1,38 @@ +"""Test configuration for MAGeCK2. + +The suite shells out to the bundled C++ helpers by bare name (``RRA``, +``mageckGSEA``), so it exercises whichever build happens to come first on PATH. +That is the correct binary after ``pip install .`` -- setup.py compiles the +helpers and installs them alongside the package -- but not during ordinary local +development: an editable install never runs that build step, and a MAGeCK v1 +package in the same environment ships commands with the *same names*. The suite +then silently tests v1's binaries and reports failures against fixes that are +present in this checkout, which is confusing and points nowhere useful. + +Put this repository's own freshly built helpers first on PATH when they exist, +so the tests always describe this checkout. Falls back to the installed ones +when the helpers have not been built (e.g. in CI, where ``pip install .`` has +already placed the right binaries on PATH). +""" + +import os +from pathlib import Path + +REPO_ROOT = Path(__file__).resolve().parent.parent + +# (binary name, directory the Makefile writes it to) +HELPER_BIN_DIRS = [ + ("RRA", REPO_ROOT / "rra" / "bin"), + ("mageckGSEA", REPO_ROOT / "gsea" / "bin"), +] + + +def pytest_configure(config): + """Prepend repo-built helper directories to PATH, most specific first.""" + prefix = [] + for name, bindir in HELPER_BIN_DIRS: + if (bindir / name).is_file(): + prefix.append(str(bindir)) + + if prefix: + os.environ["PATH"] = os.pathsep.join(prefix + [os.environ.get("PATH", "")]) From 4a82118e57fae2985cb24568fbdde9b98bebce58 Mon Sep 17 00:00:00 2001 From: Wei Li Date: Mon, 31 Aug 2026 11:38:28 -0400 Subject: [PATCH 2/3] Note the conftest in the changelog Co-Authored-By: Claude Opus 5 --- CHANGELOG.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 870499b..5779cfd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 `pathway --method gsea` — the default — now works from a pip/source install. - Pathway smoke tests (`pathway --method gsea` and `--method rra`, end to end) and a small GMT test fixture. +- `tests/conftest.py`, which puts this repository's freshly built `RRA` and + `mageckGSEA` first on `PATH` when they exist. The suite invokes the helpers by + bare name, so under an editable install — which never runs the compile step — + it silently tested whichever build came first, including a MAGeCK v1 package's + identically named binaries. That reported four `mageckGSEA` failures against + fixes present in the checkout, with nothing in the output naming the binary as + the cause. Falls back to the installed helpers when the repo ones are absent. - `count` writes `.pg_unmapped.txt` when `--unmapped-to-file` is given in paired-guide mode, recording pairs that were counted but excluded from `pg_count.txt` — because the second guide matched no entry in `--list-seq-2`, From 6aba9ff72076cd9464b941d5dace3e012523864f Mon Sep 17 00:00:00 2001 From: Wei Li Date: Mon, 31 Aug 2026 12:03:55 -0400 Subject: [PATCH 3/3] Keep testing the installed helpers, not just the build tree The PATH override applies in CI as well as locally: pip install . runs make in the source tree, so the build outputs sit in rra/bin and gsea/bin alongside the copies data_files installs. Prepending them meant the installed copies were never exercised, and a regression in helper installation could pass CI even though an installed MAGeCK2 could not find its helpers -- the exact failure 0.2.0 fixed. Measured on a venv built the way CI builds one, with the installed helpers removed and the source tree left intact: before this commit 49 passed (broken install ships green) after 2 failed test_rra_binary_on_path, test_mageckgsea_binary_on_path conftest now publishes the pre-override PATH as MAGECK2_INSTALLED_PATH, and the two tests that assert the helpers are installed look there. The rest of the suite still tests this checkout's own build, which is the point of the override. This also reports the failure better than before the conftest existed: a broken install used to surface as eight GSEA assertion failures that named nothing relevant, and now names the missing binary directly. Reported by Codex review on #38. Co-Authored-By: Claude Opus 5 --- CHANGELOG.md | 5 ++++- tests/conftest.py | 20 +++++++++++++++++--- tests/test_smoke.py | 22 +++++++++++++++++++--- 3 files changed, 40 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5779cfd..0dd826d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,7 +20,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 it silently tested whichever build came first, including a MAGeCK v1 package's identically named binaries. That reported four `mageckGSEA` failures against fixes present in the checkout, with nothing in the output naming the binary as - the cause. Falls back to the installed helpers when the repo ones are absent. + the cause. The override applies in CI too, since `pip install .` leaves the + build outputs in the source tree alongside the copies `data_files` installs; + so the two tests that assert the helpers are *installed* now look at the PATH + as it stood before the override, keeping a broken installation from passing. - `count` writes `.pg_unmapped.txt` when `--unmapped-to-file` is given in paired-guide mode, recording pairs that were counted but excluded from `pg_count.txt` — because the second guide matched no entry in `--list-seq-2`, diff --git a/tests/conftest.py b/tests/conftest.py index ceff8a5..d9a2df8 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -10,9 +10,15 @@ present in this checkout, which is confusing and points nowhere useful. Put this repository's own freshly built helpers first on PATH when they exist, -so the tests always describe this checkout. Falls back to the installed ones -when the helpers have not been built (e.g. in CI, where ``pip install .`` has -already placed the right binaries on PATH). +so the tests always describe this checkout. + +Note this override applies in CI too: ``pip install .`` runs ``make`` in the +source tree, so the build outputs are present there alongside the copies +``data_files`` installs. The installed copies would then never be exercised, and +a regression in helper installation could pass CI even though an installed +MAGeCK2 cannot find its helpers. To keep that coverage, the PATH as it stood +before this hook ran is published as ``MAGECK2_INSTALLED_PATH``, and the tests +asserting the helpers are installed look there rather than at the build tree. """ import os @@ -27,8 +33,16 @@ ] +#: The PATH before this hook ran -- i.e. where an *installed* MAGeCK2 finds its +#: helpers. Published so tests can assert on the installation rather than on the +#: build tree that gets prepended below. +INSTALLED_PATH_ENV = "MAGECK2_INSTALLED_PATH" + + def pytest_configure(config): """Prepend repo-built helper directories to PATH, most specific first.""" + os.environ.setdefault(INSTALLED_PATH_ENV, os.environ.get("PATH", "")) + prefix = [] for name, bindir in HELPER_BIN_DIRS: if (bindir / name).is_file(): diff --git a/tests/test_smoke.py b/tests/test_smoke.py index cc9f0a6..29cfd14 100644 --- a/tests/test_smoke.py +++ b/tests/test_smoke.py @@ -7,6 +7,7 @@ push in CI. """ +import os import shutil import subprocess from pathlib import Path @@ -24,14 +25,29 @@ def test_package_imports(): assert __version__ +def _installed_path(): + """The PATH as it stood before conftest prepended this repo's build tree. + + The rest of the suite deliberately tests the checkout's own helpers. These + two tests are the exception: they check that an *installed* MAGeCK2 can find + its helpers, which is what `data_files` is responsible for and what a user + running `pip install mageck2` depends on. Looking at the build tree here + would let a broken installation pass CI, since `pip install .` leaves the + build outputs in the source tree as well. + """ + return os.environ.get("MAGECK2_INSTALLED_PATH", os.environ.get("PATH", "")) + + def test_rra_binary_on_path(): - assert shutil.which("RRA") is not None, "compiled RRA binary not found on PATH" + assert ( + shutil.which("RRA", path=_installed_path()) is not None + ), "compiled RRA binary not installed onto PATH" def test_mageckgsea_binary_on_path(): assert ( - shutil.which("mageckGSEA") is not None - ), "compiled mageckGSEA binary not found on PATH" + shutil.which("mageckGSEA", path=_installed_path()) is not None + ), "compiled mageckGSEA binary not installed onto PATH" def test_mageck2_cli_runs():