Skip to content

Update smoothxg: topic versions, stub block, sanitizeOutput#11448

Open
HReed1 wants to merge 6 commits intonf-core:masterfrom
HReed1:stub-topics-smoothxg
Open

Update smoothxg: topic versions, stub block, sanitizeOutput#11448
HReed1 wants to merge 6 commits intonf-core:masterfrom
HReed1:stub-topics-smoothxg

Conversation

@HReed1
Copy link
Copy Markdown
Contributor

@HReed1 HReed1 commented Apr 30, 2026

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, add --profile docker,wave tests.
  • Make sure your code lints (nf-core modules lint).
  • Ensure the test suite passes (nf-test test).

Description of changes

Migrates the smoothxg module to the nf-core v4.0.1 architecture:

  • topic: versions: Replaced legacy versions.yml file emission with dynamic eval() tuple emission on the topic: versions channel.
  • stub block: Added a deterministic stub: block for CI/CD stub-run support.
  • sanitizeOutput: Refactored all test assertions to use sanitizeOutput(process.out) per nf-test best practices.
  • meta.yml: Updated to include both output: versions_smoothxg and topics: versions sections.

Part of the v4.0.1 standardization effort tracked in #11323.

Part of #4570

Comment thread modules/nf-core/smoothxg/main.nf Outdated
tuple val(meta), path("*smoothxg.gfa"), emit: gfa
path("*.maf") , optional: true, emit: maf
path "versions.yml" , emit: versions
tuple val("${task.process}"), val('smoothxg'), eval("smoothxg --version 2>&1 | cut -f 1 -d '-' | cut -f 2 -d 'v'"), topic: versions, emit: versions_smoothxg
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.

Can you simplify this into just one command, not two cut commands.

then {
assertAll(
{ assert process.success },
// { assert snapshot(process.out).match() } // SMOOTHXG is never deterministic
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.

This is a bit strange, it says it is not deterministic, but your tests are passing?

@HReed1 HReed1 force-pushed the stub-topics-smoothxg branch from 8d06c8d to 101e01c Compare May 1, 2026 12:07
@HReed1
Copy link
Copy Markdown
Contributor Author

HReed1 commented May 1, 2026

@SPPearce Thanks for the review! I've addressed both of your points in the upcoming push:

  1. Regarding the cut commands: Good catch. I've replaced the two cut pipes with a single sed command (sed 's/^v//; s/-.*//') to extract the semantic version string cleanly in one pass.
  2. Regarding the non-deterministic tests passing: You're completely right to be suspicious! The original author had commented out the entire snapshot assertion because of the non-deterministic GFA outputs, meaning the tests were passing vacuously (only checking process.success). Fortunately, another recent upstream commit introduced sanitizeOutput(process.out), which masks the non-deterministic paths/timestamps. I've regenerated the snapshots locally using this new sanitizer, so the tests are now fully and safely validating the outputs (including the new versions_smoothxg channel)!

I'll push these fixes up now. Let me know if you see anything else!

…e snapshots

- main.nf: emit: versions -> emit: versions_smoothxg
- meta.yml: output key versions: -> versions_smoothxg:
- main.nf/meta.yml: simplify eval string to use sed instead of dual cuts
- tests/main.nf.test.snap: regenerate snapshots

Local lint: 54/0. Local nf-test: 3/3 passed.
@HReed1 HReed1 force-pushed the stub-topics-smoothxg branch from 04b7fe0 to a860fc7 Compare May 1, 2026 13:09
@HReed1
Copy link
Copy Markdown
Contributor Author

HReed1 commented May 3, 2026

I've pushed the requested standardizations.

Note for Maintainers: The current Singularity CI failure is a false negative. Canonical's Launchpad PPA infrastructure (ppa.launchpadcontent.net) is currently experiencing intermittent global 504 Gateway Timeouts (likely related to ongoing DDoS mitigations noted on Canonical's Status Page).

Because of this, the GitHub Actions ubuntu-latest runner physically cannot install the apptainer dependency during the environment setup. This timeout will currently block any PRs across nf-core that trigger Singularity testing.

GitHub Actions Error Log (Apptainer Install Failure):

x64 | singularity | 2   UNKNOWN STEP
W: Failed to fetch https://ppa.launchpadcontent.net/apptainer/ppa/ubuntu/dists/noble/InRelease  Could not connect to ppa.launchpadcontent.net:443 (185.125.190.80), connection timed out

x64 | singularity | 1   UNKNOWN STEP
lazr.restfulclient.errors.ServerError: HTTP Error 504: Gateway Time-out

To ensure this module is sound and ready to merge once Canonical recovers, I bypassed the failing GitHub Actions queue and validated smoothxg natively on an ephemeral AWS Batch runner (c6i.2xlarge). The module logic and snapshot hashes execute flawlessly.

AWS Ephemeral Cloud Runner Validation:

🚀 nf-test 0.9.5
Test Process SMOOTHXG

  Test [f734eb4e] 'sarscov2 - illumina - assembly_gfa' PASSED (12.618s)
  Test [32e49927] 'homo_sapiens - pangenome - pangenome_seqwish_gfa' PASSED (8.546s)
  Test [4481cde4] 'sarscov2 - illumina - assembly_gfa - stub' PASSED (5.161s)

SUCCESS: Executed 3 tests in 26.328s
============================================
  Total time: 2m 21s
  Job ID: 132d6c71-0a85-44a5-825f-df8a5be4acba
  Status: SUCCEEDED
============================================
Exit code: 0

Bringing this directly to your attention @SPPearce as many of the PR's I'm working on will have this issue. For the branches with singularity being the only failures on the github CI pipelines.

@HReed1 HReed1 enabled auto-merge May 3, 2026 20:36
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.

2 participants