From 8cc93e4be6024ec0998bd6ecc50508bafd3cac21 Mon Sep 17 00:00:00 2001 From: Joshua Allen Date: Sun, 26 Apr 2026 23:10:06 -0500 Subject: [PATCH] Fix OutputFileWriter crash when produce_fastq=false and only BAM output requested The no-files guard in OutputFileWriter.__init__ was raising ValueError whenever file_handles was empty, but in BAM-only mode the top-level OFW intentionally has no open file handle (pysam manages the merged BAM directly). Guard now exempts the case where a BAM output path is set. Adds unit tests for BAM-only and VCF-only OFW construction, and integration tests for all three produce_fastq=false output combinations through the full runner, which would have caught this regression. Co-Authored-By: Claude Sonnet 4.6 --- .../utils/output_file_writer.py | 2 +- .../test_output_file_writer.py | 36 +++++++++++++++ tests/test_read_simulator/test_runner.py | 44 +++++++++++++++++++ 3 files changed, 81 insertions(+), 1 deletion(-) diff --git a/neat/read_simulator/utils/output_file_writer.py b/neat/read_simulator/utils/output_file_writer.py index 4f685e62..afe3ccf8 100755 --- a/neat/read_simulator/utils/output_file_writer.py +++ b/neat/read_simulator/utils/output_file_writer.py @@ -117,7 +117,7 @@ def __init__(self, self.vcf = vcf self.bam = bam - if not file_handles: + if not file_handles and self.bam is None: _LOG.error("output_file_writer received no files!") raise ValueError diff --git a/tests/test_read_simulator/test_output_file_writer.py b/tests/test_read_simulator/test_output_file_writer.py index 86a3db9c..a64ae040 100644 --- a/tests/test_read_simulator/test_output_file_writer.py +++ b/tests/test_read_simulator/test_output_file_writer.py @@ -124,6 +124,42 @@ def test_ofw_no_files_raises(tmp_path): OutputFileWriter(options=opts) +def test_ofw_bam_only_with_header_is_accepted(tmp_path): + """BAM-only (no FASTQ, no VCF) with a bam_header opens the BAM handle.""" + opts = _make_options(tmp_path, fq1=False, bam=True, + produce_fastq=False, produce_bam=True) + ofw = OutputFileWriter(options=opts, bam_header={"chr1": 1000}) + assert ofw.bam is not None + assert ofw.bam in ofw.files_to_write + assert ofw.fq1 is None + assert ofw.vcf is None + ofw.flush_and_close_files() + + +def test_ofw_bam_only_no_header_is_accepted(tmp_path): + """BAM-only with bam_header=None (the runner's path) must not raise. + pysam manages the merged output file so no handle is opened here.""" + opts = _make_options(tmp_path, fq1=False, bam=True, + produce_fastq=False, produce_bam=True) + ofw = OutputFileWriter(options=opts, bam_header=None) + assert ofw.bam is not None + assert ofw.fq1 is None + assert ofw.vcf is None + assert ofw.bam not in ofw.files_to_write + + +def test_ofw_vcf_only_is_accepted(tmp_path): + """VCF-only (no FASTQ, no BAM) should construct without error.""" + opts = _make_options(tmp_path, fq1=False, vcf=True, + produce_fastq=False, produce_vcf=True) + ofw = OutputFileWriter(options=opts, vcf_header={"chr1": 1000}) + assert ofw.vcf is not None + assert ofw.vcf in ofw.files_to_write + assert ofw.fq1 is None + assert ofw.bam is None + ofw.flush_and_close_files() + + def test_ofw_fq1_is_none_when_not_requested(tmp_path): ofw = _make_ofw(tmp_path, fq1=True) assert ofw.fq2 is None diff --git a/tests/test_read_simulator/test_runner.py b/tests/test_read_simulator/test_runner.py index 56a064de..68a8e8dd 100644 --- a/tests/test_read_simulator/test_runner.py +++ b/tests/test_read_simulator/test_runner.py @@ -415,3 +415,47 @@ def test_runner_with_produce_bam(tmp_path): read_simulator_runner(str(cfg), str(out_dir), "test") bam_files = list(out_dir.glob("*.bam")) assert len(bam_files) >= 1, "Expected at least one BAM output file" + + +def test_runner_bam_only_no_fastq(tmp_path): + """produce_bam=true + produce_fastq=false must write a BAM and no FASTQ.""" + ref = _write_ref(tmp_path / "ref.fa") + cfg = _write_config( + tmp_path / "conf.yml", ref, + produce_fastq="false", + produce_bam="true", + ) + out_dir = tmp_path / "out" + read_simulator_runner(str(cfg), str(out_dir), "test") + assert list(out_dir.glob("*.bam")), "Expected a BAM output file" + assert not list(out_dir.glob("*.fastq.gz")), "Expected no FASTQ output" + + +def test_runner_vcf_only_no_fastq(tmp_path): + """produce_vcf=true + produce_fastq=false must write a VCF and no FASTQ.""" + ref = _write_ref(tmp_path / "ref.fa") + cfg = _write_config( + tmp_path / "conf.yml", ref, + produce_fastq="false", + produce_vcf="true", + ) + out_dir = tmp_path / "out" + read_simulator_runner(str(cfg), str(out_dir), "test") + assert list(out_dir.glob("*.vcf.gz")), "Expected a VCF output file" + assert not list(out_dir.glob("*.fastq.gz")), "Expected no FASTQ output" + + +def test_runner_bam_and_vcf_no_fastq(tmp_path): + """produce_bam=true + produce_vcf=true + produce_fastq=false must write both.""" + ref = _write_ref(tmp_path / "ref.fa") + cfg = _write_config( + tmp_path / "conf.yml", ref, + produce_fastq="false", + produce_bam="true", + produce_vcf="true", + ) + out_dir = tmp_path / "out" + read_simulator_runner(str(cfg), str(out_dir), "test") + assert list(out_dir.glob("*.bam")), "Expected a BAM output file" + assert list(out_dir.glob("*.vcf.gz")), "Expected a VCF output file" + assert not list(out_dir.glob("*.fastq.gz")), "Expected no FASTQ output"