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

UMICollapse module: Drop external dependencies from UMICollapse module tests #7075

Merged
merged 4 commits into from
Nov 29, 2024

Conversation

MatthiasZepper
Copy link
Member

@MatthiasZepper MatthiasZepper commented Nov 24, 2024

Currently, the UMICollapse module tests run on data that is generated in the test setup{} with external dependencies (umitools/extract, bwa/index, bwa/mem, samtools/index). While this is no problem here in the modules' repository, those tests will fail on the pipeline level, if the additional modules are not installed.

While it may, on the long-term, be useful to devise a CI step detecting and installing missing modules for tests (Leon Rauschning made a neat proposal for this), I have now, due to time contraints, opted to add the required files to the test datasets (test-datasets/1390) and change the tests accordingly (this PR).

I have also updated the umi-tools dedup tests, since they so far used inputs without UMIs.

PR checklist

  • This comment contains a description of changes (with reason).
  • Ensure that the test works with either Docker / Singularity. Conda CI tests can be quite flaky:
    • For modules:
      • nf-core modules test <MODULE> --profile docker
      • nf-core modules test <MODULE> --profile singularity
      • nf-core modules test <MODULE> --profile conda
    • For subworkflows:
      • nf-core subworkflows test <SUBWORKFLOW> --profile docker
      • nf-core subworkflows test <SUBWORKFLOW> --profile singularity
      • nf-core subworkflows test <SUBWORKFLOW> --profile conda

Copy link
Contributor

@SPPearce SPPearce left a comment

Choose a reason for hiding this comment

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

Looks much better.
Think you just need to swap to using the nft-bam plugin to test the bam files then should be good.

@MatthiasZepper MatthiasZepper force-pushed the nftest_umi_bams branch 2 times, most recently from ff07ff9 to 47f19b0 Compare November 26, 2024 13:48
@MatthiasZepper MatthiasZepper added this pull request to the merge queue Nov 29, 2024
Merged via the queue into nf-core:master with commit 0b27602 Nov 29, 2024
23 checks passed
@MatthiasZepper MatthiasZepper deleted the nftest_umi_bams branch November 29, 2024 12:09
LouisLeNezet pushed a commit to LouisLeNezet/modules that referenced this pull request Dec 4, 2024
…e tests (nf-core#7075)

* Update the umi-tools dedup tests to use the new test data with UMIs.

* Update umicollapse tests to use the new UMI test data.

* Switch to nf-bam plugin for output validation.

---------

Co-authored-by: Matthias Zepper <[email protected]>
Co-authored-by: Jonathan Manning <[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.

3 participants