From e90af7f3c805df3e00a3fb29558716210f04fbf0 Mon Sep 17 00:00:00 2001 From: "Jonathan D.A. Jewell" <6759885+hyperpolymath@users.noreply.github.com> Date: Tue, 22 Sep 2026 01:28:03 +0100 Subject: [PATCH 1/2] fix(ci): install pinned fastqc and multiqc so prefilter QC is exercised CI installed four of the six tools the pipeline shells out to. FastQC and MultiQC were pinned in config/defaults/tool_versions.yml and declared required by src/core/validate.jl, but nothing on the runner ever installed them, so the prefilter QC stage, its flag plumbing out of pipeline.yml and its provenance capture had no automated coverage at all. Closes #30. Both pins are read at run time like every other tool. MultiQC comes from PyPI through the same pip path as cutadapt. FastQC is fetched from the pinned URL and refused unless it hashes to the pinned SHA256; it is unpacked to /opt rather than moved as a single file, because its launcher is a Perl script that resolves its jars relative to its own location, and reached through a symlink on PATH, which is what config/ci/tools.yml means by `path: "fastqc"`. The launcher runs `java` off PATH, so `fastqc --version` proves both that the install landed and that a JRE is present on the runner. Criterion 1 needed no change: both tools were already pinned correctly, FastQC with a SHA256 that was verified against a real download of the pinned URL. The skip is made loud rather than merely fixed. The live-probe loop in test_provenance.jl now gives each tool its own named testset, so the suite's own summary names all six and says per tool whether the probe ran; an absent tool is recorded broken on a bare checkout and fails, naming itself, when CI is set, because ci.yml is supposed to have installed it. The integration test runs the QC stage over the raw reads and asserts both logs exist, are non-empty, and carry the invocation marker that is the provenance record. The guard that asserts CI installs every tool went in vacuous and was caught by deleting the install step: matching on the tool's name passes with the step gone, because the pin-export block names every tool whether or not anything installs it. It now matches the dereference of each exported pin, which only an install step contains, and deleting either new step reddens it. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01X3hgXxWm6umMgZkjYyHnnm --- .github/workflows/ci.yml | 28 +++++++++++++++++++++++ test/integration/test_pipeline.jl | 21 +++++++++++++++++- test/unit/test_install_pins.jl | 32 +++++++++++++++++++++++++- test/unit/test_provenance.jl | 37 ++++++++++++++++++++++++------- 4 files changed, 108 insertions(+), 10 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index cce2d161..8bb12e04 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -185,6 +185,15 @@ jobs: println(io, "R_APT_VERSION=", pins["runtimes"]["r"]["apt_version"]) println(io, "BUN_VERSION=", pins["toolchain"]["bun"]["version"]) println(io, "CUTADAPT_SPEC=cutadapt==", pins["tools"]["cutadapt"]["version"]) + println(io, "MULTIQC_SPEC=multiqc==", pins["tools"]["multiqc"]["version"]) + # FastQC ships one platform-neutral zip, so its archive hangs off the + # "any" key rather than a per-platform one, and it cannot join the loop + # below. test_install_pins.jl asserts that shape, so a pin file that + # grew a linux-x86_64 fastqc entry would fail there before it failed here. + fastqc = pins["tools"]["fastqc"]["archives"]["any"] + println(io, "FASTQC_VERSION=", pins["tools"]["fastqc"]["version"]) + println(io, "FASTQC_URL=", fastqc["url"]) + println(io, "FASTQC_SHA256=", fastqc["sha256"]) for tool in ("vsearch", "swarm") archive = pins["tools"][tool]["archives"]["linux-x86_64"] println(io, uppercase(tool), "_VERSION=", pins["tools"][tool]["version"]) @@ -323,6 +332,25 @@ jobs: rm -rf swarm.tar.gz "swarm-$SWARM_VERSION-linux-x86_64" swarm --version + - name: Install multiqc + run: pip install "$MULTIQC_SPEC" + + # FastQC is a Perl launcher round a Java jar, so it is installed as a directory + # rather than a single binary: the launcher resolves its jars relative to its own + # location, and moving it out of FastQC/ breaks it. Unpacked to /opt and reached + # through a symlink, which is what config/ci/tools.yml means by `path: "fastqc"`. + # The launcher runs `java` off PATH, so `fastqc --version` proves both that the + # install landed and that a JRE is present on the runner. + - name: Install fastqc + run: | + curl -sSL -o fastqc.zip "$FASTQC_URL" + echo "$FASTQC_SHA256 fastqc.zip" | sha256sum -c - + sudo unzip -q -d /opt fastqc.zip + rm -f fastqc.zip + sudo chmod +x /opt/FastQC/fastqc + sudo ln -sf /opt/FastQC/fastqc /usr/local/bin/fastqc + fastqc --version + - name: Instantiate Julia project run: julia --project=. -e 'import Pkg; Pkg.instantiate()' diff --git a/test/integration/test_pipeline.jl b/test/integration/test_pipeline.jl index a11e7c58..27cce31b 100644 --- a/test/integration/test_pipeline.jl +++ b/test/integration/test_pipeline.jl @@ -52,7 +52,7 @@ if isdir(test_study_dir) for entry in readdir(test_study_dir; join=true) isdir(entry) || continue - for subdir in ("cutadapt", "dada2", "swarm", "vsearch", "merged") + for subdir in ("QC", "cutadapt", "dada2", "swarm", "vsearch", "merged") d = joinpath(entry, subdir) isdir(d) && rm(d; recursive=true) end @@ -103,6 +103,25 @@ continue end + # Stage 0: FastQC/MultiQC over the raw reads. The server runs this as an + # independent stage rather than a link in the chain, so nothing downstream + # consumes it -- which is exactly why it needs asserting here. Until issue #30 + # CI installed neither tool, so the whole prefilter QC stage, its flag plumbing + # out of pipeline.yml and its provenance capture had no coverage at all. + multiqc(project) + qc_dir = joinpath(project.dir, "QC") + @test isfile(joinpath(qc_dir, "multiqc_report.html")) + for logname in ("fastqc.log", "multiqc.log") + p = joinpath(qc_dir, "logs", logname) + @test isfile(p) + # Non-empty AND carrying the invocation marker. An empty log is what a stage + # that made its log directory and then died leaves behind, and it would + # satisfy `isfile` alone; the marker line is the provenance record itself, + # the same one the DADA2 assertions below grep for. + @test filesize(p) > 0 + @test any(l -> startswith(l, "[MetaManifold] cmd: "), readlines(p)) + end + # Stage 1: cutadapt trimmed = cutadapt(project) @test isdir(trimmed.dir) diff --git a/test/unit/test_install_pins.jl b/test/unit/test_install_pins.jl index 9344b6a2..c8e86087 100644 --- a/test/unit/test_install_pins.jl +++ b/test/unit/test_install_pins.jl @@ -194,9 +194,39 @@ end @test occursin("tool_versions.yml", runs) @test occursin("sha256sum -c", runs) - for tool in ("vsearch", "swarm") + # fastqc and multiqc joined this list with issue #30. Leaving them out would + # have made the guard vacuous for exactly the two tools it had just been + # extended to cover -- the shape that let the gap open in the first place. + for tool in ("vsearch", "swarm", "fastqc", "multiqc") @test !occursin(pins["tools"][tool]["version"], runs) end + + # And the gap itself: every tool the pipeline shells out to must actually be + # installed by the workflow, which is what issue #30 found missing for fastqc + # and multiqc. + # + # The assertion is on the DEREFERENCE of each exported pin, not on the tool's + # name. Written the obvious way -- does "fastqc" appear anywhere in the run + # blocks -- this guard passes with the install step deleted, because the + # pin-export block above mentions every tool by name whether or not anything + # installs it. That version was written here, and deleting the fastqc step did + # not redden it. `$FASTQC_URL` appears only where a step consumes the pin. + for (tool, ref) in (("cutadapt", "\$CUTADAPT_SPEC"), + ("multiqc", "\$MULTIQC_SPEC"), + ("fastqc", "\$FASTQC_URL"), + ("vsearch", "\$VSEARCH_URL"), + ("swarm", "\$SWARM_URL")) + @test occursin(ref, runs) + end + + # The two archive tools that are not a single binary still have to end up on + # PATH, because config/ci/tools.yml resolves every tool by bare name. + for tool in ("vsearch", "swarm", "fastqc") + @test occursin("/usr/local/bin/$tool", runs) + end + + # cd-hit is the one tool CI does not pin, so it is matched on the apt line. + @test occursin("apt-get install -y cd-hit", runs) end @testset "a required check name is a stable identifier" begin diff --git a/test/unit/test_provenance.jl b/test/unit/test_provenance.jl index 6f13306a..42e66205 100644 --- a/test/unit/test_provenance.jl +++ b/test/unit/test_provenance.jl @@ -424,16 +424,37 @@ canned(out::String, err::String = "", code::Int = 0) = ## Live probes. Gated, so that the suite passes on a machine with neither the # binaries nor the R packages installed. @testset "live probes of what is actually installed" begin + # The gate is not the same question on CI as on a laptop. ci.yml installs every + # tool in TOOL_PROBES, so an absent binary there is a provisioning regression; + # on a bare checkout it is expected. Issue #30 was this loop skipping fastqc and + # multiqc quietly for months because CI installed neither: an `@info` scrolls + # past, and a zero-failure count says nothing about which probes ran at all. + tools_must_be_present = get(ENV, "CI", "false") == "true" + for (key, probe) in PV.TOOL_PROBES - bin = PV.default_bin_resolver(key) - if isnothing(Sys.which(bin)) - @info "Provenance: skipping live probe of $key, which is not installed" - continue + # One testset per tool, so the suite's own summary names all six and shows + # per tool whether it ran, was skipped, or failed. That naming is the + # assertion; a flat loop can only report the tools it did not skip. + @testset "$key" begin + bin = PV.default_bin_resolver(key) + resolved = Sys.which(bin) + + if tools_must_be_present + # Named by the enclosing testset, so a red line reads "fastqc". + @test resolved !== nothing + end + + if isnothing(resolved) + # Broken, not passed: it shows in the summary's own column rather + # than disappearing into the count of things that went fine. + @test_skip PV.probe_tool(probe) + else + record = PV.probe_tool(probe) + @test !isempty(record.version) + @test isabspath(record.path) + @test length(record.sha256) == 64 + end end - record = PV.probe_tool(probe) - @test !isempty(record.version) - @test isabspath(record.path) - @test length(record.sha256) == 64 end r_ready = try From 21218facc68eb47d7305b60b6853e9d2c65467d3 Mon Sep 17 00:00:00 2001 From: "Jonathan D.A. Jewell" <6759885+hyperpolymath@users.noreply.github.com> Date: Tue, 22 Sep 2026 01:34:38 +0100 Subject: [PATCH 2/2] fix(test): state shell pin refs as raw strings, not escaped dollars The guard added in e90af7f asserts that each exported pin is dereferenced by a step that consumes it, and wrote those references as "\$FASTQC_URL". config/ci/lint_source.jl rejects `\$` in an interpolating string, and that step is fail-fast at position 5 of the job -- so the whole Julia test job never ran and the new install steps were never exercised on CI at all. raw"$FASTQC_URL" carries identical bytes and says the right thing: this is a shell variable reference to be found verbatim in the YAML, never a Julia interpolation. The lint exempts raw strings for exactly that reason. Re-verified: source lint clean; install pins 113/113; and deleting the Install fastqc step still reddens it at lines 223 and 229, so the guard is still connected to what it guards. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01X3hgXxWm6umMgZkjYyHnnm --- test/unit/test_install_pins.jl | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/test/unit/test_install_pins.jl b/test/unit/test_install_pins.jl index c8e86087..fe6eadc1 100644 --- a/test/unit/test_install_pins.jl +++ b/test/unit/test_install_pins.jl @@ -211,11 +211,15 @@ end # pin-export block above mentions every tool by name whether or not anything # installs it. That version was written here, and deleting the fastqc step did # not redden it. `$FASTQC_URL` appears only where a step consumes the pin. - for (tool, ref) in (("cutadapt", "\$CUTADAPT_SPEC"), - ("multiqc", "\$MULTIQC_SPEC"), - ("fastqc", "\$FASTQC_URL"), - ("vsearch", "\$VSEARCH_URL"), - ("swarm", "\$SWARM_URL")) + # raw"..." rather than "\$...": the string is a shell variable reference to be + # found verbatim in the YAML, never a Julia interpolation, and config/ci/lint_source.jl + # rejects an escaped `\$` in an interpolating string precisely to force that + # distinction to be stated. The two forms carry the same bytes; only one says why. + for (tool, ref) in (("cutadapt", raw"$CUTADAPT_SPEC"), + ("multiqc", raw"$MULTIQC_SPEC"), + ("fastqc", raw"$FASTQC_URL"), + ("vsearch", raw"$VSEARCH_URL"), + ("swarm", raw"$SWARM_URL")) @test occursin(ref, runs) end