Fix CI: Ensure confirm-pass job runs for markdown-only PRs #1618
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The nf-test workflow used paths-ignore at the workflow level, which prevented the entire workflow (including the confirm-pass job) from running when only docs/markdown files were changed. This caused PRs with only documentation changes to fail merge requirements if confirm-pass was configured as a required status check. Changes: - Remove paths-ignore from workflow trigger to ensure workflow always runs - Add dorny/paths-filter action to detect docs/markdown-only changes - Make nf-test job conditional on skip_tests output - Update confirm-pass job to always run and pass when tests are skipped - Add skip_tests output to nf-test-changes job This ensures the confirm-pass required check is always present while still skipping expensive test execution for documentation-only changes. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
This PR is against the
|
🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
|
The default 'some' quantifier returns true if ANY file matches ANY pattern, which incorrectly skips tests when a PR has mixed changes (e.g., both README.md and src/main.nf). Using 'every' ensures ALL files must match at least one pattern for tests to be skipped. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
This commit modifies nextflow.config to test that code changes properly trigger the full nf-test suite with the new CI configuration. This validates that the paths-filter correctly identifies non-documentation changes. This commit will be reverted after CI runs. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
This reverts commit 72faa29.
Copilot finished reviewing on behalf of
pinin4fjords
November 13, 2025 14:45
There was a problem hiding this comment.
Pull Request Overview
This PR fixes a CI configuration issue where pull requests containing only markdown or documentation changes could not be merged because the required confirm-pass status check didn't run. The solution removes workflow-level paths-ignore filters and instead uses conditional job execution with explicit handling for documentation-only changes.
Key changes:
- Replaced workflow-level
paths-ignorewithdorny/paths-filteraction usingpredicate-quantifier: 'every'to detect when all changed files are documentation - Made test execution conditional on
skip_testsoutput to avoid running expensive tests for documentation-only PRs - Enhanced
confirm-passjob to always run and explicitly handle both test execution and skip scenarios
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
.github/workflows/nf-test.yml |
Core workflow changes: removed paths-ignore, added dorny/paths-filter with conditional job execution, and enhanced confirm-pass job to handle skipped test scenarios |
CHANGELOG.md |
Added entry documenting the CI fix |
.nf-core.yml |
Disabled actions_nf_test lint rule to accommodate the custom workflow implementation |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
edmundmiller
requested changes
Nov 14, 2025
Contributor
edmundmiller
left a comment
edmundmiller
left a comment
There was a problem hiding this comment.
We should make this change at the GitHub Actions level.
I hadn't thought of this edge case before or no one's complained because no one else is making docs only PRs 😉
Comment on lines
-4
to
-9
| paths-ignore: | ||
| - "docs/**" | ||
| - "**/meta.yml" | ||
| - "**/*.md" | ||
| - "**/*.png" | ||
| - "**/*.svg" |
Contributor
There was a problem hiding this comment.
Is there a reason you're changing this?
Contributor
There was a problem hiding this comment.
Oh I see now.
Let's remove the' paths-ignore' part and the on: file-specific sections if that's not working for your docs PRs.
The reason we did that was to avoid spinning up a job just to check on those, but it makes sense if you're running into an actual CI requirement, and because this is RNAseq.
This was referenced Nov 14, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Fixes the CI configuration issue where PRs with only markdown/documentation changes cannot be merged because the
confirm-passrequired status check doesn't run.Problem
The
nf-testworkflow usedpaths-ignoreat the workflow level, which prevented the entire workflow (including theconfirm-passjob) from running when only docs/markdown files were changed. Ifconfirm-passis configured as a required status check in branch protection rules, these PRs cannot be merged.This affects PRs like #1613 which only modify documentation files.
Solution
Key Changes
paths-ignorefrom workflow trigger: The workflow now always runs for every PRdorny/paths-filteraction: Detects when only docs/markdown files changedpredicate-quantifier: 'every': Ensures ALL changed files must match ignore patterns for tests to be skipped (prevents skipping tests when PR has mixed doc+code changes)nf-testjob only runs whenskip_tests != 'true'confirm-passjob: Always runs and handles both test execution and skip scenariosWorkflow Behavior Scenarios
Scenario 1: Markdown-Only PR (e.g., only
docs/usage.mdandCHANGELOG.md)nf-test-changesjob runs → detects all files match ignore patterns → outputsskip_tests='true'nf-testjob is SKIPPED (condition fails:skip_tests != 'true')confirm-passjob RUNS (always) → first step executes → PASSES ✅Scenario 2: Code-Only PR (e.g., only
main.nformodules/)nf-test-changesjob runs → detects non-ignored files → outputsskip_tests='false', calculates shardsnf-testjob RUNS all test matrices (docker, conda, singularity)confirm-passjob RUNS (always) → validates test results → PASSES or FAILS based on tests ✅/❌Scenario 3: Mixed Changes PR (e.g.,
README.md+main.nf)nf-test-changesjob runs → detectsmain.nfdoesn't match ignore patterns → outputsskip_tests='false'nf-testjob RUNS full test suite (code was changed)confirm-passjob RUNS → validates test results ✅/❌Scenario 4: Edge Case - No Tests Found (shards = 0, but code changed)
nf-test-changesjob runs → outputsskip_tests='false',total_shards='0'nf-testjob is SKIPPED (condition fails:total_shards != '0')confirm-passjob RUNS → detectsnf-testwas skipped → checks for success innf-test-changes→ PASSES ✅Technical Details
Modified File
.github/workflows/nf-test.yml:paths-ignorefrom workflow trigger (lines 2-9)dorny/paths-filter@v3withpredicate-quantifier: 'every'(lines 42-52)skip_testsoutput tonf-test-changesjob (line 30)skip_tests != 'true'(line 55)nf-testjob condition to checkskip_tests(line 72)confirm-passjob to:nf-test-changesandnf-test(line 125)Why
predicate-quantifier: 'every'is CriticalWithout this setting, the default
'some'quantifier would returnskip_tests='true'if ANY file matches ANY ignore pattern. This would incorrectly skip tests for PRs that change both docs AND code.With
'every', all files must match at least one ignore pattern:README.mdalone → all files match → skip testsmain.nfalone → not all files match → run testsREADME.md+main.nf→ not all files match → run testsTest Plan
confirm-passcheck appears and passes for markdown-only changesconfirm-passcorrectly validates test results for code changes🤖 Generated with Claude Code