Update tests to match v4.4.3 API changes; CI was failing on develop

joshfactorial · claude · joshfactorial · commit ba0baf332e8e · 2026-05-18T20:22:02.000-05:00
Three sets of tests were exercising contracts that the v4.4.3 work changed:

1. tests/test_read_simulator/test_generate_reads.py (9 tests)
   generate_reads no longer returns a list of (Read, Read|None) pairs — it
   streams Read objects directly to the output_file_writer as part of the
   bounded-memory rework. Added a small `_CollectingOFW` stand-in that
   captures write_bam_record calls; the affected tests now enable
   produce_bam=True, pass a CollectingOFW, and assert on ofw.bam_records.
   For paired-end tests, the pair-structure assertions were replaced with
   forward/reverse-count assertions since pairs are no longer kept together
   in memory (mates are interleaved by position via a heap buffer).

2. tests/test_read_simulator/test_options.py (1 test)
   test_default_values expected parallel_block_size == 500000. The v4.4.3
   default is 0 — a sentinel that triggers auto-tuning from genome length
   and thread count in runner.py. Test updated to assert the sentinel.

3. tests/test_read_simulator/test_stitch_outputs.py (11 tests)
   - concat() now takes a destination Path (not a gzip handle) and writes
     raw bytes — concatenated gzip streams are valid gzip per the spec. The
     concat tests now pass a path and read it back via gzip.open.
   - merge_bam now calls pysam.cat (not pysam.merge + pysam.sort). The four
     old merge/sort/temp-file/chunk-batching tests were replaced with three
     cat-based equivalents (call_count, output path, full-input ordering).
   - The main() tests that asserted against StringIO buffers were updated
     to read back from the actual destination paths, since byte-level
     concat writes through the path, not through the handle.

All 599 tests pass locally (`pytest tests/`); should unblock the GitHub
Actions failures on develop.

Co-Authored-By: Claude Opus 4.7 (1M context) &lt;noreply@anthropic.com&gt;
diff --git a/tests/test_read_simulator/test_generate_reads.py b/tests/test_read_simulator/test_generate_reads.py
@@ -321,13 +321,37 @@ def _make_options(paired=False, seed=0):
     opts.read_len = _READ_LEN
     opts.paired_ended = paired
     opts.coverage = 5
+    # Enable BAM so generate_reads streams Read objects to the collecting OFW for
+    # inspection. FASTQ stays off — we don't need its bytes for these tests.
     opts.produce_fastq = False
-    opts.produce_bam = False
+    opts.produce_bam = True
     opts.produce_vcf = False
     opts.overwrite_output = True
     return opts
 
 
+class _CollectingOFW:
+    """
+    Minimal OutputFileWriter stand-in for generate_reads tests.
+
+    generate_reads now streams Read objects to its output_file_writer instead of
+    returning a list. Tests that want to inspect the generated reads enable
+    produce_bam=True and pass an instance of this class as the ofw. Every
+    ``write_bam_record`` call is captured into ``bam_records`` for assertion.
+    """
+    def __init__(self):
+        # generate_reads dereferences ofw.bam and ofw.files_to_write[ofw.bam].
+        # Provide a stub handle so those lookups succeed; we don't use the handle.
+        self.bam = "_collecting_bam"
+        self.fq1 = None
+        self.fq2 = None
+        self.files_to_write = {self.bam: SimpleNamespace(write=lambda *a, **kw: None)}
+        self.bam_records = []
+
+    def write_bam_record(self, read, contig_id, bam_handle, read_length):
+        self.bam_records.append(read)
+
+
 def _make_models(read_len=_READ_LEN, frag_mean=300):
     error_model = SequencingErrorModel(read_length=read_len)
     qual_model = TraditionalQualityModel()
@@ -407,46 +431,55 @@ def test_generate_reads_single_ended_returns_read_none_pairs():
     err, qual, frag = _make_models()
     opts = _make_options(paired=False)
     cv = ContigVariants()
+    ofw = _CollectingOFW()
 
-    results = generate_reads(0, ref, err, 3, qual, frag, get_uniform_gc_model(), cv,
-                             _all_span_targeted(), _nothing_discarded(),
-                             opts, None, "chr1", 0, 0)
+    generate_reads(0, ref, err, 3, qual, frag, get_uniform_gc_model(), cv,
+                   _all_span_targeted(), _nothing_discarded(),
+                   opts, ofw, "chr1", 0, 0)
 
