Skip to content

Expand N-handling test coverage + remove dead has_r2 guard - #315

Merged
joshfactorial merged 1 commit into
developfrom
test/n-handling-coverage
Jun 14, 2026
Merged

Expand N-handling test coverage + remove dead has_r2 guard#315
joshfactorial merged 1 commit into
developfrom
test/n-handling-coverage

Conversation

@joshfactorial

@joshfactorial joshfactorial commented Jun 14, 2026

Copy link
Copy Markdown
Collaborator

Summary

Follow-up to the v4.6.0 N-handling work (#313, released via #314). Closes coverage gaps and a small dead-code finding from reviewing the release PR. Mostly test-only, plus one behavior-preserving cleanup.

Tests

  • finalize_read_and_write threading — the new n_handling parameter was never exercised end-to-end. Added tests that the default (exclude) leaves a literal N in the emitted FASTQ and that telomere reaches convert_masking and fills it.
  • Paired path — added a positive control (both mates clean → fragment kept) and extended the drop test to cover read1-in-gap, not just read2-in-gap.
  • GC-bias sampling path — the prior end-to-end exclusion test only covered the uniform sampler; the GC path has its own _filter_n_regions call sites. Added a GC-path test (non-uniform model, asserted via not gc_model.is_uniform).
  • Options validation — added checks for the n_handling choice schema and n_max_fraction [0,1] range, and asserted both defaults in test_default_values.
  • Stale commenttest_check_and_log_error_choice_valid claimed the choice validator is unused by any production option; n_handling now uses it. Corrected.

Cleanup (production)

  • Removed the dead has_r2 guard in _filter_n_regions. In the paired branch read2 is always a real in-bounds window ((e - read_len, e) with e >= read_len + 10), so has_r2 was always True; the (0, 0) placeholder only occurs single-ended, which never reaches that branch. Behavior-preserving (covered by the paired N-filter tests); also drops a misleading "filtered upstream" comment.

Testing

pytest tests/test_read_simulator/ → 402 passed. gitnexus detect-changes → low risk.

🤖 Generated with Claude Code

Follow-up to the v4.6.0 N-handling work (PR #313/#314). Closes gaps found
while reviewing the release PR:

- Test that finalize_read_and_write actually threads n_handling through to
  convert_masking (default -> literal N, telomere -> TTAGGG fill); the new
  parameter was previously unexercised end-to-end.
- Add a paired positive control (both mates clean -> fragment kept) and cover
  the read1-in-gap drop direction, not just read2-in-gap.
- Add a GC-bias-path exclusion test; the prior end-to-end test only covered
  the uniform sampler, leaving the GC path's filter call sites untested.
- Validate the n_handling choice schema and n_max_fraction range, and assert
  their defaults in test_default_values.
- Fix a now-stale comment claiming the `choice` validator is unused by any
  production option (n_handling uses it).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@joshfactorial
joshfactorial merged commit 8bcd210 into develop Jun 14, 2026
1 check passed
@joshfactorial
joshfactorial deleted the test/n-handling-coverage branch June 14, 2026 17:07
@joshfactorial joshfactorial changed the title Expand N-handling test coverage Expand N-handling test coverage + remove dead has_r2 guard Jun 14, 2026
This was referenced Jun 14, 2026
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.

1 participant