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

Replace tags strategy with implicit 'tags in test' strategy #253

Merged
merged 22 commits into from
Jan 29, 2024
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 35 additions & 12 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -27,19 +27,42 @@ jobs:
name: Check for changes
runs-on: ubuntu-latest
outputs:
# Expose matched filters as job 'tags' output variable
tags: ${{ steps.filter.outputs.changes }}
changes: ${{ steps.changed_files.outputs.any_modified }}
tags: ${{ steps.list.outputs.tags }}
steps:
- name: Install Nextflow
uses: nf-core/setup-nextflow@v1
with:
version: "${{ matrix.NXF_VER }}"

- name: Cache nf-test installation
id: cache-software
uses: actions/cache@v3
with:
path: |
/usr/local/bin/nf-test
/home/runner/.nf-test/nf-test.jar
key: ${{ runner.os }}-${{ env.NFT_VER }}-nftest

- name: Install nf-test
if: steps.cache-software.outputs.cache-hit != 'true'
run: |
wget -qO- https://code.askimed.com/install/nf-test | bash
sudo mv nf-test /usr/local/bin/

- uses: actions/checkout@v3
- name: Combine all tags.yml files
id: get_username
run: find . -name "tags.yml" -not -path "./.github/*" -exec cat {} + > .github/tags.yml
- name: debug
run: cat .github/tags.yml
- uses: dorny/paths-filter@v2
id: filter
with:
filters: ".github/tags.yml"
fetch-depth: 0

- uses: tj-actions/changed-files@v42
id: changed_files
with:
dir_names: "true"

- name: nf-test list tags
id: list
if: ${{ steps.changed_files.outputs.any_modified }}
run: echo tags=$(nf-test list --silent --tags --format json ${{ steps.changed_files.outputs.all_changed_files }}) >> $GITHUB_OUTPUT

define_nxf_versions:
name: Choose nextflow versions to test against depending on target branch
Expand All @@ -56,9 +79,9 @@ jobs:
fi