-    assert isinstance(results, list)
-    assert len(results) > 0
-    for read1, read2 in results:
-        assert isinstance(read1, Read)
-        assert read2 is None
+    # In SE mode only read1 records are emitted — all forward strand.
+    assert len(ofw.bam_records) > 0
+    for read in ofw.bam_records:
+        assert isinstance(read, Read)
+        assert read.is_reverse is False
 
 
 def test_generate_reads_paired_ended_returns_read_read_pairs():
     ref = _make_reference()
     err, qual, frag = _make_models()
     opts = _make_options(paired=True)
     cv = ContigVariants()
+    ofw = _CollectingOFW()
 
-    results = generate_reads(0, ref, err, 3, qual, frag, get_uniform_gc_model(), cv,
-                             _all_span_targeted(), _nothing_discarded(),
-                             opts, None, "chr1", 0, 0)
+    generate_reads(0, ref, err, 3, qual, frag, get_uniform_gc_model(), cv,
+                   _all_span_targeted(), _nothing_discarded(),
+                   opts, ofw, "chr1", 0, 0)
 
-    assert len(results) > 0
-    for read1, read2 in results:
-        assert isinstance(read1, Read)
-        assert isinstance(read2, Read)
+    # PE mode emits both forward (r1) and reverse (r2) records, interleaved by
+    # position via the heap-buffer in generate_reads. Verify both strands appear.
+    assert len(ofw.bam_records) > 0
+    forwards = [r for r in ofw.bam_records if not r.is_reverse]
+    reverses = [r for r in ofw.bam_records if r.is_reverse]
+    assert len(forwards) > 0
+    assert len(reverses) > 0
+    # Each fragment contributes one r1 and one r2.
+    assert len(forwards) == len(reverses)
 
 
 def test_generate_reads_read_length_matches_options():
     ref = _make_reference()
     err, qual, frag = _make_models()
     opts = _make_options(paired=False)
     cv = ContigVariants()
+    ofw = _CollectingOFW()
 
-    results = generate_reads(0, ref, err, 3, qual, frag, get_uniform_gc_model(), cv,
-                             _all_span_targeted(), _nothing_discarded(),
-                             opts, None, "chr1", 0, 0)
+    generate_reads(0, ref, err, 3, qual, frag, get_uniform_gc_model(), cv,
+                   _all_span_targeted(), _nothing_discarded(),
+                   opts, ofw, "chr1", 0, 0)
 
-    for read1, _ in results:
-        assert len(read1.read_sequence) == _READ_LEN
+    assert len(ofw.bam_records) > 0
+    for read in ofw.bam_records:
+        assert len(read.read_sequence) == _READ_LEN
 
 
 # ---------------------------------------------------------------------------
@@ -459,13 +492,14 @@ def test_generate_reads_targeted_region_flag_false_filters_all():
     err, qual, frag = _make_models()
     opts = _make_options(paired=False)
     cv = ContigVariants()
+    ofw = _CollectingOFW()
     no_target = [(0, _SPAN, False)]
 
-    results = generate_reads(0, ref, err, 3, qual, frag, get_uniform_gc_model(), cv,
-                             no_target, _nothing_discarded(),
-                             opts, None, "chr1", 0, 0)
+    generate_reads(0, ref, err, 3, qual, frag, get_uniform_gc_model(), cv,
+                   no_target, _nothing_discarded(),
+                   opts, ofw, "chr1", 0, 0)
 
-    assert results == []
+    assert ofw.bam_records == []
 
 
 def test_generate_reads_discard_region_removes_all():
@@ -474,13 +508,14 @@ def test_generate_reads_discard_region_removes_all():
     err, qual, frag = _make_models()
     opts = _make_options(paired=False)
     cv = ContigVariants()
+    ofw = _CollectingOFW()
     discard_all = [(0, _SPAN, True)]
 
-    results = generate_reads(0, ref, err, 3, qual, frag, get_uniform_gc_model(), cv,
-                             _all_span_targeted(), discard_all,
-                             opts, None, "chr1", 0, 0)
+    generate_reads(0, ref, err, 3, qual, frag, get_uniform_gc_model(), cv,
+                   _all_span_targeted(), discard_all,
+                   opts, ofw, "chr1", 0, 0)
 
-    assert results == []
+    assert ofw.bam_records == []
 
 
 def test_generate_reads_discard_flag_false_keeps_reads():
@@ -489,12 +524,13 @@ def test_generate_reads_discard_flag_false_keeps_reads():
     err, qual, frag = _make_models()
     opts = _make_options(paired=False)
     cv = ContigVariants()
+    ofw = _CollectingOFW()
 
