Skip to content

template - basic pipeline level nf-test tests#3469

Merged
mirpedrol merged 34 commits into
nf-core:devfrom
maxulysse:nf-test-pipeline
Mar 21, 2025
Merged

template - basic pipeline level nf-test tests#3469
mirpedrol merged 34 commits into
nf-core:devfrom
maxulysse:nf-test-pipeline

Conversation

@maxulysse

@maxulysse maxulysse commented Feb 26, 2025

Copy link
Copy Markdown
Member

Related updated docs: nf-core/website#3216

PR checklist

  • This comment contains a description of changes (with reason)
  • CHANGELOG.md is updated
  • If you've fixed a bug or added code that should be tested, add tests!
  • Documentation in docs is updated

Comment thread CHANGELOG.md Outdated

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

we should not merge this without a corresponding PR with docs

@maxulysse

Copy link
Copy Markdown
Member Author

we should not merge this without a corresponding PR with docs

That's on the way

@maxulysse

Copy link
Copy Markdown
Member Author

Thinking that people might not want to use nf-test, so we should have a flag for this.
And then have in the linting either ci.yml or nf-test.yml, but not both as the default test should be -profile test

@maxulysse maxulysse requested a review from mashehu February 26, 2025 14:42
Comment thread nf_core/pipeline-template/.github/workflows/nf-test.yml Outdated
Comment thread nf_core/pipeline-template/nf-test.config Outdated
Comment thread nf_core/pipeline-template/nf-test.config Outdated
Comment thread nf_core/pipeline-template/tests/.nftignore
Comment thread nf_core/pipeline-template/tests/.nftignore
@mashehu mashehu dismissed their stale review February 27, 2025 09:24

docs PR is available

Comment thread nf_core/pipeline-template/.github/workflows/nf-test.yml Outdated
mashehu and others added 5 commits February 27, 2025 11:03
Comment thread nf_core/pipeline-template/.github/actions/nf-test/action.yml
Comment thread nf_core/pipeline-template/.github/actions/nf-test/action.yml
Comment thread nf_core/pipeline-template/.github/workflows/nf-test.yml Outdated
Comment thread nf_core/pipeline-template/.github/workflows/nf-test.yml Outdated
Comment thread nf_core/pipeline-template/tests/.nftignore Outdated
Comment thread nf_core/pipeline-template/tests/nextflow.config Outdated
Comment thread nf_core/pipelines/create/template_features.yml
- ".github/workflows/awsfulltest.yml"
- ".github/workflows/awstest.yml"
- ".github/workflows/ci.yml"
- ".github/workflows/nf-test.yml"

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.

when skipping tests, we should also skip nf-test.config and the tests directory

- "conf/test.config"
- "conf/test_full.config"
- ".github/workflows/ci.yml"
- ".github/workflows/nf-test.yml"

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.

skip nf-test files

.github/workflows/ci.yml
.github/workflows/nf-test.yml
.github/actions/get-shards/action.yml
.github/actions/nf-test/action.yml

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.

We should also lint for nf-test files

@mirpedrol

Copy link
Copy Markdown
Member

Thinking that people might not want to use nf-test, so we should have a flag for this.
And then have in the linting either ci.yml or nf-test.yml, but not both as the default test should be -profile test

I would say if we move to nf-test that should be the only way to test. There could be an option to skip testing, but if you test, it has to be with nf-test

@mashehu

mashehu commented Feb 27, 2025

Copy link
Copy Markdown
Contributor

💯 i guess the difference to before is now that nf-test adds some files to the template and given some people want a very bare-bone structure, we should probably have a skip feature.

@maxulysse

Copy link
Copy Markdown
Member Author

Needs some more improvments, I'm trying it out in nf-core/rnavar#178, which uncovers some issue I didn't find in sarek.
sorry about that

Co-authored-by: Júlia Mir Pedrol <mirp.julia@gmail.com>
@mashehu mashehu marked this pull request as draft February 28, 2025 19:46
@maxulysse

Copy link
Copy Markdown
Member Author
  • also update link to badge in README

Comment thread nf_core/pipeline-template/nf-test.config
Comment thread nf_core/pipeline-template/.github/workflows/nf-test.yml Outdated
shard: ${{ fromJson(needs.nf-test-changes.outputs.shard) }}
profile: [conda, docker, singularity]
isMain:
- ${{ github.base_ref == 'master' || github.base_ref == 'main' }}

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

we can think of updating this test when we fully remove master

Comment thread nf_core/pipeline-template/tests/nextflow.config

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

only one last comment, otherwise LGTM 🚀

Comment thread nf_core/pipeline-template/.github/workflows/nf-test.yml Outdated
maxulysse and others added 2 commits March 20, 2025 12:58
@maxulysse maxulysse marked this pull request as ready for review March 20, 2025 12:00
@mirpedrol

Copy link
Copy Markdown
Member

I can take care of fixing the failing test and then merging 👍

@codecov

codecov Bot commented Mar 21, 2025

Copy link
Copy Markdown

Codecov Report

Attention: Patch coverage is 88.88889% with 1 line in your changes missing coverage. Please review.

Project coverage is 76.87%. Comparing base (96097ac) to head (35befd4).
Report is 35 commits behind head on dev.

Files with missing lines Patch % Lines
nf_core/pipelines/lint/actions_nf_test.py 88.88% 1 Missing ⚠️

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@mirpedrol mirpedrol enabled auto-merge March 21, 2025 11:54
@mirpedrol mirpedrol merged commit e872751 into nf-core:dev Mar 21, 2025
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.

4 participants