Skip to content

Conversation

@dialvarezs
Copy link
Contributor

Closes #902.

PR checklist

  • 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 pipeline conventions in the contribution docs
  • If necessary, also make a PR on the nf-core/mag branch on the nf-core/test-datasets repository.
  • Make sure your code lints (nf-core pipelines lint).
  • Ensure the test suite passes (nextflow run . -profile test,docker --outdir <OUTDIR>).
  • Check for unexpected warnings in debug mode (nextflow run . -profile debug,test,docker --outdir <OUTDIR>).
  • Usage Documentation in docs/usage.md is updated.
  • Output Documentation in docs/output.md is updated.
  • CHANGELOG.md is updated.
  • README.md is updated (including new tool citations and authors/contributors).

@dialvarezs dialvarezs changed the title Snapshot for -profile test_single_end Snapshot for -profile test_single_end Oct 31, 2025
@dialvarezs dialvarezs linked an issue Oct 31, 2025 that may be closed by this pull request
@dialvarezs dialvarezs marked this pull request as ready for review November 15, 2025 03:34
@nf-core-bot

This comment was marked as resolved.

Copy link
Member

@jfy133 jfy133 left a comment

Choose a reason for hiding this comment

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

Overall LGTM,

As a general thing, I suggest wrapping all asserts in snapshot even if it's just a boolean (you can also store these in a string even for more human readability, see nf-core/createtaxdb as an example -inspired by @maxulysse )

If that particular check fails and is not in a snapshot, it can be really annoying to debug.

I have 3 minor things that may have been missed, but otherwise this is looking very comprehensive, great work @dialvarezs !

@@ -1,6 +1,12 @@
.DS_Store
Ancient_DNA/pydamage/analyze/*/*.csv
Ancient_DNA/variant_calling/*/*.vcf.gz
Copy link
Member

Choose a reason for hiding this comment

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

No string check for these? Or you could use https://github.com/seppinho/nft-vcf

.DS_Store
Ancient_DNA/pydamage/analyze/*/*.csv
Ancient_DNA/variant_calling/*/*.vcf.gz
Annotation/Prokka/**/*.{log,err,gbk,sqn}
Copy link
Member

Choose a reason for hiding this comment

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

No addition checks for gbk/sqn? These are common downstream files, so might be good to validate somehow

QC_shortreads/fastqc/*_fastqc.{html,zip}
QC_shortreads/remove_phix/*.log
Taxonomy/CAT/**/*.log
VirusIdentification/geNomad/**/*_aggregated_classification.tsv
Copy link
Member

Choose a reason for hiding this comment

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

Can be removed, as you do actually check this explicitly in genomad_results

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Generate snapshot for test_single_end

3 participants