Skip to content

Improvement to nf-tests for 2.1.0#630

Open
jscgh wants to merge 4 commits into
devfrom
pr-ci-space-changes
Open

Improvement to nf-tests for 2.1.0#630
jscgh wants to merge 4 commits into
devfrom
pr-ci-space-changes

Conversation

@jscgh

@jscgh jscgh commented May 29, 2026

Copy link
Copy Markdown
Contributor

Resolves #365

  • Stub runs now use real module containers, snapshots receive true version info
  • Fixed stub run report filenames using meta.id and meta.model (solves filename clash failure)
  • GitHub actions now shard more efficiently and request suitable volume size for pulling containers

PR checklist

  • This comment contains a description of changes (with reason).
  • Make sure your code lints (nf-core pipelines lint).
  • Ensure the test suite passes (nextflow run . -profile test,docker --outdir <OUTDIR>).
  • Check for unexpected warnings in debug mode (nextflow run . -profile debug,test,docker --outdir <OUTDIR>).
  • CHANGELOG.md is updated.

@jscgh jscgh added this to the 2.1.0 milestone May 29, 2026
@jscgh jscgh self-assigned this May 29, 2026
@github-actions

github-actions Bot commented May 29, 2026 •
edited
Loading

Copy link
Copy Markdown

nf-core pipelines lint overall result: Passed ✅ ⚠️

Posted for pipeline commit fad17c1

+| ✅ 351 tests passed       |+
#| ❔   4 tests were ignored |#
!| ❗  31 tests had warnings |!
Details

❗ Test warnings:

  • files_exist - File not found: conf/igenomes.config
  • files_exist - File not found: conf/igenomes_ignored.config
  • pipeline_todos - TODO string in main.nf: Optionally add in-text citation tools to this list.
  • pipeline_todos - TODO string in main.nf: Optionally add bibliographic entries to this list.
  • pipeline_todos - TODO string in main.nf: Only uncomment below if logic in toolCitationText/toolBibliographyText has been filled!
  • pipeline_todos - TODO string in awsfulltest.yml: You can customise AWS full pipeline tests as required
  • pipeline_todos - TODO string in nextflow.config: Specify any additional parameters here
  • pipeline_todos - TODO string in methods_description_template.yml: #Update the HTML below to your preferred methods description, e.g. add publication citation for this pipeline
  • schema_description - No description provided in schema for parameter: rosettafold2na_uniref30_link
  • schema_description - No description provided in schema for parameter: rosettafold2na_bfd_link
  • schema_description - No description provided in schema for parameter: rosettafold2na_pdb100_link
  • schema_description - No description provided in schema for parameter: rosettafold2na_weights_link
  • schema_description - No description provided in schema for parameter: rfam_full_region_link
  • schema_description - No description provided in schema for parameter: rfam_cm_link
  • schema_description - No description provided in schema for parameter: rnacentral_rfam_annotations_link
  • schema_description - No description provided in schema for parameter: rnacentral_id_mapping_link
  • schema_description - No description provided in schema for parameter: rnacentral_sequences_link
  • schema_description - No description provided in schema for parameter: rosettafold2na_uniref30_path
  • schema_description - No description provided in schema for parameter: rosettafold2na_bfd_path
  • schema_description - No description provided in schema for parameter: rosettafold2na_pdb100_path
  • schema_description - No description provided in schema for parameter: rosettafold2na_weights_path
  • local_component_structure - prepare_alphafold2_dbs.nf in subworkflows/local should be moved to a SUBWORKFLOW_NAME/main.nf structure
  • local_component_structure - prepare_rosettafold2na_dbs.nf in subworkflows/local should be moved to a SUBWORKFLOW_NAME/main.nf structure
  • local_component_structure - prepare_esmfold_dbs.nf in subworkflows/local should be moved to a SUBWORKFLOW_NAME/main.nf structure
  • local_component_structure - post_processing.nf in subworkflows/local should be moved to a SUBWORKFLOW_NAME/main.nf structure
  • local_component_structure - prepare_alphafold3_dbs.nf in subworkflows/local should be moved to a SUBWORKFLOW_NAME/main.nf structure
  • local_component_structure - prepare_rosettafold_all_atom_dbs.nf in subworkflows/local should be moved to a SUBWORKFLOW_NAME/main.nf structure
  • local_component_structure - aria2_uncompress.nf in subworkflows/local should be moved to a SUBWORKFLOW_NAME/main.nf structure
  • local_component_structure - prepare_colabfold_dbs.nf in subworkflows/local should be moved to a SUBWORKFLOW_NAME/main.nf structure
  • local_component_structure - prepare_helixfold3_dbs.nf in subworkflows/local should be moved to a SUBWORKFLOW_NAME/main.nf structure
  • local_component_structure - prepare_boltz_dbs.nf in subworkflows/local should be moved to a SUBWORKFLOW_NAME/main.nf structure

❔ Tests ignored:

✅ Tests passed:

Run details

  • nf-core/tools version 4.0.2
  • Run at 2026-05-29 02:43:42

@jscgh jscgh mentioned this pull request May 29, 2026
Merged
7 tasks
@jscgh jscgh requested a review from JoseEspinosa May 29, 2026 05:59
tlitfin
tlitfin approved these changes May 31, 2026
View reviewed changes

@tlitfin tlitfin left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🚀 - Great change on path to adding real tests. Worth checking with @JoseEspinosa before merging due to the slight change in runner resourcing but otherwise LGTM.

JoseEspinosa
JoseEspinosa reviewed Jun 2, 2026
View reviewed changes

@JoseEspinosa JoseEspinosa 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 have mixed feelings with this PR. I think it is nice to catch things like broken containers, missing tools, or version mismatches that could silently cause downstream failures but on the other hand, I wonder whether the value justifies the CI cost, as the tests are run with any commit added to a PR...

JoseEspinosa
JoseEspinosa reviewed Jun 3, 2026
View reviewed changes

@JoseEspinosa JoseEspinosa 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.

Actually, it might be nice to have the opinion of someone from infrastructure, maybe @mashehu could chip in and provide his opinion 🙏

@tlitfin

tlitfin commented Jun 3, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Comparing the old workflow with the new one, pulling real containers increases total walltime by ~4x. One potential compromise would be to only run the workflow with latest-everything to cut this in half.

I am also suspicious that we may be always running every test even for things not affected by the PR. If we could solve this issue it would also likely reduce our load on the runners.

@tlitfin

tlitfin commented Jun 19, 2026

Copy link
Copy Markdown
Contributor

@jscgh also had a nice suggestion that we could run the actions workflow after PR approval if we are concerned about resourcing.

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

Reviewers

@JoseEspinosa JoseEspinosa JoseEspinosa left review comments

@tlitfin tlitfin tlitfin approved these changes

Assignees

@jscgh jscgh

Labels

Ready for review

Projects

None yet

Milestone

2.1.0

Development

Successfully merging this pull request may close these issues.

3 participants

@jscgh @tlitfin @JoseEspinosa