test:
name: ${{ matrix.tags }} ${{ matrix.profile }} NF ${{ matrix.NXF_VER }}
name: ${{ matrix.tags }} ${{ matrix.profile }} NF-${{ matrix.NXF_VER }}
needs: [changes, define_nxf_versions]
if: needs.changes.outputs.tags != '[]'
if: needs.changes.outputs.changes
runs-on: ubuntu-latest
strategy:
fail-fast: false
Expand Down
5 changes: 2 additions & 3 deletions modules/local/multiqc_mappings_config/tests/main.nf.test
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,8 @@ nextflow_process {
name "Test process: MULTIQC_MAPPINGS_CONFIG"
script "../main.nf"
process "MULTIQC_MAPPINGS_CONFIG"
tag "modules"
tag "modules_local"
tag "multiqc_mappings_config"

tag "MULTIQC_MAPPINGS_CONFIG"
Copy link
Member

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?

Copy link
Member

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

Copy link
Contributor Author

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.

Copy link
Member

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


test("Should run without failures") {

Expand Down
2 changes: 0 additions & 2 deletions modules/local/multiqc_mappings_config/tests/tags.yml

This file was deleted.

5 changes: 2 additions & 3 deletions modules/local/sra_fastq_ftp/tests/main.nf.test
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,8 @@ nextflow_process {
name "Test process: SRA_FASTQ_FTP"
script "../main.nf"
process "SRA_FASTQ_FTP"
tag "modules"
tag "modules_local"
tag "sra_fastq_ftp"

tag "SRA_FASTQ_FTP"

test("Should run without failures") {

Expand Down
2 changes: 0 additions & 2 deletions modules/local/sra_fastq_ftp/tests/tags.yml

This file was deleted.

5 changes: 2 additions & 3 deletions modules/local/sra_ids_to_runinfo/tests/main.nf.test
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,8 @@ nextflow_process {
name "Test process: SRA_IDS_TO_RUNINFO"
script "../main.nf"
process "SRA_IDS_TO_RUNINFO"
tag "modules"
tag "modules_local"
tag "sra_ids_to_runinfo"

tag "SRA_IDS_TO_RUNINFO"

test("Should run without failures") {

Expand Down
2 changes: 0 additions & 2 deletions modules/local/sra_ids_to_runinfo/tests/tags.yml

This file was deleted.

5 changes: 2 additions & 3 deletions modules/local/sra_runinfo_to_ftp/tests/main.nf.test
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,8 @@ nextflow_process {
name "Test process: SRA_RUNINFO_TO_FTP"
script "../main.nf"
process "SRA_RUNINFO_TO_FTP"
tag "modules"
tag "modules_local"
tag "sra_runinfo_to_ftp"

tag "SRA_RUNINFO_TO_FTP"

test("Should run without failures") {

Expand Down
2 changes: 0 additions & 2 deletions modules/local/sra_runinfo_to_ftp/tests/tags.yml

This file was deleted.

5 changes: 2 additions & 3 deletions modules/local/sra_to_samplesheet/tests/main.nf.test
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,8 @@ nextflow_process {
name "Test process: SRA_TO_SAMPLESHEET"
script "../main.nf"
process "SRA_TO_SAMPLESHEET"
tag "modules"
tag "modules_local"
tag "sra_to_samplesheet"

tag "SRA_TO_SAMPLESHEET"

test("Should run without failures") {

Expand Down
2 changes: 0 additions & 2 deletions modules/local/sra_to_samplesheet/tests/tags.yml

This file was deleted.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 0 additions & 2 deletions modules/nf-core/custom/sratoolsncbisettings/tests/tags.yml

This file was deleted.

9 changes: 4 additions & 5 deletions modules/nf-core/sratools/fasterqdump/tests/main.nf.test

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 0 additions & 2 deletions modules/nf-core/sratools/fasterqdump/tests/tags.yml

This file was deleted.

6 changes: 2 additions & 4 deletions modules/nf-core/sratools/prefetch/tests/main.nf.test

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 0 additions & 2 deletions modules/nf-core/sratools/prefetch/tests/tags.yml

This file was deleted.

4 changes: 1 addition & 3 deletions modules/nf-core/untar/tests/main.nf.test

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 0 additions & 2 deletions modules/nf-core/untar/tests/tags.yml

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.

2 changes: 0 additions & 2 deletions subworkflows/nf-core/utils_nextflow_pipeline/tests/tags.yml

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.

2 changes: 0 additions & 2 deletions subworkflows/nf-core/utils_nfcore_pipeline/tests/tags.yml

This file was deleted.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 0 additions & 2 deletions subworkflows/nf-core/utils_nfvalidation_plugin/tests/tags.yml

This file was deleted.

6 changes: 4 additions & 2 deletions tests/main.nf.test
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,10 @@ nextflow_pipeline {

name "Test pipeline"
script "../main.nf"
tag "pipeline"
tag "pipeline_fetchngs"

tag "FETCHNGS"

tag "SRA"
adamrtalbot marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Member

Choose a reason for hiding this comment

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

why the SRA tag here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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?

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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.


test("Run with profile test") {

Expand Down
5 changes: 0 additions & 5 deletions tests/tags.yml

This file was deleted.

16 changes: 12 additions & 4 deletions workflows/sra/tests/main.nf.test
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,18 @@ 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"


// Subworkflows
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
// Subworkflows
// Workflow

This one is a "workflow"

tag "SRA"

// Modules
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
// Modules
// Dependencies

I'd rather we be explicit there and say dependencies, as any at this level "workflow" can contain modules and/or subworkflows

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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
Copy link
Member

Choose a reason for hiding this comment

The 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?

Copy link
Member

Choose a reason for hiding this comment

The 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....

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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.

Copy link
Member

Choose a reason for hiding this comment

The 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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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.



test("Parameters: default") {

Expand Down
14 changes: 10 additions & 4 deletions workflows/sra/tests/sra_custom_ena_metadata_fields.nf.test
Original file line number Diff line number Diff line change
Expand Up @@ -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_custom_ena_metadata_fields"

// Subworkflows
tag "SRA"

// Modules
tag "MULTIQC_MAPPINGS_CONFIG"
tag "SRA_FASTQ_FTP"
tag "SRA_IDS_TO_RUNINFO"
tag "SRA_RUNINFO_TO_FTP"
tag "SRA_TO_SAMPLESHEET"

test("Parameters: --nf_core_pipeline rnaseq --ena_metadata_fields ... --sample_mapping_fields ...") {

Expand Down
14 changes: 10 additions & 4 deletions workflows/sra/tests/sra_force_sratools_download.nf.test
Original file line number Diff line number Diff line change
Expand Up @@ -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_force_sratools_download"

// Subworkflows
tag "SRA"

// Modules
tag "MULTIQC_MAPPINGS_CONFIG"
tag "SRA_FASTQ_FTP"
tag "SRA_IDS_TO_RUNINFO"
tag "SRA_RUNINFO_TO_FTP"
tag "SRA_TO_SAMPLESHEET"

test("Parameters: --force_sratools_download") {

Expand Down
Loading
Loading