-    results = generate_reads(0, ref, err, 3, qual, frag, get_uniform_gc_model(), cv,
-                             _all_span_targeted(), _nothing_discarded(),
-                             opts, None, "chr1", 0, 0)
+    generate_reads(0, ref, err, 3, qual, frag, get_uniform_gc_model(), cv,
+                   _all_span_targeted(), _nothing_discarded(),
+                   opts, ofw, "chr1", 0, 0)
 
-    assert len(results) > 0
+    assert len(ofw.bam_records) > 0
 
 
 # ---------------------------------------------------------------------------
@@ -506,6 +542,7 @@ def test_generate_reads_variants_populated_on_reads():
     ref = _make_reference()
     err, qual, frag = _make_models()
     opts = _make_options(paired=False)
+    ofw = _CollectingOFW()
 
     cv = ContigVariants()
     snv = SingleNucleotideVariant(
@@ -516,11 +553,11 @@ def test_generate_reads_variants_populated_on_reads():
     )
     cv.add_variant(snv)
 
-    results = generate_reads(0, ref, err, 3, qual, frag, get_uniform_gc_model(), cv,
-                             _all_span_targeted(), _nothing_discarded(),
-                             opts, None, "chr1", 0, 0)
+    generate_reads(0, ref, err, 3, qual, frag, get_uniform_gc_model(), cv,
+                   _all_span_targeted(), _nothing_discarded(),
+                   opts, ofw, "chr1", 0, 0)
 
-    reads_with_mutations = [r1 for r1, _ in results if r1.mutations]
+    reads_with_mutations = [r for r in ofw.bam_records if r.mutations]
     assert len(reads_with_mutations) > 0
 
 
@@ -540,30 +577,33 @@ def test_generate_reads_paired_discard_region_removes_all():
     err, qual, frag = _make_models()
     opts = _make_options(paired=True)
     cv = ContigVariants()
+    ofw = _CollectingOFW()
     discard_all = [(0, _SPAN, True)]
 
-    results = generate_reads(0, ref, err, 3, qual, frag, get_uniform_gc_model(), cv,
-                             _all_span_targeted(), discard_all,
-                             opts, None, "chr1", 0, 0)
+    generate_reads(0, ref, err, 3, qual, frag, get_uniform_gc_model(), cv,
+                   _all_span_targeted(), discard_all,
+                   opts, ofw, "chr1", 0, 0)
 
-    assert results == []
+    assert ofw.bam_records == []
 
 
 def test_generate_reads_paired_no_discard_produces_read_pairs():
-    """Paired-end run without discard produces (Read, Read) pairs (regression guard)."""
+    """Paired-end run without discard produces both forward and reverse read records."""
     ref = _make_reference()
     err, qual, frag = _make_models()
     opts = _make_options(paired=True)
     cv = ContigVariants()
+    ofw = _CollectingOFW()
 
-    results = generate_reads(0, ref, err, 3, qual, frag, get_uniform_gc_model(), cv,
-                             _all_span_targeted(), _nothing_discarded(),
-                             opts, None, "chr1", 0, 0)
+    generate_reads(0, ref, err, 3, qual, frag, get_uniform_gc_model(), cv,
+                   _all_span_targeted(), _nothing_discarded(),
+                   opts, ofw, "chr1", 0, 0)
 
-    assert len(results) > 0
-    for read1, read2 in results:
-        assert isinstance(read1, Read)
-        assert isinstance(read2, Read)
+    forwards = [r for r in ofw.bam_records if not r.is_reverse]
+    reverses = [r for r in ofw.bam_records if r.is_reverse]
+    assert len(forwards) > 0
+    assert len(reverses) > 0
+    assert len(forwards) == len(reverses)
 
 
 # ---------------------------------------------------------------------------
diff --git a/tests/test_read_simulator/test_options.py b/tests/test_read_simulator/test_options.py
@@ -158,7 +158,9 @@ def test_default_values():
     assert opts.quality_offset == 33
     assert opts.threads == 1
     assert opts.parallel_mode == "contig"
-    assert opts.parallel_block_size == 500000
+    # Default is 0 (sentinel for auto-tune from genome length and thread count).
+    # An explicit positive int in YAML overrides; see runner for the auto-tune logic.
+    assert opts.parallel_block_size == 0
     assert opts.cleanup_splits is True
     assert opts.reuse_splits is False
     assert opts.overwrite_output is False
diff --git a/tests/test_read_simulator/test_stitch_outputs.py b/tests/test_read_simulator/test_stitch_outputs.py
