Skip to content

refactor: replace all unwrap() calls in src/lib/ with expect() #226

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
nh13 merged 1 commit into from
Apr 4, 2026
Merged

refactor: replace all unwrap() calls in src/lib/ with expect() #226

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 30 additions & 2 deletions src/commands/simulate/common.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,15 +48,24 @@ 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)
#[arg(long = "r2-quality-offset", default_value = "-2", allow_hyphen_values = true)]
pub r2_quality_offset: i8,
}

/// Parse and validate a noise standard deviation value for quality score simulation.
fn parse_noise_stddev(s: &str) -> Result<f64, String> {
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 {
Expand Down Expand Up @@ -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 {
Expand Down
15 changes: 9 additions & 6 deletions src/lib/bam_io.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -354,7 +354,10 @@ pub struct IndexingBamWriter {
impl IndexingBamWriter {
/// Create a new indexing BAM writer.
fn new(inner: MultithreadedWriter<File>, 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(),
Expand Down Expand Up @@ -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]);
}
Expand All @@ -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]);
}
Expand All @@ -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]);
}
Expand Down Expand Up @@ -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());

Expand Down Expand Up @@ -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"));
}
}
40 changes: 20 additions & 20 deletions src/lib/batched_sam_reader.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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());
}

Expand All @@ -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;
Expand Down Expand Up @@ -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
Expand All @@ -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;
}
}
Expand Down Expand Up @@ -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();

Expand All @@ -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;
}
}
Expand Down Expand Up @@ -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
Expand All @@ -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;
}
}
Expand All @@ -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!(
Expand All @@ -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());
}

Expand All @@ -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;

Expand Down Expand Up @@ -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!");
}
Expand All @@ -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");
}

Expand All @@ -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");
}

Expand All @@ -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);
Expand All @@ -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);
Expand All @@ -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);
}
}
53 changes: 40 additions & 13 deletions src/lib/fastq.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand All @@ -550,13 +561,15 @@ mod tests {
let reader = SimdFastqReader::new(Box::new(cursor) as Box<dyn BufRead + Send>);

// 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());
Expand All @@ -583,14 +596,16 @@ mod tests {
let reader = SimdFastqReader::new(Box::new(cursor) as Box<dyn BufRead + Send>);

// 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));
Expand All @@ -606,7 +621,8 @@ mod tests {
let reader = SimdFastqReader::new(Box::new(cursor) as Box<dyn BufRead + Send>);

// 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
Expand All @@ -624,17 +640,24 @@ mod tests {
let cursor = Cursor::new(fastq_data.to_vec());
let reader = SimdFastqReader::new(Box::new(cursor) as Box<dyn BufRead + Send>);

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");
Expand All @@ -653,10 +676,14 @@ mod tests {
let reader = SimdFastqReader::new(Box::new(cursor) as Box<dyn BufRead + Send>);

// 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
Expand Down
Loading
Loading