-
Notifications
You must be signed in to change notification settings - Fork 74
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
Replace tags strategy with implicit 'tags in test' strategy #253
Changes from 2 commits
61e7662
34baed6
a6c34fc
76095d3
c325d8d
fe432bc
bcae51a
b1b117d
7d88962
c36a8aa
412b059
2f6a78a
f28a336
401f91d
6cdff1c
d490e6a
00c2ea0
4d2d566
6c00433
bc25627
8946e2d
cb4f0e6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
This file was deleted.
This file was deleted.
This file was deleted.
This file was deleted.
This file was deleted.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
This file was deleted.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
This file was deleted.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
This file was deleted.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
This file was deleted.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
This file was deleted.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
This file was deleted.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
This file was deleted.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,8 +2,10 @@ nextflow_pipeline { | |
|
||
name "Test pipeline" | ||
script "../main.nf" | ||
tag "pipeline" | ||
tag "pipeline_fetchngs" | ||
|
||
tag "SRA" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why the SRA tag here? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's the immediate dependency of the pipeline. Perhaps we just run pipeline level tests at this point? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. OK I've removed it now. It will not test the workflow tests when we launch pipeline level tests anymore. We can put it back if that behaviour feels weird. |
||
|
||
tag "FETCHNGS" | ||
adamrtalbot marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
test("Run with profile test") { | ||
|
||
|
This file was deleted.
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -3,10 +3,16 @@ nextflow_workflow { | |||||
name "Test workflow: sra/main.nf" | ||||||
script "../main.nf" | ||||||
workflow "SRA" | ||||||
tag "workflows" | ||||||
tag "workflows_sra" | ||||||
tag "multiqc_mappings_config" | ||||||
tag "sra_default_parameters" | ||||||
|
||||||
// Modules | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
I'd rather we be explicit there and say dependencies, as any at this level "workflow" can contain modules and/or subworkflows There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good idea. |
||||||
tag "MULTIQC_MAPPINGS_CONFIG" | ||||||
tag "SRA_FASTQ_FTP" | ||||||
tag "SRA_IDS_TO_RUNINFO" | ||||||
tag "SRA_RUNINFO_TO_FTP" | ||||||
tag "SRA_TO_SAMPLESHEET" | ||||||
Comment on lines
+9
to
+13
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmmm...we have to be explicit with all of the workflow dependencies here now? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could be quite easy to lose track of these as you update the pipeline / workflow.... There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes we do. Not ideal, but hard to do automatically. See askimed/nf-test#179 for relevant ticket. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I had exactly the same thought as Harshil, don't think people will be very good at keeping these lists up to date. But I see the difficulties. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No magic bullet so far. I've proposed automatically testing dependencies, but I also think nf-test can only go so far and it might just have to be the resposibility of the pipeline developer. |
||||||
|
||||||
// Subworkflows | ||||||
tag "SRA" | ||||||
maxulysse marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
|
||||||
test("Parameters: default") { | ||||||
|
||||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not for this PR- but if tag is becoming redundant with the process maybe we should talk to the nf-test folks about allowing nf-test to list processes/ workflows rather than tags?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's similar to what @adamrtalbot meant when creating this issue: askimed/nf-test#178
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Correct, I've suggested it as a feature. This was the behaviour in pytest-workflow as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Gotcha, sorry for the slow uptake