Skip to content

fmhfunprofiler: fix test data and update snapshot - #12417

Open
nickp60 wants to merge 5 commits into
masterfrom
fix/fmhfunprofiler
Open

fmhfunprofiler: fix test data and update snapshot#12417
nickp60 wants to merge 5 commits into
masterfrom
fix/fmhfunprofiler

Conversation

@nickp60

@nickp60 nickp60 commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

PR checklist

I noticed that the previous test data was malformed, resulting in failure of one step of the tool; however the tool still outputs a file with a header, making it look like it was a success. Here I have fixed the tests as follows:

  • changed the sourmash sketch to use the parameters described in the fmhfunprofiler readme (kmer sizes and using protein mode on the sarscov2 proteome)
  • adjusted the test snapshot to capture actual content, not just the hash
  • added tests for relevant minigut sample against both the full database and the test database.

Closes #12416

  • 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.
  • Broadcast software version numbers to topic: versions - See version_topics
  • 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

I will change the path to the db in the tests from my personal once this PR for the data fix is reviewed and accepted.

@nickp60
nickp60 requested a review from miraep8 July 24, 2026 04:28
Comment thread modules/nf-core/fmhfunprofiler/tests/fmh-sourmash.config
Comment thread modules/nf-core/fmhfunprofiler/tests/main.nf.test
{ assert process.success },
{ assert snapshot(
path(process.out.csv[0][1]).readLines()[1..10]
).match() }

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.

Suggested change
).match() }
{ assert snapshot(
path(process.out.csv[0][1]).readLines()[1..10]
process.out.findAll { key, val -> key.startsWith('versions') }

every test should also capture the version output

Comment thread modules/nf-core/fmhfunprofiler/tests/main.nf.test

@miraep8 miraep8 left a comment

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.

Thank you Nick, agree with mashehu's requests etc but otherwise think this looks good. Thank you for detecting and fixing this!

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fmh-funprofiler silent errors

3 participants