Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improved nf test scope docs to describe how portable nf-core/modules 'tests' should be #2876

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

jfy133
Copy link
Member

@jfy133 jfy133 commented Dec 3, 2024

As discussed here: https://nfcore.slack.com/archives/C049MBCEW06/p1731521149600189

@netlify /docs/guidelines/components/modules

Copy link

netlify bot commented Dec 3, 2024

Deploy Preview for nf-core-main-site ready!

Name Link
🔨 Latest commit 8abc713
🔍 Latest deploy log https://app.netlify.com/sites/nf-core-main-site/deploys/675028bf80a05e0008f4e38c
😎 Deploy Preview https://deploy-preview-2876--nf-core-main-site.netlify.app/docs/guidelines/components/modules
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link

netlify bot commented Dec 3, 2024

Deploy Preview for nf-core-docs ready!

Name Link
🔨 Latest commit 8abc713
🔍 Latest deploy log https://app.netlify.com/sites/nf-core-docs/deploys/675028bf0934be00084637fe
😎 Deploy Preview https://deploy-preview-2876--nf-core-docs.netlify.app/docs/guidelines/components/modules
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@jfy133
Copy link
Member Author

jfy133 commented Dec 3, 2024

@christopher-hakkaart as always language clean up would be grand if you have a moment <3


Tests for modules SHOULD be be designed to be able to execute within the nf-core/modules GitHub repository CI with example test data.

Tests for modules MUST at a minimum run on the GitHub repository CI with a stub test that replicates the generation of (empty) output files.
Copy link
Contributor

Choose a reason for hiding this comment

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

and version snapshot

Copy link
Member

@mirpedrol mirpedrol left a comment

Choose a reason for hiding this comment

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

👍

Co-authored-by: Júlia Mir Pedrol <[email protected]>
@MatthiasZepper
Copy link
Member

MatthiasZepper commented Dec 3, 2024

In that discussion, there was a quite neat proposal: Using dedicated tags to distinguish the two: tag "tethered" and tag "portable" for example. (alternatively also tag "rooted" and tag "detached").

In that case, nf-core modules install could just skip the tests tagged as tethered, respectively rooted.

Otherwise, tests from the modules' repo would need to be manually deleted in the pipeline repositories after every run of nf-core modules update.

Copy link
Member

@christopher-hakkaart christopher-hakkaart left a comment

Choose a reason for hiding this comment

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

No major objections.

Comments are suggestions that I think help improve clarity but feel free to disagree.

@mirpedrol
Copy link
Member

Otherwise, tests from the modules' repo would need to be manually deleted in the pipeline repositories after every run of nf-core modules update.

You can use nf-core modules patch when removing the tests. However, this wouldn't be my preferred solution either. I think modules and subowrkflows tests should be run on the modules repo, we don't need to rerun them with pipeline tests.

Co-authored-by: Christopher Hakkaart <[email protected]>
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.

6 participants