diff --git a/src/commands/simulate/common.rs b/src/commands/simulate/common.rs index 4f1506d58..b94052b1d 100644 --- a/src/commands/simulate/common.rs +++ b/src/commands/simulate/common.rs @@ -48,8 +48,8 @@ pub struct QualityArgs { #[arg(long = "decay-rate", default_value = "0.08")] pub decay_rate: f64, - /// Standard deviation of quality noise - #[arg(long = "quality-noise", default_value = "2.0")] + /// Standard deviation of quality noise (must be finite and >= 0) + #[arg(long = "quality-noise", default_value = "2.0", value_parser = parse_noise_stddev)] pub quality_noise: f64, /// Quality offset for R2 reads (typically negative) @@ -57,6 +57,15 @@ pub struct QualityArgs { pub r2_quality_offset: i8, } +/// Parse and validate a noise standard deviation value for quality score simulation. +fn parse_noise_stddev(s: &str) -> Result { + let val: f64 = s.parse().map_err(|e| format!("invalid float: {e}"))?; + if !val.is_finite() || val < 0.0 { + return Err(format!("quality-noise must be finite and >= 0.0, got {val}")); + } + Ok(val) +} + impl QualityArgs { /// Convert to a [`PositionQualityModel`]. pub fn to_quality_model(&self) -> fgumi_lib::simulate::PositionQualityModel { @@ -600,6 +609,25 @@ mod tests { assert!(a <= 1 && b <= 1); } + #[rstest] + #[case("0.0", 0.0)] + #[case("2.5", 2.5)] + #[case("100.0", 100.0)] + fn test_parse_noise_stddev_accepts_valid(#[case] input: &str, #[case] expected: f64) { + let parsed = parse_noise_stddev(input).expect("valid noise stddev should parse"); + assert!((parsed - expected).abs() < f64::EPSILON); + } + + #[rstest] + #[case("-0.1")] + #[case("NaN")] + #[case("inf")] + #[case("-inf")] + #[case("abc")] + fn test_parse_noise_stddev_rejects_invalid(#[case] input: &str) { + assert!(parse_noise_stddev(input).is_err(), "input should be rejected: {input}"); + } + #[test] fn test_quality_args_zero_noise() { let args = QualityArgs { diff --git a/src/lib/bam_io.rs b/src/lib/bam_io.rs index 3720e0110..9011ae7a5 100644 --- a/src/lib/bam_io.rs +++ b/src/lib/bam_io.rs @@ -354,7 +354,10 @@ pub struct IndexingBamWriter { impl IndexingBamWriter { /// Create a new indexing BAM writer. fn new(inner: MultithreadedWriter, num_refs: usize) -> Self { - let block_info_rx = inner.block_info_receiver().unwrap().clone(); + let block_info_rx = inner + .block_info_receiver() + .expect("block_info_receiver must be available for IndexingBamWriter") + .clone(); Self { current_block_number: inner.current_block_number(), current_block_offset: inner.buffer_offset(), @@ -1318,7 +1321,7 @@ mod tests { let mut chained = ChainedReader::new(buffer, remaining); let mut result = Vec::new(); - chained.read_to_end(&mut result).unwrap(); + chained.read_to_end(&mut result).expect("read_to_end should succeed"); assert_eq!(result, vec![1, 2, 3, 4, 5, 6, 7, 8, 9, 10]); } @@ -1330,7 +1333,7 @@ mod tests { let mut chained = ChainedReader::new(buffer, remaining); let mut result = Vec::new(); - chained.read_to_end(&mut result).unwrap(); + chained.read_to_end(&mut result).expect("read_to_end should succeed"); assert_eq!(result, vec![1, 2, 3]); } @@ -1342,7 +1345,7 @@ mod tests { let mut chained = ChainedReader::new(buffer, remaining); let mut result = Vec::new(); - chained.read_to_end(&mut result).unwrap(); + chained.read_to_end(&mut result).expect("read_to_end should succeed"); assert_eq!(result, vec![1, 2, 3]); } @@ -1419,7 +1422,7 @@ mod tests { assert!(writer.is_ok()); // Just finish without writing records - let writer = writer.unwrap(); + let writer = writer.expect("creating writer should succeed"); let index = writer.finish(); assert!(index.is_ok()); @@ -1556,7 +1559,7 @@ mod tests { let header = Header::default(); let result = create_indexing_bam_writer("-", &header, 6, 2); assert!(result.is_err()); - let err = result.err().unwrap(); + let err = result.err().expect("result should be Err"); assert!(err.to_string().contains("stdout")); } } diff --git a/src/lib/batched_sam_reader.rs b/src/lib/batched_sam_reader.rs index a3521030e..9a90ee003 100644 --- a/src/lib/batched_sam_reader.rs +++ b/src/lib/batched_sam_reader.rs @@ -299,7 +299,7 @@ mod tests { let cursor = Cursor::new(data.as_bytes().to_vec()); let mut reader = BatchedSamReader::new(cursor, 1000); - assert!(reader.fill_buffer().unwrap()); + assert!(reader.fill_buffer().expect("fill_buffer should succeed")); assert!(!reader.buffer().is_empty()); } @@ -315,7 +315,7 @@ mod tests { let cursor = Cursor::new(data.as_bytes().to_vec()); let mut reader = BatchedSamReader::new(cursor, 500); - reader.fill_buffer().unwrap(); + reader.fill_buffer().expect("fill_buffer should succeed"); let mut batches = 0; let mut records = 0; @@ -362,7 +362,7 @@ mod tests { // Manually set small initial buffer for testing reader.set_buffer(vec![0u8; 1024]); // 1KB initial - reader.fill_buffer().unwrap(); + reader.fill_buffer().expect("fill_buffer should succeed"); let initial_capacity = reader.capacity(); // Parse until we hit a batch boundary that triggers growth @@ -373,7 +373,7 @@ mod tests { if reader.record_parsed(seq_len, line_end + 1) { grew = true; } - } else if !reader.fill_buffer().unwrap() { + } else if !reader.fill_buffer().expect("fill_buffer should succeed") { break; } } @@ -405,7 +405,7 @@ mod tests { // Small initial buffer reader.set_buffer(vec![0u8; 4096]); - reader.fill_buffer().unwrap(); + reader.fill_buffer().expect("fill_buffer should succeed"); let mut max_capacity = reader.capacity(); @@ -423,7 +423,7 @@ mod tests { // Verify never shrinks assert!(reader.capacity() >= max_capacity); - } else if !reader.fill_buffer().unwrap() { + } else if !reader.fill_buffer().expect("fill_buffer should succeed") { break; } } @@ -472,7 +472,7 @@ mod tests { reader.set_buffer(vec![0u8; 1024]); // Small buffer // First fill gets partial line - reader.fill_buffer().unwrap(); + reader.fill_buffer().expect("fill_buffer should succeed"); // Should not find complete line yet if buffer is small enough // After second fill, should have complete line @@ -482,7 +482,7 @@ mod tests { found_complete = true; break; } - if !reader.fill_buffer().unwrap() { + if !reader.fill_buffer().expect("fill_buffer should succeed") { break; } } @@ -505,11 +505,11 @@ mod tests { reader.set_buffer(vec![0u8; 512]); let initial = reader.capacity(); - reader.fill_buffer().unwrap(); + reader.fill_buffer().expect("fill_buffer should succeed"); // Don't parse anything - just refill // Second fill should trigger proactive growth if data > 50% of buffer - reader.fill_buffer().unwrap(); + reader.fill_buffer().expect("fill_buffer should succeed"); // Buffer should have grown proactively assert!( @@ -526,7 +526,7 @@ mod tests { let mut reader = BatchedSamReader::new(cursor, 1000); // Should return false (no data) - assert!(!reader.fill_buffer().unwrap()); + assert!(!reader.fill_buffer().expect("fill_buffer should succeed")); assert!(reader.buffer().is_empty()); } @@ -540,7 +540,7 @@ mod tests { let cursor = Cursor::new(data.as_bytes().to_vec()); let mut reader = BatchedSamReader::new(cursor, 1000); - reader.fill_buffer().unwrap(); + reader.fill_buffer().expect("fill_buffer should succeed"); let mut alignment_records = 0; @@ -575,7 +575,7 @@ mod tests { let mut reader = BatchedSamReader::new(cursor, 1000); let mut buf = vec![0u8; 1024]; - let n = reader.read(&mut buf).unwrap(); + let n = reader.read(&mut buf).expect("read should succeed"); assert_eq!(n, 13); assert_eq!(&buf[..n], b"Hello, World!"); } @@ -589,11 +589,11 @@ mod tests { // Read lines using BufRead let mut line = String::new(); - reader.read_line(&mut line).unwrap(); + reader.read_line(&mut line).expect("read_line should succeed"); assert_eq!(line, "Line 1\n"); line.clear(); - reader.read_line(&mut line).unwrap(); + reader.read_line(&mut line).expect("read_line should succeed"); assert_eq!(line, "Line 2\n"); } @@ -605,14 +605,14 @@ mod tests { let mut reader = BatchedSamReader::new(cursor, 1000); // Fill buffer - let buf = reader.fill_buf().unwrap(); + let buf = reader.fill_buf().expect("fill_buf should succeed"); assert_eq!(buf, b"ABCDEFGHIJ"); // Consume 5 bytes reader.consume(5); // Remaining should be FGHIJ - let buf = reader.fill_buf().unwrap(); + let buf = reader.fill_buf().expect("fill_buf should succeed"); assert_eq!(buf, b"FGHIJ"); } @@ -626,7 +626,7 @@ mod tests { reader.set_buffer(vec![0u8; 64 * 1024]); // 64KB buffer let mut buf = vec![0u8; 64 * 1024]; - let _ = reader.read(&mut buf).unwrap(); + let _ = reader.read(&mut buf).expect("read should succeed"); // Should have grown to 128KB (doubled) assert_eq!(reader.capacity(), 128 * 1024); @@ -642,7 +642,7 @@ mod tests { reader.set_buffer(vec![0u8; 64 * 1024]); // 64KB buffer let mut buf = vec![0u8; 64 * 1024]; - let _ = reader.read(&mut buf).unwrap(); + let _ = reader.read(&mut buf).expect("read should succeed"); // Should NOT have grown assert_eq!(reader.capacity(), 64 * 1024); @@ -656,7 +656,7 @@ mod tests { let mut reader = BatchedSamReader::new(cursor, 1000); let mut buf = vec![0u8; 1024]; - let n = reader.read(&mut buf).unwrap(); + let n = reader.read(&mut buf).expect("read should succeed"); assert_eq!(n, 0); } } diff --git a/src/lib/fastq.rs b/src/lib/fastq.rs index f5e3ef67e..74348daa8 100644 --- a/src/lib/fastq.rs +++ b/src/lib/fastq.rs @@ -522,9 +522,20 @@ mod tests { #[test] fn test_skip_reason_from_str_valid() { // Test various valid forms - assert_eq!(SkipReason::from_str("too few bases").unwrap(), SkipReason::TooFewBases); - assert_eq!(SkipReason::from_str("too-few-bases").unwrap(), SkipReason::TooFewBases); - assert_eq!(SkipReason::from_str("toofewbases").unwrap(), SkipReason::TooFewBases); + assert_eq!( + SkipReason::from_str("too few bases") + .expect("parsing \"too few bases\" should succeed"), + SkipReason::TooFewBases + ); + assert_eq!( + SkipReason::from_str("too-few-bases") + .expect("parsing \"too-few-bases\" should succeed"), + SkipReason::TooFewBases + ); + assert_eq!( + SkipReason::from_str("toofewbases").expect("parsing \"toofewbases\" should succeed"), + SkipReason::TooFewBases + ); } #[test] @@ -550,13 +561,15 @@ mod tests { let reader = SimdFastqReader::new(Box::new(cursor) as Box); // Simple read structure: 4M (molecular barcode) + 4T (template) - let read_structure = ReadStructure::from_str("4M4T").unwrap(); + let read_structure = + ReadStructure::from_str("4M4T").expect("parsing \"4M4T\" should succeed"); let mut iterator = ReadSetIterator::new(read_structure, reader, vec![]); let result = iterator.next(); assert!(result.is_some()); - let fastq_set = result.unwrap().unwrap(); + let fastq_set = + result.expect("iterator should yield a record").expect("reading record should succeed"); assert_eq!(fastq_set.header, b"read1"); assert_eq!(fastq_set.segments.len(), 2); assert!(fastq_set.skip_reason.is_none()); @@ -583,14 +596,16 @@ mod tests { let reader = SimdFastqReader::new(Box::new(cursor) as Box); // Read structure requires 10 bases total - let read_structure = ReadStructure::from_str("4M6T").unwrap(); + let read_structure = + ReadStructure::from_str("4M6T").expect("parsing \"4M6T\" should succeed"); let mut iterator = ReadSetIterator::new(read_structure, reader, vec![SkipReason::TooFewBases]); let result = iterator.next(); assert!(result.is_some()); - let fastq_set = result.unwrap().unwrap(); + let fastq_set = + result.expect("iterator should yield a record").expect("reading record should succeed"); assert_eq!(fastq_set.header, b"read1"); assert!(fastq_set.segments.is_empty()); assert_eq!(fastq_set.skip_reason, Some(SkipReason::TooFewBases)); @@ -606,7 +621,8 @@ mod tests { let reader = SimdFastqReader::new(Box::new(cursor) as Box); // Read structure requires 10 bases total - let read_structure = ReadStructure::from_str("4M6T").unwrap(); + let read_structure = + ReadStructure::from_str("4M6T").expect("parsing \"4M6T\" should succeed"); let mut iterator = ReadSetIterator::new(read_structure, reader, vec![]); // This should return an error because TooFewBases is not in skip_reasons @@ -624,17 +640,24 @@ mod tests { let cursor = Cursor::new(fastq_data.to_vec()); let reader = SimdFastqReader::new(Box::new(cursor) as Box); - let read_structure = ReadStructure::from_str("4M4T").unwrap(); + let read_structure = + ReadStructure::from_str("4M4T").expect("parsing \"4M4T\" should succeed"); let mut iterator = ReadSetIterator::new(read_structure, reader, vec![]); // First record - let first = iterator.next().unwrap().unwrap(); + let first = iterator + .next() + .expect("iterator should yield first record") + .expect("reading first record should succeed"); assert_eq!(first.header, b"read1"); assert_eq!(first.segments[0].seq, b"ACGT"); assert_eq!(first.segments[1].seq, b"AAAA"); // Second record - let second = iterator.next().unwrap().unwrap(); + let second = iterator + .next() + .expect("iterator should yield second record") + .expect("reading second record should succeed"); assert_eq!(second.header, b"read2"); assert_eq!(second.segments[0].seq, b"TGCA"); assert_eq!(second.segments[1].seq, b"TTTT"); @@ -653,10 +676,14 @@ mod tests { let reader = SimdFastqReader::new(Box::new(cursor) as Box); // 4M + variable T (remaining bases go to template) - let read_structure = ReadStructure::from_str("4M+T").unwrap(); + let read_structure = + ReadStructure::from_str("4M+T").expect("parsing \"4M+T\" should succeed"); let mut iterator = ReadSetIterator::new(read_structure, reader, vec![]); - let result = iterator.next().unwrap().unwrap(); + let result = iterator + .next() + .expect("iterator should yield a record") + .expect("reading record should succeed"); assert_eq!(result.segments.len(), 2); // Fixed molecular barcode segment diff --git a/src/lib/grouper.rs b/src/lib/grouper.rs index 96cea849d..a3450a26c 100644 --- a/src/lib/grouper.rs +++ b/src/lib/grouper.rs @@ -1021,8 +1021,15 @@ impl FastqGrouper { // Validate names match and get base_name (in block so names is dropped before pop) let base_name = { // Peek at the first record from each stream - let names: Vec<_> = - self.pending_records.iter().map(|q| &q.front().unwrap().name).collect(); + let names: Vec<_> = self + .pending_records + .iter() + .map(|q| { + &q.front() + .expect("pending queue must be non-empty inside all-non-empty loop") + .name + }) + .collect(); // Copy base_name immediately let base_name = strip_read_suffix(names[0]).to_vec(); @@ -1047,8 +1054,14 @@ impl FastqGrouper { }; // Pop records from all streams - let records: Vec<_> = - self.pending_records.iter_mut().map(|q| q.pop_front().unwrap()).collect(); + let records: Vec<_> = self + .pending_records + .iter_mut() + .map(|q| { + q.pop_front() + .expect("pending queue must be non-empty inside all-non-empty loop") + }) + .collect(); templates.push(FastqTemplate { name: base_name, records }); } @@ -1112,7 +1125,7 @@ mod tests { let mut grouper = SingleRecordGrouper::new(); assert!(!grouper.has_pending()); - let result = grouper.finish().unwrap(); + let result = grouper.finish().expect("finish should succeed"); assert!(result.is_none()); } @@ -1121,7 +1134,7 @@ mod tests { let mut grouper = SingleRawRecordGrouper::new(); assert!(!grouper.has_pending()); - let result = grouper.finish().unwrap(); + let result = grouper.finish().expect("finish should succeed"); assert!(result.is_none()); } @@ -1137,7 +1150,7 @@ mod tests { DecodedRecord::from_raw_bytes(raw2.clone(), GroupKey::default()), ]; - let groups = grouper.add_records(records).unwrap(); + let groups = grouper.add_records(records).expect("add_records should succeed"); assert_eq!(groups.len(), 2); assert_eq!(groups[0], raw1); assert_eq!(groups[1], raw2); @@ -1160,7 +1173,8 @@ mod tests { #[test] fn test_parse_single_fastq_record() { let data = b"@read1\nACGT\n+\nIIII\n"; - let (record, consumed) = parse_single_fastq_record(data).unwrap(); + let (record, consumed) = + parse_single_fastq_record(data).expect("parse single FASTQ record"); assert_eq!(record.name, b"read1"); assert_eq!(record.sequence, b"ACGT"); assert_eq!(record.quality, b"IIII"); @@ -1170,7 +1184,7 @@ mod tests { #[test] fn test_parse_fastq_records_multiple() { let data = b"@read1\nACGT\n+\nIIII\n@read2\nTGCA\n+\nJJJJ\n"; - let (records, leftover) = parse_fastq_records(data).unwrap(); + let (records, leftover) = parse_fastq_records(data).expect("failed to parse FASTQ records"); assert_eq!(records.len(), 2); assert_eq!(records[0].name, b"read1"); assert_eq!(records[1].name, b"read2"); @@ -1180,7 +1194,7 @@ mod tests { #[test] fn test_parse_fastq_incomplete_record() { let data = b"@read1\nACGT\n+\n"; - let (records, leftover) = parse_fastq_records(data).unwrap(); + let (records, leftover) = parse_fastq_records(data).expect("failed to parse FASTQ records"); assert!(records.is_empty()); assert_eq!(leftover, data); } @@ -1188,7 +1202,7 @@ mod tests { #[test] fn test_parse_fastq_with_leftover() { let data = b"@read1\nACGT\n+\nIIII\n@read2\nTG"; - let (records, leftover) = parse_fastq_records(data).unwrap(); + let (records, leftover) = parse_fastq_records(data).expect("failed to parse FASTQ records"); assert_eq!(records.len(), 1); assert_eq!(records[0].name, b"read1"); assert_eq!(leftover, b"@read2\nTG"); @@ -1209,11 +1223,16 @@ mod tests { let mut grouper = FastqGrouper::new(2); // Add R1 record - grouper.add_bytes_for_stream(0, b"@read1/1\nACGT\n+\nIIII\n").unwrap(); + grouper + .add_bytes_for_stream(0, b"@read1/1\nACGT\n+\nIIII\n") + .expect("add_bytes_for_stream failed"); // Add R2 record - grouper.add_bytes_for_stream(1, b"@read1/2\nTGCA\n+\nJJJJ\n").unwrap(); + grouper + .add_bytes_for_stream(1, b"@read1/2\nTGCA\n+\nJJJJ\n") + .expect("add_bytes_for_stream failed"); - let templates = grouper.drain_complete_templates().unwrap(); + let templates = + grouper.drain_complete_templates().expect("drain_complete_templates failed"); assert_eq!(templates.len(), 1); assert_eq!(templates[0].name, b"read1"); assert_eq!(templates[0].records.len(), 2); @@ -1228,13 +1247,14 @@ mod tests { // Add multiple R1 records grouper .add_bytes_for_stream(0, b"@read1/1\nACGT\n+\nIIII\n@read2/1\nAAAA\n+\nIIII\n") - .unwrap(); + .expect("add_bytes_for_stream should succeed for R1"); // Add multiple R2 records grouper .add_bytes_for_stream(1, b"@read1/2\nTGCA\n+\nJJJJ\n@read2/2\nTTTT\n+\nJJJJ\n") - .unwrap(); + .expect("add_bytes_for_stream should succeed for R2"); - let templates = grouper.drain_complete_templates().unwrap(); + let templates = + grouper.drain_complete_templates().expect("drain_complete_templates failed"); assert_eq!(templates.len(), 2); assert_eq!(templates[0].name, b"read1"); assert_eq!(templates[1].name, b"read2"); @@ -1245,24 +1265,30 @@ mod tests { let mut grouper = FastqGrouper::new(2); // Add R1 record only - grouper.add_bytes_for_stream(0, b"@read1/1\nACGT\n+\nIIII\n").unwrap(); + grouper + .add_bytes_for_stream(0, b"@read1/1\nACGT\n+\nIIII\n") + .expect("add_bytes_for_stream failed"); // No complete templates yet - let templates = grouper.drain_complete_templates().unwrap(); + let templates = + grouper.drain_complete_templates().expect("drain_complete_templates failed"); assert!(templates.is_empty()); assert!(grouper.has_pending()); // Now add R2 record - grouper.add_bytes_for_stream(1, b"@read1/2\nTGCA\n+\nJJJJ\n").unwrap(); + grouper + .add_bytes_for_stream(1, b"@read1/2\nTGCA\n+\nJJJJ\n") + .expect("add_bytes_for_stream failed"); - let templates = grouper.drain_complete_templates().unwrap(); + let templates = + grouper.drain_complete_templates().expect("drain_complete_templates failed"); assert_eq!(templates.len(), 1); } #[test] fn test_fastq_grouper_finish_empty() { let mut grouper = FastqGrouper::new(2); - let result = grouper.finish().unwrap(); + let result = grouper.finish().expect("finish should succeed"); assert!(result.is_none()); } @@ -1271,8 +1297,12 @@ mod tests { let mut grouper = FastqGrouper::new(2); // Add mismatched records - grouper.add_bytes_for_stream(0, b"@read1/1\nACGT\n+\nIIII\n").unwrap(); - grouper.add_bytes_for_stream(1, b"@read2/2\nTGCA\n+\nJJJJ\n").unwrap(); + grouper + .add_bytes_for_stream(0, b"@read1/1\nACGT\n+\nIIII\n") + .expect("add_bytes_for_stream failed"); + grouper + .add_bytes_for_stream(1, b"@read2/2\nTGCA\n+\nJJJJ\n") + .expect("add_bytes_for_stream failed"); let result = grouper.drain_complete_templates(); assert!(result.is_err()); @@ -1325,7 +1355,7 @@ mod tests { fn test_record_position_grouper_empty() { let mut grouper = RecordPositionGrouper::new(); assert!(!grouper.has_pending()); - let result = grouper.finish().unwrap(); + let result = grouper.finish().expect("finish should succeed"); assert!(result.is_none()); } @@ -1335,11 +1365,12 @@ mod tests { let key = GroupKey::single(0, 100, 0, 0, 0, 12345); let decoded = make_decoded(key, false, false, None); - let groups = grouper.add_records(vec![decoded]).unwrap(); + let groups = grouper.add_records(vec![decoded]).expect("add_records should succeed"); assert!(groups.is_empty()); // Not emitted yet assert!(grouper.has_pending()); - let final_group = grouper.finish().unwrap().expect("should emit final group"); + let final_group = + grouper.finish().expect("finish should succeed").expect("should emit final group"); assert_eq!(final_group.records.len(), 1); assert_eq!(final_group.group_key.ref_id1, 0); assert_eq!(final_group.group_key.pos1, 100); @@ -1358,10 +1389,11 @@ mod tests { make_decoded(key3, true, true, Some("4M")), ]; - let groups = grouper.add_records(records).unwrap(); + let groups = grouper.add_records(records).expect("add_records should succeed"); assert!(groups.is_empty()); // All same position — not emitted yet - let final_group = grouper.finish().unwrap().expect("should emit final group"); + let final_group = + grouper.finish().expect("finish should succeed").expect("should emit final group"); assert_eq!(final_group.records.len(), 3); } @@ -1378,7 +1410,7 @@ mod tests { make_decoded(key_pos3, false, false, None), ]; - let groups = grouper.add_records(records).unwrap(); + let groups = grouper.add_records(records).expect("add_records should succeed"); // First two positions emitted when boundary detected assert_eq!(groups.len(), 2); assert_eq!(groups[0].group_key.pos1, 100); @@ -1387,7 +1419,8 @@ mod tests { assert_eq!(groups[1].records.len(), 1); // Third position still pending - let final_group = grouper.finish().unwrap().expect("should emit final group"); + let final_group = + grouper.finish().expect("finish should succeed").expect("should emit final group"); assert_eq!(final_group.group_key.pos1, 300); } @@ -1401,10 +1434,11 @@ mod tests { make_secondary_decoded(11111), // Should be skipped ]; - let groups = grouper.add_records(records).unwrap(); + let groups = grouper.add_records(records).expect("add_records should succeed"); assert!(groups.is_empty()); - let final_group = grouper.finish().unwrap().expect("should emit final group"); + let final_group = + grouper.finish().expect("finish should succeed").expect("should emit final group"); assert_eq!(final_group.records.len(), 1); // Only primary kept } @@ -1423,10 +1457,11 @@ mod tests { make_decoded(r2_key, true, false, Some("4M")), ]; - let groups = grouper.add_records(records).unwrap(); + let groups = grouper.add_records(records).expect("add_records should succeed"); assert!(groups.is_empty()); // Same position — all in one group - let final_group = grouper.finish().unwrap().expect("should emit final group"); + let final_group = + grouper.finish().expect("finish should succeed").expect("should emit final group"); assert_eq!(final_group.records.len(), 2); } @@ -1452,12 +1487,13 @@ mod tests { make_decoded(key6, false, false, None), ]; - let groups = grouper.add_records(records).unwrap(); + let groups = grouper.add_records(records).expect("add_records should succeed"); assert_eq!(groups.len(), 2); assert_eq!(groups[0].records.len(), 3); assert_eq!(groups[1].records.len(), 2); - let final_group = grouper.finish().unwrap().expect("should emit final group"); + let final_group = + grouper.finish().expect("finish should succeed").expect("should emit final group"); assert_eq!(final_group.records.len(), 1); } @@ -1503,11 +1539,12 @@ mod tests { // Verify position keys differ (this is the bug scenario) assert_ne!(r1_key.position_key(), r2_key.position_key()); - let groups = grouper.add_records(vec![r1, r2]).unwrap(); + let groups = grouper.add_records(vec![r1, r2]).expect("add_records should succeed"); // Both should be in the same group — no emission yet assert!(groups.is_empty()); - let final_group = grouper.finish().unwrap().expect("should emit final group"); + let final_group = + grouper.finish().expect("finish should succeed").expect("should emit final group"); assert_eq!(final_group.records.len(), 2, "R1 and R2 should be in the same group"); assert_eq!(final_group.group_key.ref_id1, 5, "Group key should use R1's position"); } @@ -1525,7 +1562,7 @@ mod tests { make_decoded(r2_key, false, false, None), ]; - let groups = grouper.add_records(records).unwrap(); + let groups = grouper.add_records(records).expect("add_records should succeed"); // Position boundary with different name_hash → group emitted assert_eq!(groups.len(), 1); assert_eq!(groups[0].records.len(), 1); @@ -1602,7 +1639,7 @@ mod tests { #[test] fn test_build_templates_empty() { - let result = build_templates_from_records(vec![]).unwrap(); + let result = build_templates_from_records(vec![]).expect("build templates from records"); assert!(result.is_empty()); } @@ -1622,7 +1659,8 @@ mod tests { .build(); let decoded = DecodedRecord::new(record, key); - let templates = build_templates_from_records(vec![decoded]).unwrap(); + let templates = + build_templates_from_records(vec![decoded]).expect("build templates from records"); assert_eq!(templates.len(), 1); } @@ -1659,7 +1697,8 @@ mod tests { r2_key, ); - let templates = build_templates_from_records(vec![r1, r2]).unwrap(); + let templates = + build_templates_from_records(vec![r1, r2]).expect("build templates from records"); assert_eq!(templates.len(), 1); } @@ -1706,7 +1745,8 @@ mod tests { ), ]; - let templates = build_templates_from_records(records).unwrap(); + let templates = + build_templates_from_records(records).expect("build templates from records"); assert_eq!(templates.len(), 3); } @@ -1729,7 +1769,8 @@ mod tests { ); let decoded = DecodedRecord::from_raw_bytes(raw, key); - let templates = build_templates_from_records(vec![decoded]).unwrap(); + let templates = + build_templates_from_records(vec![decoded]).expect("build templates from records"); assert_eq!(templates.len(), 1); } @@ -1769,7 +1810,8 @@ mod tests { let records = vec![DecodedRecord::from_raw_bytes(r1, key1), DecodedRecord::from_raw_bytes(r2, key2)]; - let templates = build_templates_from_records(records).unwrap(); + let templates = + build_templates_from_records(records).expect("build templates from records"); assert_eq!(templates.len(), 1); } diff --git a/src/lib/header.rs b/src/lib/header.rs index ffad64b12..e5ff2e5e4 100644 --- a/src/lib/header.rs +++ b/src/lib/header.rs @@ -182,7 +182,10 @@ mod tests { fn test_get_last_program_id_single() { let mut header = Header::default(); let pg = Map::::default(); - header.programs_mut().add(BString::from("bwa"), pg).unwrap(); + header + .programs_mut() + .add(BString::from("bwa"), pg) + .expect("adding program to header should succeed"); assert_eq!(get_last_program_id(&header), Some("bwa".to_string())); } @@ -192,12 +195,20 @@ mod tests { // Add first program let pg1 = Map::::default(); - header.programs_mut().add(BString::from("bwa"), pg1).unwrap(); + header + .programs_mut() + .add(BString::from("bwa"), pg1) + .expect("adding program to header should succeed"); // Add second program that references the first - let pg2 = - Map::::builder().insert(tag::PREVIOUS_PROGRAM_ID, "bwa").build().unwrap(); - header.programs_mut().add(BString::from("samtools"), pg2).unwrap(); + let pg2 = Map::::builder() + .insert(tag::PREVIOUS_PROGRAM_ID, "bwa") + .build() + .expect("build should succeed"); + header + .programs_mut() + .add(BString::from("samtools"), pg2) + .expect("adding program to header should succeed"); // The last program should be samtools (not referenced by anyone) assert_eq!(get_last_program_id(&header), Some("samtools".to_string())); @@ -213,7 +224,10 @@ mod tests { fn test_make_unique_program_id_with_collision() { let mut header = Header::default(); let pg = Map::::default(); - header.programs_mut().add(BString::from("fgumi"), pg).unwrap(); + header + .programs_mut() + .add(BString::from("fgumi"), pg) + .expect("adding program to header should succeed"); assert_eq!(make_unique_program_id(&header, "fgumi"), "fgumi.1"); } @@ -223,10 +237,16 @@ mod tests { let mut header = Header::default(); let pg1 = Map::::default(); - header.programs_mut().add(BString::from("fgumi"), pg1).unwrap(); + header + .programs_mut() + .add(BString::from("fgumi"), pg1) + .expect("adding program to header should succeed"); let pg2 = Map::::default(); - header.programs_mut().add(BString::from("fgumi.1"), pg2).unwrap(); + header + .programs_mut() + .add(BString::from("fgumi.1"), pg2) + .expect("adding program to header should succeed"); assert_eq!(make_unique_program_id(&header, "fgumi"), "fgumi.2"); } @@ -234,13 +254,15 @@ mod tests { #[test] fn test_add_pg_record_empty_header() { let header = Header::default(); - let result = add_pg_record(header, "1.0.0", "fgumi test").unwrap(); + let result = + add_pg_record(header, "1.0.0", "fgumi test").expect("add_pg_record should succeed"); let programs = result.programs(); assert_eq!(programs.as_ref().len(), 1); assert!(programs.as_ref().contains_key(b"fgumi".as_slice())); // Verify the program has expected fields - let pg = programs.as_ref().get(b"fgumi".as_slice()).unwrap(); + let pg = + programs.as_ref().get(b"fgumi".as_slice()).expect("expected key should be present"); assert_eq!( pg.other_fields().get(&tag::NAME).map(std::convert::AsRef::as_ref), Some(b"fgumi".as_slice()) @@ -260,15 +282,20 @@ mod tests { fn test_add_pg_record_with_existing_fgumi() { let mut header = Header::default(); let pg = Map::::default(); - header.programs_mut().add(BString::from("fgumi"), pg).unwrap(); + header + .programs_mut() + .add(BString::from("fgumi"), pg) + .expect("adding program to header should succeed"); - let result = add_pg_record(header, "1.0.0", "fgumi test2").unwrap(); + let result = + add_pg_record(header, "1.0.0", "fgumi test2").expect("add_pg_record should succeed"); let programs = result.programs(); assert_eq!(programs.as_ref().len(), 2); assert!(programs.as_ref().contains_key(b"fgumi.1".as_slice())); // Verify PP chaining - let pg = programs.as_ref().get(b"fgumi.1".as_slice()).unwrap(); + let pg = + programs.as_ref().get(b"fgumi.1".as_slice()).expect("expected key should be present"); assert_eq!( pg.other_fields().get(&tag::PREVIOUS_PROGRAM_ID).map(std::convert::AsRef::as_ref), Some(b"fgumi".as_slice()) @@ -284,14 +311,19 @@ mod tests { .insert(tag::NAME, "bwa") .insert(tag::VERSION, "0.7.17") .build() - .unwrap(); - header.programs_mut().add(BString::from("bwa"), bwa_pg).unwrap(); - - let result = add_pg_record(header, "1.0.0", "fgumi group -i in.bam").unwrap(); + .expect("building program map should succeed"); + header + .programs_mut() + .add(BString::from("bwa"), bwa_pg) + .expect("adding program to header should succeed"); + + let result = add_pg_record(header, "1.0.0", "fgumi group -i in.bam") + .expect("add_pg_record should succeed"); let programs = result.programs(); // fgumi should chain to bwa - let pg = programs.as_ref().get(b"fgumi".as_slice()).unwrap(); + let pg = + programs.as_ref().get(b"fgumi".as_slice()).expect("expected key should be present"); assert_eq!( pg.other_fields().get(&tag::PREVIOUS_PROGRAM_ID).map(std::convert::AsRef::as_ref), Some(b"bwa".as_slice()) @@ -301,13 +333,15 @@ mod tests { #[test] fn test_add_pg_to_builder() { let builder = Header::builder(); - let builder = add_pg_to_builder(builder, "1.0.0", "fgumi extract").unwrap(); + let builder = add_pg_to_builder(builder, "1.0.0", "fgumi extract") + .expect("add_pg_to_builder should succeed"); let header = builder.build(); let programs = header.programs(); assert_eq!(programs.as_ref().len(), 1); - let pg = programs.as_ref().get(b"fgumi".as_slice()).unwrap(); + let pg = + programs.as_ref().get(b"fgumi".as_slice()).expect("expected key should be present"); assert_eq!( pg.other_fields().get(&tag::NAME).map(std::convert::AsRef::as_ref), Some(b"fgumi".as_slice()) @@ -318,7 +352,7 @@ mod tests { #[test] fn test_add_pg_record_empty_command_line() { let header = Header::default(); - let result = add_pg_record(header, "1.0.0", "").unwrap(); + let result = add_pg_record(header, "1.0.0", "").expect("add_pg_record should succeed"); let programs = result.programs(); assert_eq!(programs.as_ref().len(), 1); assert!(programs.as_ref().contains_key(b"fgumi".as_slice())); @@ -329,14 +363,16 @@ mod tests { use crate::bam_io::create_bam_writer; use tempfile::TempDir; - let dir = TempDir::new().unwrap(); + let dir = TempDir::new().expect("creating temp file/dir should succeed"); let output_path = dir.path().join("test.bam"); let header = Header::default(); - let result = add_pg_record(header, "1.0.0", "fgumi test").unwrap(); + let result = + add_pg_record(header, "1.0.0", "fgumi test").expect("add_pg_record should succeed"); // Try to write the header to a BAM file - let _writer = create_bam_writer(&output_path, &result, 1, 6).unwrap(); + let _writer = create_bam_writer(&output_path, &result, 1, 6) + .expect("creating BAM writer should succeed"); } #[test] @@ -349,20 +385,23 @@ mod tests { let header = Header::builder().add_program("SamBuilder", pg_map).build(); // Now add our fgumi @PG record - let result = add_pg_record(header, "1.0.0", "fgumi test").unwrap(); + let result = + add_pg_record(header, "1.0.0", "fgumi test").expect("add_pg_record should succeed"); let programs = result.programs(); assert_eq!(programs.as_ref().len(), 2); // fgumi should chain to SamBuilder - let pg = programs.as_ref().get(b"fgumi".as_slice()).unwrap(); + let pg = + programs.as_ref().get(b"fgumi".as_slice()).expect("expected key should be present"); assert_eq!( pg.other_fields().get(&tag::PREVIOUS_PROGRAM_ID).map(std::convert::AsRef::as_ref), Some(b"SamBuilder".as_slice()) ); // Try to write to BAM - let dir = TempDir::new().unwrap(); + let dir = TempDir::new().expect("creating temp file/dir should succeed"); let output_path = dir.path().join("test.bam"); - let _writer = create_bam_writer(&output_path, &result, 1, 6).unwrap(); + let _writer = create_bam_writer(&output_path, &result, 1, 6) + .expect("creating BAM writer should succeed"); } } diff --git a/src/lib/mi_group.rs b/src/lib/mi_group.rs index 400737dfb..eaba5d52c 100644 --- a/src/lib/mi_group.rs +++ b/src/lib/mi_group.rs @@ -1139,7 +1139,7 @@ mod tests { ]; let mut iter = MiGroupIterator::new(records.into_iter(), "MI"); - let result = iter.next().unwrap().unwrap(); + let result = iter.next().expect("iterator should yield item").expect("item should be Ok"); assert_eq!(result.0, "0"); assert_eq!(result.1.len(), 3); @@ -1158,15 +1158,15 @@ mod tests { ]; let mut iter = MiGroupIterator::new(records.into_iter(), "MI"); - let result = iter.next().unwrap().unwrap(); + let result = iter.next().expect("iterator should yield item").expect("item should be Ok"); assert_eq!(result.0, "0"); assert_eq!(result.1.len(), 2); - let result = iter.next().unwrap().unwrap(); + let result = iter.next().expect("iterator should yield item").expect("item should be Ok"); assert_eq!(result.0, "1"); assert_eq!(result.1.len(), 3); - let result = iter.next().unwrap().unwrap(); + let result = iter.next().expect("iterator should yield item").expect("item should be Ok"); assert_eq!(result.0, "2"); assert_eq!(result.1.len(), 1); @@ -1184,11 +1184,11 @@ mod tests { ]; let mut iter = MiGroupIterator::new(records.into_iter(), "MI"); - let result = iter.next().unwrap().unwrap(); + let result = iter.next().expect("iterator should yield item").expect("item should be Ok"); assert_eq!(result.0, "0"); assert_eq!(result.1.len(), 2); // Skipped record without MI - let result = iter.next().unwrap().unwrap(); + let result = iter.next().expect("iterator should yield item").expect("item should be Ok"); assert_eq!(result.0, "1"); assert_eq!(result.1.len(), 1); @@ -1205,12 +1205,12 @@ mod tests { let mut iter = MiGroupIterator::new(records.into_iter(), "MI"); // First group before error - let result = iter.next().unwrap().unwrap(); + let result = iter.next().expect("iterator should yield item").expect("item should be Ok"); assert_eq!(result.0, "0"); assert_eq!(result.1.len(), 1); // Error - let result = iter.next().unwrap(); + let result = iter.next().expect("iterator should yield item"); assert!(result.is_err()); // Iterator should be done after error @@ -1226,7 +1226,7 @@ mod tests { let records: Vec> = vec![Ok(record1), Ok(record2)]; let mut iter = MiGroupIterator::new(records.into_iter(), "RX"); - let result = iter.next().unwrap().unwrap(); + let result = iter.next().expect("iterator should yield item").expect("item should be Ok"); assert_eq!(result.0, "ACGT"); assert_eq!(result.1.len(), 2); @@ -1257,12 +1257,12 @@ mod tests { }); // First group: base MI "1" with 4 reads (2 /A + 2 /B) - let result = iter.next().unwrap().unwrap(); + let result = iter.next().expect("iterator should yield item").expect("item should be Ok"); assert_eq!(result.0, "1"); assert_eq!(result.1.len(), 4); // Second group: base MI "2" with 2 reads (1 /A + 1 /B) - let result = iter.next().unwrap().unwrap(); + let result = iter.next().expect("iterator should yield item").expect("item should be Ok"); assert_eq!(result.0, "2"); assert_eq!(result.1.len(), 2); @@ -1289,7 +1289,7 @@ mod tests { extract_mi_base(mi).to_string() }); - let result = iter.next().unwrap().unwrap(); + let result = iter.next().expect("iterator should yield item").expect("item should be Ok"); assert_eq!(result.0, "0"); assert_eq!(result.1.len(), 3); @@ -1308,12 +1308,12 @@ mod tests { }); // First group before error - let result = iter.next().unwrap().unwrap(); + let result = iter.next().expect("iterator should yield item").expect("item should be Ok"); assert_eq!(result.0, "0"); assert_eq!(result.1.len(), 1); // Error - let result = iter.next().unwrap(); + let result = iter.next().expect("iterator should yield item"); assert!(result.is_err()); // Iterator should be done after error @@ -1332,7 +1332,7 @@ mod tests { MiGroupIteratorWithTransform::new(records.into_iter(), "MI", str::to_uppercase); // All should be grouped together since they uppercase to "ABC" - let result = iter.next().unwrap().unwrap(); + let result = iter.next().expect("iterator should yield item").expect("item should be Ok"); assert_eq!(result.0, "ABC"); assert_eq!(result.1.len(), 3); @@ -1452,7 +1452,7 @@ mod tests { ]; let mut iter = RawMiGroupIterator::new(records.into_iter(), "MI"); - let result = iter.next().unwrap().unwrap(); + let result = iter.next().expect("iterator should yield item").expect("item should be Ok"); assert_eq!(result.0, "0"); assert_eq!(result.1.len(), 3); @@ -1471,15 +1471,15 @@ mod tests { ]; let mut iter = RawMiGroupIterator::new(records.into_iter(), "MI"); - let result = iter.next().unwrap().unwrap(); + let result = iter.next().expect("iterator should yield item").expect("item should be Ok"); assert_eq!(result.0, "0"); assert_eq!(result.1.len(), 2); - let result = iter.next().unwrap().unwrap(); + let result = iter.next().expect("iterator should yield item").expect("item should be Ok"); assert_eq!(result.0, "1"); assert_eq!(result.1.len(), 3); - let result = iter.next().unwrap().unwrap(); + let result = iter.next().expect("iterator should yield item").expect("item should be Ok"); assert_eq!(result.0, "2"); assert_eq!(result.1.len(), 1); @@ -1497,11 +1497,11 @@ mod tests { ]; let mut iter = RawMiGroupIterator::new(records.into_iter(), "MI"); - let result = iter.next().unwrap().unwrap(); + let result = iter.next().expect("iterator should yield item").expect("item should be Ok"); assert_eq!(result.0, "0"); assert_eq!(result.1.len(), 2); // Skipped records without MI - let result = iter.next().unwrap().unwrap(); + let result = iter.next().expect("iterator should yield item").expect("item should be Ok"); assert_eq!(result.0, "1"); assert_eq!(result.1.len(), 1); @@ -1518,12 +1518,12 @@ mod tests { let mut iter = RawMiGroupIterator::new(records.into_iter(), "MI"); // First group before error - let result = iter.next().unwrap().unwrap(); + let result = iter.next().expect("iterator should yield item").expect("item should be Ok"); assert_eq!(result.0, "0"); assert_eq!(result.1.len(), 1); // Error should be returned after flushing the pending group - let err = iter.next().unwrap(); + let err = iter.next().expect("iterator should yield item"); assert!(err.is_err()); assert!(iter.next().is_none()); @@ -1536,7 +1536,7 @@ mod tests { let mut iter = RawMiGroupIterator::new(records.into_iter(), "MI"); // Error should be returned directly - let result = iter.next().unwrap(); + let result = iter.next().expect("iterator should yield item"); assert!(result.is_err()); // Iterator should be done after error @@ -1549,7 +1549,7 @@ mod tests { vec![Ok(make_raw_bam_with_tag("RX", "ACGT")), Ok(make_raw_bam_with_tag("RX", "ACGT"))]; let mut iter = RawMiGroupIterator::new(records.into_iter(), "RX"); - let result = iter.next().unwrap().unwrap(); + let result = iter.next().expect("iterator should yield item").expect("item should be Ok"); assert_eq!(result.0, "ACGT"); assert_eq!(result.1.len(), 2); @@ -1580,12 +1580,12 @@ mod tests { }); // First group: base MI "1" with 4 reads - let result = iter.next().unwrap().unwrap(); + let result = iter.next().expect("iterator should yield item").expect("item should be Ok"); assert_eq!(result.0, "1"); assert_eq!(result.1.len(), 4); // Second group: base MI "2" with 2 reads - let result = iter.next().unwrap().unwrap(); + let result = iter.next().expect("iterator should yield item").expect("item should be Ok"); assert_eq!(result.0, "2"); assert_eq!(result.1.len(), 2); @@ -1699,12 +1699,12 @@ mod tests { RawMiGroupIterator::new(records.into_iter(), "MI").with_cell_tag(Some([b'C', b'B'])); // First group: MI=1, CB=ACGT - let result = iter.next().unwrap().unwrap(); + let result = iter.next().expect("iterator should yield item").expect("item should be Ok"); assert_eq!(result.0, "1\tACGT"); assert_eq!(result.1.len(), 2); // Second group: MI=1, CB=TGCA - let result = iter.next().unwrap().unwrap(); + let result = iter.next().expect("iterator should yield item").expect("item should be Ok"); assert_eq!(result.0, "1\tTGCA"); assert_eq!(result.1.len(), 2); @@ -1720,7 +1720,7 @@ mod tests { ]; let mut iter = RawMiGroupIterator::new(records.into_iter(), "MI").with_cell_tag(None); - let result = iter.next().unwrap().unwrap(); + let result = iter.next().expect("iterator should yield item").expect("item should be Ok"); assert_eq!(result.0, "1"); assert_eq!(result.1.len(), 2); // Both records in same group @@ -1739,12 +1739,12 @@ mod tests { RawMiGroupIterator::new(records.into_iter(), "MI").with_cell_tag(Some([b'C', b'B'])); // First group: MI=1, no CB (key = "1\t") - let result = iter.next().unwrap().unwrap(); + let result = iter.next().expect("iterator should yield item").expect("item should be Ok"); assert_eq!(result.0, "1\t"); assert_eq!(result.1.len(), 2); // Second group: MI=1, CB=ACGT (key = "1\tACGT") - let result = iter.next().unwrap().unwrap(); + let result = iter.next().expect("iterator should yield item").expect("item should be Ok"); assert_eq!(result.0, "1\tACGT"); assert_eq!(result.1.len(), 1); @@ -1767,12 +1767,12 @@ mod tests { // All three have base MI "1", but different cells // First group: base MI=1, CB=ACGT (2 records: 1/A and 1/B) - let result = iter.next().unwrap().unwrap(); + let result = iter.next().expect("iterator should yield item").expect("item should be Ok"); assert_eq!(result.0, "1\tACGT"); assert_eq!(result.1.len(), 2); // Second group: base MI=1, CB=TGCA - let result = iter.next().unwrap().unwrap(); + let result = iter.next().expect("iterator should yield item").expect("item should be Ok"); assert_eq!(result.0, "1\tTGCA"); assert_eq!(result.1.len(), 1); @@ -1814,12 +1814,13 @@ mod tests { make_raw_decoded_record("MI", "0"), ]; - let batches = grouper.add_records(records).unwrap(); + let batches = grouper.add_records(records).expect("add_records should succeed"); // Only 1 MI group so far, batch_size=10, no complete batch yet assert!(batches.is_empty()); assert!(grouper.has_pending()); - let final_batch = grouper.finish().unwrap().unwrap(); + let final_batch = + grouper.finish().expect("finish should succeed").expect("should return final batch"); assert_eq!(final_batch.groups.len(), 1); assert_eq!(final_batch.groups[0].mi, "0"); assert_eq!(final_batch.groups[0].records.len(), 3); @@ -1838,10 +1839,11 @@ mod tests { make_raw_decoded_record("MI", "2"), ]; - let batches = grouper.add_records(records).unwrap(); + let batches = grouper.add_records(records).expect("add_records should succeed"); assert!(batches.is_empty()); // 3 groups < batch_size 10 - let final_batch = grouper.finish().unwrap().unwrap(); + let final_batch = + grouper.finish().expect("finish should succeed").expect("should return final batch"); assert_eq!(final_batch.groups.len(), 3); assert_eq!(final_batch.groups[0].mi, "0"); assert_eq!(final_batch.groups[0].records.len(), 2); @@ -1863,7 +1865,7 @@ mod tests { make_raw_decoded_record("MI", "4"), ]; - let batches = grouper.add_records(records).unwrap(); + let batches = grouper.add_records(records).expect("add_records should succeed"); // 5 MI values => 4 completed groups (0,1,2,3) while "4" is still current // batch_size=2 => 2 batches of 2 groups each assert_eq!(batches.len(), 2); @@ -1877,7 +1879,8 @@ mod tests { // "4" is still pending assert!(grouper.has_pending()); - let final_batch = grouper.finish().unwrap().unwrap(); + let final_batch = + grouper.finish().expect("finish should succeed").expect("should return final batch"); assert_eq!(final_batch.groups.len(), 1); assert_eq!(final_batch.groups[0].mi, "4"); } @@ -1892,11 +1895,12 @@ mod tests { make_raw_decoded_record("MI", "0"), ]; - let batches = grouper.add_records(records).unwrap(); + let batches = grouper.add_records(records).expect("add_records should succeed"); assert!(batches.is_empty()); // Records without MI are now grouped under "" (matching MiGrouper behavior) - let final_batch = grouper.finish().unwrap().unwrap(); + let final_batch = + grouper.finish().expect("finish should succeed").expect("should return final batch"); assert_eq!(final_batch.groups.len(), 3); assert_eq!(final_batch.groups[0].mi, "0"); assert_eq!(final_batch.groups[0].records.len(), 1); @@ -1920,7 +1924,7 @@ mod tests { let mut grouper = RawMiGrouper::new("MI", 10); assert!(!grouper.has_pending()); - let final_batch = grouper.finish().unwrap(); + let final_batch = grouper.finish().expect("finish should succeed"); assert!(final_batch.is_none()); } @@ -1929,12 +1933,12 @@ mod tests { let mut grouper = RawMiGrouper::new("MI", 10); let records = vec![make_raw_decoded_record("MI", "0")]; - grouper.add_records(records).unwrap(); + grouper.add_records(records).expect("add_records should succeed"); - let batch1 = grouper.finish().unwrap(); + let batch1 = grouper.finish().expect("finish should succeed"); assert!(batch1.is_some()); - let batch2 = grouper.finish().unwrap(); + let batch2 = grouper.finish().expect("finish should succeed"); assert!(batch2.is_none()); } @@ -1947,12 +1951,12 @@ mod tests { // After adding records, current_mi is set let records = vec![make_raw_decoded_record("MI", "0")]; - grouper.add_records(records).unwrap(); + grouper.add_records(records).expect("add_records should succeed"); assert!(grouper.has_pending()); // After adding a different MI, first group moves to pending_groups let records = vec![make_raw_decoded_record("MI", "1")]; - grouper.add_records(records).unwrap(); + grouper.add_records(records).expect("add_records should succeed"); assert!(grouper.has_pending()); } @@ -1991,10 +1995,11 @@ mod tests { DecodedRecord::from_raw_bytes(rec_b, key), ]; - let batches = grouper.add_records(records).unwrap(); + let batches = grouper.add_records(records).expect("add_records should succeed"); assert!(batches.is_empty()); - let final_batch = grouper.finish().unwrap().unwrap(); + let final_batch = + grouper.finish().expect("finish should succeed").expect("should return final batch"); // Secondary was filtered out, and 1/A and 1/B transform to "1" assert_eq!(final_batch.groups.len(), 1); assert_eq!(final_batch.groups[0].mi, "1"); @@ -2022,11 +2027,12 @@ mod tests { make_decoded_record_for_mi_grouper("0"), ]; - let batches = grouper.add_records(records).unwrap(); + let batches = grouper.add_records(records).expect("add_records should succeed"); assert!(batches.is_empty()); assert!(grouper.has_pending()); - let final_batch = grouper.finish().unwrap().unwrap(); + let final_batch = + grouper.finish().expect("finish should succeed").expect("should return final batch"); assert_eq!(final_batch.groups.len(), 1); assert_eq!(final_batch.groups[0].mi, "0"); assert_eq!(final_batch.groups[0].records.len(), 3); @@ -2045,10 +2051,11 @@ mod tests { make_decoded_record_for_mi_grouper("2"), ]; - let batches = grouper.add_records(records).unwrap(); + let batches = grouper.add_records(records).expect("add_records should succeed"); assert!(batches.is_empty()); - let final_batch = grouper.finish().unwrap().unwrap(); + let final_batch = + grouper.finish().expect("finish should succeed").expect("should return final batch"); assert_eq!(final_batch.groups.len(), 3); assert_eq!(final_batch.groups[0].mi, "0"); assert_eq!(final_batch.groups[0].records.len(), 2); @@ -2070,7 +2077,7 @@ mod tests { make_decoded_record_for_mi_grouper("4"), ]; - let batches = grouper.add_records(records).unwrap(); + let batches = grouper.add_records(records).expect("add_records should succeed"); assert_eq!(batches.len(), 2); assert_eq!(batches[0].groups.len(), 2); assert_eq!(batches[0].groups[0].mi, "0"); @@ -2081,7 +2088,8 @@ mod tests { assert!(grouper.has_pending()); - let final_batch = grouper.finish().unwrap().unwrap(); + let final_batch = + grouper.finish().expect("finish should succeed").expect("should return final batch"); assert_eq!(final_batch.groups.len(), 1); assert_eq!(final_batch.groups[0].mi, "4"); } @@ -2100,7 +2108,7 @@ mod tests { let mut grouper = MiGrouper::new("MI", 10); assert!(!grouper.has_pending()); - let final_batch = grouper.finish().unwrap(); + let final_batch = grouper.finish().expect("finish should succeed"); assert!(final_batch.is_none()); } @@ -2109,12 +2117,12 @@ mod tests { let mut grouper = MiGrouper::new("MI", 10); let records = vec![make_decoded_record_for_mi_grouper("0")]; - grouper.add_records(records).unwrap(); + grouper.add_records(records).expect("add_records should succeed"); - let batch1 = grouper.finish().unwrap(); + let batch1 = grouper.finish().expect("finish should succeed"); assert!(batch1.is_some()); - let batch2 = grouper.finish().unwrap(); + let batch2 = grouper.finish().expect("finish should succeed"); assert!(batch2.is_none()); } @@ -2133,10 +2141,11 @@ mod tests { make_decoded_record_for_mi_grouper("2/A"), ]; - let batches = grouper.add_records(records).unwrap(); + let batches = grouper.add_records(records).expect("add_records should succeed"); assert!(batches.is_empty()); - let final_batch = grouper.finish().unwrap().unwrap(); + let final_batch = + grouper.finish().expect("finish should succeed").expect("should return final batch"); assert_eq!(final_batch.groups.len(), 2); assert_eq!(final_batch.groups[0].mi, "1"); assert_eq!(final_batch.groups[0].records.len(), 2); @@ -2151,11 +2160,11 @@ mod tests { assert!(!grouper.has_pending()); let records = vec![make_decoded_record_for_mi_grouper("0")]; - grouper.add_records(records).unwrap(); + grouper.add_records(records).expect("add_records should succeed"); assert!(grouper.has_pending()); let records = vec![make_decoded_record_for_mi_grouper("1")]; - grouper.add_records(records).unwrap(); + grouper.add_records(records).expect("add_records should succeed"); assert!(grouper.has_pending()); } @@ -2167,11 +2176,12 @@ mod tests { let key = crate::unified_pipeline::GroupKey::single(0, 0, 0, 0, 0, 0); let records = vec![DecodedRecord::new(record, key)]; - let batches = grouper.add_records(records).unwrap(); + let batches = grouper.add_records(records).expect("add_records should succeed"); assert!(batches.is_empty()); // Record without MI tag gets default empty string as MI value - let final_batch = grouper.finish().unwrap().unwrap(); + let final_batch = + grouper.finish().expect("finish should succeed").expect("should return final batch"); assert_eq!(final_batch.groups.len(), 1); assert_eq!(final_batch.groups[0].mi, ""); assert_eq!(final_batch.groups[0].records.len(), 1); diff --git a/src/lib/progress.rs b/src/lib/progress.rs index 2eeaa0e20..34c73aed8 100644 --- a/src/lib/progress.rs +++ b/src/lib/progress.rs @@ -365,7 +365,7 @@ mod tests { } for handle in handles { - handle.join().unwrap(); + handle.join().expect("thread should join successfully"); } // Total should be 1000 diff --git a/src/lib/read_info.rs b/src/lib/read_info.rs index 9c39bab06..804ff2af9 100644 --- a/src/lib/read_info.rs +++ b/src/lib/read_info.rs @@ -735,18 +735,18 @@ mod tests { let rg1 = Map::::builder() .insert(rg_tag::LIBRARY, String::from("libA")) .build() - .unwrap(); + .expect("building read group with library should succeed"); header = header.add_read_group(BString::from("RG1"), rg1); // Second read group with library "libB" let rg2 = Map::::builder() .insert(rg_tag::LIBRARY, String::from("libB")) .build() - .unwrap(); + .expect("building read group with library should succeed"); header = header.add_read_group(BString::from("RG2"), rg2); // Third read group with no library (should default to "unknown") - let rg3 = Map::::builder().build().unwrap(); + let rg3 = Map::::builder().build().expect("build should succeed"); header = header.add_read_group(BString::from("RG3"), rg3); let header = header.build(); @@ -829,7 +829,8 @@ mod tests { // Forward strand with 5S at start: alignment_start=100, CIGAR=5S50M // unclipped_start = 100 - 5 = 95 let record = build_record(100, "5S50M", false); - let pos = get_unclipped_position(&record).unwrap(); + let pos = + get_unclipped_position(&record).expect("get_unclipped_position should succeed"); assert_eq!(pos, 95); } @@ -838,7 +839,8 @@ mod tests { // Forward strand with 10H at start: alignment_start=100, CIGAR=10H50M // unclipped_start = 100 - 10 = 90 let record = build_record(100, "10H50M", false); - let pos = get_unclipped_position(&record).unwrap(); + let pos = + get_unclipped_position(&record).expect("get_unclipped_position should succeed"); assert_eq!(pos, 90); } @@ -847,7 +849,8 @@ mod tests { // Forward strand with 10H5S at start: alignment_start=100, CIGAR=10H5S50M // unclipped_start = 100 - 10 - 5 = 85 let record = build_record(100, "10H5S50M", false); - let pos = get_unclipped_position(&record).unwrap(); + let pos = + get_unclipped_position(&record).expect("get_unclipped_position should succeed"); assert_eq!(pos, 85); } @@ -857,7 +860,8 @@ mod tests { // alignment_end = 100 + 50 - 1 = 149 // unclipped_end = 149 + 5 = 154 let record = build_record(100, "50M5S", true); - let pos = get_unclipped_position(&record).unwrap(); + let pos = + get_unclipped_position(&record).expect("get_unclipped_position should succeed"); assert_eq!(pos, 154); } @@ -867,7 +871,8 @@ mod tests { // alignment_end = 100 + 50 - 1 = 149 // unclipped_end = 149 + 10 = 159 let record = build_record(100, "50M10H", true); - let pos = get_unclipped_position(&record).unwrap(); + let pos = + get_unclipped_position(&record).expect("get_unclipped_position should succeed"); assert_eq!(pos, 159); } @@ -877,7 +882,8 @@ mod tests { // alignment_end = 100 + 50 - 1 = 149 // unclipped_end = 149 + 5 + 10 = 164 let record = build_record(100, "50M5S10H", true); - let pos = get_unclipped_position(&record).unwrap(); + let pos = + get_unclipped_position(&record).expect("get_unclipped_position should succeed"); assert_eq!(pos, 164); } @@ -886,17 +892,28 @@ mod tests { // No clipping: alignment_start=100, CIGAR=50M // Forward: unclipped_start = 100 let forward_record = build_record(100, "50M", false); - assert_eq!(get_unclipped_position(&forward_record).unwrap(), 100); + assert_eq!( + get_unclipped_position(&forward_record) + .expect("get_unclipped_position should succeed"), + 100 + ); // Reverse: unclipped_end = 100 + 50 - 1 = 149 let reverse_record = build_record(100, "50M", true); - assert_eq!(get_unclipped_position(&reverse_record).unwrap(), 149); + assert_eq!( + get_unclipped_position(&reverse_record) + .expect("get_unclipped_position should succeed"), + 149 + ); } #[test] fn test_unmapped_read() { let record = RecordBuilder::new().sequence("ACGT").unmapped(true).build(); - assert_eq!(get_unclipped_position(&record).unwrap(), 0); + assert_eq!( + get_unclipped_position(&record).expect("get_unclipped_position should succeed"), + 0 + ); } #[test] @@ -906,7 +923,11 @@ mod tests { // Leading clips = 5H + 3S = 8 // unclipped_start = 100 - 8 = 92 let forward_record = build_record(100, "5H3S10M2I5M3D10M4S6H", false); - assert_eq!(get_unclipped_position(&forward_record).unwrap(), 92); + assert_eq!( + get_unclipped_position(&forward_record) + .expect("get_unclipped_position should succeed"), + 92 + ); // Reverse strand: same CIGAR // alignment_span = 10 + 5 + 3 + 10 = 28 (M + D consume reference) @@ -914,7 +935,11 @@ mod tests { // Trailing clips = 4S + 6H = 10 // unclipped_end = 127 + 10 = 137 let reverse_record = build_record(100, "5H3S10M2I5M3D10M4S6H", true); - assert_eq!(get_unclipped_position(&reverse_record).unwrap(), 137); + assert_eq!( + get_unclipped_position(&reverse_record) + .expect("get_unclipped_position should succeed"), + 137 + ); } } } diff --git a/src/lib/reference.rs b/src/lib/reference.rs index 20c80cefe..ec91eb6bd 100644 --- a/src/lib/reference.rs +++ b/src/lib/reference.rs @@ -417,22 +417,30 @@ mod tests { #[test] fn test_nonexistent_sequence() { - let fasta = create_default_test_fasta().unwrap(); - let reader = ReferenceReader::new(fasta.path()).unwrap(); - - let result = - reader.fetch("chr999", Position::try_from(1).unwrap(), Position::try_from(4).unwrap()); + let fasta = create_default_test_fasta().expect("creating test FASTA should succeed"); + let reader = + ReferenceReader::new(fasta.path()).expect("creating reference reader should succeed"); + + let result = reader.fetch( + "chr999", + Position::try_from(1).expect("position conversion should succeed"), + Position::try_from(4).expect("position conversion should succeed"), + ); assert!(result.is_err()); } #[test] fn test_out_of_bounds() { - let fasta = create_default_test_fasta().unwrap(); - let reader = ReferenceReader::new(fasta.path()).unwrap(); + let fasta = create_default_test_fasta().expect("creating test FASTA should succeed"); + let reader = + ReferenceReader::new(fasta.path()).expect("creating reference reader should succeed"); // chr1 is only 12 bases long - let result = - reader.fetch("chr1", Position::try_from(1).unwrap(), Position::try_from(100).unwrap()); + let result = reader.fetch( + "chr1", + Position::try_from(1).expect("position conversion should succeed"), + Position::try_from(100).expect("position conversion should succeed"), + ); assert!(result.is_err()); } @@ -694,7 +702,7 @@ mod tests { let found = find_dict_path(&fasta_path); assert!(found.is_some()); - assert_eq!(found.unwrap(), dict_path); + assert_eq!(found.expect("should find dictionary path"), dict_path); Ok(()) } @@ -712,7 +720,7 @@ mod tests { let found = find_dict_path(&fasta_path); assert!(found.is_some()); - assert_eq!(found.unwrap(), dict_path); + assert_eq!(found.expect("should find dictionary path"), dict_path); Ok(()) } @@ -733,7 +741,7 @@ mod tests { let found = find_dict_path(&fasta_path); assert!(found.is_some()); // Should find ref.dict first (fgbio/HTSJDK convention) - assert_eq!(found.unwrap(), dict_replacing); + assert_eq!(found.expect("should find dictionary path"), dict_replacing); Ok(()) } @@ -741,11 +749,11 @@ mod tests { #[test] fn test_find_dict_path_not_found() { // When no dict file exists, return None - let temp_dir = tempfile::tempdir().unwrap(); + let temp_dir = tempfile::tempdir().expect("creating temp file/dir should succeed"); let fasta_path = temp_dir.path().join("ref.fa"); // Create only the FASTA file, no dict - std::fs::write(&fasta_path, "").unwrap(); + std::fs::write(&fasta_path, "").expect("writing file should succeed"); let found = find_dict_path(&fasta_path); assert!(found.is_none()); @@ -763,7 +771,7 @@ mod tests { let found = find_dict_path(&fasta_path); assert!(found.is_some()); - assert_eq!(found.unwrap(), dict_path); + assert_eq!(found.expect("should find dictionary path"), dict_path); Ok(()) } @@ -780,7 +788,7 @@ mod tests { let found = find_dict_path(&fasta_path); assert!(found.is_some()); - assert_eq!(found.unwrap(), dict_path); + assert_eq!(found.expect("should find dictionary path"), dict_path); Ok(()) } diff --git a/src/lib/reorder_buffer.rs b/src/lib/reorder_buffer.rs index a757b32f7..4c8d3ef0c 100644 --- a/src/lib/reorder_buffer.rs +++ b/src/lib/reorder_buffer.rs @@ -151,7 +151,11 @@ impl ReorderBuffer { } // Pop the front item - let (item, size) = self.buffer.pop_front().unwrap().unwrap(); + let (item, size) = self + .buffer + .pop_front() + .expect("buffer must be non-empty when can_pop is true") + .expect("front slot must be Some when can_pop is true"); self.next_seq += 1; self.count -= 1; self.heap_bytes = self.heap_bytes.saturating_sub(size as u64); @@ -437,12 +441,12 @@ mod tests { assert_eq!(buffer.len(), 3); // Pop and verify memory decreases - let (val, size) = buffer.try_pop_next_with_size().unwrap(); + let (val, size) = buffer.try_pop_next_with_size().expect("queue should have next element"); assert_eq!(val, 100); assert_eq!(size, 1000); assert_eq!(buffer.heap_bytes(), 5000); - let (val, size) = buffer.try_pop_next_with_size().unwrap(); + let (val, size) = buffer.try_pop_next_with_size().expect("queue should have next element"); assert_eq!(val, 200); assert_eq!(size, 2000); assert_eq!(buffer.heap_bytes(), 3000); diff --git a/src/lib/simulate/family_size.rs b/src/lib/simulate/family_size.rs index 526b3c2c0..081f7f07f 100644 --- a/src/lib/simulate/family_size.rs +++ b/src/lib/simulate/family_size.rs @@ -330,7 +330,7 @@ mod tests { let samples: Vec = (0..1000).map(|_| dist.sample(&mut rng, 1)).collect(); // With high variance, we should see a range of sizes - let max = *samples.iter().max().unwrap(); + let max = *samples.iter().max().expect("samples should not be empty"); assert!(max > 10, "Expected some large family sizes with high variance"); } @@ -439,7 +439,7 @@ mod tests { #[test] fn test_empirical_empty_file() { - let temp = NamedTempFile::new().unwrap(); + let temp = NamedTempFile::new().expect("creating temp file/dir should succeed"); let result = FamilySizeDistribution::from_histogram(temp.path()); assert!(result.is_err()); @@ -447,8 +447,8 @@ mod tests { #[test] fn test_empirical_header_only() { - let mut temp = NamedTempFile::new().unwrap(); - writeln!(temp, "family_size\tcount").unwrap(); + let mut temp = NamedTempFile::new().expect("creating temp file/dir should succeed"); + writeln!(temp, "family_size\tcount").expect("writeln should succeed"); let result = FamilySizeDistribution::from_histogram(temp.path()); assert!(result.is_err()); diff --git a/src/lib/simulate/insert_size.rs b/src/lib/simulate/insert_size.rs index 8d9128213..03e15e519 100644 --- a/src/lib/simulate/insert_size.rs +++ b/src/lib/simulate/insert_size.rs @@ -203,8 +203,8 @@ mod tests { let samples: Vec = (0..1000).map(|_| model.sample(&mut rng)).collect(); // Check that we have variation - let min = *samples.iter().min().unwrap(); - let max = *samples.iter().max().unwrap(); + let min = *samples.iter().min().expect("samples should not be empty"); + let max = *samples.iter().max().expect("samples should not be empty"); assert!(max - min > 200, "Expected high variance but got narrow range"); } diff --git a/src/lib/simulate/quality.rs b/src/lib/simulate/quality.rs index 2e19447f7..1da141a8e 100644 --- a/src/lib/simulate/quality.rs +++ b/src/lib/simulate/quality.rs @@ -119,7 +119,8 @@ impl PositionQualityModel { }; // Add Gaussian noise - let noise_dist = Normal::new(0.0, self.noise_stddev).unwrap(); + let noise_dist = Normal::new(0.0, self.noise_stddev) + .expect("Normal::new must succeed with finite mean=0.0 and non-negative stddev"); let noisy = base_q + noise_dist.sample(rng); // Clamp to valid range [min_quality, 41] @@ -385,8 +386,8 @@ mod tests { let samples: Vec = (0..1000).map(|_| model.quality_at(50, &mut rng)).collect(); // With high noise, we should see variation - let min = *samples.iter().min().unwrap(); - let max = *samples.iter().max().unwrap(); + let min = *samples.iter().min().expect("samples should not be empty"); + let max = *samples.iter().max().expect("samples should not be empty"); assert!(max - min > 5, "Expected variation with high noise"); } diff --git a/src/lib/sort/keys.rs b/src/lib/sort/keys.rs index 4e4b2192e..437643023 100644 --- a/src/lib/sort/keys.rs +++ b/src/lib/sort/keys.rs @@ -969,7 +969,7 @@ mod tests { let parsed = PrimaryAlignmentInfo::from_tag_value(&value); assert!(parsed.is_some()); - let parsed = parsed.unwrap(); + let parsed = parsed.expect("parsing should succeed"); assert_eq!(parsed.tid1, 5); assert_eq!(parsed.pos1, 1000); assert!(!parsed.neg1); @@ -994,7 +994,7 @@ mod tests { let result = PrimaryAlignmentInfo::from_record(&record); assert!(result.is_some()); - let result = result.unwrap(); + let result = result.expect("result should be Some"); assert_eq!(result.tid1, 0); assert_eq!(result.pos1, 100); assert!(!result.neg1); @@ -1030,7 +1030,7 @@ mod tests { let result = PrimaryAlignmentInfo::from_tag_value(&value); assert!(result.is_some()); - let info = result.unwrap(); + let info = result.expect("result should be Some"); assert_eq!(info.tid1, 5); assert_eq!(info.pos1, 1000); assert!(!info.neg1); @@ -1063,7 +1063,7 @@ mod tests { let result = PrimaryAlignmentInfo::from_tag_value(&value); assert!(result.is_some()); - let info = result.unwrap(); + let info = result.expect("result should be Some"); assert_eq!(info.tid1, 5); assert_eq!(info.pos1, 1000); } @@ -1154,7 +1154,7 @@ mod tests { // Verify negative positions survive roundtrip let info = PrimaryAlignmentInfo::new(0, -10, false, 0, -5, true); let value = info.to_tag_value(); - let parsed = PrimaryAlignmentInfo::from_tag_value(&value).unwrap(); + let parsed = PrimaryAlignmentInfo::from_tag_value(&value).expect("parsing should succeed"); assert_eq!(parsed.tid1, 0); assert_eq!(parsed.pos1, -10); @@ -1604,8 +1604,8 @@ mod tests { natural_compare(names[i], names[i + 1]), Ordering::Less, "{} should sort before {}", - std::str::from_utf8(names[i]).unwrap(), - std::str::from_utf8(names[i + 1]).unwrap(), + std::str::from_utf8(names[i]).expect("should be valid UTF-8"), + std::str::from_utf8(names[i + 1]).expect("should be valid UTF-8"), ); } } @@ -1621,8 +1621,8 @@ mod tests { natural_compare(names[i], names[i + 1]), Ordering::Less, "{} should sort before {}", - std::str::from_utf8(names[i]).unwrap(), - std::str::from_utf8(names[i + 1]).unwrap(), + std::str::from_utf8(names[i]).expect("should be valid UTF-8"), + std::str::from_utf8(names[i + 1]).expect("should be valid UTF-8"), ); } } @@ -2104,10 +2104,11 @@ mod tests { fn test_lex_key_serialization_roundtrip() { let key = RawQuerynameLexKey::new(b"test_read".to_vec(), 42); let mut buf = Vec::new(); - key.write_to(&mut buf).unwrap(); + key.write_to(&mut buf).expect("write_to should succeed"); let mut cursor = std::io::Cursor::new(&buf); - let restored = RawQuerynameLexKey::read_from(&mut cursor).unwrap(); + let restored = + RawQuerynameLexKey::read_from(&mut cursor).expect("read_from should succeed"); assert_eq!(key, restored); } diff --git a/src/lib/sort/mod.rs b/src/lib/sort/mod.rs index 25154913b..7b849575a 100644 --- a/src/lib/sort/mod.rs +++ b/src/lib/sort/mod.rs @@ -148,15 +148,15 @@ mod tests { #[test] fn test_create_temp_dir_default() { - let dir = create_temp_dir(None).unwrap(); + let dir = create_temp_dir(None).expect("creating temp dir should succeed"); assert!(dir.path().exists()); } #[test] fn test_create_temp_dir_with_base() { - let base = tempfile::tempdir().unwrap(); + let base = tempfile::tempdir().expect("creating temp file/dir should succeed"); let subdir = base.path().join("sort_spill"); - let dir = create_temp_dir(Some(&subdir)).unwrap(); + let dir = create_temp_dir(Some(&subdir)).expect("creating temp dir should succeed"); assert!(dir.path().exists()); assert!(dir.path().starts_with(&subdir)); } diff --git a/src/lib/sort/pipeline.rs b/src/lib/sort/pipeline.rs index c689e1bbe..98874d8e6 100644 --- a/src/lib/sort/pipeline.rs +++ b/src/lib/sort/pipeline.rs @@ -398,9 +398,9 @@ mod tests { heap.push(Reverse(MergeEntry { key: 2, record: Record::default(), chunk_idx: 2 })); // Should come out in ascending order: 1, 2, 3 - assert_eq!(heap.pop().unwrap().0.key, 1); - assert_eq!(heap.pop().unwrap().0.key, 2); - assert_eq!(heap.pop().unwrap().0.key, 3); + assert_eq!(heap.pop().expect("heap should have elements").0.key, 1); + assert_eq!(heap.pop().expect("heap should have elements").0.key, 2); + assert_eq!(heap.pop().expect("heap should have elements").0.key, 3); assert!(heap.is_empty()); } diff --git a/src/lib/sort/raw.rs b/src/lib/sort/raw.rs index c8b4aa593..87d87a3fc 100644 --- a/src/lib/sort/raw.rs +++ b/src/lib/sort/raw.rs @@ -165,7 +165,7 @@ pub(crate) fn make_reader_semaphore(threads: usize) -> Arc let limit = threads.max(1); let (tx, rx) = bounded(limit); for _ in 0..limit { - tx.send(()).unwrap(); + tx.send(()).expect("semaphore channel must not be disconnected during initialization"); } Arc::new((tx, rx)) } @@ -269,8 +269,9 @@ impl GenericKeyedChunkWriter { } else { // Single-threaded BGZF with specified compression level #[allow(clippy::cast_possible_truncation)] - let level = CompressionLevel::new(compression_level as u8) - .unwrap_or_else(|| CompressionLevel::new(6).unwrap()); + let level = CompressionLevel::new(compression_level as u8).unwrap_or_else(|| { + CompressionLevel::new(6).expect("compression level 6 is always valid") + }); let writer = noodles_bgzf::io::writer::Builder::default() .set_compression_level(level) .build_from_writer(buf); @@ -2011,7 +2012,10 @@ mod tests { let lookup = LibraryLookup::from_header(&header); assert_eq!(lookup.rg_to_ordinal.len(), 1); // LibA is the only library, so it gets ordinal 1 (0 is reserved for empty/unknown) - assert_eq!(*lookup.rg_to_ordinal.get(b"rg1".as_slice()).unwrap(), 1); + assert_eq!( + *lookup.rg_to_ordinal.get(b"rg1".as_slice()).expect("rg1 should be in ordinal map"), + 1 + ); } #[test] @@ -2039,9 +2043,12 @@ mod tests { assert_eq!(lookup.rg_to_ordinal.len(), 3); // Libraries sorted alphabetically: LibA=1, LibB=2, LibC=3 - assert_eq!(*lookup.rg_to_ordinal.get(b"rg2".as_slice()).unwrap(), 1); // LibA - assert_eq!(*lookup.rg_to_ordinal.get(b"rg3".as_slice()).unwrap(), 2); // LibB - assert_eq!(*lookup.rg_to_ordinal.get(b"rg1".as_slice()).unwrap(), 3); // LibC + let rg2 = *lookup.rg_to_ordinal.get(b"rg2".as_slice()).expect("rg2"); + let rg3 = *lookup.rg_to_ordinal.get(b"rg3".as_slice()).expect("rg3"); + let rg1 = *lookup.rg_to_ordinal.get(b"rg1".as_slice()).expect("rg1"); + assert_eq!(rg2, 1); // LibA + assert_eq!(rg3, 2); // LibB + assert_eq!(rg1, 3); // LibC } #[test] @@ -2359,7 +2366,7 @@ mod tests { /// Count records in a BAM file by reading with the raw BAM reader. fn count_bam_records(path: &std::path::Path) -> u64 { use crate::sort::read_ahead::RawReadAheadReader; - let (reader, _) = create_raw_bam_reader(path, 1).unwrap(); + let (reader, _) = create_raw_bam_reader(path, 1).expect("failed to create raw BAM reader"); RawReadAheadReader::new(reader).count() as u64 } @@ -2391,10 +2398,10 @@ mod tests { .build(); } - let dir = tempfile::tempdir().unwrap(); + let dir = tempfile::tempdir().expect("failed to create temp directory"); let input = dir.path().join("input.bam"); let output = dir.path().join("output.bam"); - builder.write_bam(&input).unwrap(); + builder.write_bam(&input).expect("failed to write BAM"); // Tiny memory limit forces many chunks; low max_temp_files triggers consolidation let stats = RawExternalSorter::new(sort_order) @@ -2404,7 +2411,7 @@ mod tests { .output_compression(0) .write_index(write_index) .sort(&input, &output) - .unwrap(); + .expect("sort should succeed"); assert!( stats.chunks_written >= 5, @@ -2438,10 +2445,10 @@ mod tests { .build(); } - let dir = tempfile::tempdir().unwrap(); + let dir = tempfile::tempdir().expect("failed to create temp directory"); let input = dir.path().join("input.bam"); let output = dir.path().join("output.bam"); - builder.write_bam(&input).unwrap(); + builder.write_bam(&input).expect("failed to write BAM"); // Small memory limit + many records = guaranteed spill to multiple chunks // (no consolidation, so the semaphore must cap concurrent readers) @@ -2452,7 +2459,7 @@ mod tests { .temp_compression(0) .output_compression(0) .sort(&input, &output) - .unwrap(); + .expect("sort should succeed"); assert!( stats.chunks_written >= 2, @@ -2489,11 +2496,11 @@ mod tests { .build(); } - let dir = tempfile::tempdir().unwrap(); + let dir = tempfile::tempdir().expect("failed to create temp directory"); let input = dir.path().join("input.bam"); let output_st = dir.path().join("output_1t.bam"); let output_mt = dir.path().join("output_2t.bam"); - builder.write_bam(&input).unwrap(); + builder.write_bam(&input).expect("failed to write BAM"); // Sort single-threaded RawExternalSorter::new(sort_order) @@ -2502,7 +2509,7 @@ mod tests { .temp_compression(0) .output_compression(0) .sort(&input, &output_st) - .unwrap(); + .expect("sort should succeed"); // Sort multi-threaded (exercises par_sort_into_chunks / par_chunks_mut) RawExternalSorter::new(sort_order) @@ -2511,7 +2518,7 @@ mod tests { .temp_compression(0) .output_compression(0) .sort(&input, &output_mt) - .unwrap(); + .expect("sort should succeed"); let names_st = collect_read_names(&output_st); let names_mt = collect_read_names(&output_mt); @@ -2544,10 +2551,10 @@ mod tests { .build(); } - let dir = tempfile::tempdir().unwrap(); + let dir = tempfile::tempdir().expect("failed to create temp directory"); let input = dir.path().join("input.bam"); let output = dir.path().join("output.bam"); - builder.write_bam(&input).unwrap(); + builder.write_bam(&input).expect("failed to write BAM"); // Large memory limit so everything stays in memory (no spill chunks). RawExternalSorter::new(sort_order) @@ -2555,7 +2562,7 @@ mod tests { .threads(2) .output_compression(0) .sort(&input, &output) - .unwrap(); + .expect("sort should succeed"); let expected = (num_pairs * 2) as u64; let observed = count_bam_records(&output); @@ -2590,19 +2597,22 @@ mod tests { } let unsorted = dir.join(format!("{prefix}_unsorted.bam")); let sorted = dir.join(format!("{prefix}_sorted.bam")); - builder.write_bam(&unsorted).unwrap(); - RawExternalSorter::new(sort_order).output_compression(0).sort(&unsorted, &sorted).unwrap(); + builder.write_bam(&unsorted).expect("failed to write BAM"); + RawExternalSorter::new(sort_order) + .output_compression(0) + .sort(&unsorted, &sorted) + .expect("sort should succeed"); (sorted, names) } /// Collect all read names from a BAM file as strings. fn collect_read_names(path: &Path) -> Vec { use crate::sort::read_ahead::RawReadAheadReader; - let (reader, _) = create_raw_bam_reader(path, 1).unwrap(); + let (reader, _) = create_raw_bam_reader(path, 1).expect("failed to create raw BAM reader"); RawReadAheadReader::new(reader) .map(|rec| { let name_bytes = fgumi_raw_bam::fields::read_name(rec.as_ref()); - String::from_utf8(name_bytes.to_vec()).unwrap() + String::from_utf8(name_bytes.to_vec()).expect("read name should be valid UTF-8") }) .collect() } @@ -2610,7 +2620,7 @@ mod tests { /// Collect (`ref_id`, pos) tuples for every record in a BAM. fn collect_positions(path: &Path) -> Vec<(i32, i32)> { use crate::sort::read_ahead::RawReadAheadReader; - let (reader, _) = create_raw_bam_reader(path, 1).unwrap(); + let (reader, _) = create_raw_bam_reader(path, 1).expect("failed to create raw BAM reader"); RawReadAheadReader::new(reader) .map(|rec| { let bytes = rec.as_ref(); @@ -2626,7 +2636,7 @@ mod tests { #[test] fn test_merge_bams_coordinate_sort() { - let dir = tempfile::tempdir().unwrap(); + let dir = tempfile::tempdir().expect("failed to create temp directory"); let (bam_a, _) = create_sorted_bam(dir.path(), "a", 10, 0, SortOrder::Coordinate); let (bam_b, _) = create_sorted_bam(dir.path(), "b", 10, 10_000, SortOrder::Coordinate); @@ -2635,7 +2645,7 @@ mod tests { let count = RawExternalSorter::new(SortOrder::Coordinate) .output_compression(0) .merge_bams(&[bam_a, bam_b], &header, &merged) - .unwrap(); + .expect("sort should succeed"); // 10 pairs * 2 records * 2 inputs = 40 assert_eq!(count, 40); @@ -2650,7 +2660,7 @@ mod tests { #[test] fn test_merge_bams_template_coordinate_sort() { - let dir = tempfile::tempdir().unwrap(); + let dir = tempfile::tempdir().expect("failed to create temp directory"); let (bam_a, _) = create_sorted_bam(dir.path(), "a", 10, 0, SortOrder::TemplateCoordinate); let (bam_b, _) = create_sorted_bam(dir.path(), "b", 10, 10_000, SortOrder::TemplateCoordinate); @@ -2660,7 +2670,7 @@ mod tests { let count = RawExternalSorter::new(SortOrder::TemplateCoordinate) .output_compression(0) .merge_bams(&[bam_a, bam_b], &header, &merged) - .unwrap(); + .expect("sort should succeed"); assert_eq!(count, 40); assert_eq!(count_bam_records(&merged), 40); @@ -2668,7 +2678,7 @@ mod tests { #[test] fn test_merge_bams_queryname_sort() { - let dir = tempfile::tempdir().unwrap(); + let dir = tempfile::tempdir().expect("failed to create temp directory"); let (bam_a, _) = create_sorted_bam( dir.path(), "a", @@ -2689,7 +2699,7 @@ mod tests { let count = RawExternalSorter::new(SortOrder::Queryname(QuerynameComparator::default())) .output_compression(0) .merge_bams(&[bam_a, bam_b], &header, &merged) - .unwrap(); + .expect("sort should succeed"); assert_eq!(count, 40); assert_eq!(count_bam_records(&merged), 40); @@ -2703,7 +2713,7 @@ mod tests { #[test] fn test_merge_bams_single_input() { - let dir = tempfile::tempdir().unwrap(); + let dir = tempfile::tempdir().expect("failed to create temp directory"); let (bam_a, _) = create_sorted_bam(dir.path(), "a", 15, 0, SortOrder::Coordinate); let merged = dir.path().join("merged.bam"); @@ -2711,7 +2721,7 @@ mod tests { let count = RawExternalSorter::new(SortOrder::Coordinate) .output_compression(0) .merge_bams(&[bam_a], &header, &merged) - .unwrap(); + .expect("sort should succeed"); // 15 pairs * 2 = 30 assert_eq!(count, 30); @@ -2720,7 +2730,7 @@ mod tests { #[test] fn test_merge_bams_preserves_all_records() { - let dir = tempfile::tempdir().unwrap(); + let dir = tempfile::tempdir().expect("failed to create temp directory"); let (bam_a, names_a) = create_sorted_bam( dir.path(), "a", @@ -2741,7 +2751,7 @@ mod tests { RawExternalSorter::new(SortOrder::Queryname(QuerynameComparator::default())) .output_compression(0) .merge_bams(&[bam_a, bam_b], &header, &merged) - .unwrap(); + .expect("sort should succeed"); let merged_names: std::collections::HashSet = collect_read_names(&merged).into_iter().collect(); @@ -2754,7 +2764,7 @@ mod tests { #[test] fn test_merge_bams_many_inputs() { - let dir = tempfile::tempdir().unwrap(); + let dir = tempfile::tempdir().expect("failed to create temp directory"); let k = 8; let pairs_per_input = 5; let mut inputs = Vec::with_capacity(k); @@ -2775,7 +2785,7 @@ mod tests { let count = RawExternalSorter::new(SortOrder::Coordinate) .output_compression(0) .merge_bams(&inputs, &header, &merged) - .unwrap(); + .expect("sort should succeed"); let expected = (k * pairs_per_input * 2) as u64; // 8 * 5 * 2 = 80 assert_eq!(count, expected); diff --git a/src/lib/sort/raw_bam_reader.rs b/src/lib/sort/raw_bam_reader.rs index 475e5f834..badc6eed3 100644 --- a/src/lib/sort/raw_bam_reader.rs +++ b/src/lib/sort/raw_bam_reader.rs @@ -405,8 +405,8 @@ mod tests { let mut compressed = Vec::new(); { let mut writer = noodles_bgzf::io::Writer::new(&mut compressed); - writer.write_all(&raw_bam).unwrap(); - writer.finish().unwrap(); + writer.write_all(&raw_bam).expect("failed to write raw BAM data to BGZF"); + writer.finish().expect("failed to finish BGZF writer"); } compressed } @@ -446,8 +446,8 @@ mod tests { let mut compressed = Vec::new(); { let mut writer = noodles_bgzf::io::Writer::new(&mut compressed); - writer.write_all(b"NOT_BAM!").unwrap(); - writer.finish().unwrap(); + writer.write_all(b"NOT_BAM!").expect("failed to write non-BAM data to BGZF"); + writer.finish().expect("failed to finish BGZF writer for invalid magic test"); } let result = RawBamRecordReader::new(io::Cursor::new(compressed)); let Err(err) = result else { unreachable!("Expected error for invalid magic") }; @@ -462,7 +462,7 @@ mod tests { { let writer = noodles_bgzf::io::Writer::new(&mut compressed); // Write nothing, just finish to produce the EOF block - writer.finish().unwrap(); + writer.finish().expect("failed to finish BGZF writer for empty input test"); } let result = RawBamRecordReader::new(io::Cursor::new(compressed)); let Err(err) = result else { unreachable!("Expected error for empty input") }; @@ -478,15 +478,22 @@ mod tests { let header_text = "@HD\tVN:1.6\n"; let refs = vec![("chr1", 1000u32), ("chr2", 2000u32)]; let data = build_test_bam(header_text, &refs, &[]); - let mut reader = RawBamRecordReader::new(io::Cursor::new(data)).unwrap(); - let header_bytes = reader.skip_header().unwrap(); + let mut reader = RawBamRecordReader::new(io::Cursor::new(data)) + .expect("failed to create reader for valid BAM"); + let header_bytes = reader.skip_header().expect("failed to skip BAM header"); // The header_bytes should contain l_text + header_text + n_ref + ref data // Verify we can parse it back assert!(!header_bytes.is_empty()); // First 4 bytes: l_text - let l_text = u32::from_le_bytes(header_bytes[0..4].try_into().unwrap()) as usize; + let l_text = u32::from_le_bytes( + header_bytes + .get(0..4) + .expect("header_bytes too short for l_text") + .try_into() + .expect("l_text slice must be exactly 4 bytes"), + ) as usize; assert_eq!(l_text, header_text.len()); // Then header text @@ -495,15 +502,22 @@ mod tests { // Then n_ref let offset = 4 + l_text; - let n_ref = u32::from_le_bytes(header_bytes[offset..offset + 4].try_into().unwrap()); + let n_ref = u32::from_le_bytes( + header_bytes + .get(offset..offset + 4) + .expect("header_bytes too short for n_ref") + .try_into() + .expect("n_ref slice must be exactly 4 bytes"), + ); assert_eq!(n_ref, 2); } #[test] fn test_skip_header_twice_errors() { let data = build_test_bam("@HD\tVN:1.6\n", &[], &[]); - let mut reader = RawBamRecordReader::new(io::Cursor::new(data)).unwrap(); - reader.skip_header().unwrap(); + let mut reader = RawBamRecordReader::new(io::Cursor::new(data)) + .expect("failed to create reader for skip_header test"); + reader.skip_header().expect("first skip_header should succeed"); let result = reader.skip_header(); assert!(result.is_err()); assert!( @@ -515,7 +529,8 @@ mod tests { #[test] fn test_next_record_without_skip_header_errors() { let data = build_test_bam("@HD\tVN:1.6\n", &[], &[]); - let mut reader = RawBamRecordReader::new(io::Cursor::new(data)).unwrap(); + let mut reader = RawBamRecordReader::new(io::Cursor::new(data)) + .expect("failed to create reader for next_record test"); let result = reader.next_record(); assert!(result.is_err()); assert!( @@ -528,16 +543,17 @@ mod tests { fn test_read_single_record() { let rec = make_minimal_record(b"R"); let data = build_test_bam("@HD\tVN:1.6\n", &[], std::slice::from_ref(&rec)); - let mut reader = RawBamRecordReader::new(io::Cursor::new(data)).unwrap(); - reader.skip_header().unwrap(); + let mut reader = RawBamRecordReader::new(io::Cursor::new(data)) + .expect("failed to create reader for single record test"); + reader.skip_header().expect("failed to skip header"); - let record = reader.next_record().unwrap(); + let record = reader.next_record().expect("failed to read first record"); assert!(record.is_some(), "Expected one record"); - let record = record.unwrap(); + let record = record.expect("record should be Some"); assert_eq!(record, rec, "Record bytes should match"); // No more records - let eof = reader.next_record().unwrap(); + let eof = reader.next_record().expect("failed to read at EOF"); assert!(eof.is_none(), "Expected EOF after single record"); } @@ -548,18 +564,19 @@ mod tests { let rec_c = make_minimal_record(b"C"); let data = build_test_bam("@HD\tVN:1.6\n", &[], &[rec_a.clone(), rec_b.clone(), rec_c.clone()]); - let mut reader = RawBamRecordReader::new(io::Cursor::new(data)).unwrap(); - reader.skip_header().unwrap(); + let mut reader = RawBamRecordReader::new(io::Cursor::new(data)) + .expect("failed to create reader for multiple records test"); + reader.skip_header().expect("failed to skip header"); - let r1 = reader.next_record().unwrap().expect("record 1"); - let r2 = reader.next_record().unwrap().expect("record 2"); - let r3 = reader.next_record().unwrap().expect("record 3"); + let r1 = reader.next_record().expect("failed to read record 1").expect("record 1"); + let r2 = reader.next_record().expect("failed to read record 2").expect("record 2"); + let r3 = reader.next_record().expect("failed to read record 3").expect("record 3"); assert_eq!(r1, rec_a); assert_eq!(r2, rec_b); assert_eq!(r3, rec_c); - assert!(reader.next_record().unwrap().is_none(), "Expected EOF"); + assert!(reader.next_record().expect("failed to read at EOF").is_none(), "Expected EOF"); } #[test] @@ -567,10 +584,12 @@ mod tests { let rec_a = make_minimal_record(b"X"); let rec_b = make_minimal_record(b"Y"); let data = build_test_bam("@HD\tVN:1.6\n", &[], &[rec_a.clone(), rec_b.clone()]); - let mut reader = RawBamRecordReader::new(io::Cursor::new(data)).unwrap(); - reader.skip_header().unwrap(); + let mut reader = RawBamRecordReader::new(io::Cursor::new(data)) + .expect("failed to create reader for iterator test"); + reader.skip_header().expect("failed to skip header"); - let records: Vec> = reader.map(|r| r.unwrap()).collect(); + let records: Vec> = + reader.map(|r| r.expect("failed to read record via iterator")).collect(); assert_eq!(records.len(), 2); assert_eq!(records[0], rec_a); assert_eq!(records[1], rec_b); @@ -583,46 +602,58 @@ mod tests { let rec_c = make_minimal_record(b"C"); let data = build_test_bam("@HD\tVN:1.6\n", &[], &[rec_a.clone(), rec_b.clone(), rec_c.clone()]); - let mut reader = BatchedRawBamReader::new(io::Cursor::new(data), 2).unwrap(); - reader.skip_header().unwrap(); + let mut reader = BatchedRawBamReader::new(io::Cursor::new(data), 2) + .expect("failed to create batched reader"); + reader.skip_header().expect("failed to skip header"); // First batch: 2 records - let batch1 = reader.next_batch().unwrap().expect("batch 1"); + let batch1 = reader.next_batch().expect("failed to read batch 1").expect("batch 1"); assert_eq!(batch1.len(), 2); assert_eq!(batch1[0], rec_a); assert_eq!(batch1[1], rec_b); // Second batch: 1 record (remainder) - let batch2 = reader.next_batch().unwrap().expect("batch 2"); + let batch2 = reader.next_batch().expect("failed to read batch 2").expect("batch 2"); assert_eq!(batch2.len(), 1); assert_eq!(batch2[0], rec_c); // No more batches - assert!(reader.next_batch().unwrap().is_none(), "Expected no more batches"); + assert!( + reader.next_batch().expect("failed to read at end of batches").is_none(), + "Expected no more batches" + ); } #[test] fn test_from_buf_reader() { let data = build_test_bam("@HD\tVN:1.6\n", &[("chr1", 500)], &[]); let buf_reader = BufReader::new(io::Cursor::new(data)); - let mut reader = RawBamRecordReader::from_buf_reader(buf_reader).unwrap(); - let header_bytes = reader.skip_header().unwrap(); + let mut reader = RawBamRecordReader::from_buf_reader(buf_reader) + .expect("failed to create reader from BufReader"); + let header_bytes = reader.skip_header().expect("failed to skip header"); assert!(!header_bytes.is_empty()); // Verify no records - assert!(reader.next_record().unwrap().is_none()); + assert!(reader.next_record().expect("failed to read at EOF").is_none()); } #[test] fn test_batched_reader_eof() { let data = build_test_bam("@HD\tVN:1.6\n", &[], &[]); - let mut reader = BatchedRawBamReader::new(io::Cursor::new(data), 10).unwrap(); - reader.skip_header().unwrap(); + let mut reader = BatchedRawBamReader::new(io::Cursor::new(data), 10) + .expect("failed to create batched reader for empty BAM"); + reader.skip_header().expect("failed to skip header"); // No records, should return None immediately - assert!(reader.next_batch().unwrap().is_none(), "Expected None for empty BAM"); + assert!( + reader.next_batch().expect("failed to read first batch").is_none(), + "Expected None for empty BAM" + ); // Calling again should still return None - assert!(reader.next_batch().unwrap().is_none(), "Expected None on repeated call"); + assert!( + reader.next_batch().expect("failed to read second batch").is_none(), + "Expected None on repeated call" + ); } } diff --git a/src/lib/sort/read_ahead.rs b/src/lib/sort/read_ahead.rs index 99b53fe11..36214afa3 100644 --- a/src/lib/sort/read_ahead.rs +++ b/src/lib/sort/read_ahead.rs @@ -357,19 +357,21 @@ mod tests { Map::::new(NonZeroUsize::new(1000).expect("1000 is non-zero")); let header = Header::builder().add_reference_sequence("chr1", ref_seq).build(); - let tmp = NamedTempFile::new().unwrap(); + let tmp = NamedTempFile::new().expect("creating temp file/dir should succeed"); let path = tmp.path().to_path_buf(); { - let file = std::fs::File::create(&path).unwrap(); + let file = std::fs::File::create(&path).expect("creating file should succeed"); let mut writer = noodles::bam::io::Writer::new(file); - writer.write_header(&header).unwrap(); + writer.write_header(&header).expect("writing header should succeed"); for i in 0..num_records { let name = format!("read{i}"); let record = RecordBuf::builder().set_name(&*name).set_flags(Flags::UNMAPPED).build(); - writer.write_alignment_record(&header, &record).unwrap(); + writer + .write_alignment_record(&header, &record) + .expect("writing alignment record should succeed"); } } @@ -379,7 +381,8 @@ mod tests { #[test] fn test_read_ahead_reader_empty_bam() { let (tmp, _header) = create_test_bam_file(0); - let (reader, _header) = create_bam_reader(tmp.path(), 1).unwrap(); + let (reader, _header) = + create_bam_reader(tmp.path(), 1).expect("creating BAM reader should succeed"); let mut ra = ReadAheadReader::new(reader); assert!(ra.next_record().is_none(), "Empty BAM should yield no records"); @@ -388,7 +391,8 @@ mod tests { #[test] fn test_read_ahead_reader_single_record() { let (tmp, _header) = create_test_bam_file(1); - let (reader, _header) = create_bam_reader(tmp.path(), 1).unwrap(); + let (reader, _header) = + create_bam_reader(tmp.path(), 1).expect("creating BAM reader should succeed"); let mut ra = ReadAheadReader::new(reader); assert!(ra.next_record().is_some(), "Should yield exactly one record"); @@ -399,7 +403,8 @@ mod tests { fn test_read_ahead_reader_multiple_records() { let num = 10; let (tmp, _header) = create_test_bam_file(num); - let (reader, _header) = create_bam_reader(tmp.path(), 1).unwrap(); + let (reader, _header) = + create_bam_reader(tmp.path(), 1).expect("creating BAM reader should succeed"); let mut ra = ReadAheadReader::new(reader); let mut count = 0; @@ -413,7 +418,8 @@ mod tests { fn test_read_ahead_reader_iterator() { let num = 10; let (tmp, _header) = create_test_bam_file(num); - let (reader, _header) = create_bam_reader(tmp.path(), 1).unwrap(); + let (reader, _header) = + create_bam_reader(tmp.path(), 1).expect("creating BAM reader should succeed"); let ra = ReadAheadReader::new(reader); let records: Vec = ra.collect(); @@ -424,7 +430,8 @@ mod tests { fn test_read_ahead_reader_with_buffer_size() { let num = 10; let (tmp, _header) = create_test_bam_file(num); - let (reader, _header) = create_bam_reader(tmp.path(), 1).unwrap(); + let (reader, _header) = + create_bam_reader(tmp.path(), 1).expect("creating BAM reader should succeed"); // Use a small buffer size (4) to exercise partial-batch logic let ra = ReadAheadReader::with_buffer_size(reader, 4); @@ -438,7 +445,8 @@ mod tests { // when we drop the reader. This verifies the Drop impl does not deadlock. let num = 5000; let (tmp, _header) = create_test_bam_file(num); - let (reader, _header) = create_bam_reader(tmp.path(), 1).unwrap(); + let (reader, _header) = + create_bam_reader(tmp.path(), 1).expect("creating BAM reader should succeed"); let mut ra = ReadAheadReader::new(reader); // Read only a few records @@ -454,7 +462,8 @@ mod tests { #[test] fn test_raw_read_ahead_empty() { let (tmp, _header) = create_test_bam_file(0); - let (reader, _header) = create_raw_bam_reader(tmp.path(), 1).unwrap(); + let (reader, _header) = + create_raw_bam_reader(tmp.path(), 1).expect("creating BAM reader should succeed"); let mut ra = RawReadAheadReader::new(reader); assert!(ra.next_record().is_none(), "Empty BAM should yield no raw records"); @@ -464,7 +473,8 @@ mod tests { fn test_raw_read_ahead_multiple() { let num = 10; let (tmp, _header) = create_test_bam_file(num); - let (reader, _header) = create_raw_bam_reader(tmp.path(), 1).unwrap(); + let (reader, _header) = + create_raw_bam_reader(tmp.path(), 1).expect("creating BAM reader should succeed"); let ra = RawReadAheadReader::new(reader); let records: Vec = ra.collect(); diff --git a/src/lib/tag_reversal.rs b/src/lib/tag_reversal.rs index 77edfa24b..a58208be1 100644 --- a/src/lib/tag_reversal.rs +++ b/src/lib/tag_reversal.rs @@ -130,7 +130,8 @@ mod tests { let tag = Tag::from([b'c', b'd']); record.data_mut().insert(tag, Value::from(vec![1u16, 2, 3])); - let reversed = reverse_per_base_tags(&mut record).unwrap(); + let reversed = + reverse_per_base_tags(&mut record).expect("reverse_per_base_tags should succeed"); assert!(!reversed); // Should not reverse for positive strand if let Some(Value::Array(Array::UInt16(arr))) = record.data().get(&tag) { @@ -145,7 +146,8 @@ mod tests { let tag = Tag::from([b'c', b'd']); record.data_mut().insert(tag, Value::from(vec![1u16, 2, 3])); - let reversed = reverse_per_base_tags(&mut record).unwrap(); + let reversed = + reverse_per_base_tags(&mut record).expect("reverse_per_base_tags should succeed"); assert!(reversed); // Should reverse for negative strand if let Some(Value::Array(Array::UInt16(arr))) = record.data().get(&tag) { @@ -161,7 +163,8 @@ mod tests { .tag("ac", "ACGT") .build(); - let reversed = reverse_per_base_tags(&mut record).unwrap(); + let reversed = + reverse_per_base_tags(&mut record).expect("reverse_per_base_tags should succeed"); assert!(reversed); let tag = Tag::from([b'a', b'c']); @@ -200,7 +203,8 @@ mod tests { .tag("aq", "IIHG") .build(); - let reversed = reverse_per_base_tags(&mut record).unwrap(); + let reversed = + reverse_per_base_tags(&mut record).expect("reverse_per_base_tags should succeed"); assert!(reversed); let tag = Tag::from([b'a', b'q']); @@ -226,9 +230,10 @@ mod tests { let header = Header::default(); let mut raw = Vec::new(); - encode_record_buf(&mut raw, &header, &record_buf).unwrap(); + encode_record_buf(&mut raw, &header, &record_buf).expect("encoding record should succeed"); - let result = reverse_per_base_tags_raw(&mut raw).unwrap(); + let result = + reverse_per_base_tags_raw(&mut raw).expect("reverse_per_base_tags_raw should succeed"); assert!(result); let aux = bam_fields::aux_data_slice(&raw); @@ -254,11 +259,11 @@ mod tests { let record_buf = RecordBuilder::new().sequence("ACGT").build(); let header = Header::default(); let mut raw = Vec::new(); - encode_record_buf(&mut raw, &header, &record_buf).unwrap(); + encode_record_buf(&mut raw, &header, &record_buf).expect("encoding record should succeed"); let result = reverse_per_base_tags_raw(&mut raw); assert!(result.is_ok()); - assert!(!result.unwrap()); // positive strand => false + assert!(!result.expect("result should be Ok")); // positive strand => false } #[test] @@ -274,11 +279,11 @@ mod tests { let header = Header::default(); let mut raw = Vec::new(); - encode_record_buf(&mut raw, &header, &record_buf).unwrap(); + encode_record_buf(&mut raw, &header, &record_buf).expect("encoding record should succeed"); let result = reverse_per_base_tags_raw(&mut raw); assert!(result.is_ok()); - assert!(result.unwrap()); // negative strand => true + assert!(result.expect("result should be Ok")); // negative strand => true // Verify the cd tag was reversed: find it in the raw record's aux data let aux = bam_fields::aux_data_slice(&raw); diff --git a/src/lib/template.rs b/src/lib/template.rs index a232028ff..fa4d4d234 100644 --- a/src/lib/template.rs +++ b/src/lib/template.rs @@ -177,7 +177,7 @@ impl Builder { } else if self.r1.is_some() { return Err(anyhow!( "Multiple non-secondary, non-supplemental R1 records for read '{:?}'", - self.name.as_ref().unwrap() + self.name.as_ref().expect("name must be set before adding duplicate R1") )); } else { self.r1 = Some(record); @@ -189,7 +189,7 @@ impl Builder { } else if self.r2.is_some() { return Err(anyhow!( "Multiple non-secondary, non-supplemental R2 records for read '{:?}'", - self.name.as_ref().unwrap() + self.name.as_ref().expect("name must be set before adding duplicate R2") )); } else { self.r2 = Some(record); @@ -1579,7 +1579,7 @@ mod tests { fn create_test_record(name: &[u8], flags: u16) -> RecordBuf { RecordBuilder::new() - .name(std::str::from_utf8(name).unwrap()) + .name(std::str::from_utf8(name).expect("read name should be valid UTF-8")) .flags(Flags::from(flags)) .build() } @@ -1649,7 +1649,7 @@ mod tests { let mut builder = Builder::new(); let r1 = create_test_record(b"read1", 0); let r2 = create_test_record(b"read2", 0); - builder.push(r1).unwrap(); + builder.push(r1).expect("failed to push record to builder"); let result = builder.push(r2); assert!(result.is_err()); assert!(result.unwrap_err().to_string().contains("mismatch")); @@ -1660,7 +1660,7 @@ mod tests { let mut builder = Builder::new(); let r1a = create_test_record(b"read1", 0); let r1b = create_test_record(b"read1", 0); - builder.push(r1a).unwrap(); + builder.push(r1a).expect("failed to push record to builder"); let result = builder.push(r1b); assert!(result.is_err()); assert!(result.unwrap_err().to_string().contains("Multiple non-secondary")); @@ -1671,7 +1671,7 @@ mod tests { let mut builder = Builder::new(); let r2a = create_test_record(b"read1", FLAG_PAIRED | FLAG_READ2); let r2b = create_test_record(b"read1", FLAG_PAIRED | FLAG_READ2); - builder.push(r2a).unwrap(); + builder.push(r2a).expect("failed to push record to builder"); let result = builder.push(r2b); assert!(result.is_err()); assert!(result.unwrap_err().to_string().contains("Multiple non-secondary")); @@ -1741,8 +1741,8 @@ mod tests { assert!(r2.is_some()); // Verify the records have the expected flags - let r1 = r1.unwrap(); - let r2 = r2.unwrap(); + let r1 = r1.expect("R1 record should be present"); + let r2 = r2.expect("R2 record should be present"); assert!(r1.flags().is_first_segment()); assert!(r2.flags().is_last_segment()); @@ -1813,7 +1813,7 @@ mod tests { fn test_cigar_to_string() { use noodles::sam::alignment::record_buf::Cigar; let cigar = Cigar::default(); - assert_eq!(cigar_to_string(&cigar).unwrap(), "*"); + assert_eq!(cigar_to_string(&cigar).expect("cigar_to_string should succeed"), "*"); } #[test] @@ -1821,7 +1821,7 @@ mod tests { let ops = vec![Op::new(Kind::Match, 10), Op::new(Kind::Deletion, 2), Op::new(Kind::Match, 5)]; let cigar = CigarBuf::from(ops); - let result = cigar_to_string(&cigar).unwrap(); + let result = cigar_to_string(&cigar).expect("cigar_to_string should succeed"); assert_eq!(result, "10M2D5M"); } @@ -1965,7 +1965,7 @@ mod tests { let is_paired = (flags & FLAG_PAIRED) != 0; let mut builder = RecordBuilder::new() - .name(std::str::from_utf8(name).unwrap()) + .name(std::str::from_utf8(name).expect("read name should be valid UTF-8")) .sequence(&"A".repeat(100)) .qualities(&[30u8; 100]) .cigar("100M") @@ -2009,12 +2009,12 @@ mod tests { // R1 should have ms tag with R2's AS value (44) let r1_ms = template.records[0].data().get(&ms_tag); assert!(r1_ms.is_some(), "R1 should have ms tag"); - assert_eq!(extract_int_value(r1_ms.unwrap()), Some(44)); + assert_eq!(extract_int_value(r1_ms.expect("R1 ms tag should be present")), Some(44)); // R2 should have ms tag with R1's AS value (55) let r2_ms = template.records[1].data().get(&ms_tag); assert!(r2_ms.is_some(), "R2 should have ms tag"); - assert_eq!(extract_int_value(r2_ms.unwrap()), Some(55)); + assert_eq!(extract_int_value(r2_ms.expect("R2 ms tag should be present")), Some(55)); Ok(()) } @@ -2041,12 +2041,12 @@ mod tests { // R1 should have ms tag with R2's AS value (88) let r1_ms = template.records[0].data().get(&ms_tag); assert!(r1_ms.is_some(), "R1 should have ms tag"); - assert_eq!(extract_int_value(r1_ms.unwrap()), Some(88)); + assert_eq!(extract_int_value(r1_ms.expect("R1 ms tag should be present")), Some(88)); // R2 should have ms tag with R1's AS value (77) let r2_ms = template.records[1].data().get(&ms_tag); assert!(r2_ms.is_some(), "R2 should have ms tag"); - assert_eq!(extract_int_value(r2_ms.unwrap()), Some(77)); + assert_eq!(extract_int_value(r2_ms.expect("R2 ms tag should be present")), Some(77)); Ok(()) } @@ -2073,12 +2073,12 @@ mod tests { // R1 should have ms tag with R2's AS value (2000) let r1_ms = template.records[0].data().get(&ms_tag); assert!(r1_ms.is_some(), "R1 should have ms tag"); - assert_eq!(extract_int_value(r1_ms.unwrap()), Some(2000)); + assert_eq!(extract_int_value(r1_ms.expect("R1 ms tag should be present")), Some(2000)); // R2 should have ms tag with R1's AS value (1000) let r2_ms = template.records[1].data().get(&ms_tag); assert!(r2_ms.is_some(), "R2 should have ms tag"); - assert_eq!(extract_int_value(r2_ms.unwrap()), Some(1000)); + assert_eq!(extract_int_value(r2_ms.expect("R2 ms tag should be present")), Some(1000)); Ok(()) } @@ -2105,12 +2105,12 @@ mod tests { // R1 should have ms tag with R2's AS value let r1_ms = template.records[0].data().get(&ms_tag); assert!(r1_ms.is_some(), "R1 should have ms tag"); - assert_eq!(extract_int_value(r1_ms.unwrap()), Some(200_000)); + assert_eq!(extract_int_value(r1_ms.expect("R1 ms tag should be present")), Some(200_000)); // R2 should have ms tag with R1's AS value let r2_ms = template.records[1].data().get(&ms_tag); assert!(r2_ms.is_some(), "R2 should have ms tag"); - assert_eq!(extract_int_value(r2_ms.unwrap()), Some(100_000)); + assert_eq!(extract_int_value(r2_ms.expect("R2 ms tag should be present")), Some(100_000)); Ok(()) } @@ -2164,7 +2164,10 @@ mod tests { // R1 supplementary should have ms tag with R2's AS value (44) let r1_supp_ms = template.records[2].data().get(&ms_tag); assert!(r1_supp_ms.is_some(), "R1 supplementary should have ms tag"); - assert_eq!(extract_int_value(r1_supp_ms.unwrap()), Some(44)); + assert_eq!( + extract_int_value(r1_supp_ms.expect("R1 supplementary ms tag should be present")), + Some(44) + ); Ok(()) } @@ -2500,7 +2503,7 @@ mod tests { let is_paired = (flags & FLAG_PAIRED) != 0; let mut builder = RecordBuilder::new() - .name(std::str::from_utf8(name).unwrap()) + .name(std::str::from_utf8(name).expect("read name should be valid UTF-8")) .sequence(&"A".repeat(100)) .qualities(&[30u8; 100]) .cigar("100M") @@ -2948,7 +2951,7 @@ mod tests { let is_paired = (flags & FLAG_PAIRED) != 0; let mut builder = RecordBuilder::new() - .name(std::str::from_utf8(name).unwrap()) + .name(std::str::from_utf8(name).expect("read name should be valid UTF-8")) .sequence(&"A".repeat(read_length)) .qualities(&vec![30u8; read_length]) .cigar(&format!("{read_length}M")) @@ -3715,7 +3718,8 @@ mod tests { #[test] fn test_from_raw_records_creates_raw_mode_template() { let raw = make_minimal_raw_bam(b"read1", FLAG_PAIRED | FLAG_READ1); - let template = Template::from_raw_records(vec![raw]).unwrap(); + let template = + Template::from_raw_records(vec![raw]).expect("from_raw_records should succeed"); // Verify it's in raw-byte mode assert!(template.is_raw_byte_mode()); @@ -3729,7 +3733,8 @@ mod tests { fn test_r1_accessor_returns_none_in_raw_mode() { // After fix: calling r1() on a raw-mode Template returns None instead of panicking let raw = make_minimal_raw_bam(b"read1", FLAG_PAIRED | FLAG_READ1); - let template = Template::from_raw_records(vec![raw]).unwrap(); + let template = + Template::from_raw_records(vec![raw]).expect("from_raw_records should succeed"); // r1() should return None in raw mode (use raw_r1() instead) assert!(template.r1().is_none()); @@ -3742,7 +3747,8 @@ mod tests { // After fix: calling r2() on a raw-mode Template returns None instead of panicking let r1 = make_minimal_raw_bam(b"read1", FLAG_PAIRED | FLAG_READ1); let r2 = make_minimal_raw_bam(b"read1", FLAG_PAIRED | FLAG_READ2); - let template = Template::from_raw_records(vec![r1, r2]).unwrap(); + let template = + Template::from_raw_records(vec![r1, r2]).expect("from_raw_records should succeed"); assert!(template.is_raw_byte_mode()); @@ -3761,13 +3767,15 @@ mod tests { // R1 first, R2 second — fast path with no swap let r1 = make_minimal_raw_bam(b"read1", FLAG_PAIRED | FLAG_READ1); let r2 = make_minimal_raw_bam(b"read1", FLAG_PAIRED | FLAG_READ2); - let template = Template::from_raw_records(vec![r1, r2]).unwrap(); + let template = + Template::from_raw_records(vec![r1, r2]).expect("from_raw_records should succeed"); assert!(template.is_raw_byte_mode()); assert!(template.raw_r1().is_some()); assert!(template.raw_r2().is_some()); // Verify R1 is at index 0 (has FIRST_SEGMENT flag) - let r1_flags = crate::sort::bam_fields::flags(template.raw_r1().unwrap()); + let raw_r1 = template.raw_r1().expect("raw_r1 should be present"); + let r1_flags = crate::sort::bam_fields::flags(raw_r1); assert_ne!(r1_flags & FLAG_READ1, 0); } @@ -3776,13 +3784,15 @@ mod tests { // R2 first, R1 second — fast path should swap them let r1 = make_minimal_raw_bam(b"read1", FLAG_PAIRED | FLAG_READ1); let r2 = make_minimal_raw_bam(b"read1", FLAG_PAIRED | FLAG_READ2); - let template = Template::from_raw_records(vec![r2, r1]).unwrap(); + let template = Template::from_raw_records(vec![r2, r1]).expect("from_raw_records"); assert!(template.is_raw_byte_mode()); // After swap, R1 should be at index 0 - let r1_flags = crate::sort::bam_fields::flags(template.raw_r1().unwrap()); + let raw_r1 = template.raw_r1().expect("raw_r1 should be present"); + let r1_flags = crate::sort::bam_fields::flags(raw_r1); assert_ne!(r1_flags & FLAG_READ1, 0); - let r2_flags = crate::sort::bam_fields::flags(template.raw_r2().unwrap()); + let raw_r2 = template.raw_r2().expect("raw_r2 should be present"); + let r2_flags = crate::sort::bam_fields::flags(raw_r2); assert_ne!(r2_flags & FLAG_READ2, 0); } @@ -3803,7 +3813,8 @@ mod tests { b"read1", FLAG_PAIRED | FLAG_READ1 | crate::sort::bam_fields::flags::SECONDARY, ); - let template = Template::from_raw_records(vec![r1, sec]).unwrap(); + let template = + Template::from_raw_records(vec![r1, sec]).expect("from_raw_records should succeed"); assert!(template.is_raw_byte_mode()); assert!(template.raw_r1().is_some()); } @@ -3817,7 +3828,8 @@ mod tests { FLAG_PAIRED | FLAG_READ1 | crate::sort::bam_fields::flags::SUPPLEMENTARY, ); let r2 = make_minimal_raw_bam(b"read1", FLAG_PAIRED | FLAG_READ2); - let template = Template::from_raw_records(vec![r1, r1_supp, r2]).unwrap(); + let template = + Template::from_raw_records(vec![r1, r1_supp, r2]).expect("value should be present"); assert!(template.is_raw_byte_mode()); assert!(template.raw_r1().is_some()); assert!(template.raw_r2().is_some()); @@ -3827,7 +3839,8 @@ mod tests { fn test_from_raw_records_single_unpaired() { // Single unpaired record — should be treated as R1 let r = make_minimal_raw_bam(b"read1", 0); // no PAIRED flag - let template = Template::from_raw_records(vec![r]).unwrap(); + let template = + Template::from_raw_records(vec![r]).expect("from_raw_records should succeed"); assert!(template.is_raw_byte_mode()); assert!(template.raw_r1().is_some()); assert!(template.raw_r2().is_none()); @@ -3856,9 +3869,10 @@ mod tests { fn test_from_raw_records_all_raw_records_mut() { let r1 = make_minimal_raw_bam(b"read1", FLAG_PAIRED | FLAG_READ1); let r2 = make_minimal_raw_bam(b"read1", FLAG_PAIRED | FLAG_READ2); - let mut template = Template::from_raw_records(vec![r1, r2]).unwrap(); + let mut template = + Template::from_raw_records(vec![r1, r2]).expect("from_raw_records should succeed"); // Should be able to get mutable access - let recs = template.all_raw_records_mut().unwrap(); + let recs = template.all_raw_records_mut().expect("all_raw_records_mut should succeed"); assert_eq!(recs.len(), 2); } @@ -3866,8 +3880,9 @@ mod tests { fn test_from_raw_records_into_raw_records() { let r1 = make_minimal_raw_bam(b"read1", FLAG_PAIRED | FLAG_READ1); let r2 = make_minimal_raw_bam(b"read1", FLAG_PAIRED | FLAG_READ2); - let template = Template::from_raw_records(vec![r1, r2]).unwrap(); - let recs = template.into_raw_records().unwrap(); + let template = + Template::from_raw_records(vec![r1, r2]).expect("from_raw_records should succeed"); + let recs = template.into_raw_records().expect("into_raw_records should succeed"); assert_eq!(recs.len(), 2); } diff --git a/src/lib/umi/parallel_assigner.rs b/src/lib/umi/parallel_assigner.rs index 624e9e61f..f7b861349 100644 --- a/src/lib/umi/parallel_assigner.rs +++ b/src/lib/umi/parallel_assigner.rs @@ -815,24 +815,24 @@ mod tests { #[test] fn test_generate_neighbors() { - let umi = BitEnc::from_bytes(b"AA").unwrap(); + let umi = BitEnc::from_bytes(b"AA").expect("valid DNA sequence"); let neighbors: Vec<_> = generate_neighbors(&umi).collect(); // 2 positions × 3 alternatives = 6 neighbors assert_eq!(neighbors.len(), 6); // Check specific neighbors - assert!(neighbors.contains(&BitEnc::from_bytes(b"CA").unwrap())); - assert!(neighbors.contains(&BitEnc::from_bytes(b"GA").unwrap())); - assert!(neighbors.contains(&BitEnc::from_bytes(b"TA").unwrap())); - assert!(neighbors.contains(&BitEnc::from_bytes(b"AC").unwrap())); - assert!(neighbors.contains(&BitEnc::from_bytes(b"AG").unwrap())); - assert!(neighbors.contains(&BitEnc::from_bytes(b"AT").unwrap())); + assert!(neighbors.contains(&BitEnc::from_bytes(b"CA").expect("valid DNA sequence"))); + assert!(neighbors.contains(&BitEnc::from_bytes(b"GA").expect("valid DNA sequence"))); + assert!(neighbors.contains(&BitEnc::from_bytes(b"TA").expect("valid DNA sequence"))); + assert!(neighbors.contains(&BitEnc::from_bytes(b"AC").expect("valid DNA sequence"))); + assert!(neighbors.contains(&BitEnc::from_bytes(b"AG").expect("valid DNA sequence"))); + assert!(neighbors.contains(&BitEnc::from_bytes(b"AT").expect("valid DNA sequence"))); } #[test] fn test_generate_neighbors_k2() { - let umi = BitEnc::from_bytes(b"AA").unwrap(); + let umi = BitEnc::from_bytes(b"AA").expect("valid DNA sequence"); let neighbors = generate_neighbors_k(&umi, 2); // Should include: original, all 1-edit, all 2-edit neighbors @@ -845,11 +845,11 @@ mod tests { assert!(neighbors.contains(&umi)); // Should include 1-edit neighbors - assert!(neighbors.contains(&BitEnc::from_bytes(b"CA").unwrap())); + assert!(neighbors.contains(&BitEnc::from_bytes(b"CA").expect("valid DNA sequence"))); // Should include 2-edit neighbors - assert!(neighbors.contains(&BitEnc::from_bytes(b"CC").unwrap())); - assert!(neighbors.contains(&BitEnc::from_bytes(b"TT").unwrap())); + assert!(neighbors.contains(&BitEnc::from_bytes(b"CC").expect("valid DNA sequence"))); + assert!(neighbors.contains(&BitEnc::from_bytes(b"TT").expect("valid DNA sequence"))); } // ==================== Edge Discovery Tests ==================== @@ -857,9 +857,9 @@ mod tests { #[test] fn test_discover_edges_parallel_k1() { let umis = vec![ - (BitEnc::from_bytes(b"AAAA").unwrap(), 10), - (BitEnc::from_bytes(b"AAAT").unwrap(), 5), // 1 edit from AAAA - (BitEnc::from_bytes(b"TTTT").unwrap(), 3), // 4 edits from AAAA + (BitEnc::from_bytes(b"AAAA").expect("valid DNA sequence"), 10), + (BitEnc::from_bytes(b"AAAT").expect("valid DNA sequence"), 5), // 1 edit from AAAA + (BitEnc::from_bytes(b"TTTT").expect("valid DNA sequence"), 3), // 4 edits from AAAA ]; let edges = discover_edges_parallel_k1(&umis); @@ -872,9 +872,9 @@ mod tests { #[test] fn test_discover_edges_parallel_k2() { let umis = vec![ - (BitEnc::from_bytes(b"AAAA").unwrap(), 10), - (BitEnc::from_bytes(b"AATT").unwrap(), 5), // 2 edits from AAAA - (BitEnc::from_bytes(b"TTTT").unwrap(), 3), // 4 edits from AAAA, 2 from AATT + (BitEnc::from_bytes(b"AAAA").expect("valid DNA sequence"), 10), + (BitEnc::from_bytes(b"AATT").expect("valid DNA sequence"), 5), // 2 edits from AAAA + (BitEnc::from_bytes(b"TTTT").expect("valid DNA sequence"), 3), // 4 edits from AAAA, 2 from AATT ]; let edges = discover_edges_parallel_k(&umis, 2); @@ -889,9 +889,9 @@ mod tests { fn test_discover_edges_parallel_chain() { // Test A-B-C chain where A~B and B~C let umis = vec![ - (BitEnc::from_bytes(b"AAAA").unwrap(), 10), - (BitEnc::from_bytes(b"AAAT").unwrap(), 5), // 1 edit from AAAA - (BitEnc::from_bytes(b"AATT").unwrap(), 3), // 1 edit from AAAT, 2 from AAAA + (BitEnc::from_bytes(b"AAAA").expect("valid DNA sequence"), 10), + (BitEnc::from_bytes(b"AAAT").expect("valid DNA sequence"), 5), // 1 edit from AAAA + (BitEnc::from_bytes(b"AATT").expect("valid DNA sequence"), 3), // 1 edit from AAAT, 2 from AAAA ]; let edges = discover_edges_parallel_k1(&umis); diff --git a/src/lib/unified_pipeline/bam.rs b/src/lib/unified_pipeline/bam.rs index 4e97e384e..709895e2c 100644 --- a/src/lib/unified_pipeline/bam.rs +++ b/src/lib/unified_pipeline/bam.rs @@ -4468,7 +4468,11 @@ mod tests { // Verify the secondary serialize function works let test_data = vec![1u8, 2, 3, 4]; let mut buf = Vec::new(); - let count = (fns.secondary_serialize_fn.as_ref().unwrap())(&test_data, &mut buf).unwrap(); + let count = + (fns.secondary_serialize_fn.as_ref().expect("secondary_serialize_fn should be set"))( + &test_data, &mut buf, + ) + .expect("serialize should succeed"); assert_eq!(count, 1); assert_eq!(buf, vec![1, 2, 3, 4]); } diff --git a/src/lib/unified_pipeline/base.rs b/src/lib/unified_pipeline/base.rs index 3cdf1c286..7741d786c 100644 --- a/src/lib/unified_pipeline/base.rs +++ b/src/lib/unified_pipeline/base.rs @@ -2989,7 +2989,7 @@ pub struct PipelineStats { fn new_atomic_array() -> Box<[AtomicU64; N]> { // Create a Vec and convert to boxed slice, then try_into boxed array let v: Vec = (0..N).map(|_| AtomicU64::new(0)).collect(); - v.into_boxed_slice().try_into().unwrap() + v.into_boxed_slice().try_into().expect("Vec length matches const N") } /// Helper to create a boxed 2D array of `AtomicU64` initialized to zero. @@ -2998,14 +2998,14 @@ fn new_atomic_array() -> Box<[AtomicU64; N]> { fn new_atomic_2d_array() -> Box<[[AtomicU64; C]; R]> { let v: Vec<[AtomicU64; C]> = (0..R).map(|_| std::array::from_fn(|_| AtomicU64::new(0))).collect(); - v.into_boxed_slice().try_into().unwrap() + v.into_boxed_slice().try_into().expect("Vec length matches const R") } /// Helper to create a boxed array of `AtomicU8` initialized to a value. #[allow(clippy::unnecessary_box_returns)] fn new_atomic_u8_array(init: u8) -> Box<[AtomicU8; N]> { let v: Vec = (0..N).map(|_| AtomicU8::new(init)).collect(); - v.into_boxed_slice().try_into().unwrap() + v.into_boxed_slice().try_into().expect("Vec length matches const N") } impl Default for PipelineStats { @@ -3355,8 +3355,8 @@ impl PipelineStats { use std::fmt::Write; let mut s = String::new(); - writeln!(s, "Pipeline Statistics:").unwrap(); - writeln!(s).unwrap(); + writeln!(s, "Pipeline Statistics:").expect("write to String"); + writeln!(s).expect("write to String"); // Helper to format step stats #[allow(clippy::uninlined_format_args)] @@ -3373,7 +3373,7 @@ impl PipelineStats { } }; - writeln!(s, "Step Timing:").unwrap(); + writeln!(s, "Step Timing:").expect("write to String"); writeln!( s, "{}", @@ -3383,7 +3383,7 @@ impl PipelineStats { self.step_read_count.load(Ordering::Relaxed) ) ) - .unwrap(); + .expect("write to String"); writeln!( s, "{}", @@ -3393,7 +3393,7 @@ impl PipelineStats { self.step_decompress_count.load(Ordering::Relaxed) ) ) - .unwrap(); + .expect("write to String"); writeln!( s, "{}", @@ -3403,7 +3403,7 @@ impl PipelineStats { self.step_find_boundaries_count.load(Ordering::Relaxed) ) ) - .unwrap(); + .expect("write to String"); writeln!( s, "{}", @@ -3413,7 +3413,7 @@ impl PipelineStats { self.step_decode_count.load(Ordering::Relaxed) ) ) - .unwrap(); + .expect("write to String"); writeln!( s, "{}", @@ -3423,7 +3423,7 @@ impl PipelineStats { self.step_group_count.load(Ordering::Relaxed) ) ) - .unwrap(); + .expect("write to String"); writeln!( s, "{}", @@ -3433,7 +3433,7 @@ impl PipelineStats { self.step_process_count.load(Ordering::Relaxed) ) ) - .unwrap(); + .expect("write to String"); writeln!( s, "{}", @@ -3443,7 +3443,7 @@ impl PipelineStats { self.step_serialize_count.load(Ordering::Relaxed) ) ) - .unwrap(); + .expect("write to String"); writeln!( s, "{}", @@ -3453,7 +3453,7 @@ impl PipelineStats { self.step_compress_count.load(Ordering::Relaxed) ) ) - .unwrap(); + .expect("write to String"); writeln!( s, "{}", @@ -3463,59 +3463,71 @@ impl PipelineStats { self.step_write_count.load(Ordering::Relaxed) ) ) - .unwrap(); + .expect("write to String"); - writeln!(s).unwrap(); - writeln!(s, "Contention:").unwrap(); + writeln!(s).expect("write to String"); + writeln!(s, "Contention:").expect("write to String"); writeln!( s, " Read lock: {:>10} failed attempts", self.read_contention.load(Ordering::Relaxed) ) - .unwrap(); + .expect("write to String"); writeln!( s, " Boundary lock: {:>10} failed attempts", self.boundary_contention.load(Ordering::Relaxed) ) - .unwrap(); + .expect("write to String"); writeln!( s, " Group lock: {:>10} failed attempts", self.group_contention.load(Ordering::Relaxed) ) - .unwrap(); + .expect("write to String"); writeln!( s, " Write lock: {:>10} failed attempts", self.write_contention.load(Ordering::Relaxed) ) - .unwrap(); - - writeln!(s).unwrap(); - writeln!(s, "Queue Empty Polls:").unwrap(); - writeln!(s, " Q1 (raw): {:>10}", self.q1_empty.load(Ordering::Relaxed)).unwrap(); - writeln!(s, " Q2 (decomp): {:>10}", self.q2_empty.load(Ordering::Relaxed)).unwrap(); - writeln!(s, " Q2b (boundary): {:>10}", self.q2b_empty.load(Ordering::Relaxed)).unwrap(); - writeln!(s, " Q3 (decoded): {:>10}", self.q3_empty.load(Ordering::Relaxed)).unwrap(); - writeln!(s, " Q4 (groups): {:>10}", self.q4_empty.load(Ordering::Relaxed)).unwrap(); - writeln!(s, " Q5 (processed): {:>10}", self.q5_empty.load(Ordering::Relaxed)).unwrap(); - writeln!(s, " Q6 (serialized): {:>10}", self.q6_empty.load(Ordering::Relaxed)).unwrap(); - writeln!(s, " Q7 (compressed): {:>10}", self.q7_empty.load(Ordering::Relaxed)).unwrap(); - - writeln!(s).unwrap(); - writeln!(s, "Idle Yields: {:>10}", self.idle_yields.load(Ordering::Relaxed)).unwrap(); + .expect("write to String"); + + writeln!(s).expect("write to String"); + writeln!(s, "Queue Empty Polls:").expect("write to String"); + writeln!(s, " Q1 (raw): {:>10}", self.q1_empty.load(Ordering::Relaxed)) + .expect("write to String"); + writeln!(s, " Q2 (decomp): {:>10}", self.q2_empty.load(Ordering::Relaxed)) + .expect("write to String"); + writeln!(s, " Q2b (boundary): {:>10}", self.q2b_empty.load(Ordering::Relaxed)) + .expect("write to String"); + writeln!(s, " Q3 (decoded): {:>10}", self.q3_empty.load(Ordering::Relaxed)) + .expect("write to String"); + writeln!(s, " Q4 (groups): {:>10}", self.q4_empty.load(Ordering::Relaxed)) + .expect("write to String"); + writeln!(s, " Q5 (processed): {:>10}", self.q5_empty.load(Ordering::Relaxed)) + .expect("write to String"); + writeln!(s, " Q6 (serialized): {:>10}", self.q6_empty.load(Ordering::Relaxed)) + .expect("write to String"); + writeln!(s, " Q7 (compressed): {:>10}", self.q7_empty.load(Ordering::Relaxed)) + .expect("write to String"); + + writeln!(s).expect("write to String"); + writeln!(s, "Idle Yields: {:>10}", self.idle_yields.load(Ordering::Relaxed)) + .expect("write to String"); // NEW: Wait time for exclusive steps let boundary_wait = self.boundary_wait_ns.load(Ordering::Relaxed); let group_wait = self.group_wait_ns.load(Ordering::Relaxed); let write_wait = self.write_wait_ns.load(Ordering::Relaxed); if boundary_wait > 0 || group_wait > 0 || write_wait > 0 { - writeln!(s).unwrap(); - writeln!(s, "Lock Wait Time:").unwrap(); - writeln!(s, " Boundary lock: {:>10.1}ms", boundary_wait as f64 / 1_000_000.0).unwrap(); - writeln!(s, " Group lock: {:>10.1}ms", group_wait as f64 / 1_000_000.0).unwrap(); - writeln!(s, " Write lock: {:>10.1}ms", write_wait as f64 / 1_000_000.0).unwrap(); + writeln!(s).expect("write to String"); + writeln!(s, "Lock Wait Time:").expect("write to String"); + writeln!(s, " Boundary lock: {:>10.1}ms", boundary_wait as f64 / 1_000_000.0) + .expect("write to String"); + writeln!(s, " Group lock: {:>10.1}ms", group_wait as f64 / 1_000_000.0) + .expect("write to String"); + writeln!(s, " Write lock: {:>10.1}ms", write_wait as f64 / 1_000_000.0) + .expect("write to String"); } // NEW: Batch size statistics @@ -3526,69 +3538,69 @@ impl PipelineStats { let batch_max = self.batch_size_max.load(Ordering::Relaxed); let batch_avg = batch_sum as f64 / batch_count as f64; - writeln!(s).unwrap(); - writeln!(s, "Batch Size (records per batch):").unwrap(); - writeln!(s, " Count: {batch_count:>10}").unwrap(); - writeln!(s, " Min: {batch_min:>10}").unwrap(); - writeln!(s, " Max: {batch_max:>10}").unwrap(); - writeln!(s, " Average: {batch_avg:>10.1}").unwrap(); + writeln!(s).expect("write to String"); + writeln!(s, "Batch Size (records per batch):").expect("write to String"); + writeln!(s, " Count: {batch_count:>10}").expect("write to String"); + writeln!(s, " Min: {batch_min:>10}").expect("write to String"); + writeln!(s, " Max: {batch_max:>10}").expect("write to String"); + writeln!(s, " Average: {batch_avg:>10.1}").expect("write to String"); } // NEW: Per-thread work distribution let num_threads = self.num_threads.load(Ordering::Relaxed) as usize; if num_threads > 0 { - writeln!(s).unwrap(); - writeln!(s, "Per-Thread Work Distribution:").unwrap(); + writeln!(s).expect("write to String"); + writeln!(s, "Per-Thread Work Distribution:").expect("write to String"); // Step names for the header let step_names = ["Rd", "Dc", "Fb", "De", "Gr", "Pr", "Se", "Co", "Wr"]; // Header - write!(s, " Thread ").unwrap(); + write!(s, " Thread ").expect("write to String"); for name in &step_names { - write!(s, " {name:>6}").unwrap(); + write!(s, " {name:>6}").expect("write to String"); } - writeln!(s, " Idle ms").unwrap(); + writeln!(s, " Idle ms").expect("write to String"); // Per-thread rows for tid in 0..num_threads.min(MAX_THREADS) { - write!(s, " T{tid:<5} ").unwrap(); + write!(s, " T{tid:<5} ").expect("write to String"); for step_idx in 0..NUM_STEPS { let count = self.per_thread_step_counts[tid][step_idx].load(Ordering::Relaxed); - write!(s, " {count:>6}").unwrap(); + write!(s, " {count:>6}").expect("write to String"); } let idle_ns = self.per_thread_idle_ns[tid].load(Ordering::Relaxed); - writeln!(s, " {:>10.1}", idle_ns as f64 / 1_000_000.0).unwrap(); + writeln!(s, " {:>10.1}", idle_ns as f64 / 1_000_000.0).expect("write to String"); } // Total row - write!(s, " Total ").unwrap(); + write!(s, " Total ").expect("write to String"); for step_idx in 0..NUM_STEPS { let mut total = 0u64; for tid in 0..num_threads.min(MAX_THREADS) { total += self.per_thread_step_counts[tid][step_idx].load(Ordering::Relaxed); } - write!(s, " {total:>6}").unwrap(); + write!(s, " {total:>6}").expect("write to String"); } let total_idle: u64 = (0..num_threads.min(MAX_THREADS)) .map(|tid| self.per_thread_idle_ns[tid].load(Ordering::Relaxed)) .sum(); - writeln!(s, " {:>10.1}", total_idle as f64 / 1_000_000.0).unwrap(); + writeln!(s, " {:>10.1}", total_idle as f64 / 1_000_000.0).expect("write to String"); // Per-thread attempt statistics (shows success rate) - writeln!(s).unwrap(); - writeln!(s, "Per-Thread Attempt Success Rate:").unwrap(); + writeln!(s).expect("write to String"); + writeln!(s, "Per-Thread Attempt Success Rate:").expect("write to String"); // Header - write!(s, " Thread ").unwrap(); + write!(s, " Thread ").expect("write to String"); for name in &step_names { - write!(s, " {name:>6}").unwrap(); + write!(s, " {name:>6}").expect("write to String"); } - writeln!(s, " Total%").unwrap(); + writeln!(s, " Total%").expect("write to String"); // Per-thread rows with success rates for tid in 0..num_threads.min(MAX_THREADS) { - write!(s, " T{tid:<5} ").unwrap(); + write!(s, " T{tid:<5} ").expect("write to String"); let mut thread_attempts = 0u64; let mut thread_successes = 0u64; for step_idx in 0..NUM_STEPS { @@ -3599,17 +3611,17 @@ impl PipelineStats { thread_attempts += attempts; thread_successes += successes; if attempts == 0 { - write!(s, " - ").unwrap(); + write!(s, " - ").expect("write to String"); } else { let rate = (successes as f64 / attempts as f64) * 100.0; - write!(s, " {rate:>5.0}%").unwrap(); + write!(s, " {rate:>5.0}%").expect("write to String"); } } if thread_attempts == 0 { - writeln!(s, " -").unwrap(); + writeln!(s, " -").expect("write to String"); } else { let total_rate = (thread_successes as f64 / thread_attempts as f64) * 100.0; - writeln!(s, " {total_rate:>5.1}%").unwrap(); + writeln!(s, " {total_rate:>5.1}%").expect("write to String"); } } } @@ -3635,8 +3647,8 @@ impl PipelineStats { + self.write_contention.load(Ordering::Relaxed); if total_work_ns > 0 { - writeln!(s).unwrap(); - writeln!(s, "Thread Utilization:").unwrap(); + writeln!(s).expect("write to String"); + writeln!(s, "Thread Utilization:").expect("write to String"); let work_ms = total_work_ns as f64 / 1_000_000.0; let idle_ms = total_idle_ns as f64 / 1_000_000.0; @@ -3644,10 +3656,11 @@ impl PipelineStats { if total_thread_ms > 0.0 { let utilization = (work_ms / total_thread_ms) * 100.0; - writeln!(s, " Work time: {work_ms:>10.1}ms").unwrap(); - writeln!(s, " Idle time: {idle_ms:>10.1}ms").unwrap(); - writeln!(s, " Utilization: {utilization:>10.1}%").unwrap(); - writeln!(s, " Contention attempts: {total_contention:>7}").unwrap(); + writeln!(s, " Work time: {work_ms:>10.1}ms").expect("write to String"); + writeln!(s, " Idle time: {idle_ms:>10.1}ms").expect("write to String"); + writeln!(s, " Utilization: {utilization:>10.1}%").expect("write to String"); + writeln!(s, " Contention attempts: {total_contention:>7}") + .expect("write to String"); } } @@ -3663,8 +3676,8 @@ impl PipelineStats { // Only show throughput section if we have data if bytes_read > 0 || bytes_written > 0 { - writeln!(s).unwrap(); - writeln!(s, "Throughput:").unwrap(); + writeln!(s).expect("write to String"); + writeln!(s, "Throughput:").expect("write to String"); // Helper function to format bytes nicely let format_bytes = |bytes: u64| -> String { @@ -3702,7 +3715,7 @@ impl PipelineStats { read_ms, read_mb_s ) - .unwrap(); + .expect("write to String"); } // Decompress throughput (input → output with expansion ratio) @@ -3721,7 +3734,7 @@ impl PipelineStats { out_mb_s, expansion ) - .unwrap(); + .expect("write to String"); } // Decode throughput (records per second) @@ -3735,7 +3748,7 @@ impl PipelineStats { format_count(records_decoded), format_count(records_per_s as u64) ) - .unwrap(); + .expect("write to String"); } // Group throughput (records in → groups out) @@ -3752,7 +3765,7 @@ impl PipelineStats { format_count(records_in_per_s as u64), format_count(groups_out_per_s as u64) ) - .unwrap(); + .expect("write to String"); } // Process throughput (groups per second) @@ -3766,7 +3779,7 @@ impl PipelineStats { format_count(groups_produced), format_count(groups_per_s as u64) ) - .unwrap(); + .expect("write to String"); } // Serialize throughput @@ -3780,7 +3793,7 @@ impl PipelineStats { format_bytes(serialized_bytes), mb_per_s ) - .unwrap(); + .expect("write to String"); } // Compress throughput (input → output with compression ratio) @@ -3799,7 +3812,7 @@ impl PipelineStats { out_mb_s, compression ) - .unwrap(); + .expect("write to String"); } // Write throughput @@ -3814,21 +3827,21 @@ impl PipelineStats { write_ms, write_mb_s ) - .unwrap(); + .expect("write to String"); } } // Queue sample summary if we have any let samples = self.queue_samples.lock(); if !samples.is_empty() { - writeln!(s).unwrap(); + writeln!(s).expect("write to String"); writeln!(s, "Queue Size Timeline ({} samples at ~100ms intervals):", samples.len()) - .unwrap(); + .expect("write to String"); writeln!( s, " Time Q1 Q2 Q2b Q3 Q4 Q5 Q6 Q7 | R2 R3 R7 | R3_MB Threads" ) - .unwrap(); + .expect("write to String"); // Show all samples for sample in samples.iter() { @@ -3850,7 +3863,7 @@ impl PipelineStats { sample.reorder_sizes[2], r3_mb, ) - .unwrap(); + .expect("write to String"); // Show thread activity as compact string for &step_idx in &sample.thread_steps { if step_idx < NUM_STEPS as u8 { @@ -3866,12 +3879,12 @@ impl PipelineStats { 8 => "W", _ => "?", }; - write!(s, "{short}").unwrap(); + write!(s, "{short}").expect("write to String"); } else { - write!(s, ".").unwrap(); + write!(s, ".").expect("write to String"); } } - writeln!(s).unwrap(); + writeln!(s).expect("write to String"); } // Summary of peak reorder buffer usage @@ -3879,9 +3892,9 @@ impl PipelineStats { let peak_r3_bytes = samples.iter().map(|s| s.reorder_memory_bytes[1]).max().unwrap_or(0); let peak_r3_mb = peak_r3_bytes as f64 / 1_048_576.0; - writeln!(s).unwrap(); + writeln!(s).expect("write to String"); writeln!(s, "Peak Q3 Reorder Buffer: {peak_r3_items} items, {peak_r3_mb:.1} MB") - .unwrap(); + .expect("write to String"); } // Memory limiting statistics @@ -3889,14 +3902,16 @@ impl PipelineStats { let peak_memory = self.peak_memory_bytes.load(Ordering::Relaxed); if group_rejects > 0 || peak_memory > 0 { - writeln!(s).unwrap(); - writeln!(s, "Memory Limiting:").unwrap(); + writeln!(s).expect("write to String"); + writeln!(s, "Memory Limiting:").expect("write to String"); if group_rejects > 0 { - writeln!(s, " Group rejects (memory): {group_rejects:>10}").unwrap(); + writeln!(s, " Group rejects (memory): {group_rejects:>10}") + .expect("write to String"); } if peak_memory > 0 { let peak_mb = peak_memory as f64 / 1_048_576.0; - writeln!(s, " Peak memory usage: {peak_mb:>10.1} MB").unwrap(); + writeln!(s, " Peak memory usage: {peak_mb:>10.1} MB") + .expect("write to String"); } } @@ -5848,7 +5863,7 @@ mod tests { assert!(queues.has_error()); let err = queues.take_error(); assert!(err.is_some()); - assert_eq!(err.unwrap().to_string(), "test error"); + assert_eq!(err.expect("error was set above").to_string(), "test error"); } #[test] diff --git a/src/lib/unified_pipeline/fastq.rs b/src/lib/unified_pipeline/fastq.rs index e673d67f5..80273eb62 100644 --- a/src/lib/unified_pipeline/fastq.rs +++ b/src/lib/unified_pipeline/fastq.rs @@ -142,7 +142,10 @@ impl PairState { return None; } - let slots = self.pending.remove(&self.next_emit).unwrap(); + let slots = self + .pending + .remove(&self.next_emit) + .expect("next_emit key must exist in pending map after get() succeeded"); self.next_emit += 1; Some(slots.into_iter().flatten().collect()) } @@ -1979,7 +1982,7 @@ fn fastq_try_step_find_boundaries( .map(|c| FastqStreamBoundaries { stream_idx: c.stream_idx, data: c.data, - offsets: c.offsets.unwrap(), + offsets: c.offsets.expect("gzip chunks must have pre-computed offsets"), }) .collect(); align_stream_records(streams, serial) @@ -2171,7 +2174,7 @@ fn create_templates_from_streams( 0 => Ok(Vec::new()), 1 => { // Single-end: each record becomes its own template - let records = streams.pop().unwrap().records; + let records = streams.pop().expect("streams is non-empty in single-end branch").records; Ok(records .into_iter() .map(|r| { @@ -2186,8 +2189,8 @@ fn create_templates_from_streams( // always R2, regardless of the order produced by find_boundaries(). streams.sort_by_key(|s| s.stream_idx); let mut drain = streams.into_iter(); - let r1_records = drain.next().unwrap().records; - let r2_records = drain.next().unwrap().records; + let r1_records = drain.next().expect("sorted streams must have R1 at index 0").records; + let r2_records = drain.next().expect("sorted streams must have R2 at index 1").records; // Validate batch sizes match if r1_records.len() != r2_records.len() { @@ -3022,7 +3025,7 @@ mod tests { ]; let mut reader = Cursor::new(bgzf_empty_block); - let blocks = read_raw_blocks(&mut reader, 10).unwrap(); + let blocks = read_raw_blocks(&mut reader, 10).expect("failed to read raw blocks"); // Should read 0 blocks (EOF blocks are skipped by read_raw_blocks) assert_eq!(blocks.len(), 0); @@ -3065,8 +3068,8 @@ mod tests { } }); - producer.join().unwrap(); - consumer.join().unwrap(); + producer.join().expect("thread should not panic"); + consumer.join().expect("thread should not panic"); let pushed_count = items_pushed.load(Ordering::Relaxed); let received_count = items_received.load(Ordering::Relaxed); @@ -3110,14 +3113,16 @@ mod tests { }; // Step 1: Find boundaries - let boundary_batch = FastqFormat::find_boundaries(&boundary_state, batch).unwrap(); + let boundary_batch = + FastqFormat::find_boundaries(&boundary_state, batch).expect("find_boundaries failed"); assert_eq!(boundary_batch.streams.len(), 2); // Each stream should have 2 complete records (offsets at 0, 19, 38) assert_eq!(boundary_batch.streams[0].offsets.len(), 3); assert_eq!(boundary_batch.streams[1].offsets.len(), 3); // Step 2: Parse records - let parsed_batch = FastqFormat::parse_records(boundary_batch).unwrap(); + let parsed_batch = + FastqFormat::parse_records(boundary_batch).expect("parse_records failed"); assert_eq!(parsed_batch.streams.len(), 2); assert_eq!(parsed_batch.streams[0].stream_idx, 0); assert_eq!(parsed_batch.streams[1].stream_idx, 1); @@ -3146,7 +3151,8 @@ mod tests { serial: 0, }; - let boundary_batch1 = FastqFormat::find_boundaries(&boundary_state, batch1).unwrap(); + let boundary_batch1 = + FastqFormat::find_boundaries(&boundary_state, batch1).expect("find_boundaries failed"); assert_eq!(boundary_batch1.streams[0].offsets.len(), 2); // One complete record assert!(!boundary_state.stream_states[0].lock().leftover.is_empty()); // Leftover from incomplete @@ -3156,14 +3162,15 @@ mod tests { serial: 1, }; - let boundary_batch2 = FastqFormat::find_boundaries(&boundary_state, batch2).unwrap(); + let boundary_batch2 = + FastqFormat::find_boundaries(&boundary_state, batch2).expect("find_boundaries failed"); // Leftover + new data should form complete record assert!(boundary_batch2.streams[0].offsets.len() >= 2); assert!(boundary_state.stream_states[0].lock().leftover.is_empty()); // No more leftover // Parse both batches - let parsed1 = FastqFormat::parse_records(boundary_batch1).unwrap(); - let parsed2 = FastqFormat::parse_records(boundary_batch2).unwrap(); + let parsed1 = FastqFormat::parse_records(boundary_batch1).expect("parse_records failed"); + let parsed2 = FastqFormat::parse_records(boundary_batch2).expect("parse_records failed"); assert_eq!(parsed1.streams[0].records.len(), 1); assert_eq!(parsed1.streams[0].records[0].name, b"read1"); @@ -3199,7 +3206,8 @@ mod tests { }; // Parse the batch - let parsed = FastqFormat::parse_records(boundary_batch).unwrap(); + let parsed = FastqFormat::parse_records(boundary_batch) + .expect("parse_records failed"); assert_eq!(parsed.streams[0].stream_idx, 0); assert_eq!(parsed.streams[0].records.len(), 1); assert_eq!( @@ -3245,7 +3253,8 @@ mod tests { serial: 0, }; - let boundary_batch = FastqFormat::find_boundaries(&boundary_state, batch).unwrap(); + let boundary_batch = + FastqFormat::find_boundaries(&boundary_state, batch).expect("find_boundaries failed"); // Both streams should have exactly 2 records (the minimum) assert_eq!(boundary_batch.streams.len(), 2); @@ -3292,7 +3301,8 @@ mod tests { serial: 0, }; - let boundary_batch1 = FastqFormat::find_boundaries(&boundary_state, batch1).unwrap(); + let boundary_batch1 = + FastqFormat::find_boundaries(&boundary_state, batch1).expect("find_boundaries failed"); assert_eq!(boundary_batch1.streams[0].offsets.len() - 1, 2); // 2 records from stream 0 assert_eq!(boundary_batch1.streams[1].offsets.len() - 1, 2); // 2 records from stream 1 @@ -3309,7 +3319,8 @@ mod tests { serial: 1, }; - let boundary_batch2 = FastqFormat::find_boundaries(&boundary_state, batch2).unwrap(); + let boundary_batch2 = + FastqFormat::find_boundaries(&boundary_state, batch2).expect("find_boundaries failed"); // Stream 0: leftover(r3) + new(r4) = 2 records // Stream 1: 2 new records (r3, r4) @@ -3326,7 +3337,7 @@ mod tests { ); // Parse and verify record names - let parsed = FastqFormat::parse_records(boundary_batch2).unwrap(); + let parsed = FastqFormat::parse_records(boundary_batch2).expect("parse_records failed"); assert_eq!( parsed.streams[0].records[0].name, b"r3", "First record should be r3 from leftover" @@ -3352,7 +3363,8 @@ mod tests { serial: 0, }; - let boundary_batch1 = FastqFormat::find_boundaries(&boundary_state, batch1).unwrap(); + let boundary_batch1 = + FastqFormat::find_boundaries(&boundary_state, batch1).expect("find_boundaries failed"); // Both aligned to 1 record assert_eq!(boundary_batch1.streams[0].offsets.len() - 1, 1); assert_eq!(boundary_batch1.streams[1].offsets.len() - 1, 1); @@ -3369,14 +3381,17 @@ mod tests { serial: 1, }; - let boundary_batch2 = FastqFormat::find_boundaries(&boundary_state, batch2).unwrap(); + let boundary_batch2 = + FastqFormat::find_boundaries(&boundary_state, batch2).expect("find_boundaries failed"); // Both streams should be present (stream 0 from leftover, stream 1 from new chunk) assert_eq!(boundary_batch2.streams.len(), 2, "Both streams should be present"); // Find stream 0 and stream 1 in the result (order may vary) - let stream0 = boundary_batch2.streams.iter().find(|s| s.stream_idx == 0).unwrap(); - let stream1 = boundary_batch2.streams.iter().find(|s| s.stream_idx == 1).unwrap(); + let stream0 = + boundary_batch2.streams.iter().find(|s| s.stream_idx == 0).expect("stream not found"); + let stream1 = + boundary_batch2.streams.iter().find(|s| s.stream_idx == 1).expect("stream not found"); assert_eq!(stream0.offsets.len() - 1, 1, "Stream 0 should have 1 record from leftover"); assert_eq!(stream1.offsets.len() - 1, 1, "Stream 1 should have 1 record"); @@ -3407,7 +3422,8 @@ mod tests { serial: 0, }; - let boundary_batch = FastqFormat::find_boundaries(&boundary_state, batch).unwrap(); + let boundary_batch = + FastqFormat::find_boundaries(&boundary_state, batch).expect("find_boundaries failed"); // Both streams should have 2 records assert_eq!(boundary_batch.streams[0].offsets.len() - 1, 2); @@ -3431,7 +3447,8 @@ mod tests { serial: 0, }; - let boundary_batch = FastqFormat::find_boundaries(&boundary_state, batch).unwrap(); + let boundary_batch = + FastqFormat::find_boundaries(&boundary_state, batch).expect("find_boundaries failed"); // All 3 records should be present (no alignment needed for single stream) assert_eq!(boundary_batch.streams[0].offsets.len() - 1, 3); @@ -3455,7 +3472,8 @@ mod tests { serial: 0, }; - let _ = FastqFormat::find_boundaries(&boundary_state, batch1).unwrap(); + let _ = + FastqFormat::find_boundaries(&boundary_state, batch1).expect("find_boundaries failed"); // Verify stream 0 has leftover assert!(!boundary_state.stream_states[0].lock().leftover.is_empty()); @@ -3469,7 +3487,8 @@ mod tests { serial: 1, }; - let boundary_batch2 = FastqFormat::find_boundaries(&boundary_state, batch2).unwrap(); + let boundary_batch2 = + FastqFormat::find_boundaries(&boundary_state, batch2).expect("find_boundaries failed"); // Both streams should be present assert_eq!(boundary_batch2.streams.len(), 2); @@ -3502,7 +3521,7 @@ mod tests { serial: 42, }; - let parsed = FastqFormat::parse_records(boundary_batch).unwrap(); + let parsed = FastqFormat::parse_records(boundary_batch).expect("parse_records failed"); assert_eq!(parsed.serial, 42); assert_eq!(parsed.streams.len(), 2); // stream_idx must be preserved, not assumed from position @@ -3545,7 +3564,8 @@ mod tests { }, ]; - let templates = create_templates_from_streams(streams).unwrap(); + let templates = + create_templates_from_streams(streams).expect("create templates from streams"); assert_eq!(templates.len(), 1); assert_eq!(templates[0].records.len(), 2); @@ -3582,7 +3602,8 @@ mod tests { }, ]; - let templates = create_templates_from_streams(streams).unwrap(); + let templates = + create_templates_from_streams(streams).expect("create templates from streams"); assert_eq!(templates.len(), 1); assert_eq!(templates[0].records[0].sequence, b"ACGT", "R1 should be first"); @@ -3613,7 +3634,8 @@ mod tests { serial: 0, }; - let boundary_batch1 = FastqFormat::find_boundaries(&boundary_state, batch1).unwrap(); + let boundary_batch1 = + FastqFormat::find_boundaries(&boundary_state, batch1).expect("find_boundaries failed"); // Both aligned to 1 record; stream 0 has leftover assert_eq!(boundary_batch1.streams[0].offsets.len() - 1, 1); assert_eq!(boundary_batch1.streams[1].offsets.len() - 1, 1); @@ -3629,7 +3651,8 @@ mod tests { serial: 1, }; - let boundary_batch2 = FastqFormat::find_boundaries(&boundary_state, batch2).unwrap(); + let boundary_batch2 = + FastqFormat::find_boundaries(&boundary_state, batch2).expect("find_boundaries failed"); assert_eq!(boundary_batch2.streams.len(), 2); // The order of streams in boundary_batch2 may be [stream_idx=1, stream_idx=0] @@ -3638,13 +3661,14 @@ mod tests { let second_stream_idx = boundary_batch2.streams[1].stream_idx; // Parse records — stream_idx must be preserved - let parsed = FastqFormat::parse_records(boundary_batch2).unwrap(); + let parsed = FastqFormat::parse_records(boundary_batch2).expect("parse_records failed"); assert_eq!(parsed.streams[0].stream_idx, first_stream_idx); assert_eq!(parsed.streams[1].stream_idx, second_stream_idx); // Create templates — must produce correct R1/R2 ordering regardless // of the stream order in the Vec - let templates = create_templates_from_streams(parsed.streams).unwrap(); + let templates = + create_templates_from_streams(parsed.streams).expect("create templates from streams"); assert_eq!(templates.len(), 1); assert_eq!(templates[0].name, b"read2"); assert_eq!(templates[0].records.len(), 2); @@ -3684,7 +3708,8 @@ mod tests { let data = make_fastq_records(&[("r1", "ACGT"), ("r2", "TGCA"), ("r3", "AAAA")]); let mut cursor = Cursor::new(data); - let (buf, offsets, at_eof) = read_n_fastq_records(&mut cursor, 2).unwrap(); + let (buf, offsets, at_eof) = + read_n_fastq_records(&mut cursor, 2).expect("read N FASTQ records"); assert_eq!(offsets.len(), 3); // 2 records + initial 0 assert!(!at_eof); @@ -3702,7 +3727,8 @@ mod tests { let data = make_fastq_records(&[("r1", "ACGT")]); let mut cursor = Cursor::new(data); - let (_, offsets, at_eof) = read_n_fastq_records(&mut cursor, 5).unwrap(); + let (_, offsets, at_eof) = + read_n_fastq_records(&mut cursor, 5).expect("read N FASTQ records"); // Only 1 record available, requested 5 assert_eq!(offsets.len(), 2); // 1 record @@ -3713,7 +3739,8 @@ mod tests { fn test_read_n_fastq_records_empty_input() { let mut cursor = Cursor::new(Vec::::new()); - let (_, offsets, at_eof) = read_n_fastq_records(&mut cursor, 5).unwrap(); + let (_, offsets, at_eof) = + read_n_fastq_records(&mut cursor, 5).expect("read N FASTQ records"); assert_eq!(offsets.len(), 1); // Just the initial 0 assert!(at_eof); @@ -3748,7 +3775,7 @@ mod tests { data: b"data1".to_vec(), offsets: Some(vec![0, 5]), }); - let chunks = pair.try_pop_complete(false).unwrap(); + let chunks = pair.try_pop_complete(false).expect("try_pop_complete should succeed"); assert_eq!(chunks.len(), 2); assert!(pair.is_empty()); } @@ -3780,14 +3807,14 @@ mod tests { }); // Batch 0: complete - let chunks = pair.try_pop_complete(false).unwrap(); + let chunks = pair.try_pop_complete(false).expect("try_pop_complete should succeed"); assert_eq!(chunks.len(), 2); // Batch 1: only stream 0 — not complete without all_arrived assert!(pair.try_pop_complete(false).is_none()); // With all_arrived, batch 1 emits with just stream 0 - let chunks = pair.try_pop_complete(true).unwrap(); + let chunks = pair.try_pop_complete(true).expect("try_pop_complete should succeed"); assert_eq!(chunks.len(), 1); assert_eq!(chunks[0].stream_idx, 0); assert!(pair.is_empty()); @@ -3823,7 +3850,7 @@ mod tests { // Simulate: batches_read=2 (still reading), chunks_paired=2. // all_arrived = read_done(false) && ... → false. let all_arrived = false; - let chunks = pair.try_pop_complete(all_arrived).unwrap(); + let chunks = pair.try_pop_complete(all_arrived).expect("try_pop_complete should succeed"); assert_eq!(chunks.len(), 2); // Insert batch 1 from stream 0 only (stream 1 hit EOF earlier). @@ -3842,7 +3869,7 @@ mod tests { // Simulate: chunks_paired catches up to 3. // all_arrived = read_done(true) && 3 == 3 → true. let all_arrived = true; - let chunks = pair.try_pop_complete(all_arrived).unwrap(); + let chunks = pair.try_pop_complete(all_arrived).expect("try_pop_complete should succeed"); assert_eq!(chunks.len(), 1); assert_eq!(chunks[0].stream_idx, 0); assert!(pair.is_empty()); diff --git a/src/lib/unified_pipeline/queue.rs b/src/lib/unified_pipeline/queue.rs index 47e74901d..545b2f4a2 100644 --- a/src/lib/unified_pipeline/queue.rs +++ b/src/lib/unified_pipeline/queue.rs @@ -248,11 +248,11 @@ mod tests { assert!(queue.insert(1, 150, 10).is_ok()); // Pop in order - let (val, _) = queue.try_pop_next().unwrap(); + let (val, _) = queue.try_pop_next().expect("queue should have next element"); assert_eq!(val, 100); - let (val, _) = queue.try_pop_next().unwrap(); + let (val, _) = queue.try_pop_next().expect("queue should have next element"); assert_eq!(val, 150); - let (val, _) = queue.try_pop_next().unwrap(); + let (val, _) = queue.try_pop_next().expect("queue should have next element"); assert_eq!(val, 200); assert!(queue.try_pop_next().is_none()); diff --git a/src/lib/unified_pipeline/rebalancer.rs b/src/lib/unified_pipeline/rebalancer.rs index 586425e4d..bf7b84a89 100644 --- a/src/lib/unified_pipeline/rebalancer.rs +++ b/src/lib/unified_pipeline/rebalancer.rs @@ -349,13 +349,34 @@ mod tests { #[test] #[allow(clippy::cast_possible_truncation, clippy::cast_sign_loss)] fn test_parse_memory_limit() { - assert_eq!(parse_memory_limit("4GB").unwrap(), 4 * 1024 * 1024 * 1024); - assert_eq!(parse_memory_limit("4G").unwrap(), 4 * 1024 * 1024 * 1024); - assert_eq!(parse_memory_limit("512MB").unwrap(), 512 * 1024 * 1024); - assert_eq!(parse_memory_limit("512M").unwrap(), 512 * 1024 * 1024); - assert_eq!(parse_memory_limit("1gb").unwrap(), 1024 * 1024 * 1024); - assert_eq!(parse_memory_limit(" 2GB ").unwrap(), 2 * 1024 * 1024 * 1024); - assert_eq!(parse_memory_limit("1.5GB").unwrap(), (1.5 * 1024.0 * 1024.0 * 1024.0) as u64); + assert_eq!( + parse_memory_limit("4GB").expect("parsing \"4GB\" should succeed"), + 4 * 1024 * 1024 * 1024 + ); + assert_eq!( + parse_memory_limit("4G").expect("parsing \"4G\" should succeed"), + 4 * 1024 * 1024 * 1024 + ); + assert_eq!( + parse_memory_limit("512MB").expect("parsing \"512MB\" should succeed"), + 512 * 1024 * 1024 + ); + assert_eq!( + parse_memory_limit("512M").expect("parsing \"512M\" should succeed"), + 512 * 1024 * 1024 + ); + assert_eq!( + parse_memory_limit("1gb").expect("parsing \"1gb\" should succeed"), + 1024 * 1024 * 1024 + ); + assert_eq!( + parse_memory_limit(" 2GB ").expect("parsing \" 2GB \" should succeed"), + 2 * 1024 * 1024 * 1024 + ); + assert_eq!( + parse_memory_limit("1.5GB").expect("parsing \"1.5GB\" should succeed"), + (1.5 * 1024.0 * 1024.0 * 1024.0) as u64 + ); } #[test] diff --git a/src/lib/unified_pipeline/scheduler/balanced_chase.rs b/src/lib/unified_pipeline/scheduler/balanced_chase.rs index 08aca868d..c10b6efe6 100644 --- a/src/lib/unified_pipeline/scheduler/balanced_chase.rs +++ b/src/lib/unified_pipeline/scheduler/balanced_chase.rs @@ -252,8 +252,8 @@ mod tests { let serialize_pos = priorities.iter().position(|&s| s == PipelineStep::Serialize); // Both should be in top 3 - assert!(compress_pos.unwrap() < 3); - assert!(serialize_pos.unwrap() < 3); + assert!(compress_pos.expect("compress position should be Some") < 3); + assert!(serialize_pos.expect("serialize position should be Some") < 3); } #[test] diff --git a/src/lib/unified_pipeline/scheduler/balanced_chase_drain.rs b/src/lib/unified_pipeline/scheduler/balanced_chase_drain.rs index dffcacea3..4b6b02dae 100644 --- a/src/lib/unified_pipeline/scheduler/balanced_chase_drain.rs +++ b/src/lib/unified_pipeline/scheduler/balanced_chase_drain.rs @@ -359,7 +359,10 @@ mod tests { let compress_pos = priorities.iter().position(|&s| s == PipelineStep::Compress); let serialize_pos = priorities.iter().position(|&s| s == PipelineStep::Serialize); - assert!(compress_pos.unwrap() < serialize_pos.unwrap()); + assert!( + compress_pos.expect("compress position should be Some") + < serialize_pos.expect("serialize position should be Some") + ); } #[test] @@ -378,7 +381,10 @@ mod tests { let compress_pos = priorities.iter().position(|&s| s == PipelineStep::Compress); let serialize_pos = priorities.iter().position(|&s| s == PipelineStep::Serialize); - assert!(serialize_pos.unwrap() < compress_pos.unwrap()); + assert!( + serialize_pos.expect("serialize position should be Some") + < compress_pos.expect("compress position should be Some") + ); } #[rstest] @@ -441,7 +447,8 @@ mod tests { let compress_pos = priorities.iter().position(|&s| s == PipelineStep::Compress); let serialize_pos = priorities.iter().position(|&s| s == PipelineStep::Serialize); assert!( - compress_pos.unwrap() < serialize_pos.unwrap(), + compress_pos.expect("compress position should be Some") + < serialize_pos.expect("serialize position should be Some"), "Compress-preferred thread should try Compress before Serialize" ); } @@ -463,7 +470,10 @@ mod tests { let compress_pos = priorities.iter().position(|&s| s == PipelineStep::Compress); let serialize_pos = priorities.iter().position(|&s| s == PipelineStep::Serialize); - assert!(compress_pos.unwrap() < serialize_pos.unwrap()); + assert!( + compress_pos.expect("compress position should be Some") + < serialize_pos.expect("serialize position should be Some") + ); } #[test] @@ -482,6 +492,9 @@ mod tests { // Process should be early in the priority list (after exclusive role if any) let process_pos = priorities.iter().position(|&s| s == PipelineStep::Process); - assert!(process_pos.unwrap() <= 2, "Process should be prioritized when memory is high"); + assert!( + process_pos.expect("process position should be Some") <= 2, + "Process should be prioritized when memory is high" + ); } } diff --git a/src/lib/unified_pipeline/scheduler/mod.rs b/src/lib/unified_pipeline/scheduler/mod.rs index 1f6910d17..9af46a443 100644 --- a/src/lib/unified_pipeline/scheduler/mod.rs +++ b/src/lib/unified_pipeline/scheduler/mod.rs @@ -280,7 +280,9 @@ pub trait Scheduler: Send { // TN-1 → last exclusive step (if more than one) if thread_id == num_threads - 1 && exclusive.len() > 1 { - return Some(*exclusive.last().unwrap()); + return Some( + *exclusive.last().expect("exclusive is non-empty because len > 1 was checked"), + ); } // Interior exclusive steps: assign from both ends diff --git a/src/lib/unified_pipeline/scheduler/optimized_chase.rs b/src/lib/unified_pipeline/scheduler/optimized_chase.rs index 561453d5b..ebb5c533d 100644 --- a/src/lib/unified_pipeline/scheduler/optimized_chase.rs +++ b/src/lib/unified_pipeline/scheduler/optimized_chase.rs @@ -351,8 +351,8 @@ mod tests { let compress_pos = priorities.iter().position(|&s| s == PipelineStep::Compress); let serialize_pos = priorities.iter().position(|&s| s == PipelineStep::Serialize); - assert!(compress_pos.unwrap() < 4); - assert!(serialize_pos.unwrap() < 4); + assert!(compress_pos.expect("compress position should be Some") < 4); + assert!(serialize_pos.expect("serialize position should be Some") < 4); } #[test] diff --git a/src/lib/unified_pipeline/scheduler/ucb.rs b/src/lib/unified_pipeline/scheduler/ucb.rs index 3fc903568..83edb18f8 100644 --- a/src/lib/unified_pipeline/scheduler/ucb.rs +++ b/src/lib/unified_pipeline/scheduler/ucb.rs @@ -131,7 +131,10 @@ mod tests { // Unexplored steps should come first (infinite UCB) // Read should be later since it's well-explored - let read_pos = priorities.iter().position(|&s| s == PipelineStep::Read).unwrap(); + let read_pos = priorities + .iter() + .position(|&s| s == PipelineStep::Read) + .expect("PipelineStep::Read should be present in priorities"); assert!(read_pos > 0, "Well-explored Read should not be first"); } diff --git a/src/lib/validation.rs b/src/lib/validation.rs index ad839ce5c..7cd7bf614 100644 --- a/src/lib/validation.rs +++ b/src/lib/validation.rs @@ -290,8 +290,9 @@ mod tests { #[test] fn test_validate_file_exists_valid() { - let temp_file = NamedTempFile::new().unwrap(); - validate_file_exists(temp_file.path(), "Test file").unwrap(); + let temp_file = NamedTempFile::new().expect("creating temp file/dir should succeed"); + validate_file_exists(temp_file.path(), "Test file") + .expect("file validation should succeed"); } #[test] @@ -305,18 +306,18 @@ mod tests { #[test] fn test_validate_files_exist_all_valid() { - let temp1 = NamedTempFile::new().unwrap(); - let temp2 = NamedTempFile::new().unwrap(); + let temp1 = NamedTempFile::new().expect("creating temp file/dir should succeed"); + let temp2 = NamedTempFile::new().expect("creating temp file/dir should succeed"); let files = vec![(temp1.path().to_path_buf(), "File 1"), (temp2.path().to_path_buf(), "File 2")]; - validate_files_exist(&files).unwrap(); + validate_files_exist(&files).expect("file validation should succeed"); } #[test] fn test_validate_files_exist_one_invalid() { - let temp1 = NamedTempFile::new().unwrap(); + let temp1 = NamedTempFile::new().expect("creating temp file/dir should succeed"); let files = vec![ (temp1.path().to_path_buf(), "File 1"), @@ -345,7 +346,11 @@ mod tests { let result = validate_tag(input, "test tag"); if should_succeed { assert!(result.is_ok(), "Failed for: {description}"); - assert_eq!(result.unwrap(), expected.unwrap(), "Failed for: {description}"); + assert_eq!( + result.expect("result should be Ok"), + expected.expect("result should be Ok"), + "Failed for: {description}" + ); } else { assert!(result.is_err(), "Should have failed for: {description}"); let err_msg = result.unwrap_err().to_string(); @@ -373,7 +378,7 @@ mod tests { fn test_optional_string_to_tag_some_valid() -> Result<()> { let tag = optional_string_to_tag(Some("CB"), "cell tag")?; assert!(tag.is_some()); - assert_eq!(tag.unwrap(), Tag::from([b'C', b'B'])); + assert_eq!(tag.expect("tag should be valid"), Tag::from([b'C', b'B'])); Ok(()) } diff --git a/src/lib/variant_review.rs b/src/lib/variant_review.rs index a4b2870e0..041c090a7 100644 --- a/src/lib/variant_review.rs +++ b/src/lib/variant_review.rs @@ -317,7 +317,9 @@ mod tests { Header::builder() .add_reference_sequence( BString::from("chr1"), - Map::::new(NonZeroUsize::try_from(1000).unwrap()), + Map::::new( + NonZeroUsize::try_from(1000).expect("non-zero value 1000"), + ), ) .build() } diff --git a/src/lib/vendored/bam_codec/encoder/mod.rs b/src/lib/vendored/bam_codec/encoder/mod.rs index ca09e32a3..e8058f6b3 100644 --- a/src/lib/vendored/bam_codec/encoder/mod.rs +++ b/src/lib/vendored/bam_codec/encoder/mod.rs @@ -367,11 +367,13 @@ mod tests { let header = sam::Header::builder() .add_reference_sequence( "sq0", - Map::::new(const { NonZero::new(8).unwrap() }), + Map::::new(const { NonZero::new(8).expect("non-zero value 8") }), ) .add_reference_sequence( "sq1", - Map::::new(const { NonZero::new(13).unwrap() }), + Map::::new( + const { NonZero::new(13).expect("non-zero value 13") }, + ), ) .build(); @@ -435,7 +437,9 @@ mod tests { let header = sam::Header::builder() .add_reference_sequence( "sq0", - Map::::new(const { NonZero::new(131_072).unwrap() }), + Map::::new( + const { NonZero::new(131_072).expect("non-zero value 131_072") }, + ), ) .build(); diff --git a/src/lib/vendored/bam_codec/encoder/quality_scores.rs b/src/lib/vendored/bam_codec/encoder/quality_scores.rs index 7d184837f..2249931b1 100644 --- a/src/lib/vendored/bam_codec/encoder/quality_scores.rs +++ b/src/lib/vendored/bam_codec/encoder/quality_scores.rs @@ -129,17 +129,20 @@ mod tests { let mut buf = Vec::new(); // Valid scores - write_quality_scores_from_slice(&mut buf, 4, &[45, 35, 43, 50]).unwrap(); + write_quality_scores_from_slice(&mut buf, 4, &[45, 35, 43, 50]) + .expect("writing quality scores should succeed"); assert_eq!(buf, [45, 35, 43, 50]); // Empty scores (fill with 0xFF) buf.clear(); - write_quality_scores_from_slice(&mut buf, 3, &[]).unwrap(); + write_quality_scores_from_slice(&mut buf, 3, &[]) + .expect("writing quality scores should succeed"); assert_eq!(buf, [0xff, 0xff, 0xff]); // Zero base count with empty scores buf.clear(); - write_quality_scores_from_slice(&mut buf, 0, &[]).unwrap(); + write_quality_scores_from_slice(&mut buf, 0, &[]) + .expect("writing quality scores should succeed"); assert!(buf.is_empty()); // Invalid score (> 93) diff --git a/src/lib/vendored/bam_codec/encoder/reference_sequence_id.rs b/src/lib/vendored/bam_codec/encoder/reference_sequence_id.rs index d5f8e6cd6..19175d1ac 100644 --- a/src/lib/vendored/bam_codec/encoder/reference_sequence_id.rs +++ b/src/lib/vendored/bam_codec/encoder/reference_sequence_id.rs @@ -83,7 +83,7 @@ mod tests { let header = sam::Header::builder() .add_reference_sequence( "sq0", - Map::::new(const { NonZero::new(8).unwrap() }), + Map::::new(const { NonZero::new(8).expect("non-zero value 8") }), ) .build(); let reference_sequence_id = Some(0); diff --git a/src/lib/vendored/bam_codec/encoder/sequence.rs b/src/lib/vendored/bam_codec/encoder/sequence.rs index 7649b3b06..b3fc20be0 100644 --- a/src/lib/vendored/bam_codec/encoder/sequence.rs +++ b/src/lib/vendored/bam_codec/encoder/sequence.rs @@ -219,22 +219,22 @@ mod tests { let mut buf = Vec::new(); // Empty sequence - write_sequence_from_slice(&mut buf, 0, b"").unwrap(); + write_sequence_from_slice(&mut buf, 0, b"").expect("writing sequence should succeed"); assert!(buf.is_empty()); // Even length (4 bases -> 2 bytes) buf.clear(); - write_sequence_from_slice(&mut buf, 4, b"ACGT").unwrap(); + write_sequence_from_slice(&mut buf, 4, b"ACGT").expect("writing sequence should succeed"); assert_eq!(buf, [0x12, 0x48]); // A=1, C=2, G=4, T=8 // Odd length (3 bases -> 2 bytes, last nibble padded) buf.clear(); - write_sequence_from_slice(&mut buf, 3, b"ACG").unwrap(); + write_sequence_from_slice(&mut buf, 3, b"ACG").expect("writing sequence should succeed"); assert_eq!(buf, [0x12, 0x40]); // A=1, C=2, G=4, pad=0 // Single base buf.clear(); - write_sequence_from_slice(&mut buf, 1, b"A").unwrap(); + write_sequence_from_slice(&mut buf, 1, b"A").expect("writing sequence should succeed"); assert_eq!(buf, [0x10]); // A=1, pad=0 // Length mismatch error @@ -243,9 +243,10 @@ mod tests { // Verify matches trait-based version buf.clear(); - write_sequence_from_slice(&mut buf, 4, b"ACGT").unwrap(); + write_sequence_from_slice(&mut buf, 4, b"ACGT").expect("writing sequence should succeed"); let mut buf2 = Vec::new(); - write_sequence(&mut buf2, 4, &SequenceBuf::from(b"ACGT")).unwrap(); + write_sequence(&mut buf2, 4, &SequenceBuf::from(b"ACGT")) + .expect("writing sequence should succeed"); assert_eq!(buf, buf2); } } diff --git a/src/lib/vendored/bgzf_multithreaded.rs b/src/lib/vendored/bgzf_multithreaded.rs index bfae32c69..73497f309 100644 --- a/src/lib/vendored/bgzf_multithreaded.rs +++ b/src/lib/vendored/bgzf_multithreaded.rs @@ -385,7 +385,8 @@ pub struct Builder { impl Default for Builder { fn default() -> Self { Self { - compression_level: CompressionLevel::new(6).unwrap(), + compression_level: CompressionLevel::new(6) + .expect("compression level 6 is always valid"), worker_count: NonZero::::MIN, blocksize: BGZF_BLOCK_SIZE, } @@ -530,26 +531,35 @@ mod tests { #[test] fn test_new_and_finish() { - let mut writer = MultithreadedWriter::new(Vec::new(), CompressionLevel::new(6).unwrap()); + let mut writer = MultithreadedWriter::new( + Vec::new(), + CompressionLevel::new(6).expect("valid compression level 6"), + ); let result = writer.finish(); assert!(result.is_ok()); } #[test] fn test_roundtrip() { - let mut writer = MultithreadedWriter::new(Vec::new(), CompressionLevel::new(6).unwrap()); - writer.write_all(b"hello world").unwrap(); - let data = writer.finish().unwrap(); + let mut writer = MultithreadedWriter::new( + Vec::new(), + CompressionLevel::new(6).expect("valid compression level 6"), + ); + writer.write_all(b"hello world").expect("write_all should succeed"); + let data = writer.finish().expect("finish should succeed"); let mut reader = Reader::new(&data[..]); let mut buf = Vec::new(); - reader.read_to_end(&mut buf).unwrap(); + reader.read_to_end(&mut buf).expect("read_to_end should succeed"); assert_eq!(buf, b"hello world"); } #[test] fn test_position_tracking_initial() { - let writer = MultithreadedWriter::new(Vec::new(), CompressionLevel::new(6).unwrap()); + let writer = MultithreadedWriter::new( + Vec::new(), + CompressionLevel::new(6).expect("valid compression level 6"), + ); assert_eq!(writer.position(), 0); assert_eq!(writer.current_block_number(), 0); assert_eq!(writer.blocks_written(), 0); @@ -558,34 +568,40 @@ mod tests { #[test] fn test_position_tracking_after_writes() { - let mut writer = MultithreadedWriter::new(Vec::new(), CompressionLevel::new(6).unwrap()); + let mut writer = MultithreadedWriter::new( + Vec::new(), + CompressionLevel::new(6).expect("valid compression level 6"), + ); - writer.write_all(b"hello").unwrap(); + writer.write_all(b"hello").expect("write_all should succeed"); assert_eq!(writer.buffer_offset(), 5); - writer.flush().unwrap(); + writer.flush().expect("flush should succeed"); assert_eq!(writer.current_block_number(), 1); assert_eq!(writer.buffer_offset(), 0); - writer.finish().unwrap(); + writer.finish().expect("finish should succeed"); } #[test] fn test_block_info_notifications() { let mut writer = MultithreadedWriter::with_worker_count( - NonZero::new(2).unwrap(), + NonZero::new(2).expect("non-zero value 2"), Vec::new(), - CompressionLevel::new(6).unwrap(), + CompressionLevel::new(6).expect("valid compression level 6"), ); - let rx = writer.block_info_receiver().unwrap().clone(); + let rx = writer + .block_info_receiver() + .expect("block_info_receiver should return a receiver") + .clone(); - writer.write_all(b"block1").unwrap(); - writer.flush().unwrap(); - writer.write_all(b"block2").unwrap(); - writer.flush().unwrap(); + writer.write_all(b"block1").expect("write_all should succeed"); + writer.flush().expect("flush should succeed"); + writer.write_all(b"block2").expect("write_all should succeed"); + writer.flush().expect("flush should succeed"); - writer.finish().unwrap(); + writer.finish().expect("finish should succeed"); let infos: Vec<_> = rx.try_iter().collect(); assert_eq!(infos.len(), 2); @@ -598,21 +614,21 @@ mod tests { fn test_multiple_workers() { for worker_count in [1, 2, 4] { let mut writer = MultithreadedWriter::with_worker_count( - NonZero::new(worker_count).unwrap(), + NonZero::new(worker_count).expect("worker_count is non-zero"), Vec::new(), - CompressionLevel::new(6).unwrap(), + CompressionLevel::new(6).expect("valid compression level 6"), ); for i in 0..20 { - writer.write_all(format!("block{i}").as_bytes()).unwrap(); - writer.flush().unwrap(); + writer.write_all(format!("block{i}").as_bytes()).expect("write_all should succeed"); + writer.flush().expect("flush should succeed"); } - let data = writer.finish().unwrap(); + let data = writer.finish().expect("finish should succeed"); let mut reader = Reader::new(&data[..]); let mut buf = String::new(); - reader.read_to_string(&mut buf).unwrap(); + reader.read_to_string(&mut buf).expect("read_to_string should succeed"); for i in 0..20 { assert!(buf.contains(&format!("block{i}"))); @@ -623,28 +639,31 @@ mod tests { #[test] fn test_large_data() { let mut writer = MultithreadedWriter::with_worker_count( - NonZero::new(4).unwrap(), + NonZero::new(4).expect("non-zero value 4"), Vec::new(), - CompressionLevel::new(6).unwrap(), + CompressionLevel::new(6).expect("valid compression level 6"), ); let large_data = vec![b'A'; BGZF_BLOCK_SIZE * 5]; - writer.write_all(&large_data).unwrap(); + writer.write_all(&large_data).expect("write_all should succeed"); - let compressed = writer.finish().unwrap(); + let compressed = writer.finish().expect("finish should succeed"); let mut reader = Reader::new(&compressed[..]); let mut decompressed = Vec::new(); - reader.read_to_end(&mut decompressed).unwrap(); + reader.read_to_end(&mut decompressed).expect("read_to_end should succeed"); assert_eq!(decompressed, large_data); } #[test] fn test_eof_marker() { - let mut writer = MultithreadedWriter::new(Vec::new(), CompressionLevel::new(6).unwrap()); - writer.write_all(b"test").unwrap(); - let data = writer.finish().unwrap(); + let mut writer = MultithreadedWriter::new( + Vec::new(), + CompressionLevel::new(6).expect("valid compression level 6"), + ); + writer.write_all(b"test").expect("write_all should succeed"); + let data = writer.finish().expect("finish should succeed"); assert!(data.ends_with(BGZF_EOF)); } @@ -652,8 +671,8 @@ mod tests { #[test] fn test_builder() { let writer = Builder::default() - .set_compression_level(CompressionLevel::new(9).unwrap()) - .set_worker_count(NonZero::new(4).unwrap()) + .set_compression_level(CompressionLevel::new(9).expect("valid compression level 9")) + .set_worker_count(NonZero::new(4).expect("non-zero value 4")) .set_blocksize(32768) .build_from_writer(Vec::new());