Skip to content

Migrate GRAPHMAP2_ALIGN to nf-test#8294

Merged
SPPearce merged 4 commits intonf-core:masterfrom
LouisLeNezet:graphmap2
Apr 14, 2025
Merged

Migrate GRAPHMAP2_ALIGN to nf-test#8294
SPPearce merged 4 commits intonf-core:masterfrom
LouisLeNezet:graphmap2

Conversation

@LouisLeNezet
Copy link
Contributor

PR checklist

Closes #7590

  • This comment contains a description of changes (with reason).
  • If you've fixed a bug or added code that should be tested, add tests!
  • If you've added a new tool - have you followed the module conventions in the contribution docs
  • If necessary, include test data in your PR.
  • Remove all TODO statements.
  • Emit the versions.yml file.
  • Follow the naming conventions.
  • Follow the parameters requirements.
  • Follow the input/output options guidelines.
  • Add a resource label
  • Use BioConda and BioContainers if possible to fulfil software requirements.
  • 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.

I'm a bit concerned that the module isn't making usable sam files (but that seems to also be reflected in the github repo issues).
If it was a new module I'd ask for using samtools to make bam files, but not going to worry about that here.

{ assert process.success },
{ assert snapshot(
process.out.versions,
// sam(process.out.sam[0][1]).getSamLinesMD5() htsjdk.samtools.SAMFormatException: Error parsing text SAM file. MAPQ must be zero if RNAME is not specified
Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm, this feels a bit fishy that maybe the tool isn't making usable output files.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've open an issue on lbcb-sci/graphmap2#27

@github-project-automation github-project-automation bot moved this from Todo to In Progress in nf-test Migration Apr 14, 2025
@SPPearce SPPearce added this pull request to the merge queue Apr 14, 2025
Merged via the queue into nf-core:master with commit 3d7d473 Apr 14, 2025
19 checks passed
@github-project-automation github-project-automation bot moved this from In Progress to Done in nf-test Migration Apr 14, 2025
@LouisLeNezet LouisLeNezet deleted the graphmap2 branch April 24, 2025 07:44
famosab pushed a commit to famosab/modules that referenced this pull request Jun 3, 2025
* Migrate to nf-test

* Add stub test

* Remove comment
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

nf-test migration: graphmap2/align

2 participants