Skip SGE submission for tests with invalid qsub options, and add a self-test suite - #53
Open
milechin wants to merge 7 commits into
Open
Skip SGE submission for tests with invalid qsub options, and add a self-test suite#53milechin wants to merge 7 commits into
milechin wants to merge 7 commits into
Conversation
find_qsub.py's qsub_valid column flags tests whose qsub options were rejected by `qsub -w p` at discovery time. pkgtest.nf now branches the input channel on it: valid rows go to runTests (submitted to SGE, as before) while invalid rows go to a new reportInvalid process pinned to `executor 'local'`, so no job is submitted to the scheduler. Invalid tests are recorded as FAILED (exit_code=1, tests_failed=1, job_number=NA) with an explanatory results.txt, and merged into the same report. This avoids the prior behavior where invalid qsub_options, injected into the job's clusterOptions, made SGE reject the job at submission time -- before any in-script guard could run -- so the row silently dropped from the report. The report's 13-column layout is unchanged, so email_notif.pl and the rshiny app are unaffected. Updates README.md and CLAUDE.md to document the new behavior. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Adds the repo's first tests/ directory as a small, extensible pytest suite that validates find_qsub.py and pkgtest.nf against a checked-in fixture (a fake module pkgselfcheck/1.0 with a valid test.qsub and an invalid test.bad.qsub). test_invalid_qsub.py asserts: find_qsub sets qsub_valid True/False correctly, and the pipeline PASSES the valid test while FAILING the invalid one via reportInvalid (job_number=NA), never submitting it. Two tiers via --mode: - local (default): stubs replace `module`/`qsub` (tests/stubs/) and the pipeline runs --executor local -- no Lmod, no SGE, no `module load`. Runs in the conda env (tests/environment.yml). find_qsub.py is not modified; it runs as a subprocess with a scrubbed env (Lmod's exported `module` function removed, stubs prepended to PATH). - integration: real Lmod/qsub + real SGE on an SCC node. Shared machinery (fixture build, per-mode env, runners, CSV-by-header parsing, preflight skips) lives in conftest.py's run_env fixture; add a future test as tests/test_<feature>.py reusing it. environment.yml pins nextflow>=24 and openjdk=17: leaving them unpinned lets conda install a JDK newer than nextflow supports (Java 11-21), which fails with "cannot find Java or it's a wrong version". Docs: README.md gains a Testing section; CLAUDE.md's stale "no test suite" note now describes the suite; .gitignore excludes the runtime artifacts. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Runs the local (scheduler-free) tier of the pytest suite on push to main and on pull requests: installs Python + Nextflow + Java, points /bin/sh at bash (find_qsub.py uses the `|&` bashism, unsupported by ubuntu's dash), ensures ~/.bashrc exists (pkgtest.nf's beforeScript sources it), then runs tests/run_local.sh and uploads the JUnit results. The integration tier is SCC-only and is not run in CI. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The `get.nextflow.io` installer writes a file named `nextflow` in the current directory; run from the repo root it collided with the repo's nextflow/ directory and aborted. Run the installer in $RUNNER_TEMP. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Nextflow latest (26.x) runs task scripts with pipefail, which aborts pkgtest.nf runTests where `grep Error | wc -l` finds no match on a passing test. Pin NXF_VER to a version used on the cluster. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Nextflow runs task scripts with set -u, and pkgtest.nf runTests echoes SGE-provided vars ($NSLOTS, $QUEUE, $JOB_ID). Off-SGE (executor local, e.g. GitHub Actions) these are unset and abort the task. The local-mode test env now supplies inert defaults. Also removes the temporary CI diagnostic step. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Skip SGE submission for tests with invalid qsub options, and add a self-test suite
Summary
Builds on the existing
qsub_validcolumn (from #47) to make the pipeline act on it. Tests whose qsub options are rejected byqsub -w pare no longer submitted to SGE — they are markedFAILEDlocally instead of being silently dropped when the scheduler rejects the job at submission. Also adds the repo's first automated test suite, covering this end to end.What changed
Execution —
nextflow/pkgtest.nfbranches input rows onqsub_valid:True→runTests(submitted to SGE, unchanged behavior).True→ a newreportInvalidprocess pinned toexecutor 'local', so no job is submitted to the scheduler. It records the test asFAILED(exit_code=1,tests_failed=1,job_number=NA) with an explanatoryresults.txt.test_metrics.csv, so the report layout is unchanged andemail_notif.pl/ the rshiny app are unaffected.qsub_optionsare injected into each job'sclusterOptions, so invalid options make SGE reject the job at submission — before any in-process guard insiderunTestscould run. Handling it outsiderunTestsis the only place that works.Tests — new
tests/directory (pytest)find_qsub.py+pkgtest.nfagainst a checked-in fixture (a fake modulepkgselfcheck/1.0with one valid and one invalidtest.qsub).test_invalid_qsub.pyasserts thatfind_qsub.pysetsqsub_validcorrectly, and that the pipeline PASSES the valid test while FAILING the invalid one viareportInvalid(job_number=NA), never submitting it.--modeoption:module/qsuband the pipeline runs--executor local: no Lmod, no SGE, nomodule load. Runs in the conda env (tests/environment.yml).module, realqsub, and real SGE on an SCC node.find_qsub.pyis not modified for testing; it runs as a subprocess with a scrubbed env (Lmod's exportedmodulefunction removed, stubs prepended toPATH).tests/test_<feature>.pyreusing therun_envfixture intests/conftest.py.tests/environment.ymlpinsnextflow>=24andopenjdk=17; leaving them unpinned lets conda install a JDK newer than Nextflow supports (Java 11–21), which fails with "cannot find Java or it's a wrong version".Docs
README.mdgains a Testing section;CLAUDE.mdadded with architecture notes and the branching behavior;.gitignoreexcludes the test runtime artifacts.Testing
bash tests/run_local.sh -v→ 2 passed (Nextflow--executor local, no SGE).gdal2.csv: the invalidgdal/3.1.2row is reportedFAILED/NAwith no SGE job submitted, and the valid rows submit and PASS.How to run the tests
conda env create -f tests/environment.yml # on the SCC: module load miniconda first conda activate pkgautotest-selftest bash tests/run_local.sh -v