Skip to content

Make 22 weak unit tests assert content (#819 step 3) - #823

Merged
joshfactorial merged 3 commits into
developfrom
test/819_step3
Oct 7, 2026
Merged

joshfactorial merged 3 commits into
developfrom
test/819_step3

Conversation

@joshfactorial

Copy link
Copy Markdown
Collaborator

Step 3 of the #819 testing audit: tests that asserted existence, success, or a non-zero count instead of content. Test code only. No production behavior changes.

How the candidates were found

A scan of all 1,070 Rust tests flagged 47 with no content assertion of their own. Each was then read together with the helpers it calls:

  • 24 OK. The assertions live in a helper, e.g. chimeric_sequence_content.rs's assert_derived, model_parity.rs's byte-for-byte baselines, variants.rs's assert_literal.
  • 22 weak.
  • 1 needs a decision and is untouched (see below).

The scan cannot see a trivial assert_eq!. One such test, test_runner_max_reads_reduces_processed_count, was found by reading and is included.

Fixed (each with a hand-derived expected value and a mutation of the production code it covers, confirmed applied, failing, then passing on revert)

Tests that could not fail for the reason their name gives

  • gen_gc_bias_model::test_all_n_contig_is_skipped: chr1 was 10 bp, below the 100 bp window, so the length guard skipped it before the N path was ever reached. It is now 200 N at a depth different from chr2, and the test asserts the .bins.tsv window counts. Caveat: N handling is guarded twice, and only breaking both fails the test.
  • gen_frag_length_model::test_runner_min_reads_zero_skips_filter: its fixture gave the same model whether or not the filter ran. It now has an outlier the MAD ceiling removes at min_reads=2 and keeps at 0, and asserts both models' mean and SD.
  • test_transition_matrix_from_tsv: passed with the TSV ignored. It now asserts all four rows.
  • balanced_chimeric_offset_survives_a_fragment_no_longer_than_the_read: asserted only off >= 1, on the premise that the floor cannot hold at frag = read. It can, since 37 + 37 ≤ 151, and the test now asserts it on both sides. Removing the upper bound passed the old assertion and fails the new one.
  • test_apply_variants: the SNP's REF did not match the sequence, and the het genotypes made the outcome random. With a corrected homozygous fixture it asserts the exact sequence ATGTATGA and CIGAR MMMMDMMMM.
  • gen_mut_model::test_runner_with_indels: only asserted mutation_rate > 0. It now asserts variant-type weights of 1/3 each, mutation_rate = 3/13114 (13,114 = H1N1's non-N bases, counted independently), insertion length 2 and deletion length 3. Both indel REFs in the old fixture disagreed with H1N1 and are corrected.
  • test_runner_skips_reference_mismatch_variant: asserts the skipped SNP's context (GCT) carries no weight.

Print-only or empty

  • generate_fragments: three depth tests printed a number and asserted nothing. They now assert the placed count and depth: 22.5 ± 1.0 (4σ) for fragment coverage, and exactly 10.0 for read depth.
  • filter_reads::test_run_configuration: the body was empty with a TODO. Replaced by known-answer tests of create_map_item and RunConfiguration::from.

Exact values instead of is_some() / exists() / is_ok()

  • gen_reads config output paths (r1/r2, VCF, BAM)
  • test_create_output_file_new, test_check_parent, plus a new missing-parent test
  • sample_poisson_extreme_lambda_does_not_overflow, now bounded at ±5σ
  • test_prep_file_for_filtering, plus a gzipped-input case
  • two config-loading tests

Removed or renamed

  • Removed two max_reads smoke tests that could not see max_reads. Three existing tests pin it exactly.
  • Renamed test_overwrite_warn to test_overwrite_output_is_accepted. Nothing can capture logs in a unit test, so the warning it was named for is not asserted.

Found, not changed here

  • filter-reads output naming (bug, to be filed): it splits the whole path on . and indexes from the end. A plain x.fastq/x.vcf panics (attempt to subtract with overflow, reproduced with the binary), and a dot in a directory name produces a wrong path. The readers themselves handle plain files.
  • Decisions for the maintainer (on Testing audit before v4.0.0 #819):
    • SNPs skipped for a REF mismatch still count toward mutation_rate and snp_freq.
    • gen-mut-model does not check indel REFs against the reference.
    • A BED that names no reference contig fails as "Trinuc counts are empty. Unknown error"; test_runner_bed_unknown_contig_succeeds_or_errors_cleanly still accepts any outcome.
    • In long-read mode, balanced_chimeric_offset may return an offset past a short fragment's end.

Evidence

Full workspace suite (1,097 tests), fmt --check and clippy -D warnings pass locally.

Not verified

  • The mutation checks were run by delegated agents in isolated worktrees and by hand for this branch's own commit; their reports were reviewed and the diffs read, but not every mutation was re-run independently. Spot-checked: H1N1's 13,114 non-N bases, and the TSV row CDFs.

Refs #819.

🤖 Generated with Claude Code

joshfactorial and others added 3 commits October 6, 2026 21:00
- test_transition_matrix_from_tsv asserts all four rows of the TSV; it
  passed with the TSV ignored.
- balanced_chimeric_offset's short-fragment test asserts the floor on
  both sides; its premise that the floor cannot hold at frag=read was
  wrong, and 'off >= 1' passed with the upper bound removed.
- Drop two max_reads smoke tests that could not see max_reads; three
  tests already pin it exactly.
- The degradation-floor config test checks what was stored.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- test_runner_with_indels: assert variant-type weights (1/3 each),
  mutation_rate = 3/13114, and the fitted insertion (2) and deletion (3)
  lengths read from the written model. Fixture REFs now match H1N1.
- test_runner_skips_reference_mismatch_variant: assert the skipped SNP's
  context (GCT) carries no weight and the usable one (TCT) carries it all.
- config: assert exact r1/r2, VCF and BAM paths; rename
  test_overwrite_warn to test_overwrite_output_is_accepted, since no log
  capture exists to check the warning.
- generate_fragments: assert placed count and average depth for the
  three print-only depth tests.

Each was checked by mutating the production code it covers.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Each test now asserts a hand-derived expected value instead of existence
or is_ok(), and each was checked by mutating the covered production code
and watching it fail:

- fastq_tools test_apply_variants: fixture REF now matches the sequence,
  both variants homozygous, Q60; asserts read ATGTATGA and MMMMDMMMM.
- file_io test_create_output_file_new: reads the written bytes back.
- folder_tools test_check_parent: asserts the returned path; new
  should_panic test for a missing parent with create=false.
- sv_model extreme-lambda Poisson: +/-5 sigma bound instead of n > 0.
- filter_reads config: replaces the empty stub with known-answer tests of
  create_map_item and RunConfiguration::from.
- filter_lib test_prep_file_for_filtering: reads back through the
  returned reader and writer, both plain and gzipped input.
- frag length min_reads=0: a stray at 100,000 is kept at min_reads=0 and
  removed at min_reads=2; asserts both fitted Normal models.
- gc bias all-N contig: chr1 is now 200 bp so it reaches the N handling;
  asserts from the bin report that only chr2's two windows count.
- gen_mut_model empty transition_matrix_file: asserts the other parsed
  fields.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@joshfactorial
joshfactorial merged commit 219a5dd into develop Oct 7, 2026
7 checks passed
@joshfactorial
joshfactorial deleted the test/819_step3 branch October 7, 2026 03:59
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