Skip to content

test: add end-to-end regression tests using simulate and compare - #227

Merged
nh13 merged 2 commits into
mainfrom
nh/e2e-regression-tests
Apr 4, 2026
Merged

nh13 merged 2 commits into
mainfrom
nh/e2e-regression-tests

Conversation

@nh13

@nh13 nh13 commented Apr 4, 2026

Copy link
Copy Markdown
Member

Summary

  • Add 6 E2E regression tests that validate full pipeline determinism without golden files
  • Tests use simulate to generate synthetic data, run pipeline commands, and compare outputs
  • Covers: simulate determinism, simplex pipeline, simplex+filter, full extract-to-filter pipeline, dedup pipeline, and a sanity check that different seeds diverge
  • Feature-gated behind both compare and simulate features (runs in cargo ci-test)

Test plan

  • All 6 new E2E tests pass: cargo nextest run --features compare,simulate -E 'test(test_e2e_regression)'
  • Full test suite passes: cargo ci-test (2,213 tests, 0 failures)
  • cargo ci-fmt and cargo ci-lint clean

@nh13
nh13 temporarily deployed to github-actions April 4, 2026 06:33 — with GitHub Actions Inactive
@coderabbitai

coderabbitai Bot commented Apr 4, 2026 •

Copy link
Copy Markdown

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: e0172d14-957b-4af3-b696-6a6fce413204

📥 Commits

Reviewing files that changed from the base of the PR and between 6c068b1 and 78e6996.

📒 Files selected for processing (2)
  • tests/integration/main.rs
  • tests/integration/test_e2e_regression.rs

📝 Walkthrough

Walkthrough

Added a new integration test module test_e2e_regression that executes the fgumi binary via subprocess calls. The module contains end-to-end tests that validate individual pipeline stages (simulate, filter, dedup, extract, group, simplex) and longer workflows. Tests use temporary directories with synthesized datasets, run operations multiple times under identical parameters to verify determinism, and compare BAM outputs using the compare bams command. Additional tests confirm that different random seeds produce distinct BAM files. The module is conditionally compiled when both compare and simulate Cargo features are enabled.

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed Title clearly summarizes the main change: adding end-to-end regression tests using simulate and compare features.
Description check ✅ Passed Description directly relates to the changeset, detailing the 6 new E2E tests, their purpose, coverage, and test validation results.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch nh/e2e-regression-tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@codecov

codecov Bot commented Apr 4, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 88.80%. Comparing base (0c6b0ae) to head (78e6996).
⚠️ Report is 6 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #227      +/-   ##
==========================================
+ Coverage   88.27%   88.80%   +0.52%     
==========================================
  Files         113      113              
  Lines       53215    53215              
==========================================
+ Hits        46977    47259     +282     
+ Misses       6238     5956     -282     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 4

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@tests/integration/main.rs`:
- Around line 19-20: Add the crate-level attribute to deny unsafe code by
inserting #![deny(unsafe_code)] at the very top of the integration test crate
root (above any mod declarations), so the integration tests including the
conditional module test_e2e_regression are compiled with unsafe code banned;
ensure the attribute appears before the existing #[cfg(...)] mod
test_e2e_regression line.

In `@tests/integration/test_e2e_regression.rs`:
- Around line 288-294: The test currently compares the same BAM file to itself
using assert_bams_identical(&filtered, &filtered, ...), which will pass even for
an empty file and won't catch nondeterminism in the
extract->group->simplex->filter pipeline; fix by producing two independent
outputs (e.g., run the pipeline twice into separate variables/files like
filtered_a and filtered_b) and then call assert_bams_identical(&filtered_a,
&filtered_b, "content", ...) to validate deterministic content, or alternatively
add a real record-count assertion (check the BAM record count > 0 using the same
helper used elsewhere) against the pipeline output (referencing the filtered
variable/outputs and the assert_bams_identical helper).
- Around line 333-334: The current assertion uses assert!(!success) which passes
for any non-zero exit (including unrelated errors); update the test around
compare_bams(&bam1, &bam2, "content") to assert the specific "files differ"
failure mode instead: either change compare_bams to return a typed status and
assert it equals the specific variant (e.g., CompareResult::FilesDiffer) or, if
compare_bams returns stdout/stderr text, assert that the output contains the
canonical "files differ" message; replace the generic assert!(!success, ...)
with an assertion that checks the exact failure mode from compare_bams
(referencing compare_bams, bam1, bam2).
- Around line 34-37: The helper path_str() forces UTF-8 and panics; instead
change the pipeline so paths are preserved as OsStr: remove use of path_str(),
update fgumi() and fgumi_ok() signatures to accept an argument slice generic
over AsRef<OsStr> (e.g., &[impl AsRef<OsStr>] or a concrete
&[OsString]/&[OsStr]), and pass each argument to Command::arg using .as_ref() so
non-UTF-8 paths are preserved; update call sites to pass Path/OsStr/OsString
values and only convert to &str where truly required (not for Command::arg).
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 926022e6-ba97-4ec0-9462-6dbee1a682b1

📥 Commits

Reviewing files that changed from the base of the PR and between ea6e3fc and 6c068b1.

📒 Files selected for processing (2)
  • tests/integration/main.rs
  • tests/integration/test_e2e_regression.rs

Comment thread tests/integration/main.rs
Comment thread tests/integration/test_e2e_regression.rs Outdated
Comment thread tests/integration/test_e2e_regression.rs Outdated
Comment thread tests/integration/test_e2e_regression.rs Outdated
nh13 added 2 commits April 4, 2026 10:54
Add 6 E2E regression tests that validate full pipeline determinism
without golden files. Tests use simulate to generate synthetic data,
run pipeline commands, and compare outputs:

- Simulate determinism (same seed → identical output)
- Simplex pipeline determinism (grouped → simplex → compare)
- Simplex + filter pipeline determinism
- Full pipeline (fastq → extract → group → simplex → filter)
- Dedup pipeline determinism
- Different seeds produce different output (sanity check)

Tests are feature-gated behind both `compare` and `simulate` features,
so they run as part of `cargo ci-test`.
@nh13
nh13 force-pushed the nh/e2e-regression-tests branch from 6c068b1 to 78e6996 Compare April 4, 2026 17:55
@nh13
nh13 temporarily deployed to github-actions April 4, 2026 17:55 — with GitHub Actions Inactive
@nh13
nh13 merged commit 0ee001f into main Apr 4, 2026
7 checks passed
@nh13
nh13 deleted the nh/e2e-regression-tests branch April 4, 2026 20:48
@nh13 nh13 mentioned this pull request Apr 4, 2026

This branch was previously deployed

1 inactive deployment
github-actions — 78e6996e Deployed Apr 4, 2026 by nh13 via coverage #864
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.

1 participant