Skip to content

FEAT: Refactor CI to only test modified files - #1123

Open
maxulysse wants to merge 3 commits into
masterfrom
refactor-ci-dynamic-testing
Open

FEAT: Refactor CI to only test modified files#1123
maxulysse wants to merge 3 commits into
masterfrom
refactor-ci-dynamic-testing

Conversation

@maxulysse

Copy link
Copy Markdown
Member

Description

Replace the hardcoded 154-profile test matrix with a dynamic detect-changes job that only tests profiles affected by the PR.

Inspired by nf-core/modules' approach with detect-nf-test-changes.

Changes

.github/workflows/main.yml

  • New detect-changes job: Uses git diff with fetch-depth: 0 between base and head to determine which files changed, then builds a JSON test matrix dynamically
  • Dynamic profile_test matrix: Instead of a hardcoded list of 154 profiles, the matrix is built from fromJson(needs.detect-changes.outputs.tests)
  • Smart test selection:
    • Push to master → test all profiles
    • Core infrastructure (main.yml, nextflow.config, nfcore_custom.config, configtest.nf, bin/) → test all profiles
    • conf/<name>.config changes → test only that profile
    • conf/sanger/farm22.config or conf/uppmax/rackham.config changes → test the parent profile
    • conf/pipeline/<pipeline>/<name>.config changes → test the matching pipeline profiles with -c pipeline/<pipeline>.config --custom_config_base ${GITHUB_WORKSPACE}
    • Only docs changed → skip tests entirely
  • Pipeline tests: New type: pipeline test steps that run nextflow run configtest.nf -profile <name> -c pipeline/<pipeline>.config --custom_config_base ${GITHUB_WORKSPACE}

bin/cchecker.py

  • Removed GitHub Actions matrix validation (no longer a hardcoded matrix)
  • Now validates internal consistency:
    • All profiles in nfcore_custom.config have config files in conf/
    • All config files in conf/ are registered in nfcore_custom.config
    • All pipeline-specific configs are referenced in pipeline/<pipeline>.config

Testing

This PR should be tested by creating a PR that modifies a single config file and verifying only that profile's test runs.

@maxulysse maxulysse changed the title Refactor CI to only test modified files FEAT: Refactor CI to only test modified files May 29, 2026

@jfy133 jfy133 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this needs somone else to look a bit more thoroughly too, but I guess I can follow the bash bits - but not the python stuff.

Don't forget to update docs on this Repo but also on the website docs to remove the requirement to modify the action file when merged!

fi

# Core infrastructure files that should trigger a full test
if echo "$CHANGED" | grep -qE '^(\.github/workflows/main\.yml|nextflow\.config|nfcore_custom\.config|configtest\.nf|bin/|pipeline/[^/]+\.config)$'; then

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bit ugly to hard code this into the code, is there not a way we can have a separate file that gets loaded here?

fi

# If only docs/README/etc changed, skip tests
if ! echo "$CHANGED" | grep -qvE '^(docs/|README\.md|CITATION\.cff|LICENSE|\.editorconfig|\.gitattributes|\.pre-commit-config\.yaml|\.prettierrc\.yml|\.prettierignore|\.github/(PULL_REQUEST_TEMPLATE\.md|CODEOWNERS|generate_codeowners\.sh))$'; then

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same above withteh harcdoing

fetch-depth: 0
- id: detect
run: |
set -euo pipefail

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a very very long bit of bash to be in the action itself could this not be moved into a standalone action thingy or a little script?

Comment thread bin/cchecker.py
### Check Github Config now
tests = set()
### Ignore these profiles
ignore_me = ["czbiohub_aws"]

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this why it's now failing and not before? Why was this being ignored in teh first place?

@maxulysse

Copy link
Copy Markdown
Member Author

I think this needs somone else to look a bit more thoroughly too, but I guess I can follow the bash bits - but not the python stuff.

Don't forget to update docs on this Repo but also on the website docs to remove the requirement to modify the action file when merged!

yeah, sorry, this is still a draft.
I wanted prek to be merged before continuing onto that one

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants