Skip to content

Update nf-test snapshot#52

Merged
BenjaminWehnert1008 merged 9 commits into
devfrom
updated_test_snapshot
Jun 2, 2026
Merged

Update nf-test snapshot#52
BenjaminWehnert1008 merged 9 commits into
devfrom
updated_test_snapshot

Conversation

@BenjaminWehnert1008
Copy link
Copy Markdown
Collaborator

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/deepmutscan 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).

@BenjaminWehnert1008
Copy link
Copy Markdown
Collaborator Author

@mashehu I've updated the test snapshot, but nf-test still fails, do you have an idea why and also how to proceed? Thanks!

@mashehu
Copy link
Copy Markdown
Contributor

mashehu commented May 20, 2026

looks like the test produces more files in the CI then you currently have. How did you update your snapshots?

@BenjaminWehnert1008
Copy link
Copy Markdown
Collaborator Author

looks like the test produces more files in the CI then you currently have. How did you update your snapshots?

nf-test test tests/default.nf.test --profile docker --update-snapshot

@mashehu
Copy link
Copy Markdown
Contributor

mashehu commented May 20, 2026

Can you try it with nf-test test tests/default.nf.test --profile +docker --update-snapshot otherwise the test profile might be overwritten https://www.nf-test.com/docs/configuration/#managing-profiles

@BenjaminWehnert1008
Copy link
Copy Markdown
Collaborator Author

BenjaminWehnert1008 commented May 20, 2026

@mashehu, it seems to not fully resolve it...

@mashehu
Copy link
Copy Markdown
Contributor

mashehu commented May 22, 2026

I excluded a bit more unstable R files from the snapshot (should just check names for them). Can you try to recreate the snapshot on your side again @BenjaminWehnert1008 ?

@BenjaminWehnert1008
Copy link
Copy Markdown
Collaborator Author

@mashehu @MaximilianStammnitz after further expanding the ignore file, the test is now running without any failures. Is it ready to be merged? And what is the next step? Thanks! :)

@MaximilianStammnitz
Copy link
Copy Markdown
Collaborator

@mashehu @MaximilianStammnitz after further expanding the ignore file, the test is now running without any failures. Is it ready to be merged? And what is the next step? Thanks! :)

amazing @BenjaminWehnert1008, I hope this looks good enough for the merge !!

Comment thread tests/.nftignore
Comment on lines +20 to +21
**/fitness_estimation_mutscan_edgeR.tsv
**/fitness_estimation_mutscan_limma.tsv
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

hmm, tsv files usually are quite stable. any clue what is different on the different platforms here?

Copy link
Copy Markdown
Collaborator Author

@BenjaminWehnert1008 BenjaminWehnert1008 May 26, 2026

Choose a reason for hiding this comment

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

@mashehu yes, this is weird... I'm also wondering. The edgeR and limma outputs contain heavily calculated stats (logFoldChange, PValue, etc.) with up to 14+ decimal places - maybe they're different because of differences in rounding?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

ah, that could be the case.
can you maybe then use https://github.com/lukfor/nft-csv to get at least a partial snapshot instead of ignoring them completely?

Comment thread .gitignore Outdated
BenjaminWehnert1008 and others added 2 commits May 26, 2026 15:29
Co-authored-by: Matthias Hörtenhuber <mashehu@users.noreply.github.com>
@BenjaminWehnert1008 BenjaminWehnert1008 merged commit 220fc76 into dev Jun 2, 2026
9 checks passed
@BenjaminWehnert1008 BenjaminWehnert1008 deleted the updated_test_snapshot branch June 2, 2026 13:22
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