Add pipeline level nf-tests for template #2992

adamrtalbot · 2024-05-21T19:09:07Z

This PR adds nf-test to the pipeline template, based on the work in nf-core/fetchngs and nf-core/rnaseq. Adds a simple default test and replaces the nextflow run command with nf-test test for more advanced testing. The default test just checks for a successful exit status, which should keep it lightweight and similar to existing tests.

see nf-core/website#2536 for related documentation to PR.

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

mashehu

We need the tooling (especially linting) in place before we can merge this into the template. Would also be nice if it would not break the CI for pipelines with nf-test, because this feels like a quite bullish push.

.github/workflows/create-test-wf.yml

Co-authored-by: Matthias Hörtenhuber <[email protected]>

nf_core/pipeline-template/tests/nextflow.config

.github/workflows/pytest.yml

.github/workflows/rich-codex.yml

Co-authored-by: Matthias Hörtenhuber <[email protected]>

maxulysse · 2024-05-21T19:45:50Z

nf_core/pipeline-template/tests/nextflow.config

+params {
+    // Base directory for nf-core/modules test data
+    modules_testdata_base_path = 's3://ngi-igenomes/testdata/nf-core/modules/'
+


Suggested change

pipelines_testdata_base_path = 's3://ngi-igenomes/testdata/nf-core/PIPELNE/'

Should be done on line 11

maxulysse

GOOD FOR ME as long as you deal with the pipelines_testdata_base_path params

adamrtalbot · 2024-05-21T19:55:48Z

Would also be nice if it would not break the CI for pipelines with nf-test

This shouldn't break any pipelines with existing nf-tests, in fact it should complement them pretty well. There may be merge conflicts but all of the existing nf-tested pipelines should be mostly compatible. I've checked fetchngs, rnaseq, sarek, methylseq and phageannotator which are as far as I'm aware the main nf-tested pipelines.

adamrtalbot · 2024-05-21T19:59:12Z

nf_core/pipeline-template/tests/default.main.nf.test

+
+    name "Test pipeline with default settings"
+    script "../main.nf"
+    tag  {{ name }}


@mashehu what is wrong with this jinja templating? From my understanding this should be a tag of the pipeline name but it is being interpreted as a closure.

adamrtalbot · 2024-05-21T20:11:17Z

We need the tooling (especially linting)

@mashehu what should we lint for here? We need to check that outdir = "$outputDir", but outside of that everything is reasonably flexible. Perhaps we could make sure profile "test" is default but I'm not sure that's always going to be true (i.e. weird pipelines). Tags are no longer required, naming can't be that strict. Should we just check for existence of files?

mashehu · 2024-05-22T03:28:00Z

Would also be nice if it would not break the CI for pipelines with nf-test

This shouldn't break any pipelines with existing nf-tests, in fact it should complement them pretty well. There may be merge conflicts but all of the existing nf-tested pipelines should be mostly compatible. I've checked fetchngs, rnaseq, sarek, methylseq and phageannotator which are as far as I'm aware the main nf-tested pipelines.

Sorry, I meant CI for pipelines without nf-test. Or do you think we can push easily every pipeline to switch to nf-test?

mashehu · 2024-05-22T03:30:41Z

We need the tooling (especially linting)

@mashehu what should we lint for here? We need to check that outdir = "$outputDir", but outside of that everything is reasonably flexible. Perhaps we could make sure profile "test" is default but I'm not sure that's always going to be true (i.e. weird pipelines). Tags are no longer required, naming can't be that strict. Should we just check for existence of files?

Yes, at least file existence and a minimum set of assertions. See linting for components.

mirpedrol · 2024-05-22T09:41:18Z

nf_core/pipeline-template/tests/default.main.nf.test

+
+    name "Test pipeline with default settings"
+    script "../main.nf"
+    // tag  {{ name }}


Was this failing? I tested it, and it seems to work, it's replaced by the pipeline name

update: I think it has to go between quotes tag "{{ name }}"

mirpedrol · 2024-05-22T10:02:41Z

nf_core/pipeline-template/tests/default.main.nf.test

+
+        then {
+            assertAll(
+                { assert workflow.success }


We should add "TODO nf-core" comments and link to documentation on how to assert different files, are more tests with other profiles, etc.

adamrtalbot · 2024-05-22T14:02:57Z

Or do you think we can push easily every pipeline to switch to nf-test?

I personally would prefer to push people to nf-test rather than nextflow run, because we can actually test the outputs. Replacing nextflow run with nf-test shouldn't be too hard, especially if we only test for pipeline success by default.

adamrtalbot · 2024-05-22T20:39:47Z

Yes, at least file existence and a minimum set of assertions. See linting for components.

@mashehu added quick linting check, will flesh it out later: 15ccd18

codecov · 2024-06-21T15:55:52Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 76.11%. Comparing base (edc9b41) to head (a483dca).

❗ Current head a483dca differs from pull request most recent head 2738965

Please upload reports for the commit 2738965 to get more accurate results.

Additional details and impacted files

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

adamrtalbot · 2024-07-09T17:08:39Z

fatal: unable to access 'https://gitlab.com/nf-core/modules-test.git/': Failed to connect to gitlab.com port 443 after 129691 ms: Connection timed out

Annoying.

mirpedrol

LGTM! sorry for the late review :)

adamrtalbot and others added 2 commits May 21, 2024 15:08

Add pipeline level nf-tests for template

b17664c

[automated] Update CHANGELOG.md

e71d283

adamrtalbot marked this pull request as draft May 21, 2024 19:11

mashehu reviewed May 21, 2024

View reviewed changes

.github/workflows/create-test-wf.yml Outdated Show resolved Hide resolved

Update .github/workflows/create-test-wf.yml

1eb5703

Co-authored-by: Matthias Hörtenhuber <[email protected]>

adamrtalbot commented May 21, 2024

View reviewed changes

nf_core/pipeline-template/tests/nextflow.config Show resolved Hide resolved

adamrtalbot added 3 commits May 21, 2024 15:31

Update nf_core/pipeline-template/tests/nextflow.config

17a59e8

Add nf-test to template test

44bef2c

Use specific version of nf-test installer GHA

de5074a

mashehu reviewed May 21, 2024

View reviewed changes

.github/workflows/pytest.yml Outdated Show resolved Hide resolved

mashehu reviewed May 21, 2024

View reviewed changes

.github/workflows/rich-codex.yml Outdated Show resolved Hide resolved

Apply suggestions from code review

7999631

Co-authored-by: Matthias Hörtenhuber <[email protected]>

maxulysse reviewed May 21, 2024

View reviewed changes

maxulysse approved these changes May 21, 2024

View reviewed changes

Use pipeline specific path for testing

42eecbe

adamrtalbot mentioned this pull request May 21, 2024

Update CI workflow for pipelines #2854

Open

adamrtalbot commented May 21, 2024

View reviewed changes

Comment out broken tag

f46da78

adamrtalbot added 6 commits May 21, 2024 16:15

CI fixes

0edfd44

Remove CD

f8ab7e6

Assume GHA reverts to previous WD

bbd64b3

Fix path changing for good?

0a999f4

Fix path changing for good?

9d9b228

combine steps

c042acc

mirpedrol reviewed May 22, 2024

View reviewed changes

mirpedrol mentioned this pull request May 22, 2024

Add pipeline nf-test tests #2994

Closed

12 tasks

adamrtalbot added 4 commits May 22, 2024 09:28

nf-test does not write things into tests dir

9e06d40

Merge branch 'dev' into add_pipeline_nf-tests_to_template

164ce92

quote pipeline name tag in nf-test

196538e

Remove recursive from mv command

d81da57

adamrtalbot added 2 commits May 22, 2024 10:17

Add TODO nf-core

a483dca

Add quick check for linting files

15ccd18

adamrtalbot marked this pull request as ready for review June 21, 2024 15:55

Merge branch 'dev' into add_pipeline_nf-tests_to_template

daffd6b

adamrtalbot added 4 commits June 26, 2024 14:15

Merge branch 'dev' into add_pipeline_nf-tests_to_template

6a5fbb3

Fix tests/nextflow.config path in linting

2738965

Merge branch 'dev' into add_pipeline_nf-tests_to_template

8665980

ci update

90e8b6d

adamrtalbot mentioned this pull request Jul 9, 2024

Add nf-test to pipeline template CI workflow #2881

Merged

4 tasks

adamrtalbot added 2 commits July 9, 2024 15:56

Merge branch 'dev' into add_pipeline_nf-tests_to_template

60a4700

Revert default ci.yml in template to replace with original CI workflow

629a4ce

adamrtalbot changed the base branch from dev to pipeline-nf-test July 15, 2024 13:25

Merge branch 'pipeline-nf-test' into add_pipeline_nf-tests_to_template

ae00415

mirpedrol approved these changes Aug 2, 2024

View reviewed changes

adamrtalbot merged commit 55d07ae into nf-core:pipeline-nf-test Aug 2, 2024

adamrtalbot deleted the add_pipeline_nf-tests_to_template branch August 2, 2024 08:56


	pipelines_testdata_base_path = 's3://ngi-igenomes/testdata/nf-core/PIPELNE/'

Add pipeline level nf-tests for template #2992

Add pipeline level nf-tests for template #2992

Uh oh!

Conversation

adamrtalbot commented May 21, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR checklist

Uh oh!

mashehu left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

maxulysse May 21, 2024

Choose a reason for hiding this comment

Uh oh!

adamrtalbot Jun 21, 2024

Choose a reason for hiding this comment

Uh oh!

maxulysse left a comment

Choose a reason for hiding this comment

Uh oh!

adamrtalbot commented May 21, 2024

Uh oh!

adamrtalbot May 21, 2024

Choose a reason for hiding this comment

Uh oh!

adamrtalbot commented May 21, 2024

Uh oh!

mashehu commented May 22, 2024

Uh oh!

mashehu commented May 22, 2024

Uh oh!

mirpedrol May 22, 2024

Choose a reason for hiding this comment

Uh oh!

mirpedrol May 22, 2024

Choose a reason for hiding this comment

Uh oh!

mirpedrol May 22, 2024

Choose a reason for hiding this comment

Uh oh!

adamrtalbot commented May 22, 2024

Uh oh!

adamrtalbot commented May 22, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented Jun 21, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

adamrtalbot commented Jul 9, 2024

Uh oh!

mirpedrol left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

adamrtalbot commented May 21, 2024 •

edited

Loading

adamrtalbot commented May 22, 2024 •

edited

Loading

codecov bot commented Jun 21, 2024 •

edited

Loading