diff --git a/.claude/development-notes/README.md b/.claude/development-notes/README.md index 2240025..df67a14 100644 --- a/.claude/development-notes/README.md +++ b/.claude/development-notes/README.md @@ -75,6 +75,7 @@ One file per subject, not per source file — the reasoning crosses file boundar | `gates-that-stopped-checking.md` | the failure this project is prone to: a change moves what a checker points at, and the checker keeps exiting 0 over nothing | | `host-glibc-floor.md` | v3.1.1's analysis environment installed only on the machine that froze it: virtual packages, the four guards, and why a solve cannot answer it | | `someone-elses-machine.md` | four defects a green suite could not see, because the suite runs where the assumption holds; and the second machine that found three of them in an hour | +| `macos-support.md` | the sizing, after the manual was found claiming a platform the pinned environments cannot solve on: three blockers, and why exporting is what makes it single-platform | | `brainstorming.md` | ideas for later releases: what each would buy, what it would break, and where it sits | | `module-queue.md` | the plan from v3.1.1: thirteen modules at one a week, published without a release, and the two shape questions the roster raised | diff --git a/.claude/development-notes/macos-support.md b/.claude/development-notes/macos-support.md new file mode 100644 index 0000000..12827b2 --- /dev/null +++ b/.claude/development-notes/macos-support.md @@ -0,0 +1,48 @@ +# What macOS support would actually take + +**Written 2026-09-23, against the tree at `6e12735`, during the 3.1.3 cycle.** Z asked whether the new tab completion would work on a Mac. It would not, and neither does anything else, although the manual had claimed macOS since before v3.1.0. Z's call the same day: **drop it for now, correct the claim, and plan it properly** - *"It is real work. I'm still fixing the backend. I need to get this to become more stable before we start working on MacOS."* + +This note is the sizing, so the next attempt starts from measurements rather than from a survey. + +## The claim that was wrong + +Five places said macOS was supported, including the Requirements table and `README.md`. One said more than that: the analysis-environment section claimed the compiler is *"GCC on Linux, clang on macOS"*, describing a per-platform behavior the shipped file cannot have. All six now say Linux, and `#### Why not Windows` became `#### Why Linux only` (keeping the `#why-not-windows` anchor, which one row links to). + +**This is the glibc bug one level up.** That was a single pin excluding older Linux; this was a whole platform the documentation promised. Same cause, recorded in [[someone-elses-machine]]: it has only ever been installed on one machine. + +## The three blockers, in the order they bite + +### 1. The environments cannot solve at all + +`install/environment.yml` pins `ld_impl_linux-64`. `install/environment-analysis.yml` carries **12** `linux-64` pins - `sysroot_linux-64`, `gcc_impl_linux-64`, `gxx_impl_linux-64`, `kernel-headers_linux-64` and the rest of the toolchain. Those package names exist for `linux-64` by construction and for no other platform, so `conda env create` fails before anything else is reached. + +**This arrived with the pinned exports.** A hand-written spec names what you ask for and solves per platform; an export names exactly what one machine got. The reproducibility that makes the export right is the same property that makes it single-platform. + +So macOS needs its own exported files - and **two of them**, because conda treats `osx-64` and `osx-arm64` as different platforms. Each has to be exported on that hardware and verified there. That is the part that cannot be done from here. + +### 2. Three GNU-only idioms in the shipped wrapper + +| | | +|---|---| +| `PoolSeqFlow:86` | `readlink -f` - `-f` is a GNU extension; it fails at startup, before dispatch | +| `PoolSeqFlow:178` | `sort -V` - BSD `sort` has no version sort | +| `PoolSeqFlow:359` | `sed -i "..."` - BSD requires a backup suffix, `sed -i ''`, and errors without one | + +Only the completion was fixed, because it was being written that day: `lib/poolseqflow-completion.bash` follows symlinks a hop at a time with plain `readlink` and a loop guard. That is better code on Linux too, so it stayed. + +### 3. The suite itself has to run there + +Proving macOS support means running the suite on the Mac, and the suite is not portable either. Found without looking hard: `stat -c` in `run_tests.sh` and `02_launcher`, `find -printf` in `04_pipeline`, GNU `sed -i` in `04_pipeline`, `05_guards` and `06_dryrun`. There will be more - a full audit was started and stopped when the decision was made. + +**And the filesystem differs.** APFS is case-insensitive by default, so any two fixtures differing only in case are the same file there. + +## What else to think about before starting + +- **`check-host-floor.sh` reasons about `__glibc`**, which does not exist on macOS - conda uses `__osx`. It needs to know which platform it is checking rather than assuming. +- **`export-environment.sh` and `prep-version.sh`** write and validate one pair of files. They would need to know about three platforms, and a release would not be exportable from one machine. +- **Every module compiles its hot path on the user's machine.** So the analysis environment needs a working clang toolchain pinned for each macOS platform, not just R. +- **`bash 3.2`.** macOS ships the 2007 GPLv2 bash as `/bin/bash`. The wrapper's shebang is `#!/usr/bin/env bash`, so it would take whatever is first on PATH - conda's newer bash if an environment is active, the system one otherwise. Worth deciding deliberately rather than discovering. + +## The honest summary + +Not a flag and not an afternoon. It is: three sets of pinned environment files instead of one, a release process that can export them, three wrapper fixes, an unknown number of suite fixes, and a machine to verify all of it on. Nothing here is hard; it is just genuinely a platform port, and the pinning that makes this project reproducible is exactly what makes it one. diff --git a/.claude/development-notes/someone-elses-machine.md b/.claude/development-notes/someone-elses-machine.md index bf97d8e..c79e312 100644 --- a/.claude/development-notes/someone-elses-machine.md +++ b/.claude/development-notes/someone-elses-machine.md @@ -28,7 +28,23 @@ fi `rehash` is zsh's name for `hash -r`. The wrapper is `#!/usr/bin/env bash`, so wherever `ZSH_VERSION` reaches a bash script, conda picks a command bash does not have. Measured on the server: `bash -c 'echo $ZSH_VERSION'` printed `5.9`, so something there exports it - zsh does not, and the maintainer's machine does not, which is exactly why it had never appeared. -**Fixed by defining it rather than by silencing conda.** `lib/wrapper_lib.sh` carries `rehash() { hash -r; }`; the backslash in `\rehash` suppresses aliases, not functions, so that is what runs. Suppressing conda's stderr was the first instinct and was wrong: it would hide conda's real failures, and refreshing the command table is what conda was asking for. +**NOT FIXED, on purpose. Z's ruling, 2026-09-23.** Three fixes were written and all three were rejected, and the reason they were all wrong is the same: **the wrapper is not what is broken.** + +`eval "$(conda shell.bash hook)"` has been line 17 of `PoolSeqFlow` since **v1.0.0**, written by Z alone, and it is unchanged through every release to 3.1.2 - only its line number moved as the header grew. It works, and it is the only way a script gets `conda activate`: a shell function does not cross a process boundary, so a user who has run `conda init` gives their *interactive* shell the function and gives a script nothing. Measured - parent reports `conda is a function`, the child one process later reports `conda is a file` and `conda activate` fails with `CondaError: Run 'conda init' before 'conda activate'`. + +**And asking for the bash hook does not get a bash-only hook.** `conda shell.bash hook` emits `__conda_hashr` with the `ZSH_VERSION` branch still in it, at line 20 of its own output. That is conda's, not ours. + +So on a machine where something exports `ZSH_VERSION` into a bash process, conda believes a false claim the environment made about itself and calls a command bash does not have. **The correct response is to do nothing**, because correcting another machine's environment is not this tool's business. The noise is cosmetic: the consequence of the zsh branch is that `hash -r` does not run, and the wrapper invokes nothing before it activates, so there is no stale entry to refresh. + +The three rejected fixes, and what each got wrong: + +- **Suppressing conda's stderr.** Would hide conda's real failures, and the command-table refresh is what conda was actually asking for. +- **`rehash() { hash -r; }` in `lib/wrapper_lib.sh`.** Made a bash script carry zsh's vocabulary to satisfy a claim that was not true. It was also in the wrong place - `wrapper_lib.sh` is sourced at line 96 and the hook is evaluated at line 44, so the function did not exist for the eval that first raised the error. +- **`unset ZSH_VERSION POSH_VERSION` before the hook.** Z: *"You are still making assumptions about the shell. We don't deal with that."* Process-local or not, it is the tool reaching into variables it does not own to compensate for someone else's misconfiguration. + +**What this costs**: the `rehash: command not found` line comes back on that server. Z accepted that knowingly. + +**The general form is still worth keeping**, and it is why this one sits oddly beside the other three in this note. Each of them is a string standing in for a real question, and here the real question is *which shell is this*. The difference is that the other three were our strings, in our code, answering questions about our own behavior - and this one is conda asking a question we have no standing to answer. **It was visible in one arm and not another for a reason worth keeping.** `uninstall` called `conda deactivate` bare while `uninstall_all` had `2>/dev/null || true`, so the same noise was hidden in one place and shown in the other. That inconsistency is what made it findable - the user could say "during uninstall but not uninstall_all", which pointed straight at the difference. The `|| true` also closed a real hazard: under `set -e` a non-zero `conda deactivate` would have abandoned the uninstall with the environment half removed. diff --git a/CLAUDE.md b/CLAUDE.md index 9400f26..94892ee 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -106,22 +106,22 @@ bash test/run_tests.sh --suite 07_analysis --case citation | you changed | run | |---|---| -| `bin/` | `05_helpers` — except the three `check_*.sh`, which are `02_launcher` | +| `bin/` | `03_helpers` — except the three `check_*.sh`, which are `02_launcher` | | `PoolSeqFlow`, install/uninstall, the check scripts | `02_launcher` | | `bin/config_migrate.sh`, the templates | `01_migrate` | -| step 0, parameter resolution, the change guards | `04_guards` | -| wiring, channels, promotion, a step's script | `03_pipeline` | +| step 0, parameter resolution, the change guards | `05_guards` | +| wiring, channels, promotion, a step's script | `04_pipeline` | | `dryrun.nf`, `dryrun`/`dryclean` | `06_dryrun` | | version strings, packaging, syntax | `00_static` | | a module library under `modules/lib/` | `analysis_rlib` — no JVM, 3 seconds | | `analysis/lib/nf/`, the frame | the analysis seam you touched: `analysis_frame`, `analysis_plan`, `analysis_verify`, `analysis_design`, `analysis_time`, `analysis_series`, `analysis_modules`, `analysis_results` | | a module | `--suite `; its cases travel with it under `modules//test/` | -`--fast` runs everything that does not start a JVM; what it skips is `03_pipeline`, `04_guards`, and the pipeline halves of `06_dryrun` and the analysis suites. +`--fast` runs everything that does not start a JVM; what it skips is `04_pipeline`, `05_guards`, and the pipeline halves of `06_dryrun` and the analysis suites. **`bash test/run_tests.sh --changed` picks the suites for you**, from what each suite declares it runs expanded through the include graph. `dev/scripts/select-tests.py ` shows the reasoning without running anything. It errs wide — a change to `test/lib/` or to the selector selects everything — so a narrow answer is trustworthy and a wide one is only expensive. -**Every suite declares what it may cost** — `static`, `jvm` or `pipeline` — in a `# cost:` line in its own header. `--cost static` is the set that completes with nothing installed: `00_static`, `01_migrate`, `02_launcher`, `05_helpers`, `08_analysis_rlib`. `--fast` is a different axis and still a case-level switch, so the two compose. +**Every suite declares what it may cost** — `static`, `jvm` or `pipeline` — in a `# cost:` line in its own header. `--cost static` is the set that completes with nothing installed: `00_static`, `01_migrate`, `02_launcher`, `03_helpers`, `07_analysis_rlib`. `--fast` is a different axis and still a case-level switch, so the two compose. **`--suite` and `--case` accumulate and match by name, not by number.** `--suite analysis_time --suite analysis_series` runs both, and `--suite analysis` runs all nine. Every run prints the scope it selected, so a narrowed run cannot be mistaken for a full one; renumbering a suite therefore costs nothing, because nothing addresses one by its number. diff --git a/PoolSeqFlow b/PoolSeqFlow index 1cdf4f4..09f925a 100755 --- a/PoolSeqFlow +++ b/PoolSeqFlow @@ -401,6 +401,53 @@ deploy_payload() { ;; esac echo "" + install_completion "$dest" +} + +# Where a user's own bash completions live, per the XDG base directory specification. The same +# path bash-completion searches for a file named after the command. +completion_dir() { + printf '%s/bash-completion/completions' "${XDG_DATA_HOME:-$HOME/.local/share}" +} + +# Put the completion where bash will find it by name, and say what zsh needs. +# +# Copied rather than symlinked, so uninstalling one version does not leave the file pointing +# into a directory that is gone. A failure here does not fail the install: completion is a +# convenience and the tool works without it. +install_completion() { + local src="$1/lib/poolseqflow-completion.bash" dir + [ -f "$src" ] || return 0 + dir=$(completion_dir) + mkdir -p "$dir" 2>/dev/null || { echo "Could not create $dir; tab completion not installed."; return 0; } + if ! cp "$src" "$dir/PoolSeqFlow" 2>/dev/null; then + echo "Could not write $dir/PoolSeqFlow; tab completion not installed." + return 0 + fi + echo "Tab completion installed:" + echo " $dir/PoolSeqFlow" + echo "" + echo " bash picks it up in your next shell." + echo " zsh does not read that directory. Add this to ~/.zshrc:" + echo "" + echo " autoload -U +X bashcompinit && bashcompinit" + echo " . $dir/PoolSeqFlow" + echo "" +} + +# Remove the completion, but only when no other installed version still provides one. +remove_completion() { + local prefix file + prefix=$(install_prefix) + file="$(completion_dir)/PoolSeqFlow" + [ -f "$file" ] || return 0 + # Another version left installed keeps it: the file is the same for every version, and the + # command it completes still exists. + if ls -d "$prefix"/opt/PoolSeqFlow-*/ > /dev/null 2>&1; then + return 0 + fi + rm -f "$file" 2>/dev/null || true + echo "Removed $file" } # Every PoolSeqFlow environment except this version's own and the legacy unversioned one. @@ -1300,14 +1347,7 @@ case $COMMAND in bash "$(install_prefix)/opt/PoolSeqFlow-${VERSION}/bin/check_install.sh" echo "Installation complete." echo "" - echo "The analysis layer was copied with everything else, so 'PoolSeqFlow analysis'" - echo "answers now. THAT DOES NOT MEAN THE ANALYSIS LAYER IS INSTALLED. The scripts" - echo "weigh nothing and travel with the release so that they can never be a version out" - echo "of step with the pipeline; the weight is the environment, which carries R and" - echo "which nothing here creates. Until you create it, every module refuses and says so." - echo "" - echo "The pipeline is complete without it. Add the analysis layer whenever you want it," - echo "or never:" + echo "To optionally install the analysis layer, run:" echo " $(basename "$0") analysis install" ;; @@ -1324,12 +1364,9 @@ case $COMMAND in check) run_check "$CHECK_TARGET" ;; - run|resume) + run) # Resuming is what `run` already does: every step skips itself when its outputs are # already in storage. Nextflow's own -resume is not used. - if [ "$COMMAND" = "resume" ]; then - echo "Note: 'resume' is deprecated - 'run' already resumes automatically." - fi require_install require_project_config require_migrated_config @@ -2028,6 +2065,7 @@ EOF fi if [ "$HAVE_PAYLOAD" -eq 1 ]; then remove_payload "$TARGET_VERSION" + remove_completion elif [ -n "$TARGET_VERSION" ]; then echo "No pipeline installed at $PREFIX/opt/PoolSeqFlow-${TARGET_VERSION}." fi @@ -2084,6 +2122,7 @@ EOF for v in $PAYLOADS; do remove_payload "$v" || true done + remove_completion echo "" echo "All PoolSeqFlow environments and installations removed." ;; diff --git a/README.md b/README.md index 80759e8..591bead 100644 --- a/README.md +++ b/README.md @@ -12,7 +12,7 @@ ### 📖 [Read the documentation →](https://ozankiratli.github.io/PoolSeqFlow/) -> **Platform note:** PoolSeqFlow is developed and tested on **Linux and macOS**. Windows is not supported — the resume logic relies on symbolic links and Unix-style paths that are not compatible with native Windows filesystems. +> **Platform note:** PoolSeqFlow is developed and tested on **Linux**. macOS and Windows are not supported: the shipped conda environments are pinned to `linux-64` builds, and the resume logic relies on symbolic links and Unix-style paths that are not compatible with native Windows filesystems. --- @@ -55,7 +55,7 @@ Raw FASTQ reads ## Quick start -Requires Linux or macOS and [conda](https://docs.conda.io/en/miniconda.html). Every bioinformatics tool is installed for you into an isolated environment, pinned to an exact build. +Requires Linux and [conda](https://docs.conda.io/en/miniconda.html). Every bioinformatics tool is installed for you into an isolated environment, pinned to an exact build. ```bash # 1. Download the latest release diff --git a/dev/RELEASING.md b/dev/RELEASING.md index 0f8f50d..a1aaced 100644 --- a/dev/RELEASING.md +++ b/dev/RELEASING.md @@ -62,15 +62,20 @@ Every check must say `ok`. A failure here is a compatibility problem between thi ### Prove the frozen environment installs on an older host than this one ``` -./PoolSeqFlow analysis install -dev/scripts/check-host-floor.sh +dev/scripts/check-exported-floor.sh ``` +It solves each exported file into a scratch environment of its own, reads what that really requires of its host, and discards it. Minutes, and a real network solve. + +**It replaces `./PoolSeqFlow analysis install` + `check-host-floor.sh`, which could not answer this and was unsafe besides.** The bump is step 6, so here the wrapper still declares the *old* version. `analysis install` therefore looks for `PoolSeqFlow--analysis`, finds it, prints `already exists` and creates nothing — and `check-host-floor.sh` defaults to that same old name, so the step read out the floor of the release being replaced while looking like a check on the new one. And removing the old environment first, which is the obvious way to make the install actually run, is worse: it builds the **new** file under the **old** version's name, leaving the release you have not replaced yet pointing at an environment that is no longer its own. + **This is the step v3.1.1 did not have, and a cluster found what it missed.** The analysis environment carries a compiler, because every module builds its hot path on the user's machine, and a compiler is built against a particular glibc. `conda update --all` takes the newest build of everything *this* machine can install, so `sysroot_linux-64` climbs to the maintainer's own glibc unless something says otherwise — v3.1.1 shipped `sysroot_linux-64=2.39`, which declares `__glibc >=2.39`, and no host below that could solve it. It installed perfectly here and nowhere older. `prep-version.sh` now pins the floor into the scratch environment before it updates, and `export-environment.sh` refuses to write a file that breaks it, so this should pass without incident. Run it anyway: those two guard the package whose *version* is the glibc it targets, and **every other package carries its constraint in conda metadata instead**. `libsanitizer=16.2.0` requires `__glibc >=2.17` and nothing in the string `16.2.0` says so. This reads `conda-meta/*.json` in the installed environment, which is where the real constraints are, and reports the highest lower bound anything actually imposes. -It needs the environment to exist, which is why it comes after an install rather than instead of one. A dry-run solve cannot substitute: measured 2026-09-21, `conda env create --dry-run --json` returns no `depends` key at all — 0 of 190 records carried one. +It needs an environment that really exists, which is why the script builds one rather than asking. A dry-run solve cannot substitute: measured 2026-09-21, `conda env create --dry-run --json` returns no `depends` key at all — 0 of 190 records carried one. + +**And it asks a different question from the one `prep-version.sh` already answered at `[4/5]`.** That checks the floor of the **cloned and updated** scratch environments, before the export, which is what catches an update raising the floor while the solve is still in hand. This checks the file that was then **written**, solved from nothing the way a user's install resolves it. A clone carries whatever the source environment held; a file names constraints and lets the solver choose again. If it refuses, the floor is a release decision and not a solve artifact. Raising it drops machines, so it takes a line in the manual's Requirements, a move of `HOST_GLIBC_FLOOR` in `export-environment.sh`, and a CHANGELOG entry saying which machines just lost support. @@ -242,6 +247,10 @@ It builds the tarball into `modules/repo/`, reads `kind`, `contract`, `frame`, ` Sync `dev` with `main` so the version bump and the CHANGELOG come back, then carry on. The first commits after a release are usually the things this protocol found and deferred. +``` +git merge --ff-only main +``` + --- ## What to do when a step fails diff --git a/dev/scripts/check-exported-floor.sh b/dev/scripts/check-exported-floor.sh new file mode 100755 index 0000000..cfce40e --- /dev/null +++ b/dev/scripts/check-exported-floor.sh @@ -0,0 +1,118 @@ +#!/usr/bin/env bash +# +# Solve a shipped environment file into a scratch environment, read what it really requires of +# its host, and throw the scratch away. +# +# Usage: dev/scripts/check-exported-floor.sh [environment-file ...] +# defaults to both: install/environment.yml and install/environment-analysis.yml +# +# Minutes, and a real network solve. Run it by hand during a release, after the export. +# +# WHAT IT REPLACES, AND WHY THAT DID NOT WORK +# ------------------------------------------- +# dev/RELEASING.md step 2 used to say: +# +# ./PoolSeqFlow analysis install +# dev/scripts/check-host-floor.sh +# +# In the middle of a release cycle the wrapper still declares the OLD version, because the bump +# is step 6. So `analysis install` looks for PoolSeqFlow--analysis, finds it already there, +# prints "Environment ... already exists" and creates nothing - and check-host-floor.sh then +# defaults to that same old name. The step installed nothing and reported on the release being +# replaced, while reading as a check on the new one. +# +# Removing the old environment first, which is the obvious way to make the install actually run, +# is worse than the no-op: it builds the NEW file under the OLD version's name, so the release +# that has not been replaced yet is left pointing at an environment that is no longer its own. +# Z, 2026-09-24: *"Even if it ran it still would be wrong because it would install an updated +# env for an older version."* Hence a scratch name that belongs to no release, and a discard. +# +# WHAT THIS ANSWERS THAT prep-version.sh DOES NOT +# ----------------------------------------------- +# prep-version.sh checks the floor at [4/5] against the CLONED AND UPDATED scratch environments, +# before it exports. That catches an update raising the floor while the solve is still in hand. +# It does not check the file that was then written. This does: the exported file is solved from +# scratch, the way a user's install resolves it, and the floor is read out of the result. +# +# The two are not the same question. A clone carries whatever was in the source environment; a +# file names constraints and lets the solver choose again. + +set -uo pipefail + +cd "$(dirname "$0")/../.." || exit 1 + +SCRATCH="PoolSeqFlow-floorcheck" +FILES=("$@") +[ "${#FILES[@]}" -gt 0 ] || FILES=(install/environment.yml install/environment-analysis.yml) + +# CONDA HAS TO BE REACHABLE, AND "conda said no" IS NOT "conda did not run". +# +# Release scripts here call `conda env list` and read an empty answer as "not present". On a +# machine whose shell function is set up for another shell family, a non-interactive bash gets +# `__conda_exe: permission denied` and the same empty answer, so a sound release is refused for +# a reason that has nothing to do with the environments. This checks the tool works at all +# before believing anything it says. +if ! command -v conda > /dev/null 2>&1; then + echo "ERROR: conda is not on PATH." >&2 + exit 1 +fi +if ! conda env list > /dev/null 2>&1; then + echo "ERROR: 'conda env list' failed, so nothing it reports can be trusted." >&2 + echo " Source the hook first, then run this again:" >&2 + echo " . \"\$(dirname \"\$(dirname \"\$(command -v conda)\")\")/etc/profile.d/conda.sh\"" >&2 + exit 1 +fi + +env_exists() { + conda env list | awk '{print $1}' | grep -qxF "$1" +} + +if env_exists "$SCRATCH"; then + echo "ERROR: '$SCRATCH' already exists, left over from an earlier run." >&2 + echo " Investigate or discard it, then start again:" >&2 + echo " conda env remove -n $SCRATCH --yes" >&2 + exit 1 +fi + +# Removed however this exits, including on an interrupt: a scratch environment left behind makes +# the next run refuse, and it is large. +discard() { + if env_exists "$SCRATCH"; then + echo " discarding $SCRATCH" + conda env remove --name "$SCRATCH" --yes > /dev/null 2>&1 || true + fi +} +trap discard EXIT INT TERM + +STATUS=0 +for file in "${FILES[@]}"; do + if [ ! -f "$file" ]; then + echo "ERROR: no such file: $file" >&2 + STATUS=1 + continue + fi + echo "" + echo "=== $file" + echo " solving into $SCRATCH (minutes)..." + # -n is required: the exported files carry no `name:` key, which 00_static asserts, so that + # a user's install cannot be named by whoever ran the export. + if ! conda env create --name "$SCRATCH" --file "$file" --yes > /tmp/floorcheck.$$.log 2>&1; then + echo " REFUSED: the file does not solve on this host." >&2 + sed 's/^/ /' /tmp/floorcheck.$$.log >&2 + rm -f /tmp/floorcheck.$$.log + STATUS=1 + discard + continue + fi + rm -f /tmp/floorcheck.$$.log + bash dev/scripts/check-host-floor.sh "$SCRATCH" || STATUS=1 + discard +done + +echo "" +if [ "$STATUS" -eq 0 ]; then + echo "Every file solves from scratch and holds the floor it declares." +else + echo "At least one file did not. Nothing was changed; the scratch environment is gone." >&2 +fi +exit "$STATUS" diff --git a/dev/scripts/debug-case.sh b/dev/scripts/debug-case.sh index bd16d23..4795fbf 100755 --- a/dev/scripts/debug-case.sh +++ b/dev/scripts/debug-case.sh @@ -4,7 +4,7 @@ # machine and not another can be compared directly. # # Usage: dev/scripts/debug-case.sh [label] -# dev/scripts/debug-case.sh 04_guards unusable_multirun mine +# dev/scripts/debug-case.sh 05_guards unusable_multirun mine # dev/scripts/debug-case.sh 15_analysis_results one_pdf yours # # WHY THIS EXISTS diff --git a/dev/scripts/prep-version.sh b/dev/scripts/prep-version.sh index 9476dcd..fd767eb 100755 --- a/dev/scripts/prep-version.sh +++ b/dev/scripts/prep-version.sh @@ -363,9 +363,7 @@ if [ "$TEST_STATUS" -ne 0 ]; then say " Collecting what the failing runs left behind..." harvest_artifacts | tee -a "$LOGDIR/summary.txt" say " Each failing case's run.out is there, with the .nextflow.log beside it." - say " Read run.out first: three cases assert on Nextflow's own status line" - say " ([SUCCESS]/[FAILED] completed=N failed=N cached=N), and its absence is a" - say " different fault from a number in it being wrong." + say " Read run.out first: it is what the case itself saw." say "" say " Machine state either side of the run:" say " $LOGDIR/system-before.txt" @@ -442,13 +440,4 @@ for e in "$UPDATE_ENV" "$UPDATE_ANALYSIS_ENV"; do done say "" -say "Done. Next:" -say " git diff install/environment.yml install/environment-analysis.yml" -say " dev/scripts/bump-version.sh $NEW" -say " ./PoolSeqFlow install # builds PoolSeqFlow-$NEW" -say " ./PoolSeqFlow analysis install # builds PoolSeqFlow-$NEW-analysis" -say " ./PoolSeqFlow check install" -say "" -say "Both release environments are built by those installs, from the two exported files, so" -say "what ships and what was tested are the same set - and the files, not long-lived" -say "environments, are what carry them forward." +say "Done. The release steps are in dev/RELEASING.md." diff --git a/lib/poolseqflow-completion.bash b/lib/poolseqflow-completion.bash new file mode 100644 index 0000000..2a17b78 --- /dev/null +++ b/lib/poolseqflow-completion.bash @@ -0,0 +1,112 @@ +#!/usr/bin/env bash +# +# Tab completion for PoolSeqFlow, in bash and in zsh. +# +# SOURCED, never run. `PoolSeqFlow install` puts it in the user's bash-completion directory, +# where bash finds it by the command's name. zsh does not read that directory, so it needs two +# lines in ~/.zshrc, and install prints them: +# +# autoload -U +X bashcompinit && bashcompinit +# . ~/.local/share/bash-completion/completions/PoolSeqFlow +# +# NOTHING HERE RUNS THE WRAPPER. It evaluates `conda shell.bash hook` before it dispatches, which +# costs about 0.4s, and a keystroke cannot. Every list below is a literal or comes from the +# filesystem. + +# The installation directory the command word resolves to. $bindir/PoolSeqFlow is a symlink into +# $prefix/opt/PoolSeqFlow-/, so the resolved file's directory is the installation. +# +# Symlinks are followed a hop at a time with plain `readlink`, not `readlink -f`: the `-f` is a +# GNU extension that BSD and macOS do not carry. A relative target resolves against the +# directory of the link that named it. +_poolseqflow_install_dir() { + local exe dir hops=0 + exe=$(command -v "$1" 2>/dev/null) || return 1 + while [ -L "$exe" ]; do + hops=$((hops + 1)) + [ "$hops" -gt 20 ] && return 1 + dir=$(cd "$(dirname "$exe")" 2>/dev/null && pwd -P) || return 1 + exe=$(readlink "$exe") || return 1 + case $exe in + /*) ;; + *) exe="$dir/$exe" ;; + esac + done + dir=$(cd "$(dirname "$exe")" 2>/dev/null && pwd -P) || return 1 + printf '%s' "$dir" +} + +# The modules installed into that installation, one per line. `lib` holds the shared libraries +# and is never named by a user, so it is not offered. +_poolseqflow_modules() { + local dir entry + dir=$(_poolseqflow_install_dir "$1") || return 0 + [ -d "$dir/analysis/modules" ] || return 0 + for entry in "$dir"/analysis/modules/*/; do + [ -d "$entry" ] || continue + entry=${entry%/} + entry=${entry##*/} + [ "$entry" = "lib" ] && continue + printf '%s\n' "$entry" + done +} + +# Fill COMPREPLY with the words of $1 that start with $2, one array element each. +# +# A read loop rather than `mapfile`, which is a bash builtin zsh does not have. It also keeps +# each match one element without relying on word splitting, which differs between the two shells +# outside the `emulate -L sh` that zsh's bashcompinit wraps this call in. +_poolseqflow_reply() { + local line + COMPREPLY=() + while IFS= read -r line; do + [ -n "$line" ] || continue + COMPREPLY+=("$line") + done < <(compgen -W "$1" -- "$2") +} + +_poolseqflow() { + local cur cmd top modules + cur=${COMP_WORDS[COMP_CWORD]} + cmd=${COMP_WORDS[0]} + # Every verb the wrapper dispatches, which 00_static checks against its `case`. + top="install init init_multi check run dryrun dryclean migrate_config clean reset" + top="$top analysis version cite list uninstall uninstall_all" + + if [ "$COMP_CWORD" -eq 1 ]; then + _poolseqflow_reply "$top" "$cur" + return + fi + + case ${COMP_WORDS[1]} in + check) + [ "$COMP_CWORD" -eq 2 ] && _poolseqflow_reply "install project" "$cur" + ;; + analysis) + modules=$(_poolseqflow_modules "$cmd" | tr '\n' ' ') + case $COMP_CWORD in + 2) + _poolseqflow_reply \ + "install check modules complete version cite uninstall $modules" "$cur" + ;; + 3) + if [ "${COMP_WORDS[2]}" = modules ]; then + _poolseqflow_reply "list available install uninstall" "$cur" + else + _poolseqflow_reply "nocpp" "$cur" + fi + ;; + 4) + # `modules install ` offers nothing: the names come from the + # catalogue, which is read over the network. + [ "${COMP_WORDS[2]}" = modules ] && [ "${COMP_WORDS[3]}" = uninstall ] && + _poolseqflow_reply "$modules" "$cur" + ;; + esac + ;; + esac +} + +# Both names the installer creates: the plain symlink and every versioned one on PATH. +# shellcheck disable=SC2046 +complete -F _poolseqflow PoolSeqFlow $(compgen -c PoolSeqFlow- 2>/dev/null) diff --git a/lib/wrapper_lib.sh b/lib/wrapper_lib.sh index c3033a1..0ff2e45 100644 --- a/lib/wrapper_lib.sh +++ b/lib/wrapper_lib.sh @@ -9,12 +9,6 @@ # POOLSEQFLOW_PREFIX from the caller, ENV_FILE for analysis_r_packages, and # POOLSEQFLOW_MODULE_INDEX where the user overrides the catalogue location. -# Where installations live: POOLSEQFLOW_PREFIX, else an installed wrapper's own location, -# else ~/.local. -# Not dead code. conda's `__conda_hashr` calls `\rehash` - zsh's name for `hash -r` - whenever -# ZSH_VERSION is set, including inside a bash script, where the command does not exist. -rehash() { hash -r; } - # Whether a directory is on PATH, compared as resolved directories rather than as strings: a # trailing slash, a doubled slash or a symlinked home each defeat `case ":$PATH:" in *":$dir:"*`. # An entry that does not resolve is skipped rather than failing the loop. @@ -34,6 +28,8 @@ dir_on_path() { return 1 } +# Where installations live: POOLSEQFLOW_PREFIX, else an installed wrapper's own location, +# else ~/.local. install_prefix() { if [ -n "${POOLSEQFLOW_PREFIX:-}" ]; then printf '%s' "$POOLSEQFLOW_PREFIX" diff --git a/manual/PoolSeqFlow-manual.md b/manual/PoolSeqFlow-manual.md index 542fb48..14ef654 100644 --- a/manual/PoolSeqFlow-manual.md +++ b/manual/PoolSeqFlow-manual.md @@ -15,7 +15,7 @@ A Nextflow pipeline for allele frequency analysis from pooled Illumina sequencin !!! info "Platform support" - PoolSeqFlow is developed and tested on **Linux and macOS**. Windows is not supported — the resume logic relies on symbolic links and Unix-style paths that do not behave correctly on native Windows filesystems. + PoolSeqFlow is developed and tested on **Linux**. macOS and Windows are not supported: the shipped conda environments are pinned to `linux-64` builds, and the resume logic relies on symbolic links and Unix-style paths that do not behave correctly on native Windows filesystems. --- @@ -137,8 +137,8 @@ If PoolSeqFlow contributes to published work, please cite it — the DOI and a f | | | |---|---| -| **Operating system** | Linux or macOS. Windows is not supported — see [below](#why-not-windows) | -| **glibc** | 2.28 or newer, on Linux — RHEL and Rocky 8, Debian 10, Ubuntu 18.10, or anything later. `ldd --version` prints what you have. CentOS 7 is the one common machine below it, and is end of life. [Why →](#glibc-floor) | +| **Operating system** | Linux. macOS and Windows are not supported — see [below](#why-not-windows) | +| **glibc** | 2.28 or newer — RHEL and Rocky 8, Debian 10, Ubuntu 18.10, or anything later. `ldd --version` prints what you have. CentOS 7 is the one common machine below it, and is end of life. [Why →](#glibc-floor) | | **Conda** | [Conda or Miniconda](https://docs.conda.io/en/miniconda.html) | | **Git** | Optional, for cloning | | **Working directory** | `mainDir` — where you launch the pipeline. Holds your reads, the reference, this project's configuration and everything actively processed. It has to persist between runs; it is not scratch space | @@ -163,9 +163,11 @@ Everything else is installed for you. `./PoolSeqFlow install` builds an isolated Pinning is deliberate. Pool-seq results depend on the exact behavior of the pileup and filtering tools, and an unpinned environment would make two runs of the same config non-comparable. -#### Why not Windows +#### Why Linux only { #why-not-windows } -The pipeline moves each file it produces to where it belongs and leaves a symbolic link behind, and it relies on Unix path semantics throughout. Neither behaves correctly on native Windows filesystems, and WSL only works under some filesystem configurations — which is not a guarantee worth documenting. See [Symbolic links instead of copies](#symbolic-links-instead-of-copies). +**Windows.** The pipeline moves each file it produces to where it belongs and leaves a symbolic link behind, and it relies on Unix path semantics throughout. Neither behaves correctly on native Windows filesystems, and WSL only works under some filesystem configurations — which is not a guarantee worth documenting. See [Symbolic links instead of copies](#symbolic-links-instead-of-copies). + +**macOS.** Every tool a release runs is pinned to an exact build in `install/environment.yml` and `install/environment-analysis.yml`, and those files are exports from a Linux machine: they name builds that exist for `linux-64` and for no other platform. Conda cannot solve either of them on a Mac, so the install fails before anything else is reached. Supporting macOS means a second set of pinned files exported and verified on a Mac — one for Apple Silicon and one for Intel, since conda treats them as different platforms — and that is a piece of work rather than a flag. Earlier releases listed macOS as supported; that was never true of the pinned environments, and saying so was the error. #### The glibc floor { #glibc-floor } @@ -195,7 +197,7 @@ Already have it installed and upgrading from an earlier version? Read [Upgrading ## Install -Check the [requirements](#getting-started) first if you have not — in particular that you are on Linux or macOS, and that conda is available. +Check the [requirements](#getting-started) first if you have not — in particular that you are on Linux, and that conda is available. ### 1. Get PoolSeqFlow @@ -247,6 +249,23 @@ Installing takes a while the first time; later installs reuse the conda package Once this is done the folder you downloaded has served its purpose. Everything from here uses the installed command, from your own project directory. +#### Tab completion { #tab-completion } + +Installing also writes a completion for `PoolSeqFlow` into `~/.local/share/bash-completion/completions/`, or wherever `XDG_DATA_HOME` points. **In bash it works in your next shell and there is nothing to do.** + +**zsh does not read that directory**, so it needs two lines in `~/.zshrc` — the installer prints them with the right path filled in: + +```bash +autoload -U +X bashcompinit && bashcompinit +. ~/.local/share/bash-completion/completions/PoolSeqFlow +``` + +It completes every command, the two targets `check` takes, the analysis commands, and **the modules you actually have installed** — so `PoolSeqFlow analysis ` lists the modules in that installation's store rather than a fixed list. `analysis modules install ` deliberately offers nothing: those names come from the catalogue over the network, and a keystroke should not make a network request. + +Uninstalling the last installed version removes the completion. Another version left installed keeps it, since the command it completes is still there. + +It never runs `PoolSeqFlow` itself. Starting the wrapper means starting conda, which takes long enough to be felt on a keypress, so the completion reads what it needs from the filesystem instead. + --- @@ -307,6 +326,32 @@ The paths are yours to choose; the structure inside them is not. A project ready The file names are examples. `readPattern` is what finds the reads — `*_R{1,2}.fq.gz` by default, which is what matches the pairs above — and `referenceFile` and `gffFile` name the two in `Reference/`. +**The reads may sit in subfolders of `Data/` instead, or as well.** However your sequencing facility handed them over is how you can leave them — one folder per sample, one per run or lane, nested as deep as you like, or all together as above. All three of these are read the same way: + +``` +Data/ Data/ Data/ +├── Sample1_R1.fq.gz ├── Sample1/ ├── run_A/ +├── Sample1_R2.fq.gz │ ├── Sample1_R1.fq.gz │ ├── Sample1_R1.fq.gz +├── Sample2_R1.fq.gz │ └── Sample1_R2.fq.gz │ └── Sample1_R2.fq.gz +└── Sample2_R2.fq.gz └── Sample2/ └── run_B/ + ├── Sample2_R1.fq.gz ├── Sample2_R1.fq.gz + └── Sample2_R2.fq.gz └── Sample2_R2.fq.gz +``` + +**A sample is named by its file, never by its folder.** `Sample1_R1.fq.gz` is sample `Sample1` wherever it lies, so `metadata.csv` does not change and neither does `readPattern` — the folders are yours to arrange and the pipeline does not read meaning into them. A pair whose mates are in two *different* folders is still that pair, and is taken as one. + +What step 0 checks is the pair itself: **every sample has exactly one of each mate.** Three ways to break it, all of which the reader would otherwise accept in silence: + +| in `Data/` | what it is | +|---|---| +| `Sample1_R1.fq.gz` with no `Sample1_R2.fq.gz` anywhere | a sample that would be left out of the run without a word | +| `run_A/Sample1_R1.fq.gz` and `run_B/Sample1_R1.fq.gz` | the same mate twice, which would be aligned against itself | +| a full pair under both `run_A/` and `run_B/` | one sample over again, each copy overwriting the other's results | + +If two folders really do hold two different samples, give them two names. If they are one sample sequenced twice, that is two rows in `metadata.csv` sharing an `RG_Sample` — see [What a unit is](#what-a-unit-is). + +**Hidden folders are skipped, and the run says so.** Anything beginning with a dot is not searched — `.snapshot` on NetApp storage holds a copy of every file per snapshot, and searching it would find every sample many times over. If one of them does hold reads, step 0 names it so you know what was left out. + `init` never overwrites. Running it again in a project you have already filled in reports what is there and changes nothing. It copies `parameters.config` for you because that is a settings file you edit in place. It does **not** write `metadata.csv`, because that is a table describing your experiment — which FASTQ pairs are one pool, how many individuals each holds, and the order your result columns come out in — and a copied one would describe someone else's. Write it yourself, starting from `metadata.csv.example`, and read [Metadata](#metadata) before your first run rather than after it. @@ -3941,7 +3986,7 @@ What the table is really for is the decision it supports. For `association`, rea `dev/scripts/bench-compiled-paths.R` is what produced the table, and re-running it on your own machine is how you find out what these numbers are where you work. -**The analysis environment already has a compiler.** Conda's `r-base` depends on one — GCC on Linux, clang on macOS — because R needs a toolchain to build packages from source, so an environment built by `PoolSeqFlow analysis install` can compile on every platform this ships to. +**The analysis environment already has a compiler.** Conda's `r-base` depends on GCC, because R needs a toolchain to build packages from source, so an environment built by `PoolSeqFlow analysis install` can always compile. Each module publishes its compiled source into the results folder whether or not the run used it, and the header of the module's own script beside it names the path that produced the numbers. @@ -4797,7 +4842,7 @@ Exit **3** means the FastQC per-base composition table did not have the expected #### Symbolic link errors -Confirm you are on Linux or macOS. Windows — including WSL under some filesystem configurations — is not supported. Also check that `storageDir` is still mounted and was not cleared while the run was in flight. +Confirm you are on Linux. macOS and Windows — including WSL under some filesystem configurations — are not supported. Also check that `storageDir` is still mounted and was not cleared while the run was in flight. #### A step fails and I cannot tell why diff --git a/modules/association/test/association.sh b/modules/association/test/association.sh index 2a28629..430c8d7 100644 --- a/modules/association/test/association.sh +++ b/modules/association/test/association.sh @@ -1,13 +1,14 @@ #!/bin/bash # association, against the analytic corpus its own tools build. # cost: jvm +# env: analysis # covers: modules/association/ modules/lib/ # covers: test/tools/freq_corpus.py # covers: analysis.nf modules/association/main.nf # # The fixtures and helpers every analysis suite shares are in test/lib/analysis.sh. # -# THE PIPELINE IS ASSUMED TO WORK. That is 03_pipeline's business, and re-proving it here would +# THE PIPELINE IS ASSUMED TO WORK. That is 04_pipeline's business, and re-proving it here would # cost minutes a case. # # Every expectation is `test/tools/freq_corpus.py`'s, computed by plain Python loops that share @@ -215,8 +216,11 @@ test_both_paths_through_the_parse_agree() { association_corpus "$sb" association_direct "$sb/plain" "$ASSOCIATION_OPTIONS" association_direct "$sb/compiled" "${ASSOCIATION_OPTIONS/\"usecpp\":false/\"usecpp\":true}" + # A FAILURE, NOT A SKIP. The analysis environment pins gcc_linux-64 and gxx_linux-64, so a + # compiled path that does not build is a broken release rather than a machine without a + # compiler. As a skip this read as "no compiler" and the run still exited 0. if [ ! -s "$sb/compiled/association.tsv" ]; then - skip_case "the compiled path did not build: $(tail -3 "$sb/compiled/out.txt")" + fail_case "the compiled path did not build: $(tail -3 "$sb/compiled/out.txt")" return fi diff -q "$sb/plain/association.tsv" "$sb/compiled/association.tsv" >/dev/null \ diff --git a/modules/basicstats/test/basicstats.sh b/modules/basicstats/test/basicstats.sh index bb89eaa..3f25f68 100644 --- a/modules/basicstats/test/basicstats.sh +++ b/modules/basicstats/test/basicstats.sh @@ -1,13 +1,14 @@ #!/bin/bash # basicstats, against the analytic corpus its own tools build. # cost: jvm +# env: analysis # covers: modules/basicstats/ modules/lib/ # covers: test/tools/freq_corpus.py # covers: analysis.nf modules/basicstats/main.nf # # The fixtures and helpers every analysis suite shares are in test/lib/analysis.sh. # -# THE PIPELINE IS ASSUMED TO WORK. That is 03_pipeline's business, and re-proving it here would +# THE PIPELINE IS ASSUMED TO WORK. That is 04_pipeline's business, and re-proving it here would # cost minutes a case. # Run the module's R directly over the corpus, under one set of options, into $1. @@ -42,6 +43,31 @@ basicstats_direct() { } +# THE MODULE RUN ONCE FOR THE WHOLE SUITE. Four cases below assert against the same published +# folder, and each was paying for its own baseline copy and its own two Nextflow launches - +# eight launches to check a file list, three DOIs, three greps and one set of numbers. +# +# The folder is copied out of the sandbox rather than read in place, because analysis_ready +# wipes $ANALYSIS_SB and cases run in alphabetical order, so another case may rebuild it +# between two of these. +# +# Sets BASICSTATS_PUBLISHED to the copy and BASICSTATS_STATUS to what the run returned. Returns +# non-zero when the case should stop, having already skipped or failed. +BASICSTATS_PUBLISHED="" +BASICSTATS_STATUS="" +basicstats_published() { + [ -n "$BASICSTATS_PUBLISHED" ] && return 0 + analysis_ready single || return 1 + if ! have_analysis_r; then skip_case "no analysis environment"; return 1; fi + analysis_plant_results "$ANALYSIS_SB/store/Output" + BASICSTATS_STATUS=$(analysis_run_module basicstats) + local keep; keep=$(guard_path "$TEST_TMPDIR/basicstats-published") + rm -rf "$keep" + cp -a "$ANALYSIS_SB/main/Analysis/Results/basicstats" "$keep" 2>/dev/null || true + BASICSTATS_PUBLISHED="$keep" + return 0 +} + # --------------------------------------------------------------------------------------- # basicstats, which a release ships. Unlike every module above it this one is not planted by # the case - it is in the store because the release carries it, which is also why an error in @@ -49,13 +75,10 @@ basicstats_direct() { # The fixture is six pools of one library each, exp_population over three levels and exp_time # over two, at the template's poolSize 100 and ploidy 2. test_basicstats_publishes_a_row_for_every_pool() { - analysis_ready single || return - if ! have_analysis_r; then skip_case "no analysis environment"; return; fi - analysis_plant_results "$ANALYSIS_SB/store/Output" - local status; status=$(analysis_run_module basicstats) - assert_status 0 "$status" "basicstats should run; see $ANALYSIS_SB/run.out" + basicstats_published || return + assert_status 0 "$BASICSTATS_STATUS" "basicstats should run; see $ANALYSIS_SB/run.out" - local folder="$ANALYSIS_SB/main/Analysis/Results/basicstats" + local folder="$BASICSTATS_PUBLISHED" assert_file "$folder/design.tsv" "the design table is published" assert_file "$folder/basicstats.R" "and the script that produced it" assert_file "$folder/CITATIONS.md" "and what to cite for it" @@ -72,12 +95,9 @@ test_basicstats_publishes_a_row_for_every_pool() { # THE METHODS ARE CITED, NOT ONLY THE SOFTWARE. A diversity estimate a reader cannot trace to a # definition is one they cannot check, and the two n_eff forms in circulation differ. test_basicstats_cites_the_statistics_it_computes() { - analysis_ready single || return - if ! have_analysis_r; then skip_case "no analysis environment"; return; fi - analysis_plant_results "$ANALYSIS_SB/store/Output" - analysis_run_module basicstats > /dev/null + basicstats_published || return - local folder="$ANALYSIS_SB/main/Analysis/Results/basicstats" + local folder="$BASICSTATS_PUBLISHED" local cites; cites=$(cat "$folder/CITATIONS.md" 2>/dev/null) assert_contains "$cites" "10.1073/pnas.70.12.3321" "Nei, for the diversity statistic" assert_contains "$cites" "10.1534/genetics.118.300900" "Hivert, for the effective sample size" @@ -91,12 +111,9 @@ test_basicstats_cites_the_statistics_it_computes() { # not have. What is published is the shared library folded into the module's own script, so the # functions that computed the numbers are in the file. test_basicstats_publishes_the_library_it_computed_with() { - analysis_ready single || return - if ! have_analysis_r; then skip_case "no analysis environment"; return; fi - analysis_plant_results "$ANALYSIS_SB/store/Output" - analysis_run_module basicstats > /dev/null + basicstats_published || return - local script; script=$(cat "$ANALYSIS_SB/main/Analysis/Results/basicstats/basicstats.R" 2>/dev/null) + local script; script=$(cat "$BASICSTATS_PUBLISHED/basicstats.R" 2>/dev/null) assert_contains "$script" "n_eff <- function" "n_eff travels with the result" assert_contains "$script" "site_diversity <- function" "and so does gene diversity" assert_contains "$script" "analysis frame 2026" "under the frame version that defined them" @@ -106,13 +123,10 @@ test_basicstats_publishes_the_library_it_computed_with() { # and the three tables it computes from them are published. The arithmetic in them is checked # by the case below this one, which calls the same R directly and costs no JVM. test_basicstats_publishes_what_it_measured() { - analysis_ready single || return - if ! have_analysis_r; then skip_case "no analysis environment"; return; fi - analysis_plant_results "$ANALYSIS_SB/store/Output" - local status; status=$(analysis_run_module basicstats) - assert_status 0 "$status" "basicstats should run; see $ANALYSIS_SB/run.out" + basicstats_published || return + assert_status 0 "$BASICSTATS_STATUS" "basicstats should run; see $ANALYSIS_SB/run.out" - local folder="$ANALYSIS_SB/main/Analysis/Results/basicstats" + local folder="$BASICSTATS_PUBLISHED" assert_file "$folder/sites.tsv" "the site counts are published" assert_file "$folder/depth.tsv" "and the depth summaries" assert_file "$folder/diversity.tsv" "and the diversity" @@ -281,7 +295,6 @@ test_a_merged_pool_reports_a_bound() { # setting is undiscoverable. test_a_depth_plot_is_drawn_only_for_named_sequences() { if ! have_analysis_r; then skip_case "no analysis environment"; return; fi - if ! have_analysis_r_package ggplot2; then skip_case "no ggplot2"; return; fi local sb; sb=$(guard_path "$TEST_TMPDIR/basicstats-plots") rm -rf "$sb"; mkdir -p "$sb" python3 "$REPO_ROOT/test/tools/freq_corpus.py" "$sb" "$sb" @@ -335,8 +348,6 @@ test_every_path_through_the_hot_loop_agrees() { "$name: nor a depth summary" done - # Rcpp needs a compiler, which is not shipped and is not on every machine. - if ! have_analysis_rcpp; then skip_case "no Rcpp and compiler in the analysis environment"; return; fi basicstats_direct "$sb/cpp" '{"minReads":2,"binSize":4,"workers":1,"usecpp":true}' "$sb" assert_eq "" "$(diff "$sb/ref/diversity.tsv" "$sb/cpp/diversity.tsv" 2>&1)" \ "the compiled path must agree with the R it replaces: $(cat "$sb/cpp/out.txt" 2>/dev/null)" @@ -352,7 +363,6 @@ test_every_path_through_the_hot_loop_agrees() { test_the_parallel_path_agrees_with_the_sequential_one() { local rscript; rscript=$(analysis_rscript) if [ -z "$rscript" ]; then skip_case "no analysis environment"; return; fi - if ! have_analysis_r_package doFuture; then skip_case "no doFuture"; return; fi local sb; sb=$(guard_path "$TEST_TMPDIR/basicstats-parallel") rm -rf "$sb"; mkdir -p "$sb" python3 "$REPO_ROOT/test/tools/freq_corpus.py" "$sb" "$sb" @@ -377,8 +387,6 @@ test_the_parallel_path_agrees_with_the_sequential_one() { test_a_worker_compiles_the_hot_path_for_itself() { local rscript; rscript=$(analysis_rscript) if [ -z "$rscript" ]; then skip_case "no analysis environment"; return; fi - if ! have_analysis_r_package doFuture; then skip_case "no doFuture"; return; fi - if ! have_analysis_r_package Rcpp; then skip_case "no Rcpp in the analysis environment"; return; fi local sb; sb=$(guard_path "$TEST_TMPDIR/basicstats-parallel-cpp") rm -rf "$sb"; mkdir -p "$sb" python3 "$REPO_ROOT/test/tools/freq_corpus.py" "$sb" "$sb" diff --git a/modules/mds/test/mds.sh b/modules/mds/test/mds.sh index 894c9f1..19cced9 100644 --- a/modules/mds/test/mds.sh +++ b/modules/mds/test/mds.sh @@ -1,13 +1,14 @@ #!/bin/bash # mds, against the analytic corpus its own tools build. # cost: jvm +# env: analysis # covers: modules/mds/ modules/lib/ # covers: test/tools/freq_corpus.py # covers: analysis.nf modules/mds/main.nf # # The fixtures and helpers every analysis suite shares are in test/lib/analysis.sh. # -# THE PIPELINE IS ASSUMED TO WORK. That is 03_pipeline's business, and re-proving it here would +# THE PIPELINE IS ASSUMED TO WORK. That is 04_pipeline's business, and re-proving it here would # cost minutes a case. # # Every expectation is `test/tools/freq_corpus.py`'s, computed by plain Python loops that share @@ -74,9 +75,18 @@ table_gap() { # $1 and $2 agree to within rounding; $3 names what was being compared. assert_tables_agree() { local gap; gap=$(table_gap "$1" "$2") - if ! awk -v g="$gap" 'BEGIN { exit !(g + 0 < 1e-9 && g != "") }' 2>/dev/null; then - fail_case "$3: the tables differ by $gap" + # THE ANSWER IS NUMERIC OR IT IS A REFUSAL. table_gap replies with a number, or with one of + # its own sentinels - "shape" when the dimensions or names differ, "text" when a character + # column does - or with R's error text on stderr when a file is missing or unreadable. awk + # coerces every non-numeric string to 0, so `g + 0 < 1e-9` passed all three: measured + # 2026-09-23, gap="shape", gap="text" and gap="Error in read.delim" each passed. Both + # sentinels were dead from the day they were written, and so was the missing-table case. + if ! printf '%s' "$gap" | grep -qE '^[0-9]+(\.[0-9]+)?([eE][-+]?[0-9]+)?$'; then + fail_case "$3: $gap" + return fi + awk -v g="$gap" 'BEGIN { exit !(g + 0 < 1e-9) }' \ + || fail_case "$3: the tables differ by $gap" } # A COHORT OF $2 POOLS INTO $1, the corpus's six plus copies of the sixth, with an `exp_cage` @@ -186,8 +196,9 @@ test_the_sampling_correction_is_applied_and_positive() { mds_corpus "$sb" mds_direct "$sb/run" "$MDS_OPTIONS" - local a b raw distance correction + local a b raw distance correction rows=0 while IFS=$'\t' read -r a b _ distance raw correction; do + rows=$((rows + 1)) # %.17g, because assert_close's tolerance is absolute and awk's default six significant # figures lands outside it on a number this small. assert_close "$correction" \ @@ -197,6 +208,10 @@ test_the_sampling_correction_is_applied_and_positive() { fail_case "$a-$b: the correction is $correction, so nothing was subtracted" fi done < <(awk -F'\t' 'NR > 1' "$sb/run/distance.tsv") + # A published table with no rows would otherwise run the loop zero times and pass, which is + # the state the module is in when it is most broken. Its sibling guards with `[ ! -s ]`; a + # row count also catches a table that carries its header and nothing else. + [ "$rows" -gt 0 ] || fail_case "nothing published"$'\n'"$(cat "$sb/run/out.txt" 2>/dev/null)" } # A BIN BOUNDARY MUST NOT MOVE A NUMBER. The distances are sums over sites, so a bin falls @@ -372,7 +387,16 @@ test_the_points_can_carry_a_color_and_a_shape() { # A ggplot2 warning about an unknown label or a dropped shape is written to stderr, which # a Nextflow task swallows; out.txt is where a case can still see it. assert_not_contains "$(cat "$sb/run/out.txt")" "Warning" "and it must draw without warning" - assert_tables_agree "$sb/run/distance.tsv" "$sb/run/distance.tsv" "self-comparison sanity" + + # A COLOR AND A SHAPE ARE PRESENTATION, so the numbers must not move. That is what the + # keyed run is compared against here. The line this replaces compared distance.tsv with + # ITSELF and was labeled "self-comparison sanity", so it answered 0e+00 whatever the keys + # had done - the only assertion in the case about the keys affecting anything. + mds_direct "$sb/plain" "$MDS_OPTIONS" + assert_tables_agree "$sb/run/distance.tsv" "$sb/plain/distance.tsv" \ + "a color and a shape key changed the distances" + assert_tables_agree "$sb/run/mds.tsv" "$sb/plain/mds.tsv" \ + "a color and a shape key changed the coordinates" } # SIX SHAPES AND NO MORE. ggplot2 assigns none to a seventh level and leaves those pools off the diff --git a/parameters.config.template b/parameters.config.template index 644151c..736c87b 100644 --- a/parameters.config.template +++ b/parameters.config.template @@ -15,7 +15,7 @@ params { // `./PoolSeqFlow migrate_config` carries the old value over. storageDir = "/path/to/permanent/storage" // The files you place, all relative to mainDir: - // mainDir/Data/ the reads (dataSource names it) + // mainDir/Data/ the reads, in it or in subfolders of it (dataSource names it) // mainDir/Reference/ reference + annotation (referenceFile, gffFile) // mainDir/metadata.csv what your samples ARE (metadataFile) dataSource = 'Data' @@ -239,7 +239,10 @@ params { // The decompressed reference, written by step 1. There is no `gff` beside it: snpEff // builds its database from the file you placed, gzipped or not. reference = "${params.dir.dictionaries}/${params.referenceFile.replace('.gz', '')}" - reads = "${params.dir.data}/${params.readPattern}" + // The `**` is what lets the reads sit in subfolders of Data/ as well as directly in it - + // one folder per sample, one per sequencing run, or none at all. It matches across + // directories INCLUDING none, which `**/` does not: `**/` finds only the nested ones. + reads = "${params.dir.data}/**${params.readPattern}" // How each tool is invoked. Replace a command with a full path to use a system binary diff --git a/scripts/0_verify_environment.nf b/scripts/0_verify_environment.nf index 85da495..25bb938 100644 --- a/scripts/0_verify_environment.nf +++ b/scripts/0_verify_environment.nf @@ -356,8 +356,11 @@ process CheckData { log_message "The data source is set to: ${check.dataSource}" - # Check for FASTQ files - FASTQ_COUNT=\$(find \$DATADIR ${read_pattern} | wc -l) + # Check for FASTQ files. Hidden directories below the root are pruned, the same way the + # sample match and the read channel prune them: reads may be nested, so a `.snapshot` + # would otherwise be counted once per snapshot and the pairing test below would read a + # count that describes the storage rather than the data. + FASTQ_COUNT=\$(find \$DATADIR -mindepth 1 -name '.*' -type d -prune -o ${read_pattern} -print | wc -l) if [ \$FASTQ_COUNT -eq 0 ]; then log_message "No FASTQ files found in data directory!" log_message "Expected pattern: ${check.readPattern}" @@ -523,13 +526,98 @@ SAMPLEIDS log_message "METADATA SAMPLE MATCH: FAIL" STATUS="FAIL" else - sample_ids=\$(find ${dataDir} ${readPattern} | while read -r fq; do + # ONE LINE PER READ FILE, as sampleIDdirectory. Deliberately NOT deduplicated: + # a sample's two mates give two lines, which is how the counts below see a mate + # that is missing or a name that appears twice. + # + # Hidden directories below the data root are pruned, to agree with the read + # channel: `.snapshot` on NetApp storage holds a copy of every read per snapshot, + # and since the reads may be nested the glob walks into those. -mindepth 1 so the + # root is never itself pruned - a project under ~/.local would find nothing. + find ${dataDir} -mindepth 1 -name '.*' -type d -prune -o ${readPattern} -print \\ + | while read -r fq; do base=\$(basename "\$fq") + fname="\$base" + fqdir=\$(dirname "\$fq") ${stripMate} - echo "\$base" - done | sort -u) + printf '%s\\t%s\\t%s\\n' "\$base" "\$fname" "\$fqdir" + done | sort > read_sources.txt + sample_ids=\$(cut -f1 read_sources.txt | sort -u) MATCHED="yes" + + # EXACTLY ONE OF EACH MATE PER SAMPLE, AND THE FOLDERS DO NOT MATTER. + # + # A pair is a pair wherever it is filed: mates in two different directories group + # correctly, because a sample is named by its file and never by its folder. What is + # checked is the pair itself - two files carrying two different names. + # + # Two things it catches, both of which the read channel accepts in silence: + # + # a mate with no partner fromFilePairs emits NOTHING for it, so the sample + # is simply absent from the run. The check this + # replaces asked only whether the file count was + # even, which two orphans satisfy between them. + # the same mate twice two copies of an R1 in two folders are handed on + # as a "pair", and the pipeline would align R1 + # against R1. Measured, not inferred. + # + # A full pair duplicated across folders lands here too, as four files for a sample + # that takes two: each copy would be written to the outputs named after the sample. + for sample in \$sample_ids; do + n_files=\$(awk -F'\\t' -v s="\$sample" '\$1 == s' read_sources.txt | wc -l) + n_names=\$(awk -F'\\t' -v s="\$sample" '\$1 == s { print \$2 }' read_sources.txt \\ + | sort -u | wc -l) + [ "\$n_files" -eq 2 ] && [ "\$n_names" -eq 2 ] && continue + + log_message "Sample '\$sample' does not have exactly one of each mate:" + awk -F'\\t' -v s="\$sample" '\$1 == s { print \$3 "/" \$2 }' read_sources.txt \\ + | sort | while read -r where; do log_message " \$where"; done + if [ "\$n_names" -lt "\$n_files" ]; then + log_message "The same file name appears more than once, so this is one sample" + log_message "over again - and each copy would be written to the outputs named" + log_message "after it. Keep one, or give them names of their own." + else + log_message "readPattern '${check.readPattern}' takes the two mates together," + log_message "and a mate with no partner is left out of the run without a word." + fi + MATCHED="no" + done + + # A SUBFOLDER OF Data/ HOLDING NO READS AT ALL. Empty, or full of something else - + # either way the reads that were meant to be in it are not, and a run that simply + # ignored it would process fewer samples than the user believes it has. + # A parent of a folder that does hold reads is not itself empty. + find ${dataDir} -mindepth 1 -name '.*' -type d -prune -o -type d -print \\ + | while read -r subdir; do + if [ -z "\$(find "\$subdir" ${readPattern} -print 2>/dev/null | head -1)" ]; then + echo "\$subdir" + fi + done > empty_dirs.txt + if [ -s empty_dirs.txt ]; then + log_message "These folders under ${dataDir} hold no reads matching '${check.readPattern}':" + while IFS= read -r d; do log_message " \$d"; done < empty_dirs.txt + log_message "Reads may sit in subfolders, so an empty one is either a layout that" + log_message "did not finish copying or a pattern that does not match its files." + MATCHED="no" + fi + + # HIDDEN FOLDERS ARE SKIPPED, AND SAYING SO IS THE POINT. Silently ignoring a folder + # that holds reads is how a user loses samples without being told. Only reported + # when one actually holds something the pattern matches. + find ${dataDir} -mindepth 1 -type d -name '.*' -print 2>/dev/null \\ + | while read -r hidden; do + if [ -n "\$(find "\$hidden" ${readPattern} -print 2>/dev/null | head -1)" ]; then + echo "\$hidden" + fi + done > hidden_dirs.txt + if [ -s hidden_dirs.txt ]; then + log_message "NOTE: these hidden folders hold reads and were skipped:" + while IFS= read -r d; do log_message " \$d"; done < hidden_dirs.txt + log_message "Nothing inside them is used. On NetApp storage '.snapshot' is the" + log_message "usual one, and it holds a copy of every read per snapshot." + fi + # Reads with no row: a hard failure. for sample in \$sample_ids; do if ! grep -qxF "\$sample" metadata_ids.txt; then diff --git a/scripts/2_trim_reads.nf b/scripts/2_trim_reads.nf index 6b5d625..b4afae8 100644 --- a/scripts/2_trim_reads.nf +++ b/scripts/2_trim_reads.nf @@ -6,9 +6,24 @@ include { sampleTrimOptions } from './metadata.nf' // The (variant, sample) read channel, and what derives every sample id. Takes the variant LIST, // not a channel: channel.fromFilePairs globs while the DAG is built and fixes N there. One glob // per step-2 variant, not per run. +// True when a read lies under a directory whose name begins with a dot, counted from the data +// root DOWN. Only below the root, because the project itself may sit under one - ~/.local is +// the default install prefix - and that says nothing about the reads. +// +// The case this exists for is `.snapshot`, which NetApp exposes read-only inside every +// directory on a large share of HPC storage and which holds a copy of every file per snapshot. +// Since the reads may now be nested, `**` walks into those too: measured, a single flat sample +// on such a mount is found once for itself and once per snapshot. +def hiddenBelow(String root, String path) { + def rel = path.startsWith(root) ? path.substring(root.length()) : path + return rel.tokenize('/').any { part -> part.startsWith('.') } +} + def readPairChannel(List variants) { def per = variants.collect { variant -> + def dataRoot = "${variant.dir.data}".toString() channel.fromFilePairs("${variant.reads}", checkIfExists: true) + .filter { _id, files -> !hiddenBelow(dataRoot, "${files[0]}".toString()) } .map { id, files -> tuple(variant, id, files[0], files[1]) } } return per.size() == 1 ? per[0] : per.inject { a, b -> a.mix(b) } diff --git a/scripts/resolve_parameters.nf b/scripts/resolve_parameters.nf index 8241237..f1a5a8b 100644 --- a/scripts/resolve_parameters.nf +++ b/scripts/resolve_parameters.nf @@ -100,8 +100,11 @@ def collectNames(Map m, String prefix, List out) { } // Every name that IS a parameter, so a multi-run column naming something else can be refused. -// The derived names are unioned in: the template leaves them commented out, so they do not exist -// in params until resolveParameters() runs. +// +// The derived names are unioned in because some of them are not in params yet: parameters.config +// leaves the knobs and the whole `cores` block commented out, so they come into existence only +// when resolveParameters() computes them. The seven paths below - referencePath through reads - +// are live there and so already counted; the union makes the set whole either way. def knownParameterNames() { return (collectNames(params, '', []) + derivedParameterNames()).unique().sort() } @@ -213,7 +216,14 @@ def deriveRunPaths(Map p) { p.multiRunPath = "${p.mainDir}/${p.multiRunFile}" p.reference = "${p.dir.dictionaries}/${p.referenceFile.replace('.gz', '')}" p.gff = "${p.dir.dictionaries}/${p.gffFile.replace('.gz', '')}" - p.reads = "${p.dir.data}/${p.readPattern}" + // `**` so the reads may sit in subfolders of Data/ as well as directly in it. It matches + // across directories INCLUDING none, which `**/` does not - `**/` finds only the nested + // ones and would stop every flat project. + // + // This is the value a run uses: it lands in each variant map and readPairChannel globs + // `variant.reads`. parameters.config declares the same path and that copy is not consulted, + // so editing `reads` there has no effect. + p.reads = "${p.dir.data}/**${p.readPattern}" if (!sensitivityIsPinned()) p.filterFalsePositives.sensitivity = derivedSensitivity(p) if (!snpEffDbIsPinned()) p.snpEff.db = derivedSnpEffDb(p) diff --git a/test/README.md b/test/README.md index dadb9db..15e3d78 100644 --- a/test/README.md +++ b/test/README.md @@ -40,17 +40,17 @@ the same arrangement `dev/` uses. | `suites/00_static.sh` | Syntax, release packaging, version consistency. No data needed | | `suites/01_migrate.sh` | `bin/config_migrate.sh`, against configs written for earlier releases | | `suites/02_launcher.sh` | `./PoolSeqFlow` environment handling, against a stub conda | -| `suites/03_pipeline.sh` | End-to-end runs against the fixture. The slow one | -| `suites/04_guards.sh` | The step 0 change guards, via step-0-only runs | -| `suites/05_helpers.sh` | Unit coverage for `bin/`, called directly. No conda, no fixture | +| `suites/04_pipeline.sh` | End-to-end runs against the fixture. The slow one | +| `suites/05_guards.sh` | The step 0 change guards, via step-0-only runs | +| `suites/03_helpers.sh` | Unit coverage for `bin/`, called directly. No conda, no fixture | | `suites/06_dryrun.sh` | The layout preview, and what `dryclean` will and will not delete | The analysis layer is nine suites rather than one. It runs **no** pipeline — artifacts are planted — but most of it still starts a Nextflow run per case, which is what makes running only the seam you touched worth doing. | Path | What it is | |---|---| -| `suites/07_analysis_frame.sh` | What the frame is, what it reads, and what keeps it optional | -| `suites/08_analysis_rlib.sh` | The shared R library, called directly. **No Nextflow at all** — seconds, not minutes | +| `suites/08_analysis_frame.sh` | What the frame is, what it reads, and what keeps it optional | +| `suites/07_analysis_rlib.sh` | The shared R library, called directly. **No Nextflow at all** — seconds, not minutes | | `suites/09_analysis_modules.sh` | The module store, and what a module gets from the frame: its library, settings and artifact classes | | `suites/10_analysis_verify.sh` | What the frame checks before a module reads anything | | `suites/11_analysis_plan.sh` | Which results an invocation covers, and where each lands | @@ -65,9 +65,9 @@ The analysis layer is nine suites rather than one. It runs **no** pipeline — a | class | what it promises | suites | |---|---|---| -| `static` | completes with **nothing installed**; a case wanting a tool skips rather than building | `00_static` `01_migrate` `02_launcher` `05_helpers` `08_analysis_rlib` | -| `jvm` | starts Nextflow per case, against planted artifacts | `04_guards` `06_dryrun`, the other analysis suites, and a module's own | -| `pipeline` | runs the pipeline itself against the fixture | `03_pipeline` | +| `static` | completes with **nothing installed**; a case wanting a tool skips rather than building | `00_static` `01_migrate` `02_launcher` `03_helpers` `07_analysis_rlib` | +| `jvm` | starts Nextflow per case, against planted artifacts | `05_guards` `06_dryrun`, the other analysis suites, and a module's own | +| `pipeline` | runs the pipeline itself against the fixture | `04_pipeline` | The line between `static` and `jvm` is whether a case **builds** something. Asking `have_tools` and skipping is static — that is how `00_static` holds its `nextflow lint` case. Calling for a baseline or a pipeline run is not, and `00_static` refuses a suite that does both. @@ -80,7 +80,7 @@ suite. Run the script directly to see the reasoning before committing to it: ``` $ dev/scripts/select-tests.py analysis/lib/nf/plan.nf -$ dev/scripts/select-tests.py bin/depth_cutoff.py # 03_pipeline 04_guards 05_helpers +$ dev/scripts/select-tests.py bin/depth_cutoff.py # 04_pipeline 05_guards 03_helpers ``` **Two halves, and only one of them is maintained by hand.** The graph is derived — `include {} @@ -114,12 +114,12 @@ startup**, flat, cached or not. Nothing in the pipeline dominates that at fixtur suite runtime is essentially a count of `nextflow run` invocations. Two consequences worth knowing before adding a case: -- Prefer a **unit test in `05_helpers.sh`** over an end-to-end one. `bin/classify_manifest.sh` +- Prefer a **unit test in `03_helpers.sh`** over an end-to-end one. `bin/classify_manifest.sh` exists as a separate script for exactly this reason — its edge cases (a value containing `=`, an empty value, no trailing newline, an unparseable line) are milliseconds there and a JVM start each through a pipeline run. When new guard logic is worth testing thoroughly, extract it to `bin/` first. -- Never give a case its own setup run. `04_guards.sh` builds one verified project and each +- Never give a case its own setup run. `05_guards.sh` builds one verified project and each case works on a copy — 22ms against 21s. Doing it per case was most of that suite's runtime and tested nothing. @@ -204,7 +204,7 @@ Two details worth knowing when writing assertions against the frequency tables: ## Known gaps - **`depth2freq.awk` and `MajorAlleleToRef.py` still have no unit coverage** and are exercised - end to end only. Most of this gap has closed since it was written — `05_helpers.sh` now covers + end to end only. Most of this gap has closed since it was written — `03_helpers.sh` now covers `classify_manifest.sh`, `find_artifact.sh`, `depth_cutoff.py`, `filterFalsePositives.sh`, both parsers and `atomic_mv.sh`, and `config_migrate.sh` has a suite of its own — but those two are the ones left. diff --git a/test/lib/analysis.sh b/test/lib/analysis.sh index a48f4ff..2cedad7 100644 --- a/test/lib/analysis.sh +++ b/test/lib/analysis.sh @@ -10,7 +10,7 @@ # The analysis layer: what it ships as, which results an invocation covers, and what it # refuses before computing anything. # -# THE PIPELINE IS ASSUMED TO WORK. That is 03_pipeline's business, and running it here would +# THE PIPELINE IS ASSUMED TO WORK. That is 04_pipeline's business, and running it here would # cost minutes to produce files this suite only ever counts. Published artifacts are planted # instead; what cannot be planted is the identity record beside the results, because # .poolseqflow_params holds the manifest exactly as the pipeline resolves it and a hand-written diff --git a/test/lib/sandbox.sh b/test/lib/sandbox.sh index 4f1da43..83c9393 100644 --- a/test/lib/sandbox.sh +++ b/test/lib/sandbox.sh @@ -449,10 +449,43 @@ _run_entry() { local sb="$1" entry="$2"; shift 2 local proj="${SANDBOX_PROJECT_DIR:-$sb/main}" local out="${SANDBOX_RUN_OUT:-$sb/run.out}" + # THE RUN INHERITS THE ACTIVATED ENVIRONMENT AND SETS NEITHER PATH NOR JAVA_HOME. The suite + # declares which environment its work happens in, run_tests.sh activates it once for the + # block, and `conda activate` is what the tool itself does before a run. + # + # WHAT THE HAND-ROLLED VERSION GOT WRONG. It exported JAVA_HOME as $TEST_CONDA_ENV, while + # openjdk's own activate.d script exports $CONDA_PREFIX/lib/jvm and JAVA_LD_LIBRARY_PATH + # beside it. $TEST_CONDA_ENV has no lib/server, so it is not a JAVA_HOME; the runs worked + # only because JAVA_CMD was given explicitly and Nextflow prefers it. It also pointed every + # analysis entry at the PIPELINE environment, which carries no Rscript, pandoc or typst - + # those runs worked only because the analysis environment was activated for the whole + # process and sat on the tail of PATH, which in turn meant a tool missing from the pipeline + # environment was quietly answered by the analysis one. + # + # A refusal rather than a fallback when the wrong environment is active: a run under the + # other one would be measuring software the release does not use for this entry. + local want="$TEST_CONDA_ENV" + case "$entry" in + analysis.nf|analysis/*) want="$TEST_ANALYSIS_ENV" ;; + esac ( cd "$proj" || exit 1 - export JAVA_HOME="$TEST_CONDA_ENV" JAVA_CMD="$TEST_CONDA_ENV/bin/java" - export PATH="$TEST_CONDA_ENV/bin:$PATH" + # THE ENTRY DECIDES, NOT THE SUITE. The suite's own block activation covers the common + # case and this costs nothing then; an entry needing the OTHER environment switches + # here. An analysis suite building its baseline is exactly that - analysis_ready calls + # run_verify_only, which is a pipeline entry - so the two cannot be split by suite. + # + # Inside the subshell, so the block's activation is untouched by it. + if [ -n "$want" ] && [ "${CONDA_PREFIX:-}" != "$want" ]; then + conda activate "$want" 2>/dev/null || true + fi + # A refusal, never a fallback: running under the other environment would measure + # software the release does not use for this entry, and would pass while doing it. + if [ -n "$want" ] && [ "${CONDA_PREFIX:-}" != "$want" ]; then + printf '%s needs %s activated, but CONDA_PREFIX is %s\n' \ + "$entry" "$want" "${CONDA_PREFIX:-}" > "$out" + exit 91 + fi export NXF_HOME="$sb/nxfhome" NXF_VER="${TEST_NXF_VER:-26.04.6}" # What the wrapper exports: a module is launched as its own entry script, so nothing # Nextflow computes points at the installation. SANDBOX_INSTALL_OVERRIDE is for the case @@ -504,8 +537,8 @@ sandbox_config_flat() { local sb="$1" ( cd "$sb/main" || exit 1 - export JAVA_HOME="$TEST_CONDA_ENV" JAVA_CMD="$TEST_CONDA_ENV/bin/java" - export PATH="$TEST_CONDA_ENV/bin:$PATH" + # Inherits the activated environment, like _run_entry. Its one caller, 05_guards, + # declares `# env: pipeline`. export NXF_HOME="$sb/nxfhome" NXF_VER="${TEST_NXF_VER:-26.04.6}" export POOLSEQFLOW_HOME="$sb/install" nextflow config -flat "$sb/install" 2>/dev/null @@ -823,6 +856,9 @@ run_launcher_with_envs() { # The one payload file that is not placeholder-able: the wrapper SOURCES it, so an empty # lib/ makes every launcher case fail before it reaches what it is testing. cp "$REPO_ROOT/lib/wrapper_lib.sh" "$sb/lib/" + # Real too, because `install` copies it out to the user's completion directory and a case + # asserts what landed there. An empty placeholder would install an empty completion. + cp "$REPO_ROOT/lib/poolseqflow-completion.bash" "$sb/lib/" # A project to stand in, for the arms that read one. Its content is whatever the case set: # `storageDir` is the key require_migrated_config turns on, so a case chooses between a @@ -842,8 +878,13 @@ run_launcher_with_envs() { # Installs go inside the sandbox, never into the operator's real ~/.local. Without this # a launcher test would deploy a stub payload over a working installation. LAUNCHER_PREFIX="$sb/prefix" + # XDG_DATA_HOME goes in the sandbox too. `install` writes the tab completion under it and + # `uninstall` removes it, so without this every launcher case that installs would reach + # into the operator's own ~/.local/share and the uninstall cases would delete from it. + LAUNCHER_XDG="$sb/xdg" LAUNCHER_OUTPUT=$(cd "$sb" && PATH="$sb/stub/bin:$PATH" \ - POOLSEQFLOW_PREFIX="$LAUNCHER_PREFIX" ./PoolSeqFlow "$@" 2>&1) + POOLSEQFLOW_PREFIX="$LAUNCHER_PREFIX" XDG_DATA_HOME="$LAUNCHER_XDG" \ + ./PoolSeqFlow "$@" 2>&1) LAUNCHER_STATUS=$? } diff --git a/test/run_tests.sh b/test/run_tests.sh index 6b97376..355ce56 100755 --- a/test/run_tests.sh +++ b/test/run_tests.sh @@ -150,19 +150,6 @@ have_analysis_r() { } export -f have_analysis_r -# Whether the environment a module runs in can build a compiled path: Rcpp AND the toolchain it -# drives. Rcpp alone is not enough. Its compiler is conda's own - -# x86_64-conda-linux-gnu-c++ - which lives in that environment's bin and nowhere else, so the -# search has to happen with that bin on PATH or Rcpp reports "tools not found". -have_analysis_rcpp() { - have_analysis_r_package Rcpp || return 1 - local cxx - cxx=$("$TEST_ANALYSIS_ENV/bin/R" CMD config CXX 2>/dev/null | awk '{print $1}') - [ -n "$cxx" ] || return 1 - PATH="$TEST_ANALYSIS_ENV/bin:$PATH" command -v "$cxx" > /dev/null 2>&1 -} -export -f have_analysis_rcpp - # The analysis environment, which is where a module actually runs. Found rather than assumed: # the wrapper creates it with `conda env create -n`, naming it and leaving the directory to # conda. TEST_ANALYSIS_ENV points it at another one. @@ -177,22 +164,15 @@ fi TEST_ANALYSIS_ENV="${TEST_ANALYSIS_ENV:-}" export TEST_ANALYSIS_ENV -# ONE ACTIVATION FOR THE WHOLE RUN, because that is how the tool runs a module. -# -# `PoolSeqFlow analysis ` does `conda activate ` before it starts, and a -# case that only puts the environment's bin on PATH is testing something else. The difference is -# not theoretical: conda's R compiles with conda's own x86_64-conda-linux-gnu-c++, which lives in -# that environment and is on no other PATH, so every compiled-path case failed with -# -# sh: x86_64-conda-linux-gnu-c++: command not found -# WARNING: The tools required to build C++ code for R were not found. +# THE conda SHELL FUNCTION, DEFINED BUT NOT USED YET. Sourcing the hook is what makes +# `conda activate` exist in a script at all; the activation itself happens further down, around +# the block of suites that declare `# env: analysis`, so nothing else in the run has an +# environment on its PATH. # -# until the suite was run against the environment's R rather than the machine's. -# -# The ANALYSIS environment is the one activated, not the pipeline's: only one can be, and -# _run_entry puts the pipeline environment's bin at the front of PATH for every Nextflow run, so -# that side is served explicitly while this side needs the activation scripts. Silent when conda -# is not reachable - the cases that need it check have_analysis_r and skip. +# It used to activate here, for the whole process. That put the analysis environment behind +# every one of the 500-odd cases that do not want it, and made the pipeline suites unable to +# notice a missing tool: both environments carry nextflow, samtools, bcftools and rsync, so +# anything dropped from the pipeline environment was quietly answered by the analysis one. # # THE HOOK IS FOUND FROM THE ENVIRONMENT'S OWN PATH, NOT FROM `conda info --base`. That command # prints a plugin's load error onto stdout alongside the answer - anaconda-anon-usage does it on @@ -201,11 +181,8 @@ export TEST_ANALYSIS_ENV # lives at /envs/, so the base is two directories up and needs nothing to say so. if [ -n "$TEST_ANALYSIS_ENV" ]; then _conda_hook="$(dirname "$(dirname "$TEST_ANALYSIS_ENV")")/etc/profile.d/conda.sh" - if [ -f "$_conda_hook" ]; then - # shellcheck disable=SC1091 - . "$_conda_hook" - conda activate "$TEST_ANALYSIS_ENV" 2>/dev/null || true - fi + # shellcheck disable=SC1091 + [ -f "$_conda_hook" ] && . "$_conda_hook" unset _conda_hook fi @@ -238,14 +215,6 @@ pdf_text() { } export -f pdf_text -# True when a named package is installed in the analysis environment. -have_analysis_r_package() { - [ -n "$TEST_ANALYSIS_ENV" ] || return 1 - "$TEST_ANALYSIS_ENV/bin/Rscript" --vanilla \ - -e "quit(status = !requireNamespace('$1', quietly = TRUE))" > /dev/null 2>&1 -} -export -f have_analysis_r_package - # What a suite may cost, declared in its own header as `# cost: `: # # static completes with nothing installed. A case wanting a tool skips rather than @@ -266,6 +235,33 @@ suite_cost() { printf '%s' "${declared:-pipeline}" } +# Which conda environment a suite's work happens inside, declared in its own header as +# `# env: `: `pipeline`, `analysis`, or absent for neither. +# +# THE TOOL RUNS IN AN ACTIVATED ENVIRONMENT, SO THE SUITE DOES TOO. `PoolSeqFlow run` activates +# the pipeline environment and `PoolSeqFlow analysis ` activates the analysis one, and +# a case that only puts a bin on PATH is testing something the release does not do. What that +# cost, measured 2026-09-23: activation runs `etc/conda/activate.d/openjdk_activate.sh`, which +# exports JAVA_HOME as $CONDA_PREFIX/lib/jvm and JAVA_LD_LIBRARY_PATH beside it, while +# _run_entry had been setting JAVA_HOME to $CONDA_PREFIX - a directory with no lib/server in it, +# so not a JAVA_HOME at all - and JAVA_LD_LIBRARY_PATH not at all. Every pipeline case had been +# launching its JVM under an environment no user has. +# +# For the analysis side it is the compiler: Rcpp drives conda's own x86_64-conda-linux-gnu-c++, +# which is on no PATH but that environment's, and without activation every compiled path fails +# with `sh: x86_64-conda-linux-gnu-c++: command not found`. +# +# Absent means the suite genuinely runs outside both, and the four that declare nothing are the +# static ones: 00_static, 01_migrate, 02_launcher and 03_helpers. They reach a tool by explicit +# path where they need one at all - 00_static's lint case sets PATH per invocation, 03_helpers +# calls bcftools through BCFTOOLS_BIN - and 02_launcher and 06_dryrun drive the wrapper against +# a STUB conda on purpose, which has to stay ahead of anything real on PATH. +suite_env() { + local declared + declared=$(sed -n '1,12s/^# env: *//p' "$1" | head -1) + printf '%s' "${declared:-none}" +} + # True when `name` contains any of the remaining arguments, or when there are none. No filters # means everything, which is what makes an unfiltered run the whole suite. matches_any() { @@ -300,6 +296,18 @@ fi TEST_TMPDIR=$(mktemp -d "${TMPDIR:-/tmp}/poolseqflow-test.XXXXXX") export TEST_TMPDIR +# XDG_DATA_HOME FOR THE WHOLE RUN, so nothing a case installs reaches the operator's own home. +# `PoolSeqFlow install` writes the tab completion under it and `uninstall` removes it again - +# the one thing the wrapper puts outside its own prefix. +# +# Set here rather than in run_launcher_with_envs, which is where it was first put: 02_launcher +# invokes the wrapper inline in several cases rather than through that helper, and those calls +# went straight to ~/.local/share. Measured - the directory appeared there on the first run. +# One export covers every invocation however a case makes it. +XDG_DATA_HOME="$TEST_TMPDIR/xdg" +export XDG_DATA_HOME +mkdir -p "$XDG_DATA_HOME" + # A second working area on a DIFFERENT filesystem, when the machine has one to offer. Moving an # artifact between two volumes is a different code path from moving it within one, and it is the # path both atomic_mv.sh data-loss defects lived in; TEST_TMPDIR alone cannot reach it. @@ -350,6 +358,20 @@ for suite in "$REPO_ROOT"/modules/*/test/*.sh; do SUITES+=("$suite") done +# THE SUITES THAT NEED AN ENVIRONMENT GO LAST, so they form one block and one activation covers +# all of them. Ordering is the whole mechanism: without it 07_analysis_rlib sits in the middle of +# the numbered suites and the run would have to activate and deactivate around it. +_plain=(); _needs_env=() +for suite in "${SUITES[@]}"; do + if [ "$(suite_env "$suite")" = "none" ]; then + _plain+=("$suite") + else + _needs_env+=("$suite") + fi +done +SUITES=("${_plain[@]+"${_plain[@]}"}" "${_needs_env[@]+"${_needs_env[@]}"}") +unset _plain _needs_env + if [ "$LIST_ONLY" -eq 1 ]; then echo "Suites:" for suite in "${SUITES[@]}"; do @@ -402,11 +424,28 @@ fi SUITES_RUN=0 CURRENT_SUITE="" +ACTIVE_ENV="none" for suite in "${SUITES[@]}"; do name=$(basename "$suite" .sh) matches_any "$name" "${SUITE_FILTERS[@]+"${SUITE_FILTERS[@]}"}" || continue matches_any "$(suite_cost "$suite")" "${COST_FILTERS[@]+"${COST_FILTERS[@]}"}" || continue SUITES_RUN=$((SUITES_RUN + 1)) + + # ONE ACTIVATION, AT THE BOUNDARY. The suites are ordered so that everything needing an + # environment is contiguous, so this fires once on the way in and once on the way out. + # Silent when conda is not reachable: the cases inside check have_analysis_r and skip. + want=$(suite_env "$suite") + if [ "$want" != "$ACTIVE_ENV" ]; then + [ "$ACTIVE_ENV" = "none" ] || conda deactivate 2>/dev/null || true + case "$want" in + pipeline) [ -n "$TEST_CONDA_ENV" ] \ + && conda activate "$TEST_CONDA_ENV" 2>/dev/null || true ;; + analysis) [ -n "$TEST_ANALYSIS_ENV" ] \ + && conda activate "$TEST_ANALYSIS_ENV" 2>/dev/null || true ;; + esac + ACTIVE_ENV="$want" + fi + # Suites marked slow read this to decide whether to skip themselves. export TEST_FAST="$FAST" CURRENT_SUITE="$name" diff --git a/test/suites/00_static.sh b/test/suites/00_static.sh index d73b5ab..91cf9bd 100644 --- a/test/suites/00_static.sh +++ b/test/suites/00_static.sh @@ -136,9 +136,13 @@ test_release_archive_excludes_development_material() { # One-line loops, which is how all of these are written. The multi-line one in 9_completion.nf # carries the same guard inside its body and is not matched here. test_glob_loops_publishing_artifacts_are_guarded() { - local unguarded - unguarded=$(cd "$REPO_ROOT" && grep -n 'for [A-Za-z_]* in [^;]*\*[^;]*; *do' scripts/*.nf \ - | grep 'atomic_mv\.sh' | grep -v '\[ -e ' || true) + local loops unguarded + loops=$(cd "$REPO_ROOT" && grep -n 'for [A-Za-z_]* in [^;]*\*[^;]*; *do' scripts/*.nf \ + | grep 'atomic_mv\.sh' || true) + # A POSITIVE CONTROL. The assertion below is about an absence, so a change to how these + # loops are written empties the search and the case passes over nothing at all. + [ -n "$loops" ] || { fail_case "no glob loops calling atomic_mv.sh were found at all"; return; } + unguarded=$(printf '%s\n' "$loops" | grep -v '\[ -e ' || true) [ -z "$unguarded" ] || fail_case \ "glob loops calling atomic_mv.sh with no existence guard:"$'\n'"$unguarded" } @@ -756,11 +760,14 @@ test_install_payload_matches_the_release_archive() { # this rather than turning up in someone's log tree months later. The one-writer-per-file rule # is carried by the file NAME, which is what makes the flattening safe. test_every_process_logs_into_its_workflows_own_directory() { - local offenders - offenders=$(grep -rh 'dir_log = "' "$REPO_ROOT"/scripts/*.nf "$REPO_ROOT"/dryrun.nf \ - | sed 's|.*dir_log = "||; s|".*||' \ - | grep -vE '^[$]\{(run\.dir\.logs|params\.dir\.allLogs)\}/[0-9A-Za-z_]+$' \ - | sort -u) + local declared offenders + declared=$(grep -rh 'dir_log = "' "$REPO_ROOT"/scripts/*.nf "$REPO_ROOT"/dryrun.nf \ + | sed 's|.*dir_log = "||; s|".*||' | sort -u) + # A POSITIVE CONTROL, for the same reason: the assertion is about an absence, and a change + # to how dir_log is written would empty the extraction rather than fail the case. + [ -n "$declared" ] || { fail_case "no dir_log assignments were found at all"; return; } + offenders=$(printf '%s\n' "$declared" \ + | grep -vE '^[$]\{(run\.dir\.logs|params\.dir\.allLogs)\}/[0-9A-Za-z_]+$' || true) assert_eq "" "$offenders" \ "a log directory should be the workflow's own, with nothing nested below it" } @@ -802,14 +809,19 @@ test_the_per_sample_parameter_table_is_the_same_on_both_sides() { # not exist. Checked against the template rather than against a list here, so adding a column # for a parameter that was never added to parameters.config fails. test_every_per_sample_parameter_names_a_real_parameter() { - local leaf - sed -n '/^PARAM_COLUMNS = {/,/^}/p' "$REPO_ROOT/bin/parse_metadata.py" \ - | sed -n 's/.*"param_[A-Za-z0-9]*": "\([A-Za-z0-9._]*\)".*/\1/p' \ - | while read -r parameter; do - leaf="${parameter##*.}" - grep -qE "^[[:space:]]*${leaf}[[:space:]]*=" "$REPO_ROOT/parameters.config.template" \ - || fail_case "param_ column overrides '$parameter', which parameters.config.template does not define" - done + local leaf parameter checked=0 + # Fed by a redirect rather than a pipe. The last stage of a pipeline runs in a subshell, so + # `... | while read` reaches fail_case but its CASE_FAILED never returns to run_case - this + # case reported PASS for any template at all until 2026-09-23. + while read -r parameter; do + checked=$((checked + 1)) + leaf="${parameter##*.}" + grep -qE "^[[:space:]]*${leaf}[[:space:]]*=" "$REPO_ROOT/parameters.config.template" \ + || fail_case "param_ column overrides '$parameter', which parameters.config.template does not define" + done < <(sed -n '/^PARAM_COLUMNS = {/,/^}/p' "$REPO_ROOT/bin/parse_metadata.py" \ + | sed -n 's/.*"param_[A-Za-z0-9]*": "\([A-Za-z0-9._]*\)".*/\1/p') + # An extraction that matches nothing would otherwise pass over an empty loop. + [ "$checked" -gt 0 ] || fail_case "no param_ columns found in bin/parse_metadata.py" } # EVERY citations.json IS GENERATED, and this is what stops one being edited by hand. @@ -1439,3 +1451,42 @@ PY ) assert_eq "" "$out" "every declared manual anchor must exist:"$'\n'"$out" } + +# THE COMPLETION OFFERS EXACTLY WHAT THE WRAPPER DISPATCHES, and this is checked by running the +# completion rather than by reading its source, so how the list is written cannot fool it. +# +# A hand-kept list beside a `case` is the shape that rots: the wrapper gains a verb, the +# completion does not, and nothing says so. Strict equality both ways, with no exception list - +# an exception list is the next thing to go stale. +test_the_completion_offers_every_verb_the_wrapper_takes() { + local dispatched offered + dispatched=$(sed -n '/^case "\?\$COMMAND"\?/,/^esac/p' "$REPO_ROOT/PoolSeqFlow" \ + | sed -n 's/^ \([a-z_|]*\))$/\1/p' | tr '|' '\n' | sort -u) + offered=$(bash -c ' + . "$1/lib/poolseqflow-completion.bash" + COMP_WORDS=(PoolSeqFlow ""); COMP_CWORD=1 + _poolseqflow + printf "%s\n" "${COMPREPLY[@]}"' _ "$REPO_ROOT" | sort -u) + + [ -n "$dispatched" ] || { fail_case "no verbs were extracted from the wrapper's case"; return; } + [ -n "$offered" ] || { fail_case "the completion offered nothing at all"; return; } + assert_eq "$dispatched" "$offered" "the completion and the wrapper must agree on the verbs" +} + +# The second level, for the two subcommands that have a fixed set. `analysis` also offers the +# installed modules, which vary by machine, so only the fixed words are compared. +test_the_completion_offers_the_subcommands_each_verb_takes() { + local out + out=$(bash -c ' + . "$1/lib/poolseqflow-completion.bash" + reply() { COMP_WORDS=("${@:2}" ""); COMP_CWORD=$1; _poolseqflow; printf "%s\n" "${COMPREPLY[@]}"; } + printf "check: %s\n" "$(reply 2 PoolSeqFlow check | sort | tr "\n" " ")" + printf "modules: %s\n" "$(reply 3 PoolSeqFlow analysis modules | sort | tr "\n" " ")" + ' _ "$REPO_ROOT") + + assert_contains "$out" "check: install project " \ + "check takes the two targets its usage names" + assert_contains "$out" "modules: available install list uninstall " \ + "analysis modules takes the four verbs its usage names" +} + diff --git a/test/suites/02_launcher.sh b/test/suites/02_launcher.sh index 5360cca..1f72312 100644 --- a/test/suites/02_launcher.sh +++ b/test/suites/02_launcher.sh @@ -462,8 +462,6 @@ test_usage_and_implementation_agree() { done < <(printf '%s\n' "$advertised") while read -r cmd; do [ -n "$cmd" ] || continue - # `resume` is an accepted deprecated alias, deliberately not advertised. - [ "$cmd" = "resume" ] && continue printf '%s\n' "$advertised" | grep -qx "$cmd" \ || fail_case "'$cmd' is implemented but not advertised in usage" done < <(printf '%s\n' "$implemented") @@ -1604,3 +1602,35 @@ test_every_environment_removal_passes_minus_y() { without=$(grep -n 'conda env remove -n "' "$REPO_ROOT/PoolSeqFlow" | grep -v -- '-y' || true) assert_eq "" "$without" "every conda env remove should pass -y" } + +# THE COMPLETION IS INSTALLED WHERE BASH LOOKS FOR IT, and the message says what zsh needs, +# because zsh does not read that directory and would otherwise get nothing with no explanation. +test_install_puts_the_tab_completion_where_bash_finds_it() { + run_launcher_with_envs "base $VERSIONED_ENV" install + assert_status 0 "$LAUNCHER_STATUS" "install should succeed" + + local file="$LAUNCHER_XDG/bash-completion/completions/PoolSeqFlow" + assert_file "$file" "the completion should be installed under XDG_DATA_HOME" + assert_contains "$(cat "$file" 2>/dev/null)" "complete -F _poolseqflow" \ + "and it should be the real completion, not an empty placeholder" + assert_contains "$LAUNCHER_OUTPUT" "bashcompinit" "the message should tell a zsh user what to add" +} + +# UNINSTALLING THE LAST VERSION TAKES IT WITH IT. A completion left behind completes a command +# that is gone, and it is the one file `install` writes outside its own prefix. +test_uninstall_removes_the_tab_completion() { + run_launcher_with_envs "base $VERSIONED_ENV" install + local file="$LAUNCHER_XDG/bash-completion/completions/PoolSeqFlow" + assert_file "$file" "the completion should be there before the uninstall" + + # The same sandbox, so the installation the first call made is what this one removes. The + # `<<< y` answers the confirmation, as the other uninstall cases do. + local sb; sb=$(dirname "$LAUNCHER_PREFIX") + local out; out=$( cd "$sb" && PATH="$sb/stub/bin:$PATH" \ + POOLSEQFLOW_PREFIX="$LAUNCHER_PREFIX" XDG_DATA_HOME="$LAUNCHER_XDG" \ + ./PoolSeqFlow uninstall 2>&1 <<< y ) + assert_count 0 "$(find "$LAUNCHER_PREFIX/opt" -maxdepth 1 -name 'PoolSeqFlow-*' | wc -l)" \ + "the payload should be gone, or the completion is right to stay" + assert_eq "" "$(ls "$LAUNCHER_XDG/bash-completion/completions" 2>/dev/null)" \ + "the completion should go with the last installation:"$'\n'"$out" +} diff --git a/test/suites/05_helpers.sh b/test/suites/03_helpers.sh similarity index 100% rename from test/suites/05_helpers.sh rename to test/suites/03_helpers.sh diff --git a/test/suites/03_pipeline.sh b/test/suites/04_pipeline.sh similarity index 93% rename from test/suites/03_pipeline.sh rename to test/suites/04_pipeline.sh index 94620a5..cb275e4 100644 --- a/test/suites/03_pipeline.sh +++ b/test/suites/04_pipeline.sh @@ -1,6 +1,7 @@ #!/bin/bash # End-to-end runs against the committed fixture. The slow suite; --fast skips it. # cost: pipeline +# env: pipeline # covers: poolseqflow.nf scripts/ nextflow.config bin/ # # One full run is shared by the cases that only inspect its results, because a run costs @@ -71,7 +72,7 @@ test_frequency_table_columns_follow_the_metadata_order() { # THE ONE THING THE UNIT TESTS CANNOT SEE: that the pool sizes are read off the real metadata # file in a real run. # -# bin/filterFalsePositives.sh is covered case by case in 05_helpers, but every one of those +# bin/filterFalsePositives.sh is covered case by case in 03_helpers, but every one of those # calls it directly with a -p string written by the test. This reads the step 0 report from the # run that already happened, so it costs nothing, and it fails if poolSizes() stops resolving # the fallback to the global poolSize or stops being reported at all. @@ -547,7 +548,7 @@ test_fastqc_zips_are_promoted_but_htmls_go_straight_to_storage() { # E1q reachable a second way, when the pair is promoted between the volumes. # # Runs against a copy of the finished project, so the cost is a run of skips rather than a -# fresh analysis. Copying is safe for the same reason 04_guards relies on it: the stored +# fresh analysis. Copying is safe for the same reason 05_guards relies on it: the stored # manifest excludes mainDir, storageDir and every dir.* entry, so it still matches after the # move. This also exercises the other half of E1p - the rebuilt sample's aligned BAM has been # promoted, so Align has to find it in permanent storage rather than where it wrote it. @@ -1340,3 +1341,93 @@ test_the_depth_table_costs_no_extra_task() { assert_eq "2" "$(task_count "$PIPELINE_SB" "VCF2Frequencies:CalculateFrequencies")" \ "one task per split file, writing both its depth table and its frequency table" } + +# READS MAY SIT IN SUBFOLDERS OF Data/, or directly in it, or both at once. +# +# THIS RUNS STEP 2, NOT STEP 0, because step 0 cannot answer it. Step 0 finds reads with +# `find -name`, which has always recursed, so a nested layout passed its sample match long +# before the reads could actually be staged - the divergence this change closes. What had to +# move is `params.reads`, which readPairChannel globs, and only a step that consumes that +# channel exercises it. Written against step 0 first, where reverting the glob left the case +# passing. +test_reads_are_found_in_subfolders_of_data() { + have_tools || { skip_case "no conda environment"; return; } + [ "${TEST_FAST:-0}" = "1" ] && { skip_case "--fast"; return; } + local sb status s + sb=$(make_pipeline_sandbox "nested-reads") + write_sandbox_config "$sb" + + # One sample in a folder of its own, one two levels down, the rest left flat. + mkdir -p "$sb/main/Data/TestSample1" "$sb/main/Data/batch2/TestSample2" + mv "$sb/main/Data/TestSample1_R"*.fq.gz "$sb/main/Data/TestSample1/" + mv "$sb/main/Data/TestSample2_R"*.fq.gz "$sb/main/Data/batch2/TestSample2/" + + status=$(run_trim_only "$sb") + assert_status 0 "$status" "step 2 should complete over a mixed layout; see $sb/run.out" + + # Every sample trimmed, wherever its reads were: the nested two are the point, and the + # flat ones prove `**` still matches at depth zero. + for n in 1 2 3 4 5 6; do + s="TestSample$n" + assert_count 2 "$(find "$sb/main/Utilized" -name "${s}_R[12]_clipped.fq.gz" 2>/dev/null | wc -l)" \ + "$s should have been trimmed whatever folder its reads were in" + done +} + +# HIDDEN FOLDERS ARE EXCLUDED FROM THE READ CHANNEL, not only from step 0's search. +# +# THIS RUNS STEP 2, because step 0 cannot answer it. The two prune independently - step 0 with +# `find -prune`, the channel with a filter in readPairChannel - and a case that runs only step 0 +# passes with the channel's filter deleted outright. Measured: it did. +# +# `.snapshot` is NetApp's, exposed read-only inside every directory on much HPC storage and +# holding a copy of every file per snapshot. Unfiltered, the channel emits the sample twice and +# trims it twice, with both runs writing the outputs named after it. +test_hidden_folders_are_excluded_from_the_read_channel() { + have_tools || { skip_case "no conda environment"; return; } + [ "${TEST_FAST:-0}" = "1" ] && { skip_case "--fast"; return; } + local sb status samples + sb=$(make_pipeline_sandbox "hidden-channel") + write_sandbox_config "$sb" + samples=$(find "$sb/main/Data" -name '*_R1.fq.gz' | wc -l) + + mkdir -p "$sb/main/Data/.snapshot/nightly" + cp "$sb/main/Data/TestSample1_R1.fq.gz" "$sb/main/Data/.snapshot/nightly/" + cp "$sb/main/Data/TestSample1_R2.fq.gz" "$sb/main/Data/.snapshot/nightly/" + + status=$(run_trim_only "$sb") + assert_status 0 "$status" "the snapshot copy must not disturb the run; see $sb/run.out" + + # One trim per sample, and the count is what shows the copy was excluded: unfiltered, the + # channel emits TestSample1 twice and this reads one higher than the sample count. + assert_count "$samples" "$(task_count "$sb" TrimQcClip:TrimReads)" \ + "each sample should be trimmed once, the hidden copy not at all" +} + +# `reads` IN parameters.config IS NOT CONSULTED, and this pins that. +# +# deriveRunPaths() assigns p.reads directly - not through fill(), which is what respects a user +# setting - so the value a run globs is always the one it computes, into each variant map. The +# copy in parameters.config exists so a reader can see how the path is built. +# +# Worth a case because the asymmetry is invisible: the knobs beside it in the same file ARE +# respected, since deriveInto() fills them only when absent. And because it cost real time - +# the `**` was added to parameters.config.template first and nothing changed, which reads as +# the feature not working rather than as the line not being read. +test_the_reads_setting_in_the_config_is_not_consulted() { + have_tools || { skip_case "no conda environment"; return; } + [ "${TEST_FAST:-0}" = "1" ] && { skip_case "--fast"; return; } + local sb status samples + sb=$(make_pipeline_sandbox "reads-override") + samples=$(find "$sb/main/Data" -name '*_R1.fq.gz' | wc -l) + # A path that matches nothing anywhere. If it were read, the run would find no reads at all. + write_sandbox_config "$sb" \ + 's|^ reads .*| reads = "/nonexistent/nothing/**_R{1,2}.fq.gz"|' + assert_contains "$(cat "$sb/main/parameters.config")" "/nonexistent/nothing/" \ + "the sandbox config should carry the bogus path, or this case proves nothing" + + status=$(run_trim_only "$sb") + assert_status 0 "$status" "the derived value should be used regardless; see $sb/run.out" + assert_count "$samples" "$(task_count "$sb" TrimQcClip:TrimReads)" \ + "every sample should still be trimmed, from the path the resolver computed" +} diff --git a/test/suites/04_guards.sh b/test/suites/05_guards.sh similarity index 84% rename from test/suites/04_guards.sh rename to test/suites/05_guards.sh index 7a3794a..f07ea22 100644 --- a/test/suites/04_guards.sh +++ b/test/suites/05_guards.sh @@ -1,6 +1,7 @@ #!/bin/bash # The step 0 change guards: what invalidates existing outputs and what merely gets recorded. # cost: jvm +# env: pipeline # covers: scripts/0_verify_environment.nf scripts/resolve_parameters.nf scripts/variants.nf # covers: scripts/metadata.nf bin/parse_metadata.py bin/parse_multirun.py # covers: bin/classify_manifest.sh @@ -1115,3 +1116,210 @@ c,200 assert_contains "$report" "was a,50" "for the run that held the old value" assert_contains "$report" "now a,200" "and now holds another run\'s" } + +# THE COLLISION THE SUBFOLDERS MAKE REACHABLE. Two directories holding the same file name are +# one sample twice over: both become that sample, and each would overwrite the other's results. +# Impossible while the reads had to be flat, which is why nothing checked for it before. +test_one_sample_name_in_two_folders_is_refused() { + have_tools || { skip_case "no conda environment"; return; } + local sb status out + sb=$(make_pipeline_sandbox "duplicate-reads") + write_sandbox_config "$sb" + + mkdir -p "$sb/main/Data/run1" "$sb/main/Data/run2" + cp "$sb/main/Data/TestSample1_R1.fq.gz" "$sb/main/Data/run1/TestSample1_R1.fq.gz" + cp "$sb/main/Data/TestSample1_R2.fq.gz" "$sb/main/Data/run1/TestSample1_R2.fq.gz" + cp "$sb/main/Data/TestSample1_R1.fq.gz" "$sb/main/Data/run2/TestSample1_R1.fq.gz" + cp "$sb/main/Data/TestSample1_R2.fq.gz" "$sb/main/Data/run2/TestSample1_R2.fq.gz" + + status=$(run_verify_only "$sb") + out=$(cat "$sb/store/Output/Reports/0_verify_environment.txt" 2>/dev/null)$(cat "$sb/run.out") + assert_status 1 "$status" "one name in two folders should stop the run" + assert_contains "$out" "does not have exactly one of each mate" "and say what is wrong" + assert_contains "$out" "The same file name appears more than once" \ + "naming the reason rather than the folders" + assert_contains "$out" "/Data/run1" "naming the first directory" + assert_contains "$out" "/Data/run2" "and the second" + assert_contains "$out" "METADATA SAMPLE MATCH: FAIL" "and fail the check that owns it" +} + +# A MATE WITH NO PARTNER IS DROPPED BY THE READ CHANNEL WITHOUT A WORD. +# +# fromFilePairs emits nothing at all for a lone file - measured, not inferred - so the sample +# simply is not in the run. The check this replaces was that the FASTQ count is even, which two +# samples each missing a mate satisfy between them. +test_a_sample_missing_one_mate_is_refused() { + have_tools || { skip_case "no conda environment"; return; } + local sb status out + sb=$(make_pipeline_sandbox "orphan-mate") + write_sandbox_config "$sb" + # TWO samples lose a mate, so the FASTQ count stays EVEN. That is the whole point: the + # check this replaces asked only whether the total was divisible by two, which two orphans + # satisfy between them. One orphan is caught earlier, by DATA FILES CHECK. + rm -f "$sb/main/Data/TestSample5_R2.fq.gz" "$sb/main/Data/TestSample6_R2.fq.gz" + + status=$(run_verify_only "$sb") + out=$(cat "$sb/store/Output/Reports/0_verify_environment.txt" 2>/dev/null)$(cat "$sb/run.out") + assert_status 1 "$status" "two half-present samples should stop the run" + assert_contains "$out" "All FASTQ files are properly paired" \ + "the count is even, so the check this replaces is satisfied" + assert_contains "$out" "TestSample5' does not have exactly one of each mate" "naming the first" + assert_contains "$out" "TestSample6' does not have exactly one of each mate" "and the second" + assert_contains "$out" "METADATA SAMPLE MATCH: FAIL" "and failing the check that owns it" +} + +# A SUBFOLDER OF Data/ WITH NO READS IN IT. Reads may be nested now, so an empty folder is +# either a copy that did not finish or a readPattern that does not match what is in it - +# and a run that ignored it would process fewer samples than the user believes it has. +test_a_data_subfolder_with_no_reads_is_refused() { + have_tools || { skip_case "no conda environment"; return; } + local sb status out + sb=$(make_pipeline_sandbox "empty-subfolder") + write_sandbox_config "$sb" + mkdir -p "$sb/main/Data/TestSample7" + + status=$(run_verify_only "$sb") + out=$(cat "$sb/store/Output/Reports/0_verify_environment.txt" 2>/dev/null)$(cat "$sb/run.out") + assert_status 1 "$status" "a folder holding no reads should stop the run" + assert_contains "$out" "hold no reads matching" "saying what it looked for" + assert_contains "$out" "Data/TestSample7" "and naming the folder" +} + +# HIDDEN FOLDERS ARE SKIPPED, AND THE RUN SAYS SO RATHER THAN GOING QUIET. +# +# `.snapshot` is NetApp's, exposed read-only inside every directory on much HPC storage, and it +# holds a copy of every read per snapshot. Since the reads may be nested the glob would walk +# into those and find every sample twice, so they are pruned - but a folder full of reads that +# is ignored in silence is how a user loses samples without being told. +test_hidden_folders_are_skipped_and_reported() { + have_tools || { skip_case "no conda environment"; return; } + local sb status out + sb=$(make_pipeline_sandbox "hidden-reads") + write_sandbox_config "$sb" + mkdir -p "$sb/main/Data/.snapshot/nightly" + cp "$sb/main/Data/TestSample1_R1.fq.gz" "$sb/main/Data/.snapshot/nightly/" + cp "$sb/main/Data/TestSample1_R2.fq.gz" "$sb/main/Data/.snapshot/nightly/" + + status=$(run_verify_only "$sb") + out=$(cat "$sb/store/Output/Reports/0_verify_environment.txt" 2>/dev/null)$(cat "$sb/run.out") + # The copy is pruned, so TestSample1 still has two files in one directory and nothing fails. + assert_status 0 "$status" "a hidden copy must not fail the run; see $sb/run.out" + assert_contains "$out" "hidden folders hold reads and were skipped" "but it must be said" + assert_contains "$out" "Data/.snapshot" "naming the folder" + assert_contains "$out" "METADATA SAMPLE MATCH: PASS" \ + "and the real reads still match their rows" +} + +# A PAIR IS A PAIR WHEREVER IT IS FILED. Mates in two different folders group correctly, +# because a sample is named by its file and never by its folder, so this must not be refused. +# Z, 2026-09-24: *"mates in separate folders should be fine ... We just need to be looking for +# pairs. Not where they stored."* +test_mates_in_separate_folders_are_accepted() { + have_tools || { skip_case "no conda environment"; return; } + local sb status out + sb=$(make_pipeline_sandbox "split-pair") + write_sandbox_config "$sb" + mkdir -p "$sb/main/Data/first" "$sb/main/Data/second" + mv "$sb/main/Data/TestSample1_R1.fq.gz" "$sb/main/Data/first/" + mv "$sb/main/Data/TestSample1_R2.fq.gz" "$sb/main/Data/second/" + + status=$(run_verify_only "$sb") + out=$(cat "$sb/store/Output/Reports/0_verify_environment.txt" 2>/dev/null)$(cat "$sb/run.out") + assert_status 0 "$status" "a pair split across folders should verify; see $sb/run.out" + assert_contains "$out" "METADATA SAMPLE MATCH: PASS" "the pair is still a pair" + assert_not_contains "$out" "exactly one of each mate" "and nothing should be refused" +} + +# THE SAME MATE TWICE IS NOT A PAIR, and the read channel cannot tell. Two copies of an R1 in +# two folders are handed on as a pair of files, and the pipeline would align R1 against R1 - +# measured against fromFilePairs, which reports n=2 for them. Only reachable since the reads +# may be nested, because one directory cannot hold a name twice. +test_the_same_mate_in_two_folders_is_refused() { + have_tools || { skip_case "no conda environment"; return; } + local sb status out + sb=$(make_pipeline_sandbox "same-mate") + write_sandbox_config "$sb" + mkdir -p "$sb/main/Data/copyA" "$sb/main/Data/copyB" + mv "$sb/main/Data/TestSample2_R1.fq.gz" "$sb/main/Data/copyA/" + cp "$sb/main/Data/copyA/TestSample2_R1.fq.gz" "$sb/main/Data/copyB/" + rm -f "$sb/main/Data/TestSample2_R2.fq.gz" + + status=$(run_verify_only "$sb") + out=$(cat "$sb/store/Output/Reports/0_verify_environment.txt" 2>/dev/null)$(cat "$sb/run.out") + assert_status 1 "$status" "two copies of one mate are not a pair" + assert_contains "$out" "TestSample2' does not have exactly one of each mate" "naming the sample" + assert_contains "$out" "The same file name appears more than once" "and the reason" +} + +# THE RECORD HOLDS WHAT YOU SET, NOT WHAT IS DERIVED FROM IT. +# +# analysisParams() names the seven derived paths in skipKey, so they never reach +# .poolseqflow_params. That is what lets the pipeline change how a path is BUILT without telling +# every existing project its outputs are invalid: `reads` gained a `**` in 3.2.0 and no project +# noticed, because only dataSource and readPattern are compared. +# +# Recording one of them would also break a project that simply moved volumes, since they are +# absolute. Written as a case because the cost of losing it is paid by users, silently, on an +# upgrade that looks routine. +test_the_record_holds_settings_and_not_derived_paths() { + guards_ready || return + local rec; rec="$GUARD_SB/store/Output/.poolseqflow_params" + assert_file "$rec" "the first run should have recorded the settings" + + local body; body=$(cat "$rec" 2>/dev/null) + assert_contains "$body" "dataSource=" "what names the folder is a setting" + assert_contains "$body" "readPattern=" "and so is what matches the files" + + local name + for name in reads reference gff referencePath gffPath metadataPath multiRunPath referenceFa; do + assert_not_contains "$body" "$name=" \ + "$name is derived and absolute, so recording it would refuse a project that moved" + done +} + +# CHANGING WHAT MATCHES THE READS IS A CHANGE TO THE READS. readPattern is a setting, so unlike +# the derived `reads` it is recorded and compared - the other half of the case above. +test_a_read_pattern_change_invalidates_the_trimming() { + guards_ready || return + # Something step 2 produced, or the guard adopts the edit as a new baseline and passes. + mkdir -p "$GUARD_SB/main/Utilized/Trimmed" + : > "$GUARD_SB/main/Utilized/Trimmed/TestSample1_R1_clipped.fq.gz" + + write_sandbox_config "$GUARD_SB" \ + 's|^ readPattern .*| readPattern = "*_R{1,2}.fastq.gz"|' + local status report + status=$(run_verify_only "$GUARD_SB") + report=$(guard_report) + assert_status 1 "$status" "a readPattern change should fail against existing outputs" + assert_contains "$report" "readPattern" "naming what differs" +} + +# THE DATA FOLDER IS NOT THERE AT ALL. dataSource may name any folder under mainDir, so a typo +# in it lands here rather than on a missing-reads message. +test_a_missing_data_folder_is_refused() { + have_tools || { skip_case "no conda environment"; return; } + local sb status out + sb=$(make_pipeline_sandbox "no-data-folder") + write_sandbox_config "$sb" "s|^ dataSource .*| dataSource = 'NotThere'|" + + status=$(run_verify_only "$sb") + out=$(cat "$sb/store/Output/Reports/0_verify_environment.txt" 2>/dev/null)$(cat "$sb/run.out") + assert_status 1 "$status" "a dataSource naming nothing should stop the run" + assert_contains "$out" "Data directory" "saying which directory it looked for" +} + +# THE FOLDER IS THERE AND THE PATTERN MATCHES NOTHING IN IT. Distinct from the case above: the +# reads exist, and the setting that finds them does not describe them. +test_a_read_pattern_matching_nothing_is_refused() { + have_tools || { skip_case "no conda environment"; return; } + local sb status out + sb=$(make_pipeline_sandbox "pattern-matches-nothing") + write_sandbox_config "$sb" \ + 's|^ readPattern .*| readPattern = "*_L00{1,2}.fastq.gz"|' + + status=$(run_verify_only "$sb") + out=$(cat "$sb/store/Output/Reports/0_verify_environment.txt" 2>/dev/null)$(cat "$sb/run.out") + assert_status 1 "$status" "a pattern matching nothing should stop the run" + assert_contains "$out" "No FASTQ files found" "saying nothing matched" + assert_contains "$out" "Expected pattern" "and what it was looking for" +} diff --git a/test/suites/06_dryrun.sh b/test/suites/06_dryrun.sh index e5076ef..469ae7e 100644 --- a/test/suites/06_dryrun.sh +++ b/test/suites/06_dryrun.sh @@ -1,6 +1,7 @@ #!/bin/bash # The dry run: where the work would go, shown before any of it is done. # cost: jvm +# env: pipeline # covers: dryrun.nf scripts/variants.nf scripts/resolve_parameters.nf # # What these are about is the promise the subcommand makes. A preview verifies the project and diff --git a/test/suites/08_analysis_rlib.sh b/test/suites/07_analysis_rlib.sh similarity index 96% rename from test/suites/08_analysis_rlib.sh rename to test/suites/07_analysis_rlib.sh index d5ae8b6..b4aa02c 100644 --- a/test/suites/08_analysis_rlib.sh +++ b/test/suites/07_analysis_rlib.sh @@ -1,11 +1,12 @@ #!/bin/bash -# The shared R library, called directly. No Nextflow, no conda, no fixture. +# The shared R library, called directly against the release's own R. No Nextflow, no fixture. # cost: static +# env: analysis # covers: modules/lib/ test/tools/r_lib_tests.R test/tools/split_counts.R test/tools/pool_sensitivity.R # # The fixtures and helpers every analysis suite shares are in test/lib/analysis.sh. # -# THE PIPELINE IS ASSUMED TO WORK. That is 03_pipeline's business, and re-proving it here would +# THE PIPELINE IS ASSUMED TO WORK. That is 04_pipeline's business, and re-proving it here would # cost minutes a case. # BASE R, AND NOTHING ELSE. These functions are unit-tested against whatever R is on the diff --git a/test/suites/07_analysis_frame.sh b/test/suites/08_analysis_frame.sh similarity index 99% rename from test/suites/07_analysis_frame.sh rename to test/suites/08_analysis_frame.sh index f28103e..f53cef1 100644 --- a/test/suites/07_analysis_frame.sh +++ b/test/suites/08_analysis_frame.sh @@ -1,12 +1,13 @@ #!/bin/bash # The analysis frame: what it is, what it reads, what keeps it optional. # cost: jvm +# env: analysis # covers: analysis.nf analysis/frame.config analysis/frame.version analysis/lib/nf/paths.nf # covers: analysis/analysis.config.template # # The fixtures and helpers every analysis suite shares are in test/lib/analysis.sh. # -# THE PIPELINE IS ASSUMED TO WORK. That is 03_pipeline's business, and re-proving it here would +# THE PIPELINE IS ASSUMED TO WORK. That is 04_pipeline's business, and re-proving it here would # cost minutes a case. # --------------------------------------------------------------------------------------- diff --git a/test/suites/09_analysis_modules.sh b/test/suites/09_analysis_modules.sh index f843383..70ecfd9 100644 --- a/test/suites/09_analysis_modules.sh +++ b/test/suites/09_analysis_modules.sh @@ -1,13 +1,14 @@ #!/bin/bash # The module store, and what a module gets from the frame. # cost: jvm +# env: analysis # covers: analysis/modules.nf analysis/lib/nf/modules.nf analysis/lib/nf/store.nf # covers: analysis/lib/nf/paths.nf # covers: analysis.nf # # The fixtures and helpers every analysis suite shares are in test/lib/analysis.sh. # -# THE PIPELINE IS ASSUMED TO WORK. That is 03_pipeline's business, and re-proving it here would +# THE PIPELINE IS ASSUMED TO WORK. That is 04_pipeline's business, and re-proving it here would # cost minutes a case. # --------------------------------------------------------------------------------------- diff --git a/test/suites/10_analysis_verify.sh b/test/suites/10_analysis_verify.sh index 5a66e4b..c9d1068 100644 --- a/test/suites/10_analysis_verify.sh +++ b/test/suites/10_analysis_verify.sh @@ -1,12 +1,13 @@ #!/bin/bash # Verification: what the frame checks before a module reads anything. # cost: jvm +# env: analysis # covers: analysis/0_verify_analysis.nf analysis/lib/nf/citations.nf # covers: analysis.nf # # The fixtures and helpers every analysis suite shares are in test/lib/analysis.sh. # -# THE PIPELINE IS ASSUMED TO WORK. That is 03_pipeline's business, and re-proving it here would +# THE PIPELINE IS ASSUMED TO WORK. That is 04_pipeline's business, and re-proving it here would # cost minutes a case. # --------------------------------------------------------------------------------------- diff --git a/test/suites/11_analysis_plan.sh b/test/suites/11_analysis_plan.sh index 400fa63..73257fb 100644 --- a/test/suites/11_analysis_plan.sh +++ b/test/suites/11_analysis_plan.sh @@ -1,12 +1,13 @@ #!/bin/bash # Which results an invocation covers, and which directory each lands in. # cost: jvm +# env: analysis # covers: analysis/lib/nf/plan.nf # covers: analysis.nf # # The fixtures and helpers every analysis suite shares are in test/lib/analysis.sh. # -# THE PIPELINE IS ASSUMED TO WORK. That is 03_pipeline's business, and re-proving it here would +# THE PIPELINE IS ASSUMED TO WORK. That is 04_pipeline's business, and re-proving it here would # cost minutes a case. # --------------------------------------------------------------------------------------- diff --git a/test/suites/12_analysis_design.sh b/test/suites/12_analysis_design.sh index 7bb6512..5887c61 100644 --- a/test/suites/12_analysis_design.sh +++ b/test/suites/12_analysis_design.sh @@ -1,12 +1,13 @@ #!/bin/bash # The experimental design, and the pools every frequency is read against. # cost: jvm +# env: analysis # covers: analysis/lib/nf/design.nf analysis/lib/nf/pools.nf # covers: analysis.nf # # The fixtures and helpers every analysis suite shares are in test/lib/analysis.sh. # -# THE PIPELINE IS ASSUMED TO WORK. That is 03_pipeline's business, and re-proving it here would +# THE PIPELINE IS ASSUMED TO WORK. That is 04_pipeline's business, and re-proving it here would # cost minutes a case. # EVERY analysis records the design the project was in, so a project whose design contradicts @@ -744,7 +745,7 @@ test_the_report_says_when_there_is_no_phenotype() { # # The detection limits below are hand-computed from 1/(2*ploidy*poolSize), which is a third # copy of the equation - the Groovy one in resolve_parameters.nf and the awk one in -# bin/filterFalsePositives.sh are tied together by 05_helpers, and these numbers tie this one +# bin/filterFalsePositives.sh are tied together by 03_helpers, and these numbers tie this one # to both. test_the_verification_report_states_the_pool_sizes() { analysis_ready single || return diff --git a/test/suites/13_analysis_time.sh b/test/suites/13_analysis_time.sh index 122e606..213b521 100644 --- a/test/suites/13_analysis_time.sh +++ b/test/suites/13_analysis_time.sh @@ -1,12 +1,13 @@ #!/bin/bash # The time axis, and the date parsing behind it. # cost: jvm +# env: analysis # covers: analysis/lib/nf/time.nf analysis/lib/nf/design.nf # covers: analysis.nf # # The fixtures and helpers every analysis suite shares are in test/lib/analysis.sh. # -# THE PIPELINE IS ASSUMED TO WORK. That is 03_pipeline's business, and re-proving it here would +# THE PIPELINE IS ASSUMED TO WORK. That is 04_pipeline's business, and re-proving it here would # cost minutes a case. # Time is not guessed. 20240307 reads as a number as readily as a date, which keeps the order diff --git a/test/suites/14_analysis_series.sh b/test/suites/14_analysis_series.sh index 35d184f..30b1ee8 100644 --- a/test/suites/14_analysis_series.sh +++ b/test/suites/14_analysis_series.sh @@ -2,12 +2,13 @@ # Series: which pools are one thing measured repeatedly, and what a time axis does to a unit. # The same units and conditions without a time axis are 12_analysis_design's. # cost: jvm +# env: analysis # covers: analysis/lib/nf/design.nf analysis/lib/nf/time.nf # covers: analysis.nf # # The fixtures and helpers every analysis suite shares are in test/lib/analysis.sh. # -# THE PIPELINE IS ASSUMED TO WORK. That is 03_pipeline's business, and re-proving it here would +# THE PIPELINE IS ASSUMED TO WORK. That is 04_pipeline's business, and re-proving it here would # cost minutes a case. # --------------------------------------------------------------------------------------- @@ -23,6 +24,14 @@ TestSample2,PoolB,Pop1,T1' local out; out=$(analysis_output) assert_contains "$out" "2 pools at the same exp_time" "the refusal counts them" assert_contains "$out" "'T1': PoolA, PoolB" "and names the timepoint and the pools" + + # BOTH REMEDIES, because the refusal is ambiguous between them and offering only one is + # wrong half the time: technical replicates that were meant to be merged belong under one + # RG_Sample, not in a new exp_ column. + assert_contains "$out" "analysis.design.biologicalRep or technicalRep" \ + "one remedy is to tell them apart and declare what they are" + assert_contains "$out" "give the rows the same" \ + "and the other is to merge them, which is the pipeline's job and not a series" } test_a_series_key_naming_the_time_column_refuses() { @@ -235,21 +244,3 @@ S6,P6,control,2,L1,T2' assert_contains "$(analysis_report "$ANALYSIS_SB")" "1-2 technical" \ "one unit was sequenced twice and the other once" } - -# The same-key-same-timepoint refusal is ambiguous between the two remedies, and offering only -# one of them is wrong half the time: technical replicates that were meant to be merged belong -# under one RG_Sample, not in a new exp_ column. -test_the_duplicate_pool_refusal_offers_both_remedies() { - analysis_ready single || return - analysis_write_metadata "$ANALYSIS_SB" 'SampleID,RG_Sample,exp_population,exp_time -TestSample1,PoolA,Pop1,T1 -TestSample2,PoolB,Pop1,T1' - analysis_write_metadata_config "$ANALYSIS_SB" " timeVar { kind = 'categorical' }" - local status; status=$(run_analysis "$ANALYSIS_SB" verify) - assert_status 1 "$status" "two pools at one point is still a refusal" - local out; out=$(analysis_output) - assert_contains "$out" "analysis.design.biologicalRep or technicalRep" \ - "one remedy is to tell them apart and declare what they are" - assert_contains "$out" "give the rows the same" \ - "and the other is to merge them, which is the pipeline's job and not a series" -} diff --git a/test/suites/15_analysis_results.sh b/test/suites/15_analysis_results.sh index 0c27620..b293d84 100644 --- a/test/suites/15_analysis_results.sh +++ b/test/suites/15_analysis_results.sh @@ -1,6 +1,7 @@ #!/bin/bash # What a module writes: where it lands, what it carries, and completion. # cost: jvm +# env: analysis # covers: analysis/lib/nf/results.nf analysis/lib/nf/outputs.nf analysis/complete.nf # covers: bin/write_citations.py # covers: analysis.nf @@ -8,7 +9,7 @@ # # The fixtures and helpers every analysis suite shares are in test/lib/analysis.sh. # -# THE PIPELINE IS ASSUMED TO WORK. That is 03_pipeline's business, and re-proving it here would +# THE PIPELINE IS ASSUMED TO WORK. That is 04_pipeline's business, and re-proving it here would # cost minutes a case. # --------------------------------------------------------------------------------------- @@ -436,22 +437,16 @@ test_an_intermediate_comes_back_from_permanent_storage() { assert_file "$archived/matrix.tsv.provenance" "record included" assert_file "$archived/bystander.txt" \ "and nothing else in permanent storage was carried off with them" -} - -# The stage the copy lands in belongs to the transfer, not to Analysis/Main. Left behind it -# would be read as an intermediate by the next `find` that walks Main. -test_a_copy_back_leaves_no_staging_directory() { - analysis_writer_ready || return - local status; status=$(analysis_run_module writer) - assert_status 0 "$status" "the first run should derive the intermediate" - - analysis_archive_main - analysis_folder_name "'from_storage'" - status=$(analysis_run_module writer) - assert_status 0 "$status" "the module should run against the archived intermediate" + # AND NO STAGING DIRECTORY SURVIVES IT. The stage the copy lands in belongs to the + # transfer, not to Analysis/Main; left behind it would be read as an intermediate by the + # next `find` that walks Main. + # + # Asserted here rather than in a case of its own, which repeated this whole setup for one + # line and had nothing in it proving the copy back had happened at all - so the count was + # over a directory that need never have existed. The assertions above are that proof. local leftovers - leftovers=$(find "$(analysis_main_dir)" -maxdepth 1 -name '.restore.*' 2>/dev/null | wc -l) + leftovers=$(find "$main" -maxdepth 1 -name '.restore.*' 2>/dev/null | wc -l) assert_eq "0" "$leftovers" "no staging directory survives the copy" }