From 15e85b7789e8b74914c9bb9887809e144414421f Mon Sep 17 00:00:00 2001 From: noamteyssier <22600644+noamteyssier@users.noreply.github.com> Date: Mon, 6 Jul 2026 17:52:39 -0700 Subject: [PATCH 01/21] tests: improve coverage throughout --- src/bq/header.rs | 136 +++++++++++ src/bq/reader.rs | 199 ++++++++++++++++ src/bq/writer.rs | 315 +++++++++++++++++++++++++ src/cbq/core/block.rs | 459 ++++++++++++++++++++++++++++++++++++ src/record/binseq_record.rs | 155 ++++++++++++ 5 files changed, 1264 insertions(+) diff --git a/src/bq/header.rs b/src/bq/header.rs index 42b845e..61adf24 100644 --- a/src/bq/header.rs +++ b/src/bq/header.rs @@ -338,3 +338,139 @@ impl FileHeader { Self::from_bytes(&buffer) } } + +#[cfg(test)] +mod tests { + use super::*; + + // ==================== FileHeaderBuilder Tests ==================== + + #[test] + fn test_builder_default() { + let builder = FileHeaderBuilder::default(); + // Missing slen should fail to build + assert!(builder.build().is_err()); + } + + #[test] + fn test_builder_bitsize() { + let header = FileHeaderBuilder::new() + .slen(64) + .bitsize(BitSize::Four) + .build() + .unwrap(); + assert_eq!(header.bits, BitSize::Four); + } + + #[test] + fn test_builder_missing_slen() { + let result = FileHeaderBuilder::new().xlen(10).build(); + assert!(result.is_err()); + } + + // ==================== FileHeader Constructor Tests ==================== + + #[test] + fn test_header_new() { + let header = FileHeader::new(BitSize::Two, 100, true); + assert_eq!(header.slen, 100); + assert_eq!(header.xlen, 0); + assert!(header.flags); + assert!(!header.is_paired()); + } + + #[test] + fn test_header_new_extended() { + let header = FileHeader::new_extended(BitSize::Four, 100, 50, false); + assert_eq!(header.slen, 100); + assert_eq!(header.xlen, 50); + assert_eq!(header.bits, BitSize::Four); + assert!(header.is_paired()); + } + + #[test] + fn test_set_bitsize() { + let mut header = FileHeader::new(BitSize::Two, 100, false); + header.set_bitsize(BitSize::Four); + assert_eq!(header.bits, BitSize::Four); + } + + // ==================== from_bytes Tests ==================== + + #[test] + fn test_from_bytes_invalid_format_version() { + let header = FileHeader::new(BitSize::Two, 32, false); + let mut buffer = [0u8; SIZE_HEADER]; + let mut cursor = std::io::Cursor::new(&mut buffer[..]); + header.write_bytes(&mut cursor).unwrap(); + buffer[4] = 99; // corrupt format version + let result = FileHeader::from_bytes(&buffer); + assert!(result.is_err()); + } + + #[test] + fn test_from_bytes_four_bit_size() { + let header = FileHeader::new(BitSize::Four, 32, false); + let mut buffer = [0u8; SIZE_HEADER]; + let mut cursor = std::io::Cursor::new(&mut buffer[..]); + header.write_bytes(&mut cursor).unwrap(); + let parsed = FileHeader::from_bytes(&buffer).unwrap(); + assert_eq!(parsed.bits, BitSize::Four); + } + + #[test] + fn test_from_bytes_invalid_bitsize() { + let header = FileHeader::new(BitSize::Two, 32, false); + let mut buffer = [0u8; SIZE_HEADER]; + let mut cursor = std::io::Cursor::new(&mut buffer[..]); + header.write_bytes(&mut cursor).unwrap(); + buffer[13] = 99; // corrupt bitsize byte + let result = FileHeader::from_bytes(&buffer); + assert!(result.is_err()); + } + + #[test] + fn test_from_bytes_invalid_magic() { + let buffer = [0u8; SIZE_HEADER]; + let result = FileHeader::from_bytes(&buffer); + assert!(result.is_err()); + } + + // ==================== from_buffer Tests ==================== + + #[test] + fn test_from_buffer_too_small() { + let buffer = [0u8; 10]; + let result = FileHeader::from_buffer(&buffer); + assert!(result.is_err()); + } + + #[test] + fn test_from_buffer_valid() { + let header = FileHeader::new(BitSize::Two, 32, false); + let mut buffer = Vec::new(); + header.write_bytes(&mut buffer).unwrap(); + buffer.extend_from_slice(&[0u8; 16]); // trailing data beyond header + let parsed = FileHeader::from_buffer(&buffer).unwrap(); + assert_eq!(parsed.slen, 32); + } + + // ==================== from_reader Tests ==================== + + #[test] + fn test_from_reader_valid() { + let header = FileHeader::new_extended(BitSize::Two, 32, 16, true); + let mut buffer = Vec::new(); + header.write_bytes(&mut buffer).unwrap(); + let mut cursor = std::io::Cursor::new(buffer); + let parsed = FileHeader::from_reader(&mut cursor).unwrap(); + assert_eq!(parsed, header); + } + + #[test] + fn test_from_reader_truncated() { + let mut cursor = std::io::Cursor::new(vec![0u8; 5]); + let result = FileHeader::from_reader(&mut cursor); + assert!(result.is_err()); + } +} diff --git a/src/bq/reader.rs b/src/bq/reader.rs index 365b17f..ad2d11b 100644 --- a/src/bq/reader.rs +++ b/src/bq/reader.rs @@ -1312,4 +1312,203 @@ mod tests { } } } + + // ==================== get_buffer_slice Tests ==================== + + #[test] + fn test_get_buffer_slice_valid() { + let reader = MmapReader::new(TEST_BQ_FILE).unwrap(); + let num_records = reader.num_records().min(10); + let slice = reader.get_buffer_slice(0..num_records); + assert!(slice.is_ok()); + assert_eq!( + slice.unwrap().len(), + num_records * reader.config.record_size_u64() + ); + } + + #[test] + fn test_get_buffer_slice_out_of_range() { + let reader = MmapReader::new(TEST_BQ_FILE).unwrap(); + let num_records = reader.num_records(); + let slice = reader.get_buffer_slice(0..(num_records + 100)); + assert!(slice.is_err()); + } + + // ==================== MmapReader Error Path Tests ==================== + + #[test] + fn test_mmap_reader_directory_is_incompatible() { + // Directories cannot be memory-mapped as regular files + let result = MmapReader::new("./data"); + assert!(result.is_err(), "Should fail when given a directory"); + } + + #[test] + fn test_mmap_reader_truncated_file() { + use std::io::Write as _; + + let path = "test_truncated_reader.bq"; + { + let header = crate::bq::FileHeaderBuilder::new().slen(64).build().unwrap(); + let mut file = std::fs::File::create(path).unwrap(); + header.write_bytes(&mut file).unwrap(); + // Write a partial record (not a full multiple of the record size) + file.write_all(&[0u8; 4]).unwrap(); + } + + let result = MmapReader::new(path); + assert!(result.is_err(), "Should fail on truncated record data"); + + std::fs::remove_file(path).unwrap(); + } + + // ==================== StreamReader Tests ==================== + + fn build_stream_bytes(paired: bool) -> Vec { + use crate::SequencingRecordBuilder; + use crate::bq::{FileHeaderBuilder, WriterBuilder}; + + let header = if paired { + FileHeaderBuilder::new().slen(64).xlen(32).build().unwrap() + } else { + FileHeaderBuilder::new().slen(64).build().unwrap() + }; + + let mut writer = WriterBuilder::default() + .header(header) + .build(Vec::new()) + .unwrap(); + + for i in 0..5 { + let s_seq = vec![b"ACGT"[i % 4]; 64]; + let x_seq = vec![b"TGCA"[i % 4]; 32]; + let record = if paired { + SequencingRecordBuilder::default() + .s_seq(&s_seq) + .x_seq(&x_seq) + .build() + .unwrap() + } else { + SequencingRecordBuilder::default() + .s_seq(&s_seq) + .build() + .unwrap() + }; + writer.push(record).unwrap(); + } + writer.flush().unwrap(); + writer.into_inner() + } + + #[test] + fn test_stream_reader_new_and_with_capacity() { + let data = build_stream_bytes(false); + let cursor = std::io::Cursor::new(data.clone()); + let _reader = StreamReader::new(cursor); + + let cursor = std::io::Cursor::new(data); + let _reader = StreamReader::with_capacity(cursor, 64); + } + + #[test] + fn test_stream_reader_read_header() { + let data = build_stream_bytes(false); + let mut reader = StreamReader::new(std::io::Cursor::new(data)); + let header = reader.read_header().unwrap(); + assert_eq!(header.slen, 64); + + // Second call should hit the cached path + let header_again = reader.read_header().unwrap(); + assert_eq!(header_again.slen, 64); + } + + #[test] + fn test_stream_reader_next_record_unpaired() { + let data = build_stream_bytes(false); + let mmap_reader = { + let path = "test_stream_reader_compare.bq"; + std::fs::write(path, &data).unwrap(); + let reader = MmapReader::new(path).unwrap(); + std::fs::remove_file(path).unwrap(); + reader + }; + + let mut reader = StreamReader::new(std::io::Cursor::new(data)); + let mut count = 0; + while let Some(record) = reader.next_record() { + let record = record.unwrap(); + let expected = mmap_reader.get(count).unwrap(); + assert_eq!(record.index(), expected.index()); + assert_eq!(record.sbuf(), expected.sbuf()); + count += 1; + } + assert_eq!(count, mmap_reader.num_records()); + } + + #[test] + fn test_stream_reader_next_record_paired() { + let data = build_stream_bytes(true); + let mut reader = StreamReader::new(std::io::Cursor::new(data)); + let mut count = 0; + while let Some(record) = reader.next_record() { + let record = record.unwrap(); + assert!(record.is_paired()); + count += 1; + } + assert_eq!(count, 5); + } + + #[test] + #[should_panic(expected = "attempt to subtract with overflow")] + fn test_stream_reader_small_buffer_forces_multiple_fills() { + // Known bug (tracked separately, not fixed here): once a tiny buffer + // capacity forces `fill_buffer` to shift remaining bytes to the start + // of the internal buffer (reader.rs:716-719), `buffer_pos` no longer + // reflects the absolute stream offset. `next_record`'s id calculation + // (reader.rs:801) assumes it still does, and underflows. This test + // pins current (buggy) behavior so it fails loudly once fixed -- + // at which point it should be rewritten to assert sequential ids + // across the fill boundary instead. + let data = build_stream_bytes(false); + let mut reader = StreamReader::with_capacity(std::io::Cursor::new(data), 40); + while let Some(record) = reader.next_record() { + record.unwrap(); + } + } + + #[test] + fn test_stream_reader_partial_record_error() { + let mut data = build_stream_bytes(false); + // Truncate the data in the middle of the last record + data.truncate(data.len() - 4); + let mut reader = StreamReader::new(std::io::Cursor::new(data)); + + let mut saw_error = false; + while let Some(record) = reader.next_record() { + if record.is_err() { + saw_error = true; + break; + } + } + assert!(saw_error, "Expected a partial record error"); + } + + #[test] + fn test_stream_reader_set_default_quality_score() { + let data = build_stream_bytes(false); + let mut reader = StreamReader::new(std::io::Cursor::new(data)); + reader.set_default_quality_score(42); + if let Some(Ok(record)) = reader.next_record() { + assert!(record.squal().iter().all(|&q| q == 42)); + } + } + + #[test] + fn test_stream_reader_into_inner() { + let data = build_stream_bytes(false); + let reader = StreamReader::new(std::io::Cursor::new(data.clone())); + let cursor = reader.into_inner(); + assert_eq!(cursor.into_inner(), data); + } } diff --git a/src/bq/writer.rs b/src/bq/writer.rs index 084d215..89e9946 100644 --- a/src/bq/writer.rs +++ b/src/bq/writer.rs @@ -794,6 +794,7 @@ mod testing { use std::{fs::File, io::BufWriter}; use super::*; + use crate::SequencingRecordBuilder; use crate::bq::{FileHeaderBuilder, SIZE_HEADER}; #[test] @@ -860,4 +861,318 @@ mod testing { assert_eq!(inner.len(), SIZE_HEADER); Ok(()) } + + // ==================== Encoder Tests ==================== + + #[test] + fn test_encoder_new() { + let header = FileHeaderBuilder::new().slen(8).build().unwrap(); + let encoder = Encoder::new(header); + assert!(matches!(encoder.policy, Policy::IgnoreSequence)); + } + + #[test] + fn test_encoder_encode_single_wrong_length() { + let header = FileHeaderBuilder::new().slen(8).build().unwrap(); + let mut encoder = Encoder::new(header); + let result = encoder.encode_single(b"ACGT"); + assert!(result.is_err()); + } + + #[test] + fn test_encoder_encode_single_invalid_ignored() { + let header = FileHeaderBuilder::new().slen(8).build().unwrap(); + let mut encoder = Encoder::with_policy(header, Policy::IgnoreSequence); + let result = encoder.encode_single(b"ACGTNNNN").unwrap(); + assert!(result.is_none()); + } + + #[test] + fn test_encoder_encode_single_invalid_corrected() { + let header = FileHeaderBuilder::new().slen(8).build().unwrap(); + let mut encoder = Encoder::with_policy(header, Policy::SetToA); + let result = encoder.encode_single(b"ACGTNNNN").unwrap(); + assert!(result.is_some()); + } + + #[test] + fn test_encoder_encode_paired_wrong_primary_length() { + let header = FileHeaderBuilder::new().slen(8).xlen(8).build().unwrap(); + let mut encoder = Encoder::new(header); + let result = encoder.encode_paired(b"ACGT", b"ACGTACGT"); + assert!(result.is_err()); + } + + #[test] + fn test_encoder_encode_paired_wrong_extended_length() { + let header = FileHeaderBuilder::new().slen(8).xlen(8).build().unwrap(); + let mut encoder = Encoder::new(header); + let result = encoder.encode_paired(b"ACGTACGT", b"ACGT"); + assert!(result.is_err()); + } + + #[test] + fn test_encoder_encode_paired_invalid_ignored() { + let header = FileHeaderBuilder::new().slen(8).xlen(8).build().unwrap(); + let mut encoder = Encoder::with_policy(header, Policy::IgnoreSequence); + let result = encoder + .encode_paired(b"ACGTNNNN", b"ACGTACGT") + .unwrap(); + assert!(result.is_none()); + } + + #[test] + fn test_encoder_encode_paired_invalid_corrected() { + let header = FileHeaderBuilder::new().slen(8).xlen(8).build().unwrap(); + let mut encoder = Encoder::with_policy(header, Policy::SetToA); + let result = encoder + .encode_paired(b"ACGTNNNN", b"NNNNACGT") + .unwrap(); + assert!(result.is_some()); + } + + // ==================== WriterBuilder Tests ==================== + + #[test] + fn test_writer_builder_missing_header() { + let result = WriterBuilder::default().build(Vec::new()); + assert!(result.is_err()); + } + + // ==================== Deprecated Writer Methods ==================== + + #[test] + #[allow(deprecated)] + fn test_write_record_deprecated() -> Result<()> { + let mut writer = WriterBuilder::default() + .header(FileHeaderBuilder::new().slen(8).build()?) + .build(Vec::new())?; + let wrote = writer.write_record(None, b"ACGTACGT")?; + assert!(wrote); + Ok(()) + } + + #[test] + #[allow(deprecated)] + fn test_write_record_deprecated_skipped() -> Result<()> { + let mut writer = WriterBuilder::default() + .header(FileHeaderBuilder::new().slen(8).build()?) + .build(Vec::new())?; + let wrote = writer.write_record(None, b"NNNNNNNN")?; + assert!(!wrote); + Ok(()) + } + + #[test] + #[allow(deprecated)] + fn test_write_paired_record_deprecated() -> Result<()> { + let mut writer = WriterBuilder::default() + .header(FileHeaderBuilder::new().slen(8).xlen(8).flags(true).build()?) + .build(Vec::new())?; + let wrote = writer.write_paired_record(Some(5), b"ACGTACGT", b"TTGGCCAA")?; + assert!(wrote); + Ok(()) + } + + // ==================== push() Tests ==================== + + #[test] + fn test_push_single() -> Result<()> { + let mut writer = WriterBuilder::default() + .header(FileHeaderBuilder::new().slen(8).build()?) + .build(Vec::new())?; + let record = SequencingRecordBuilder::default() + .s_seq(b"ACGTACGT") + .build()?; + assert!(writer.push(record)?); + Ok(()) + } + + #[test] + fn test_push_single_invalid_skipped() -> Result<()> { + let mut writer = WriterBuilder::default() + .header(FileHeaderBuilder::new().slen(8).build()?) + .build(Vec::new())?; + let record = SequencingRecordBuilder::default() + .s_seq(b"NNNNNNNN") + .build()?; + assert!(!writer.push(record)?); + Ok(()) + } + + #[test] + fn test_push_paired() -> Result<()> { + let mut writer = WriterBuilder::default() + .header(FileHeaderBuilder::new().slen(8).xlen(8).build()?) + .build(Vec::new())?; + let record = SequencingRecordBuilder::default() + .s_seq(b"ACGTACGT") + .x_seq(b"TTGGCCAA") + .build()?; + assert!(writer.push(record)?); + Ok(()) + } + + #[test] + fn test_push_paired_invalid_skipped() -> Result<()> { + let mut writer = WriterBuilder::default() + .header(FileHeaderBuilder::new().slen(8).xlen(8).build()?) + .build(Vec::new())?; + let record = SequencingRecordBuilder::default() + .s_seq(b"NNNNNNNN") + .x_seq(b"TTGGCCAA") + .build()?; + assert!(!writer.push(record)?); + Ok(()) + } + + #[test] + fn test_push_paired_mismatch() -> Result<()> { + let mut writer = WriterBuilder::default() + .header(FileHeaderBuilder::new().slen(8).xlen(8).build()?) + .build(Vec::new())?; + // Writer expects paired records but record has no x_seq + let record = SequencingRecordBuilder::default() + .s_seq(b"ACGTACGT") + .build()?; + let result = writer.push(record); + assert!(result.is_err()); + Ok(()) + } + + #[test] + fn test_push_with_flag() -> Result<()> { + let mut writer = WriterBuilder::default() + .header(FileHeaderBuilder::new().slen(8).flags(true).build()?) + .build(Vec::new())?; + let record = SequencingRecordBuilder::default() + .s_seq(b"ACGTACGT") + .flag(99) + .build()?; + assert!(writer.push(record)?); + Ok(()) + } + + // ==================== Writer Misc Tests ==================== + + #[test] + fn test_new_encoder() -> Result<()> { + let writer = WriterBuilder::default() + .header(FileHeaderBuilder::new().slen(8).build()?) + .build(Vec::new())?; + let encoder = writer.new_encoder(); + assert!(encoder.sbuffer.is_empty()); + Ok(()) + } + + #[test] + fn test_writer_flush() -> Result<()> { + let mut writer = WriterBuilder::default() + .header(FileHeaderBuilder::new().slen(8).build()?) + .build(Vec::new())?; + writer.flush()?; + Ok(()) + } + + #[test] + fn test_writer_ingest() -> Result<()> { + let mut main_writer = WriterBuilder::default() + .header(FileHeaderBuilder::new().slen(8).build()?) + .build(Vec::new())?; + + let header = main_writer.header(); + let mut other_writer = WriterBuilder::default() + .header(header) + .headless(true) + .build(Vec::new())?; + + let record = SequencingRecordBuilder::default() + .s_seq(b"ACGTACGT") + .build()?; + other_writer.push(record)?; + + main_writer.ingest(&mut other_writer)?; + assert!(other_writer.by_ref().is_empty()); + assert_eq!(main_writer.by_ref().len(), SIZE_HEADER + 8); + Ok(()) + } + + #[test] + fn test_writer_policy_accessor() -> Result<()> { + let writer = WriterBuilder::default() + .header(FileHeaderBuilder::new().slen(8).build()?) + .policy(Policy::SetToA) + .build(Vec::new())?; + assert!(matches!(writer.policy(), Policy::SetToA)); + Ok(()) + } + + // ==================== StreamWriter Tests ==================== + + #[test] + fn test_stream_writer_new() -> Result<()> { + let writer = StreamWriter::new( + Vec::new(), + FileHeaderBuilder::new().slen(8).build()?, + Policy::default(), + false, + )?; + let inner = writer.into_inner()?; + assert_eq!(inner.len(), SIZE_HEADER); + Ok(()) + } + + #[test] + #[allow(deprecated)] + fn test_stream_writer_deprecated_methods() -> Result<()> { + let mut writer = StreamWriter::new( + Vec::new(), + FileHeaderBuilder::new().slen(8).xlen(8).build()?, + Policy::default(), + false, + )?; + assert!(writer.write_record(None, b"ACGTACGT")?); + assert!(writer.write_paired_record(None, b"ACGTACGT", b"TTGGCCAA")?); + writer.flush()?; + Ok(()) + } + + #[test] + fn test_stream_writer_push() -> Result<()> { + let mut writer = StreamWriter::new( + Vec::new(), + FileHeaderBuilder::new().slen(8).build()?, + Policy::default(), + false, + )?; + let record = SequencingRecordBuilder::default() + .s_seq(b"ACGTACGT") + .build()?; + assert!(writer.push(record)?); + writer.flush()?; + let inner = writer.into_inner()?; + assert_eq!(inner.len(), SIZE_HEADER + 8); + Ok(()) + } + + // ==================== StreamWriterBuilder Tests ==================== + + #[test] + fn test_stream_writer_builder_missing_header() { + let result = StreamWriterBuilder::default().build(Vec::new()); + assert!(result.is_err()); + } + + #[test] + fn test_stream_writer_builder_with_policy_and_headless() -> Result<()> { + let inner = Vec::new(); + let writer = StreamWriterBuilder::default() + .header(FileHeaderBuilder::new().slen(8).build()?) + .policy(Policy::SetToA) + .headless(true) + .build(inner)?; + let inner = writer.into_inner()?; + assert!(inner.is_empty()); + Ok(()) + } } diff --git a/src/cbq/core/block.rs b/src/cbq/core/block.rs index fbfe4fc..bfd4952 100644 --- a/src/cbq/core/block.rs +++ b/src/cbq/core/block.rs @@ -964,3 +964,462 @@ impl BinseqRecord for RefRecord<'_> { } } } + +#[cfg(test)] +mod tests { + use super::*; + use crate::Error; + use crate::SequencingRecordBuilder; + use crate::cbq::core::FileHeaderBuilder; + use zstd::zstd_safe; + + fn full_header(block_size: usize) -> FileHeader { + FileHeaderBuilder::default() + .is_paired(true) + .with_headers(true) + .with_qualities(true) + .with_flags(true) + .with_block_size(block_size) + .build() + } + + fn unpaired_header(block_size: usize) -> FileHeader { + FileHeaderBuilder::default() + .is_paired(false) + .with_headers(false) + .with_qualities(false) + .with_flags(false) + .with_block_size(block_size) + .build() + } + + fn full_record<'a>( + s_seq: &'a [u8], + x_seq: &'a [u8], + s_qual: &'a [u8], + x_qual: &'a [u8], + ) -> SequencingRecord<'a> { + SequencingRecordBuilder::default() + .s_seq(s_seq) + .x_seq(x_seq) + .s_header(b"read1") + .x_header(b"read2") + .s_qual(s_qual) + .x_qual(x_qual) + .flag(42) + .build() + .unwrap() + } + + // ==================== push()/validate_record() error paths ==================== + + #[test] + fn test_push_record_size_exceeds_block() { + let mut block = ColumnarBlock::new(unpaired_header(4)); + let record = SequencingRecordBuilder::default() + .s_seq(b"ACGTACGTACGT") + .build() + .unwrap(); + let result = block.push(record); + assert!(matches!( + result, + Err(Error::WriteError( + WriteError::RecordSizeExceedsMaximumBlockSize(_, _) + )) + )); + } + + #[test] + fn test_push_block_full() { + // Block fits exactly one 8-byte-encoded record (32bp -> 1 word == 8 bytes) + let mut block = ColumnarBlock::new(unpaired_header(8)); + let seq = vec![b'A'; 32]; + block.push(SequencingRecordBuilder::default().s_seq(&seq).build().unwrap()) + .unwrap(); + + let result = block.push(SequencingRecordBuilder::default().s_seq(&seq).build().unwrap()); + assert!(matches!( + result, + Err(Error::CbqError(CbqError::BlockFull { .. })) + )); + } + + #[test] + fn test_push_paired_mismatch() { + let mut block = ColumnarBlock::new(full_header(1 << 16)); + // Full header requires paired, flags, headers, and qualities but this + // record is missing all of them. + let record = SequencingRecordBuilder::default() + .s_seq(b"ACGT") + .build() + .unwrap(); + let result = block.push(record); + assert!(matches!( + result, + Err(Error::WriteError(WriteError::ConfigurationMismatch { + attribute: "paired", + .. + })) + )); + } + + #[test] + fn test_push_headers_mismatch() { + let header = FileHeaderBuilder::default() + .is_paired(false) + .with_headers(true) + .with_qualities(false) + .with_flags(false) + .with_block_size(1 << 16) + .build(); + let mut block = ColumnarBlock::new(header); + let record = SequencingRecordBuilder::default() + .s_seq(b"ACGT") + .build() + .unwrap(); + let result = block.push(record); + assert!(matches!( + result, + Err(Error::WriteError(WriteError::ConfigurationMismatch { + attribute: "headers", + .. + })) + )); + } + + #[test] + fn test_push_qualities_mismatch() { + let header = FileHeaderBuilder::default() + .is_paired(false) + .with_headers(false) + .with_qualities(true) + .with_flags(false) + .with_block_size(1 << 16) + .build(); + let mut block = ColumnarBlock::new(header); + let record = SequencingRecordBuilder::default() + .s_seq(b"ACGT") + .build() + .unwrap(); + let result = block.push(record); + assert!(matches!( + result, + Err(Error::WriteError(WriteError::ConfigurationMismatch { + attribute: "qualities", + .. + })) + )); + } + + #[test] + fn test_usage() { + let mut block = ColumnarBlock::new(unpaired_header(1 << 16)); + assert_eq!(block.usage(), 0.0); + let seq = vec![b'A'; 32]; + block + .push(SequencingRecordBuilder::default().s_seq(&seq).build().unwrap()) + .unwrap(); + assert!(block.usage() > 0.0); + } + + // ==================== Internal error paths (direct private-method calls) ==================== + + #[test] + fn test_add_sequence_missing_xseq() { + let mut block = ColumnarBlock::new(full_header(1 << 16)); + let record = SequencingRecordBuilder::default() + .s_seq(b"ACGT") + .build() + .unwrap(); + let result = block.add_sequence(&record); + assert!(matches!( + result, + Err(Error::WriteError(WriteError::ConfigurationMismatch { + attribute: "x_seq", + .. + })) + )); + } + + #[test] + fn test_add_flag_missing() { + let header = FileHeaderBuilder::default() + .is_paired(false) + .with_headers(false) + .with_qualities(false) + .with_flags(true) + .with_block_size(1 << 16) + .build(); + let mut block = ColumnarBlock::new(header); + let record = SequencingRecordBuilder::default() + .s_seq(b"ACGT") + .build() + .unwrap(); + let result = block.add_flag(&record); + assert!(matches!( + result, + Err(Error::WriteError(WriteError::ConfigurationMismatch { + attribute: "flag", + .. + })) + )); + } + + #[test] + fn test_add_headers_missing_sheader() { + let header = FileHeaderBuilder::default() + .is_paired(false) + .with_headers(true) + .with_qualities(false) + .with_flags(false) + .with_block_size(1 << 16) + .build(); + let mut block = ColumnarBlock::new(header); + let record = SequencingRecordBuilder::default() + .s_seq(b"ACGT") + .build() + .unwrap(); + let result = block.add_headers(&record); + assert!(matches!( + result, + Err(Error::WriteError(WriteError::ConfigurationMismatch { + attribute: "s_header", + .. + })) + )); + } + + #[test] + fn test_add_headers_missing_xheader() { + let header = FileHeaderBuilder::default() + .is_paired(true) + .with_headers(true) + .with_qualities(false) + .with_flags(false) + .with_block_size(1 << 16) + .build(); + let mut block = ColumnarBlock::new(header); + let record = SequencingRecordBuilder::default() + .s_seq(b"ACGT") + .s_header(b"read1") + .build() + .unwrap(); + let result = block.add_headers(&record); + assert!(matches!( + result, + Err(Error::WriteError(WriteError::ConfigurationMismatch { + attribute: "x_header", + .. + })) + )); + } + + #[test] + fn test_add_quality_missing_squal() { + let header = FileHeaderBuilder::default() + .is_paired(false) + .with_headers(false) + .with_qualities(true) + .with_flags(false) + .with_block_size(1 << 16) + .build(); + let mut block = ColumnarBlock::new(header); + let record = SequencingRecordBuilder::default() + .s_seq(b"ACGT") + .build() + .unwrap(); + let result = block.add_quality(&record); + assert!(matches!( + result, + Err(Error::WriteError(WriteError::ConfigurationMismatch { + attribute: "s_qual", + .. + })) + )); + } + + #[test] + fn test_add_quality_missing_xqual() { + let header = FileHeaderBuilder::default() + .is_paired(true) + .with_headers(false) + .with_qualities(true) + .with_flags(false) + .with_block_size(1 << 16) + .build(); + let mut block = ColumnarBlock::new(header); + let record = SequencingRecordBuilder::default() + .s_seq(b"ACGT") + .s_qual(b"IIII") + .build() + .unwrap(); + let result = block.add_quality(&record); + assert!(matches!( + result, + Err(Error::WriteError(WriteError::ConfigurationMismatch { + attribute: "x_qual", + .. + })) + )); + } + + #[test] + fn test_take_incomplete_cannot_ingest() { + let mut block = ColumnarBlock::new(unpaired_header(8)); + let seq = vec![b'A'; 32]; + block + .push(SequencingRecordBuilder::default().s_seq(&seq).build().unwrap()) + .unwrap(); + + let mut other = ColumnarBlock::new(unpaired_header(8)); + other + .push(SequencingRecordBuilder::default().s_seq(&seq).build().unwrap()) + .unwrap(); + + let result = block.take_incomplete(&other); + assert!(matches!( + result, + Err(Error::CbqError(CbqError::CannotIngestBlock { .. })) + )); + } + + // ==================== Full round-trip through streaming (read_from/decompress_columns) ==================== + + #[test] + fn test_full_feature_roundtrip_streaming() { + let s_seq = b"ACGTNACGT"; + let x_seq = b"TTGGNCCAA"; + let s_qual = vec![b'I'; s_seq.len()]; + let x_qual = vec![b'J'; x_seq.len()]; + + let header = full_header(1 << 16); + let mut block = ColumnarBlock::new(header); + block + .push(full_record(s_seq, x_seq, &s_qual, &x_qual)) + .unwrap(); + + let mut cctx = zstd_safe::CCtx::create(); + let mut buffer = Vec::new(); + let block_header = block.flush_to(&mut buffer, &mut cctx).unwrap().unwrap(); + + // Stream the compressed columns back through `read_from` + `decompress_columns`, + // matching what the streaming `Reader` does. + let mut cursor = std::io::Cursor::new(buffer[std::mem::size_of::()..].to_vec()); + let mut reader_block = ColumnarBlock::new(header); + reader_block.read_from(&mut cursor, block_header).unwrap(); + reader_block.decompress_columns().unwrap(); + + let range = BlockRange::new(0, block_header.num_records); + let rec = reader_block.iter_records(range).next().unwrap(); + + assert_eq!(rec.flag(), Some(42)); + assert_eq!(rec.sheader(), b"read1"); + assert_eq!(rec.xheader(), b"read2"); + assert!(rec.is_paired()); + assert!(rec.has_quality()); + assert_eq!(rec.squal(), s_qual.as_slice()); + assert_eq!(rec.xqual(), x_qual.as_slice()); + + let mut sbuf = Vec::new(); + rec.decode_s(&mut sbuf).unwrap(); + assert_eq!(sbuf, s_seq); + + let mut xbuf = Vec::new(); + rec.decode_x(&mut xbuf).unwrap(); + assert_eq!(xbuf, x_seq); + } + + // ==================== Full round-trip through decompress_from_bytes (mmap path) ==================== + + #[test] + fn test_full_feature_roundtrip_from_bytes() { + let s_seq = b"ACGTNACGT"; + let x_seq = b"TTGGNCCAA"; + let s_qual = vec![b'I'; s_seq.len()]; + let x_qual = vec![b'J'; x_seq.len()]; + + let header = full_header(1 << 16); + let mut block = ColumnarBlock::new(header); + block + .push(full_record(s_seq, x_seq, &s_qual, &x_qual)) + .unwrap(); + + let mut cctx = zstd_safe::CCtx::create(); + let mut buffer = Vec::new(); + let block_header = block.flush_to(&mut buffer, &mut cctx).unwrap().unwrap(); + + // Skip the serialized block header - `decompress_from_bytes` expects + // only the compressed column bytes, matching `MmapReader::load_block`. + let column_bytes = &buffer[std::mem::size_of::()..]; + + let mut dctx = zstd_safe::DCtx::create(); + let mut reader_block = ColumnarBlock::new(header); + reader_block + .decompress_from_bytes(column_bytes, block_header, &mut dctx) + .unwrap(); + + let range = BlockRange::new(0, block_header.num_records); + let rec = reader_block.iter_records(range).next().unwrap(); + + assert_eq!(rec.flag(), Some(42)); + assert_eq!(rec.sheader(), b"read1"); + assert_eq!(rec.xheader(), b"read2"); + assert!(rec.is_paired()); + assert!(rec.has_quality()); + assert_eq!(rec.squal(), s_qual.as_slice()); + assert_eq!(rec.xqual(), x_qual.as_slice()); + assert_eq!(rec.sseq(), s_seq); + assert_eq!(rec.xseq(), x_seq); + } + + // ==================== Default header fallback (no headers stored) ==================== + + #[test] + fn test_default_header_fallback_uses_record_index() { + let header = unpaired_header(1 << 16); + let mut block = ColumnarBlock::new(header); + block + .push(SequencingRecordBuilder::default().s_seq(b"ACGT").build().unwrap()) + .unwrap(); + + let mut cctx = zstd_safe::CCtx::create(); + let mut buffer = Vec::new(); + let block_header = block.flush_to(&mut buffer, &mut cctx).unwrap().unwrap(); + + let mut cursor = std::io::Cursor::new(buffer[std::mem::size_of::()..].to_vec()); + let mut reader_block = ColumnarBlock::new(header); + reader_block.read_from(&mut cursor, block_header).unwrap(); + reader_block.decompress_columns().unwrap(); + + let range = BlockRange::new(0, block_header.num_records); + let rec = reader_block.iter_records(range).next().unwrap(); + assert_eq!(rec.sheader(), b"0"); + assert_eq!(rec.xheader(), b"0"); + assert!(!rec.is_paired()); + assert!(!rec.has_quality()); + assert_eq!(rec.flag(), None); + } + + #[test] + #[should_panic(expected = "sbuf is not implemented for cbq")] + fn test_sbuf_unimplemented() { + let header = unpaired_header(1 << 16); + let mut block = ColumnarBlock::new(header); + block + .push(SequencingRecordBuilder::default().s_seq(b"ACGT").build().unwrap()) + .unwrap(); + + let mut cctx = zstd_safe::CCtx::create(); + let mut buffer = Vec::new(); + let block_header = block.flush_to(&mut buffer, &mut cctx).unwrap().unwrap(); + + let mut cursor = std::io::Cursor::new(buffer[std::mem::size_of::()..].to_vec()); + let mut reader_block = ColumnarBlock::new(header); + reader_block.read_from(&mut cursor, block_header).unwrap(); + reader_block.decompress_columns().unwrap(); + + let range = BlockRange::new(0, block_header.num_records); + let rec = reader_block.iter_records(range).next().unwrap(); + let _ = rec.sbuf(); + } +} diff --git a/src/record/binseq_record.rs b/src/record/binseq_record.rs index 1fb0370..246f201 100644 --- a/src/record/binseq_record.rs +++ b/src/record/binseq_record.rs @@ -114,3 +114,158 @@ pub trait BinseqRecord { !self.squal().is_empty() } } + +#[cfg(test)] +mod tests { + use super::*; + + /// Minimal implementation exercising only the trait's required methods, + /// so that the default-method implementations get covered. + struct MockRecord { + bitsize: BitSize, + index: u64, + flag: Option, + sbuf: Vec, + xbuf: Vec, + slen: u64, + xlen: u64, + squal: Vec, + } + + impl BinseqRecord for MockRecord { + fn bitsize(&self) -> BitSize { + self.bitsize + } + fn index(&self) -> u64 { + self.index + } + fn flag(&self) -> Option { + self.flag + } + fn sheader(&self) -> &[u8] { + b"seq_header" + } + fn xheader(&self) -> &[u8] { + b"" + } + fn slen(&self) -> u64 { + self.slen + } + fn xlen(&self) -> u64 { + self.xlen + } + fn sbuf(&self) -> &[u64] { + &self.sbuf + } + fn xbuf(&self) -> &[u64] { + &self.xbuf + } + fn squal(&self) -> &[u8] { + &self.squal + } + } + + fn unpaired_record() -> MockRecord { + let seq = b"ACGTACGTAC"; + let mut sbuf = Vec::new(); + BitSize::Two.encode(seq, &mut sbuf).unwrap(); + MockRecord { + bitsize: BitSize::Two, + index: 7, + flag: Some(3), + sbuf, + xbuf: Vec::new(), + slen: seq.len() as u64, + xlen: 0, + squal: Vec::new(), + } + } + + fn paired_record() -> MockRecord { + let sseq = b"ACGTACGTAC"; + let xseq = b"TTGGCCAATT"; + let mut sbuf = Vec::new(); + let mut xbuf = Vec::new(); + BitSize::Two.encode(sseq, &mut sbuf).unwrap(); + BitSize::Two.encode(xseq, &mut xbuf).unwrap(); + MockRecord { + bitsize: BitSize::Two, + index: 1, + flag: None, + sbuf, + xbuf, + slen: sseq.len() as u64, + xlen: xseq.len() as u64, + squal: vec![b'I'; sseq.len()], + } + } + + #[test] + fn test_default_index_and_flag() { + let record = unpaired_record(); + assert_eq!(record.index(), 7); + assert_eq!(record.flag(), Some(3)); + } + + #[test] + fn test_decode_s() { + let record = unpaired_record(); + let mut buf = Vec::new(); + record.decode_s(&mut buf).unwrap(); + assert_eq!(buf, b"ACGTACGTAC"); + } + + #[test] + fn test_decode_x() { + let record = paired_record(); + let mut buf = Vec::new(); + record.decode_x(&mut buf).unwrap(); + assert_eq!(buf, b"TTGGCCAATT"); + } + + #[test] + fn test_decode_s_alloc() { + let record = unpaired_record(); + let buf = record.decode_s_alloc().unwrap(); + assert_eq!(buf, b"ACGTACGTAC"); + } + + #[test] + fn test_decode_x_alloc() { + let record = paired_record(); + let buf = record.decode_x_alloc().unwrap(); + assert_eq!(buf, b"TTGGCCAATT"); + } + + #[test] + #[should_panic(expected = "does not implement direct sequence access")] + fn test_sseq_default_panics() { + let record = unpaired_record(); + let _ = record.sseq(); + } + + #[test] + #[should_panic(expected = "does not implement direct sequence access")] + fn test_xseq_default_panics() { + let record = unpaired_record(); + let _ = record.xseq(); + } + + #[test] + fn test_is_paired() { + assert!(!unpaired_record().is_paired()); + assert!(paired_record().is_paired()); + } + + #[test] + fn test_has_quality() { + assert!(!unpaired_record().has_quality()); + assert!(paired_record().has_quality()); + } + + #[test] + fn test_default_xqual_is_empty() { + let record = unpaired_record(); + assert!(record.xqual().is_empty()); + } +} From 90dfea6e7303d338e4470fea1dc837de6fcbd5c6 Mon Sep 17 00:00:00 2001 From: noamteyssier <22600644+noamteyssier@users.noreply.github.com> Date: Mon, 6 Jul 2026 17:56:16 -0700 Subject: [PATCH 02/21] fix: bug in bq stream reader with incorrect id for small buffers --- src/bq/reader.rs | 37 ++++++++++++++++++++++++------------- 1 file changed, 24 insertions(+), 13 deletions(-) diff --git a/src/bq/reader.rs b/src/bq/reader.rs index ad2d11b..2a705d0 100644 --- a/src/bq/reader.rs +++ b/src/bq/reader.rs @@ -599,6 +599,14 @@ pub struct StreamReader { /// Length of valid data in the buffer buffer_len: usize, + + /// Number of records returned so far, used to assign each record's id + /// + /// This is tracked independently of `buffer_pos` because `fill_buffer` + /// shifts remaining bytes to the start of the buffer and resets + /// `buffer_pos` whenever a mid-stream refill is needed, so `buffer_pos` + /// no longer reflects the absolute stream offset once that happens. + records_read: u64, } impl StreamReader { @@ -641,6 +649,7 @@ impl StreamReader { buffer_pos: 0, buffer_len: 0, default_quality_score: DEFAULT_QUALITY_SCORE, + records_read: 0, } } @@ -797,10 +806,12 @@ impl StreamReader { self.qbuf.resize(max_size, self.default_quality_score); } - // Create record with incremental ID (based on read position) - let id = (record_start - SIZE_HEADER) / record_size; + // Create record with an incremental ID, tracked independently of + // buffer position since `fill_buffer` may have shifted the buffer + let id = self.records_read; + self.records_read += 1; Some(Ok(RefRecord::new( - id as u64, + id, record_u64s, &self.qbuf, config, @@ -1460,21 +1471,21 @@ mod tests { } #[test] - #[should_panic(expected = "attempt to subtract with overflow")] fn test_stream_reader_small_buffer_forces_multiple_fills() { - // Known bug (tracked separately, not fixed here): once a tiny buffer - // capacity forces `fill_buffer` to shift remaining bytes to the start - // of the internal buffer (reader.rs:716-719), `buffer_pos` no longer - // reflects the absolute stream offset. `next_record`'s id calculation - // (reader.rs:801) assumes it still does, and underflows. This test - // pins current (buggy) behavior so it fails loudly once fixed -- - // at which point it should be rewritten to assert sequential ids - // across the fill boundary instead. + // Use a tiny buffer capacity so `fill_buffer` must shift remaining + // bytes to the start of the internal buffer mid-stream (reader.rs + // 716-719). Record ids are tracked via a dedicated `records_read` + // counter (not derived from `buffer_pos`), so they must still come + // back sequential across that shift boundary. let data = build_stream_bytes(false); let mut reader = StreamReader::with_capacity(std::io::Cursor::new(data), 40); + let mut expected_id = 0u64; while let Some(record) = reader.next_record() { - record.unwrap(); + let record = record.unwrap(); + assert_eq!(record.index(), expected_id); + expected_id += 1; } + assert_eq!(expected_id, 5); } #[test] From ba5814d6642feb4ab56ccfb6c50ccdff40c95ef4 Mon Sep 17 00:00:00 2001 From: noamteyssier <22600644+noamteyssier@users.noreply.github.com> Date: Mon, 6 Jul 2026 18:05:28 -0700 Subject: [PATCH 03/21] tests: improve coverage --- src/record/sequencing_record.rs | 83 ++++++++++++++++++ src/vbq/header.rs | 143 ++++++++++++++++++++++++++++++++ src/vbq/index.rs | 124 +++++++++++++++++++++++++++ 3 files changed, 350 insertions(+) diff --git a/src/record/sequencing_record.rs b/src/record/sequencing_record.rs index 95b6fd0..b5da435 100644 --- a/src/record/sequencing_record.rs +++ b/src/record/sequencing_record.rs @@ -401,3 +401,86 @@ impl<'a> SequencingRecordBuilder<'a> { }) } } + +#[cfg(test)] +mod tests { + use super::*; + + // ==================== SequencingRecord::new() and accessors ==================== + + #[test] + fn test_new_constructor() { + let record = SequencingRecord::new( + b"ACGT", + Some(b"IIII"), + Some(b"s_hdr"), + Some(b"TGCA"), + Some(b"FFFF"), + Some(b"x_hdr"), + Some(7), + ); + assert_eq!(record.s_seq(), b"ACGT"); + assert_eq!(record.s_qual(), Some(b"IIII".as_slice())); + assert_eq!(record.s_header(), Some(b"s_hdr".as_slice())); + assert_eq!(record.x_seq(), Some(b"TGCA".as_slice())); + assert_eq!(record.x_qual(), Some(b"FFFF".as_slice())); + assert_eq!(record.x_header(), Some(b"x_hdr".as_slice())); + assert_eq!(record.flag(), Some(7)); + } + + #[test] + fn test_new_constructor_minimal() { + let record = SequencingRecord::new(b"ACGT", None, None, None, None, None, None); + assert_eq!(record.s_seq(), b"ACGT"); + assert_eq!(record.s_qual(), None); + assert_eq!(record.s_header(), None); + assert_eq!(record.x_seq(), None); + assert_eq!(record.x_qual(), None); + assert_eq!(record.x_header(), None); + assert_eq!(record.flag(), None); + assert!(!record.is_paired()); + assert!(!record.has_flags()); + assert!(!record.has_headers()); + assert!(!record.has_qualities()); + } + + // ==================== SequencingRecordBuilder opt_* setters ==================== + + #[test] + fn test_builder_opt_setters_some() { + let record = SequencingRecordBuilder::default() + .s_seq(b"ACGT") + .opt_s_header(Some(b"s_hdr")) + .opt_x_seq(Some(b"TGCA")) + .opt_x_qual(Some(b"FFFF")) + .opt_flag(Some(9)) + .build() + .unwrap(); + assert_eq!(record.s_header(), Some(b"s_hdr".as_slice())); + assert_eq!(record.x_seq(), Some(b"TGCA".as_slice())); + assert_eq!(record.x_qual(), Some(b"FFFF".as_slice())); + assert_eq!(record.flag(), Some(9)); + } + + #[test] + fn test_builder_opt_setters_none() { + let record = SequencingRecordBuilder::default() + .s_seq(b"ACGT") + .opt_s_header(None) + .opt_x_seq(None) + .opt_x_qual(None) + .opt_flag(None) + .build() + .unwrap(); + assert_eq!(record.s_header(), None); + assert_eq!(record.x_seq(), None); + assert_eq!(record.x_qual(), None); + assert_eq!(record.flag(), None); + } + + #[test] + fn test_builder_missing_sequence() { + let result = SequencingRecordBuilder::default().build(); + assert!(result.is_err()); + } +} diff --git a/src/vbq/header.rs b/src/vbq/header.rs index 16476bd..29a88b3 100644 --- a/src/vbq/header.rs +++ b/src/vbq/header.rs @@ -557,3 +557,146 @@ impl BlockHeader { self.size as usize + SIZE_BLOCK_HEADER } } + +#[cfg(test)] +mod tests { + use super::*; + + // ==================== FileHeaderBuilder Tests ==================== + + #[test] + fn test_builder_block_and_bitsize() { + let header = FileHeaderBuilder::new() + .block(4096) + .bitsize(BitSize::Four) + .build(); + assert_eq!(header.block, 4096); + assert_eq!(header.bits, BitSize::Four); + } + + #[test] + fn test_builder_defaults() { + let header = FileHeaderBuilder::new().build(); + assert_eq!(header.block, BLOCK_SIZE); + assert!(!header.qual); + assert!(!header.compressed); + assert!(!header.paired); + assert!(!header.headers); + assert!(!header.flags); + } + + // ==================== FileHeader Constructor Tests ==================== + + #[test] + fn test_file_header_new() { + let header = FileHeader::new(true, true, true, BitSize::Four, true, true); + assert_eq!(header.block, BLOCK_SIZE); + assert!(header.qual); + assert!(header.compressed); + assert!(header.paired); + assert_eq!(header.bits, BitSize::Four); + assert!(header.headers); + assert!(header.flags); + assert!(header.is_paired()); + } + + #[test] + fn test_file_header_default() { + let header = FileHeader::default(); + assert_eq!(header.block, BLOCK_SIZE); + assert!(!header.is_paired()); + } + + #[test] + fn test_set_bitsize() { + let mut header = FileHeader::default(); + header.set_bitsize(BitSize::Four); + assert_eq!(header.bits, BitSize::Four); + } + + // ==================== FileHeader from_bytes/from_reader Tests ==================== + + #[test] + fn test_file_header_roundtrip() { + let header = FileHeader::new(true, false, true, BitSize::Two, true, true); + let mut buffer = Vec::new(); + header.write_bytes(&mut buffer).unwrap(); + let mut cursor = std::io::Cursor::new(buffer); + let parsed = FileHeader::from_reader(&mut cursor).unwrap(); + assert_eq!(parsed, header); + } + + #[test] + fn test_file_header_from_bytes_four_bit() { + let header = FileHeader::new(false, false, false, BitSize::Four, false, false); + let mut buffer = [0u8; SIZE_HEADER]; + { + let mut cursor = std::io::Cursor::new(&mut buffer[..]); + header.write_bytes(&mut cursor).unwrap(); + } + let parsed = FileHeader::from_bytes(&buffer).unwrap(); + assert_eq!(parsed.bits, BitSize::Four); + } + + #[test] + fn test_file_header_from_bytes_invalid_magic() { + let buffer = [0u8; SIZE_HEADER]; + let result = FileHeader::from_bytes(&buffer); + assert!(result.is_err()); + } + + #[test] + fn test_file_header_from_bytes_invalid_format() { + let header = FileHeader::default(); + let mut buffer = [0u8; SIZE_HEADER]; + { + let mut cursor = std::io::Cursor::new(&mut buffer[..]); + header.write_bytes(&mut cursor).unwrap(); + } + buffer[4] = 99; + let result = FileHeader::from_bytes(&buffer); + assert!(result.is_err()); + } + + #[test] + fn test_file_header_from_bytes_invalid_bitsize() { + let header = FileHeader::default(); + let mut buffer = [0u8; SIZE_HEADER]; + { + let mut cursor = std::io::Cursor::new(&mut buffer[..]); + header.write_bytes(&mut cursor).unwrap(); + } + buffer[16] = 99; + let result = FileHeader::from_bytes(&buffer); + assert!(result.is_err()); + } + + #[test] + fn test_file_header_from_reader_truncated() { + let mut cursor = std::io::Cursor::new(vec![0u8; 5]); + let result = FileHeader::from_reader(&mut cursor); + assert!(result.is_err()); + } + + // ==================== BlockHeader Tests ==================== + + #[test] + fn test_block_header_from_bytes_invalid_magic() { + let buffer = [0u8; SIZE_BLOCK_HEADER]; + let result = BlockHeader::from_bytes(&buffer); + assert!(result.is_err()); + } + + #[test] + fn test_block_header_roundtrip() { + let header = BlockHeader::new(2048, 42); + let mut buffer = Vec::new(); + header.write_bytes(&mut buffer).unwrap(); + let mut fixed = [0u8; SIZE_BLOCK_HEADER]; + fixed.copy_from_slice(&buffer); + let parsed = BlockHeader::from_bytes(&fixed).unwrap(); + assert_eq!(parsed.size, 2048); + assert_eq!(parsed.records, 42); + assert!(!parsed.is_empty()); + } +} diff --git a/src/vbq/index.rs b/src/vbq/index.rs index 3f9e880..7b2c8e1 100644 --- a/src/vbq/index.rs +++ b/src/vbq/index.rs @@ -629,3 +629,127 @@ impl BlockIndex { .unwrap_or_default() } } + +#[cfg(test)] +mod tests { + use super::*; + + /// Writes a minimal raw VBQ file (file header + block headers/data, with + /// **no** embedded index) to `path`, suitable for `BlockIndex::from_vbq`. + /// + /// `from_vbq` scans block-by-block from just after the file header to + /// EOF, so it requires the file to end exactly at the last block's data + /// -- unlike files produced by `vbq::Writer`, which always append an + /// embedded index on `finish()` that `from_vbq` cannot parse as a block. + fn write_raw_vbq_file(path: &str, blocks: &[(u64, u32)]) { + let mut buffer = Vec::new(); + FileHeader::default().write_bytes(&mut buffer).unwrap(); + for &(size, records) in blocks { + BlockHeader::new(size, records) + .write_bytes(&mut buffer) + .unwrap(); + buffer.extend(std::iter::repeat_n(0u8, size as usize)); + } + std::fs::write(path, buffer).unwrap(); + } + + // ==================== BlockIndex::from_vbq Tests ==================== + + #[test] + fn test_from_vbq() { + let path = "test_index_from_vbq.vbq"; + write_raw_vbq_file(path, &[(64, 3), (128, 5)]); + + let index = BlockIndex::from_vbq(path).unwrap(); + std::fs::remove_file(path).unwrap(); + + assert_eq!(index.n_blocks(), 2); + assert_eq!(index.num_records(), 8); + assert_eq!(index.ranges()[0].cumulative_records, 0); + assert_eq!(index.ranges()[1].cumulative_records, 3); + } + + #[test] + fn test_from_vbq_nonexistent_file() { + let result = BlockIndex::from_vbq("./data/does_not_exist.vbq"); + assert!(result.is_err()); + } + + #[test] + fn test_n_blocks() { + let path = "test_index_n_blocks.vbq"; + write_raw_vbq_file(path, &[(32, 1)]); + + let index = BlockIndex::from_vbq(path).unwrap(); + std::fs::remove_file(path).unwrap(); + + assert_eq!(index.n_blocks(), index.ranges().len()); + } + + #[test] + fn test_pprint() { + let path = "test_index_pprint.vbq"; + write_raw_vbq_file(path, &[(32, 1)]); + + // Just verify it doesn't panic + let index = BlockIndex::from_vbq(path).unwrap(); + std::fs::remove_file(path).unwrap(); + index.pprint(); + } + + #[test] + fn test_num_records_empty_index() { + let index = BlockIndex::new(IndexHeader::new(0)); + assert_eq!(index.num_records(), 0); + assert_eq!(index.n_blocks(), 0); + } + + // ==================== IndexHeader Tests ==================== + + #[test] + fn test_index_header_from_reader_invalid_magic() { + let mut buffer = [0u8; INDEX_HEADER_SIZE]; + LittleEndian::write_u64(&mut buffer[0..8], 0xDEAD_BEEF); + let result = IndexHeader::from_reader(&mut Cursor::new(buffer)); + assert!(result.is_err()); + } + + #[test] + fn test_index_header_roundtrip() { + let header = IndexHeader::new(12345); + let mut buffer = Vec::new(); + header.write_bytes(&mut buffer).unwrap(); + let parsed = IndexHeader::from_reader(&mut Cursor::new(buffer)).unwrap(); + assert_eq!(parsed.bytes, 12345); + assert_eq!(parsed.magic, INDEX_MAGIC); + } + + // ==================== BlockIndex round-trip through write_bytes/from_bytes ==================== + + #[test] + fn test_block_index_write_and_from_bytes_roundtrip() { + let mut index = BlockIndex::new(IndexHeader::new(999)); + index.add_range(BlockRange::new(32, 100, 5, 0)); + index.add_range(BlockRange::new(164, 200, 7, 5)); + + let mut buffer = Vec::new(); + index.write_bytes(&mut buffer).unwrap(); + + let parsed = BlockIndex::from_bytes(&buffer).unwrap(); + assert_eq!(parsed.n_blocks(), 2); + assert_eq!(parsed.num_records(), 12); + assert_eq!(parsed.ranges()[0].start_offset, 32); + assert_eq!(parsed.ranges()[1].cumulative_records, 5); + } + + #[test] + fn test_block_index_write_range_skips_empty_blocks() { + let mut index = BlockIndex::new(IndexHeader::new(0)); + index.add_range(BlockRange::new(32, 0, 0, 0)); + index.add_range(BlockRange::new(64, 50, 3, 0)); + + let mut buffer = Vec::new(); + index.write_range(&mut buffer).unwrap(); + assert_eq!(buffer.len(), SIZE_BLOCK_RANGE); + } +} From ee9f663dc5fcd6ef61816540034602b22aa4ad97 Mon Sep 17 00:00:00 2001 From: noamteyssier <22600644+noamteyssier@users.noreply.github.com> Date: Mon, 6 Jul 2026 18:29:11 -0700 Subject: [PATCH 04/21] style: fmt --- src/bq/reader.rs | 12 ++++----- src/bq/writer.rs | 16 +++++++----- src/cbq/core/block.rs | 59 +++++++++++++++++++++++++++++++++++-------- 3 files changed, 63 insertions(+), 24 deletions(-) diff --git a/src/bq/reader.rs b/src/bq/reader.rs index 2a705d0..2be87ef 100644 --- a/src/bq/reader.rs +++ b/src/bq/reader.rs @@ -810,12 +810,7 @@ impl StreamReader { // buffer position since `fill_buffer` may have shifted the buffer let id = self.records_read; self.records_read += 1; - Some(Ok(RefRecord::new( - id, - record_u64s, - &self.qbuf, - config, - ))) + Some(Ok(RefRecord::new(id, record_u64s, &self.qbuf, config))) } /// Consumes the stream reader and returns the inner reader @@ -1361,7 +1356,10 @@ mod tests { let path = "test_truncated_reader.bq"; { - let header = crate::bq::FileHeaderBuilder::new().slen(64).build().unwrap(); + let header = crate::bq::FileHeaderBuilder::new() + .slen(64) + .build() + .unwrap(); let mut file = std::fs::File::create(path).unwrap(); header.write_bytes(&mut file).unwrap(); // Write a partial record (not a full multiple of the record size) diff --git a/src/bq/writer.rs b/src/bq/writer.rs index 89e9946..e457b13 100644 --- a/src/bq/writer.rs +++ b/src/bq/writer.rs @@ -915,9 +915,7 @@ mod testing { fn test_encoder_encode_paired_invalid_ignored() { let header = FileHeaderBuilder::new().slen(8).xlen(8).build().unwrap(); let mut encoder = Encoder::with_policy(header, Policy::IgnoreSequence); - let result = encoder - .encode_paired(b"ACGTNNNN", b"ACGTACGT") - .unwrap(); + let result = encoder.encode_paired(b"ACGTNNNN", b"ACGTACGT").unwrap(); assert!(result.is_none()); } @@ -925,9 +923,7 @@ mod testing { fn test_encoder_encode_paired_invalid_corrected() { let header = FileHeaderBuilder::new().slen(8).xlen(8).build().unwrap(); let mut encoder = Encoder::with_policy(header, Policy::SetToA); - let result = encoder - .encode_paired(b"ACGTNNNN", b"NNNNACGT") - .unwrap(); + let result = encoder.encode_paired(b"ACGTNNNN", b"NNNNACGT").unwrap(); assert!(result.is_some()); } @@ -967,7 +963,13 @@ mod testing { #[allow(deprecated)] fn test_write_paired_record_deprecated() -> Result<()> { let mut writer = WriterBuilder::default() - .header(FileHeaderBuilder::new().slen(8).xlen(8).flags(true).build()?) + .header( + FileHeaderBuilder::new() + .slen(8) + .xlen(8) + .flags(true) + .build()?, + ) .build(Vec::new())?; let wrote = writer.write_paired_record(Some(5), b"ACGTACGT", b"TTGGCCAA")?; assert!(wrote); diff --git a/src/cbq/core/block.rs b/src/cbq/core/block.rs index bfd4952..2050bf5 100644 --- a/src/cbq/core/block.rs +++ b/src/cbq/core/block.rs @@ -1034,10 +1034,21 @@ mod tests { // Block fits exactly one 8-byte-encoded record (32bp -> 1 word == 8 bytes) let mut block = ColumnarBlock::new(unpaired_header(8)); let seq = vec![b'A'; 32]; - block.push(SequencingRecordBuilder::default().s_seq(&seq).build().unwrap()) + block + .push( + SequencingRecordBuilder::default() + .s_seq(&seq) + .build() + .unwrap(), + ) .unwrap(); - let result = block.push(SequencingRecordBuilder::default().s_seq(&seq).build().unwrap()); + let result = block.push( + SequencingRecordBuilder::default() + .s_seq(&seq) + .build() + .unwrap(), + ); assert!(matches!( result, Err(Error::CbqError(CbqError::BlockFull { .. })) @@ -1117,7 +1128,12 @@ mod tests { assert_eq!(block.usage(), 0.0); let seq = vec![b'A'; 32]; block - .push(SequencingRecordBuilder::default().s_seq(&seq).build().unwrap()) + .push( + SequencingRecordBuilder::default() + .s_seq(&seq) + .build() + .unwrap(), + ) .unwrap(); assert!(block.usage() > 0.0); } @@ -1268,12 +1284,22 @@ mod tests { let mut block = ColumnarBlock::new(unpaired_header(8)); let seq = vec![b'A'; 32]; block - .push(SequencingRecordBuilder::default().s_seq(&seq).build().unwrap()) + .push( + SequencingRecordBuilder::default() + .s_seq(&seq) + .build() + .unwrap(), + ) .unwrap(); let mut other = ColumnarBlock::new(unpaired_header(8)); other - .push(SequencingRecordBuilder::default().s_seq(&seq).build().unwrap()) + .push( + SequencingRecordBuilder::default() + .s_seq(&seq) + .build() + .unwrap(), + ) .unwrap(); let result = block.take_incomplete(&other); @@ -1304,7 +1330,8 @@ mod tests { // Stream the compressed columns back through `read_from` + `decompress_columns`, // matching what the streaming `Reader` does. - let mut cursor = std::io::Cursor::new(buffer[std::mem::size_of::()..].to_vec()); + let mut cursor = + std::io::Cursor::new(buffer[std::mem::size_of::()..].to_vec()); let mut reader_block = ColumnarBlock::new(header); reader_block.read_from(&mut cursor, block_header).unwrap(); reader_block.decompress_columns().unwrap(); @@ -1379,14 +1406,20 @@ mod tests { let header = unpaired_header(1 << 16); let mut block = ColumnarBlock::new(header); block - .push(SequencingRecordBuilder::default().s_seq(b"ACGT").build().unwrap()) + .push( + SequencingRecordBuilder::default() + .s_seq(b"ACGT") + .build() + .unwrap(), + ) .unwrap(); let mut cctx = zstd_safe::CCtx::create(); let mut buffer = Vec::new(); let block_header = block.flush_to(&mut buffer, &mut cctx).unwrap().unwrap(); - let mut cursor = std::io::Cursor::new(buffer[std::mem::size_of::()..].to_vec()); + let mut cursor = + std::io::Cursor::new(buffer[std::mem::size_of::()..].to_vec()); let mut reader_block = ColumnarBlock::new(header); reader_block.read_from(&mut cursor, block_header).unwrap(); reader_block.decompress_columns().unwrap(); @@ -1406,14 +1439,20 @@ mod tests { let header = unpaired_header(1 << 16); let mut block = ColumnarBlock::new(header); block - .push(SequencingRecordBuilder::default().s_seq(b"ACGT").build().unwrap()) + .push( + SequencingRecordBuilder::default() + .s_seq(b"ACGT") + .build() + .unwrap(), + ) .unwrap(); let mut cctx = zstd_safe::CCtx::create(); let mut buffer = Vec::new(); let block_header = block.flush_to(&mut buffer, &mut cctx).unwrap().unwrap(); - let mut cursor = std::io::Cursor::new(buffer[std::mem::size_of::()..].to_vec()); + let mut cursor = + std::io::Cursor::new(buffer[std::mem::size_of::()..].to_vec()); let mut reader_block = ColumnarBlock::new(header); reader_block.read_from(&mut cursor, block_header).unwrap(); reader_block.decompress_columns().unwrap(); From 6b2848cb7e7150c763197fc05c50e40e9d10a60d Mon Sep 17 00:00:00 2001 From: noamteyssier <22600644+noamteyssier@users.noreply.github.com> Date: Mon, 6 Jul 2026 18:30:33 -0700 Subject: [PATCH 05/21] ci: skip unnecessary runs --- .github/workflows/ci.yml | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index ddad108..c8a1424 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -1,6 +1,9 @@ name: CI -on: [push, pull_request] +on: + push: + branches: [main] + pull_request: env: CARGO_TERM_COLOR: always From 601e58ae7825508a07617e216d3978e76896de03 Mon Sep 17 00:00:00 2001 From: noamteyssier <22600644+noamteyssier@users.noreply.github.com> Date: Mon, 6 Jul 2026 18:42:34 -0700 Subject: [PATCH 06/21] style: clippy fix suggestions --- src/bq/reader.rs | 34 ++++++++++++++++------------------ src/parallel.rs | 4 ++++ src/utils/fastx.rs | 2 ++ src/vbq/writer.rs | 1 + src/write.rs | 10 +++++++++- 5 files changed, 32 insertions(+), 19 deletions(-) diff --git a/src/bq/reader.rs b/src/bq/reader.rs index 2be87ef..ed5eecb 100644 --- a/src/bq/reader.rs +++ b/src/bq/reader.rs @@ -682,27 +682,25 @@ impl StreamReader { /// * The header data is invalid /// * End of stream is reached before the full header can be read pub fn read_header(&mut self) -> Result<&FileHeader> { - if self.header.is_some() { - return Ok(self - .header - .as_ref() - .expect("Missing header when expected in stream")); - } - - // Ensure we have enough data for the header - while self.buffer_len - self.buffer_pos < SIZE_HEADER { - self.fill_buffer()?; - } + if self.header.is_none() { + // Ensure we have enough data for the header + while self.buffer_len - self.buffer_pos < SIZE_HEADER { + self.fill_buffer()?; + } - // Parse header - let header_slice = &self.buffer[self.buffer_pos..self.buffer_pos + SIZE_HEADER]; - let header = FileHeader::from_buffer(header_slice)?; + // Parse header + let header_slice = &self.buffer[self.buffer_pos..self.buffer_pos + SIZE_HEADER]; + let header = FileHeader::from_buffer(header_slice)?; - self.header = Some(header); - self.config = Some(RecordConfig::from_header(&header)); - self.buffer_pos += SIZE_HEADER; + self.header = Some(header); + self.config = Some(RecordConfig::from_header(&header)); + self.buffer_pos += SIZE_HEADER; + } - Ok(self.header.as_ref().unwrap()) + Ok(self + .header + .as_ref() + .expect("header was just populated above")) } /// Fills the internal buffer with more data from the reader diff --git a/src/parallel.rs b/src/parallel.rs index 901bbf7..605633b 100644 --- a/src/parallel.rs +++ b/src/parallel.rs @@ -13,6 +13,10 @@ use crate::{ /// distinction between BINSEQ readers is not important. /// /// For more specialized workflows see [`bq::MmapReader`], [`vbq::MmapReader`], and [`cbq::MmapReader`]. +// `cbq::MmapReader` is intrinsically larger than the other variants (it holds a reusable +// `ColumnarBlock` decode buffer). Boxing it would shrink this enum but is a breaking change to +// the variant's public field type, so it's left as-is rather than churn downstream consumers. +#[allow(clippy::large_enum_variant)] pub enum BinseqReader { Bq(bq::MmapReader), Vbq(vbq::MmapReader), diff --git a/src/utils/fastx.rs b/src/utils/fastx.rs index 46ab7e4..3d13007 100644 --- a/src/utils/fastx.rs +++ b/src/utils/fastx.rs @@ -86,6 +86,7 @@ impl FastxEncoderBuilder { /// .run()?; /// # Ok::<(), binseq::Error>(()) /// ``` + #[must_use] pub fn input>(mut self, path: P) -> Self { self.input = Some(FastxInput::Single(path.as_ref().to_path_buf())); self @@ -125,6 +126,7 @@ impl FastxEncoderBuilder { /// .run()?; /// # Ok::<(), binseq::Error>(()) /// ``` + #[must_use] pub fn input_paired>(mut self, r1: P, r2: P) -> Self { self.input = Some(FastxInput::Paired( r1.as_ref().to_path_buf(), diff --git a/src/vbq/writer.rs b/src/vbq/writer.rs index 8aead27..71d05f6 100644 --- a/src/vbq/writer.rs +++ b/src/vbq/writer.rs @@ -446,6 +446,7 @@ impl Writer { } #[deprecated(note = "use `push` method with SequencingRecord instead")] + #[allow(clippy::too_many_arguments)] pub fn write_paired_record( &mut self, flag: Option, diff --git a/src/write.rs b/src/write.rs index 5686f1b..fe23240 100644 --- a/src/write.rs +++ b/src/write.rs @@ -430,7 +430,10 @@ impl BinseqWriterBuilder { .with_headers(self.headers) .with_flags(self.flags) .with_optional_block_size(self.block_size) - .with_optional_compression_level(self.compression_level.map(|level| level as usize)) + .with_optional_compression_level( + self.compression_level + .map(|level| usize::try_from(level).unwrap_or(0)), + ) .build(); let inner = if self.headless { @@ -447,6 +450,11 @@ impl BinseqWriterBuilder { /// /// This enum wraps the three format-specific writers (BQ, VBQ, CBQ) and provides /// a unified interface for writing sequence data. +// `cbq::ColumnarBlockWriter` is intrinsically larger than the other variants (it holds a +// reusable `ColumnarBlock` encode buffer). Boxing it would shrink this enum but is a breaking +// change to the variant's public field type, so it's left as-is rather than churn downstream +// consumers. +#[allow(clippy::large_enum_variant)] pub enum BinseqWriter { /// BQ format writer Bq(bq::Writer), From dab2eea7588cf43fa6a32129233b73a0f23856b2 Mon Sep 17 00:00:00 2001 From: noamteyssier <22600644+noamteyssier@users.noreply.github.com> Date: Mon, 6 Jul 2026 18:42:51 -0700 Subject: [PATCH 07/21] ci: add clippy check --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index c8a1424..a18fd28 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -27,7 +27,7 @@ jobs: - name: Formatting run: cargo fmt --check - name: Linting - run: cargo clippy --verbose + run: cargo clippy --all-features -- -D warnings example_grep: runs-on: ubuntu-latest From 7dbe81ed25362e64458e4422624ca056a8bfb560 Mon Sep 17 00:00:00 2001 From: noamteyssier <22600644+noamteyssier@users.noreply.github.com> Date: Mon, 6 Jul 2026 18:43:16 -0700 Subject: [PATCH 08/21] ci: update checkout version --- .github/workflows/ci.yml | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index a18fd28..3db7631 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -12,7 +12,7 @@ jobs: build: runs-on: ubuntu-latest steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v7 - name: Build run: cargo build --verbose - name: Run tests @@ -23,7 +23,7 @@ jobs: fmt_lint: runs-on: ubuntu-latest steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v7 - name: Formatting run: cargo fmt --check - name: Linting @@ -35,7 +35,7 @@ jobs: matrix: ext: [bq, vbq, cbq] steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v7 - name: run example ${{ matrix.ext }} run: cargo run --release --example grep -- ./data/subset.${{ matrix.ext }} "ACGTACGT" @@ -45,7 +45,7 @@ jobs: matrix: ext: [bq, vbq, cbq] steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v7 - name: run example ${{ matrix.ext }} run: cargo run --release --example parallel_range -- ./data/subset.${{ matrix.ext }} 4 30 200 @@ -55,7 +55,7 @@ jobs: matrix: ext: [bq, vbq, cbq] steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v7 - name: run example (single) ${{ matrix.ext }} run: cargo run --release --example write -- ./data/subset_R1.fastq.gz -o ./output.${{ matrix.ext }} - name: run example (paired) ${{ matrix.ext }} @@ -67,6 +67,6 @@ jobs: matrix: ext: [bq, vbq, cbq] steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v7 - name: run example ${{ matrix.ext }} run: cargo run --release --example read -- ./data/subset.${{ matrix.ext }} From e7ada38d520e2b63019c560731d805244870f8ba Mon Sep 17 00:00:00 2001 From: noamteyssier <22600644+noamteyssier@users.noreply.github.com> Date: Mon, 6 Jul 2026 18:43:53 -0700 Subject: [PATCH 09/21] ci: added additional lint allowances --- Cargo.toml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Cargo.toml b/Cargo.toml index 6e31503..81cf6e8 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -41,4 +41,7 @@ paraseq = ["dep:paraseq", "dep:parking_lot"] [lints.clippy] pedantic = { level = "warn", priority = -1 } cast_possible_truncation = "allow" +cast_precision_loss = "allow" missing_errors_doc = "allow" +struct_excessive_bools = "allow" +fn_params_excessive_bools = "allow" From ade8aef19bedd211bc9b1c1f007a7b1af4486902 Mon Sep 17 00:00:00 2001 From: noamteyssier <22600644+noamteyssier@users.noreply.github.com> Date: Tue, 7 Jul 2026 07:37:16 -0700 Subject: [PATCH 10/21] style: clippy fix suggestions --- src/bq/reader.rs | 52 +++++++++++++++++++++++-------------------- src/cbq/core/block.rs | 2 +- src/cbq/read.rs | 8 +++---- src/cbq/write.rs | 8 +------ src/error.rs | 48 +++++++++++++++++++-------------------- src/parallel.rs | 31 ++++++++++++++------------ src/policy.rs | 2 +- src/vbq/reader.rs | 7 +++--- 8 files changed, 78 insertions(+), 80 deletions(-) diff --git a/src/bq/reader.rs b/src/bq/reader.rs index ed5eecb..fe230b9 100644 --- a/src/bq/reader.rs +++ b/src/bq/reader.rs @@ -1035,9 +1035,8 @@ mod tests { #[test] fn test_mmap_reader_is_paired() { let reader = MmapReader::new(TEST_BQ_FILE).unwrap(); - let is_paired = reader.is_paired(); - // Test that the method returns a boolean - assert!(is_paired || !is_paired); // Always true, tests the method works + // The fixture file contains paired records + assert!(reader.is_paired()); } #[test] @@ -1125,8 +1124,7 @@ mod tests { // All quality scores should be the custom score assert!( squal.iter().all(|&q| q == custom_score), - "All quality scores should be {}", - custom_score + "All quality scores should be {custom_score}" ); } } @@ -1184,8 +1182,7 @@ mod tests { let final_count = *count.lock().unwrap(); assert_eq!( final_count, expected_count, - "Should process exactly {} records", - expected_count + "Should process exactly {expected_count} records" ); } } @@ -1198,8 +1195,16 @@ mod tests { let header = reader.header(); let config = RecordConfig::from_header(&header); - assert_eq!(config.slen, header.slen as u64, "Sequence length mismatch"); - assert_eq!(config.xlen, header.xlen as u64, "Extended length mismatch"); + assert_eq!( + config.slen, + u64::from(header.slen), + "Sequence length mismatch" + ); + assert_eq!( + config.xlen, + u64::from(header.xlen), + "Extended length mismatch" + ); assert_eq!(config.bitsize, header.bits, "Bit size mismatch"); } @@ -1246,17 +1251,17 @@ mod tests { fn test_ref_record_paired_data() { let reader = MmapReader::new(TEST_BQ_FILE).unwrap(); - if reader.is_paired() { - if let Ok(record) = reader.get(0) { - let xbuf = record.xbuf(); - let xlen = record.xlen(); - - if xlen > 0 { - assert!( - !xbuf.is_empty(), - "Extended buffer should not be empty for paired" - ); - } + if reader.is_paired() + && let Ok(record) = reader.get(0) + { + let xbuf = record.xbuf(); + let xlen = record.xlen(); + + if xlen > 0 { + assert!( + !xbuf.is_empty(), + "Extended buffer should not be empty for paired" + ); } } } @@ -1291,12 +1296,11 @@ mod tests { for i in 0..num_records { let record = reader.get(i); - assert!(record.is_ok(), "Should get record at index {}", i); + assert!(record.is_ok(), "Should get record at index {i}"); assert_eq!( record.unwrap().index() as usize, i, - "Record index mismatch at {}", - i + "Record index mismatch at {i}" ); } } @@ -1311,7 +1315,7 @@ mod tests { for &idx in &indices { let record = reader.get(idx); - assert!(record.is_ok(), "Should get record at index {}", idx); + assert!(record.is_ok(), "Should get record at index {idx}"); assert_eq!(record.unwrap().index() as usize, idx); } } diff --git a/src/cbq/core/block.rs b/src/cbq/core/block.rs index 2050bf5..b463f22 100644 --- a/src/cbq/core/block.rs +++ b/src/cbq/core/block.rs @@ -1125,7 +1125,7 @@ mod tests { #[test] fn test_usage() { let mut block = ColumnarBlock::new(unpaired_header(1 << 16)); - assert_eq!(block.usage(), 0.0); + assert!(block.usage().abs() < f64::EPSILON); let seq = vec![b'A'; 32]; block .push( diff --git a/src/cbq/read.rs b/src/cbq/read.rs index ab07913..2058bf6 100644 --- a/src/cbq/read.rs +++ b/src/cbq/read.rs @@ -355,9 +355,8 @@ mod tests { #[test] fn test_mmap_reader_is_paired() { let reader = MmapReader::new(TEST_CBQ_FILE).unwrap(); - let is_paired = reader.is_paired(); - // Test that the method returns a boolean - assert!(is_paired || !is_paired); + // The fixture file contains paired records + assert!(reader.is_paired()); } #[test] @@ -445,8 +444,7 @@ mod tests { let final_count = *count.lock().unwrap(); assert_eq!( final_count, expected_count, - "Should process exactly {} records", - expected_count + "Should process exactly {expected_count} records" ); } } diff --git a/src/cbq/write.rs b/src/cbq/write.rs index 8ff0b53..68de3f7 100644 --- a/src/cbq/write.rs +++ b/src/cbq/write.rs @@ -435,13 +435,7 @@ mod tests { (0..n_seq) .map(|i| { (0..seq_len) - .map(|j| { - if i % 5 == 0 { - b'N' - } else { - BASES[(i as usize + j) % 4] - } - }) + .map(|j| if i % 5 == 0 { b'N' } else { BASES[(i + j) % 4] }) .collect::>() }) .collect() diff --git a/src/error.rs b/src/error.rs index 9354801..ddbde48 100644 --- a/src/error.rs +++ b/src/error.rs @@ -371,30 +371,30 @@ mod testing { #[test] fn test_header_error_invalid_magic_number() { - let error = HeaderError::InvalidMagicNumber(0xDEADBEEF); - let error_str = format!("{}", error); + let error = HeaderError::InvalidMagicNumber(0xDEAD_BEEF); + let error_str = format!("{error}"); assert!(error_str.contains("0xdeadbeef") || error_str.contains("3735928559")); } #[test] fn test_header_error_invalid_format_version() { let error = HeaderError::InvalidFormatVersion(99); - let error_str = format!("{}", error); + let error_str = format!("{error}"); assert!(error_str.contains("99")); } #[test] fn test_header_error_invalid_bit_size() { let error = HeaderError::InvalidBitSize(8); - let error_str = format!("{}", error); - assert!(error_str.contains("8")); + let error_str = format!("{error}"); + assert!(error_str.contains('8')); assert!(error_str.contains("[2,4]")); } #[test] fn test_header_error_invalid_size() { let error = HeaderError::InvalidSize(100, 200); - let error_str = format!("{}", error); + let error_str = format!("{error}"); assert!(error_str.contains("100")); assert!(error_str.contains("200")); } @@ -407,7 +407,7 @@ mod testing { requested_index: 150, max_index: 100, }; - let error_str = format!("{}", error); + let error_str = format!("{error}"); assert!(error_str.contains("150")); assert!(error_str.contains("100")); } @@ -415,21 +415,21 @@ mod testing { #[test] fn test_read_error_file_truncation() { let error = ReadError::FileTruncation(12345); - let error_str = format!("{}", error); + let error_str = format!("{error}"); assert!(error_str.contains("12345")); } #[test] fn test_read_error_partial_record() { let error = ReadError::PartialRecord(42); - let error_str = format!("{}", error); + let error_str = format!("{error}"); assert!(error_str.contains("42")); } #[test] fn test_read_error_invalid_block_magic_number() { - let error = ReadError::InvalidBlockMagicNumber(0xBADC0DE, 1000); - let error_str = format!("{}", error); + let error = ReadError::InvalidBlockMagicNumber(0x0BAD_C0DE, 1000); + let error_str = format!("{error}"); assert!(error_str.contains("1000")); } @@ -442,7 +442,7 @@ mod testing { expected: true, actual: false, }; - let error_str = format!("{}", error); + let error_str = format!("{error}"); assert!(error_str.contains("paired")); assert!(error_str.contains("true")); assert!(error_str.contains("false")); @@ -454,7 +454,7 @@ mod testing { expected: 100, got: 150, }; - let error_str = format!("{}", error); + let error_str = format!("{error}"); assert!(error_str.contains("100")); assert!(error_str.contains("150")); } @@ -462,14 +462,14 @@ mod testing { #[test] fn test_write_error_invalid_nucleotide_sequence() { let error = WriteError::InvalidNucleotideSequence("ACGTNX".to_string()); - let error_str = format!("{}", error); + let error_str = format!("{error}"); assert!(error_str.contains("ACGTNX")); } #[test] fn test_write_error_record_size_exceeds_max() { let error = WriteError::RecordSizeExceedsMaximumBlockSize(2000, 1024); - let error_str = format!("{}", error); + let error_str = format!("{error}"); assert!(error_str.contains("2000")); assert!(error_str.contains("1024")); } @@ -482,7 +482,7 @@ mod testing { obs_primary: false, obs_extended: false, }; - let error_str = format!("{}", error); + let error_str = format!("{error}"); assert!(error_str.contains("Missing required sequence length")); } @@ -494,7 +494,7 @@ mod testing { max_block_size: 1024, record_size: 2048, }; - let error_str = format!("{}", error); + let error_str = format!("{error}"); assert!(error_str.contains("1024")); assert!(error_str.contains("2048")); } @@ -506,7 +506,7 @@ mod testing { record_size: 200, block_size: 1024, }; - let error_str = format!("{}", error); + let error_str = format!("{error}"); assert!(error_str.contains("900")); assert!(error_str.contains("200")); assert!(error_str.contains("1024")); @@ -518,7 +518,7 @@ mod testing { self_block_size: 1024, other_block_size: 2048, }; - let error_str = format!("{}", error); + let error_str = format!("{error}"); assert!(error_str.contains("1024")); assert!(error_str.contains("2048")); } @@ -528,7 +528,7 @@ mod testing { #[test] fn test_builder_error_missing_slen() { let error = BuilderError::MissingSlen; - let error_str = format!("{}", error); + let error_str = format!("{error}"); assert!(error_str.contains("Missing sequence length")); } @@ -537,7 +537,7 @@ mod testing { #[test] fn test_extension_error_unsupported() { let error = ExtensionError::UnsupportedExtension("test.xyz".to_string()); - let error_str = format!("{}", error); + let error_str = format!("{error}"); assert!(error_str.contains("test.xyz")); } @@ -588,7 +588,7 @@ mod testing { #[test] fn test_error_debug_output() { let error = Error::WriteError(WriteError::MissingHeader); - let debug_str = format!("{:?}", error); + let debug_str = format!("{error:?}"); assert!(debug_str.contains("WriteError")); } @@ -599,7 +599,7 @@ mod testing { fn test_fastx_error_empty_file() { use super::FastxEncodingError; let error = FastxEncodingError::EmptyFastxFile; - let error_str = format!("{}", error); + let error_str = format!("{error}"); assert!(error_str.contains("Empty FASTX file")); } @@ -608,7 +608,7 @@ mod testing { fn test_fastx_error_missing_input() { use super::FastxEncodingError; let error = FastxEncodingError::MissingInput; - let error_str = format!("{}", error); + let error_str = format!("{error}"); assert!(error_str.contains("not provided with any input")); } } diff --git a/src/parallel.rs b/src/parallel.rs index 605633b..8d7c190 100644 --- a/src/parallel.rs +++ b/src/parallel.rs @@ -264,8 +264,8 @@ mod testing { #[test] fn test_parallel_processor() { for ext in ["bq", "vbq", "cbq"] { - eprintln!("Testing {}", ext); - let reader = BinseqReader::new(&format!("./data/subset.{}", ext)).unwrap(); + eprintln!("Testing {ext}"); + let reader = BinseqReader::new(format!("./data/subset.{ext}")).unwrap(); let num_records = reader.num_records().unwrap(); let processor = TestProcessor::default(); assert!(reader.process_parallel(processor.clone(), 0).is_ok()); @@ -276,8 +276,8 @@ mod testing { #[test] fn test_parallel_processor_range() { for ext in ["bq", "vbq", "cbq"] { - eprintln!("Testing {}", ext); - let reader = BinseqReader::new(&format!("./data/subset.{}", ext)).unwrap(); + eprintln!("Testing {ext}"); + let reader = BinseqReader::new(format!("./data/subset.{ext}")).unwrap(); let processor = TestProcessor::default(); assert!( reader @@ -291,8 +291,8 @@ mod testing { #[test] fn test_parallel_processor_out_of_range_start() { for ext in ["bq", "vbq", "cbq"] { - eprintln!("Testing {}", ext); - let reader = BinseqReader::new(&format!("./data/subset.{}", ext)).unwrap(); + eprintln!("Testing {ext}"); + let reader = BinseqReader::new(format!("./data/subset.{ext}")).unwrap(); let processor = TestProcessor::default(); assert!( reader @@ -305,8 +305,8 @@ mod testing { #[test] fn test_parallel_processor_out_of_range_end() { for ext in ["bq", "vbq", "cbq"] { - eprintln!("Testing {}", ext); - let reader = BinseqReader::new(&format!("./data/subset.{}", ext)).unwrap(); + eprintln!("Testing {ext}"); + let reader = BinseqReader::new(format!("./data/subset.{ext}")).unwrap(); let processor = TestProcessor::default(); assert!( reader @@ -317,10 +317,13 @@ mod testing { } #[test] + // A backwards range (start > end) is intentionally passed here to verify + // that the function rejects it as invalid, not iterated over. + #[allow(clippy::reversed_empty_ranges)] fn test_parallel_processor_backwards_range() { for ext in ["bq", "vbq", "cbq"] { - eprintln!("Testing {}", ext); - let reader = BinseqReader::new(&format!("./data/subset.{}", ext)).unwrap(); + eprintln!("Testing {ext}"); + let reader = BinseqReader::new(format!("./data/subset.{ext}")).unwrap(); let processor = TestProcessor::default(); assert!(reader.process_parallel_range(processor, 0, 100..0).is_err()); } @@ -330,8 +333,8 @@ mod testing { fn test_set_decode_block() { for ext in ["bq", "vbq", "cbq"] { for opt in [true, false] { - eprintln!("Testing {} - decode {}", ext, opt); - let mut reader = BinseqReader::new(&format!("./data/subset.{}", ext)).unwrap(); + eprintln!("Testing {ext} - decode {opt}"); + let mut reader = BinseqReader::new(format!("./data/subset.{ext}")).unwrap(); reader.set_decode_block(opt); let num_records = reader.num_records().unwrap(); let processor = TestProcessor::default(); @@ -345,8 +348,8 @@ mod testing { fn test_set_default_quality_score() { for ext in ["bq", "vbq", "cbq"] { let default_score = b'#'; - eprintln!("Testing {} - default score: {}", ext, default_score); - let mut reader = BinseqReader::new(&format!("./data/subset.{}", ext)).unwrap(); + eprintln!("Testing {ext} - default score: {default_score}"); + let mut reader = BinseqReader::new(format!("./data/subset.{ext}")).unwrap(); reader.set_default_quality_score(default_score); let num_records = reader.num_records().unwrap(); let processor = TestProcessor::default(); diff --git a/src/policy.rs b/src/policy.rs index 6f8f39e..c09d326 100644 --- a/src/policy.rs +++ b/src/policy.rs @@ -405,7 +405,7 @@ mod tests { #[test] fn test_policy_debug() { let policy = Policy::SetToA; - let debug_str = format!("{:?}", policy); + let debug_str = format!("{policy:?}"); assert!(debug_str.contains("SetToA")); } diff --git a/src/vbq/reader.rs b/src/vbq/reader.rs index 23592f9..f9925ac 100644 --- a/src/vbq/reader.rs +++ b/src/vbq/reader.rs @@ -1389,9 +1389,8 @@ mod tests { #[test] fn test_mmap_reader_is_paired() { let reader = MmapReader::new(TEST_VBQ_FILE).unwrap(); - let is_paired = reader.is_paired(); - // Test that the method returns a boolean - assert!(is_paired || !is_paired); + // The fixture file contains paired records + assert!(reader.is_paired()); } #[test] @@ -1399,7 +1398,7 @@ mod tests { let reader = MmapReader::new(TEST_VBQ_FILE).unwrap(); let header = &reader.header; assert!(header.block > 0, "Expected non-zero block size"); - assert_eq!(header.magic, 0x51455356, "Expected VSEQ magic number"); + assert_eq!(header.magic, 0x5145_5356, "Expected VSEQ magic number"); } // ==================== RecordBlock Tests ==================== From 826126f2bd0bea5729e6149192a77807d66b772d Mon Sep 17 00:00:00 2001 From: noamteyssier <22600644+noamteyssier@users.noreply.github.com> Date: Tue, 7 Jul 2026 07:37:29 -0700 Subject: [PATCH 11/21] ci: added clippy warnings --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 3db7631..d145442 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -27,7 +27,7 @@ jobs: - name: Formatting run: cargo fmt --check - name: Linting - run: cargo clippy --all-features -- -D warnings + run: cargo clippy --all-features --all-targets -- -D warnings example_grep: runs-on: ubuntu-latest From ee703e99ec23e70fca340098dff6bab748f6a18d Mon Sep 17 00:00:00 2001 From: noamteyssier <22600644+noamteyssier@users.noreply.github.com> Date: Tue, 7 Jul 2026 07:37:45 -0700 Subject: [PATCH 12/21] style: clippy fix suggestions --- examples/auto-write.rs | 20 +++++++++----------- examples/grep.rs | 2 +- examples/parallel_range.rs | 24 ++++++++++++------------ examples/write.rs | 22 ++++++++++------------ 4 files changed, 32 insertions(+), 36 deletions(-) diff --git a/examples/auto-write.rs b/examples/auto-write.rs index 1a57292..8bcc88c 100644 --- a/examples/auto-write.rs +++ b/examples/auto-write.rs @@ -64,17 +64,15 @@ impl Args { fn format(&self) -> Format { if let Some(format) = self.format { format - } else { - if let Some(output) = &self.output { - match output.split(".").last() { - Some("bq") => Format::Bq, - Some("vbq") => Format::Vbq, - Some("cbq") => Format::Cbq, - _ => Format::default(), - } - } else { - Format::default() + } else if let Some(output) = &self.output { + match output.split('.').next_back() { + Some("bq") => Format::Bq, + Some("vbq") => Format::Vbq, + Some("cbq") => Format::Cbq, + _ => Format::default(), } + } else { + Format::default() } } fn bitsize(&self) -> BitSize { @@ -87,7 +85,7 @@ impl Args { /// Creates an output file handle fn ohandle(&self) -> Result { let path = if let Some(output) = &self.output { - output.to_string() + output.clone() } else { format!("{}{}", &self.prefix, self.format().extension()) }; diff --git a/examples/grep.rs b/examples/grep.rs index fcbc5d7..303851a 100644 --- a/examples/grep.rs +++ b/examples/grep.rs @@ -37,7 +37,7 @@ impl GrepCounter { } impl ParallelProcessor for GrepCounter { fn process_record(&mut self, record: R) -> binseq::Result<()> { - if self.match_sequence(&record.sseq()) || self.match_sequence(&record.xseq()) { + if self.match_sequence(record.sseq()) || self.match_sequence(record.xseq()) { self.local_count += 1; } diff --git a/examples/parallel_range.rs b/examples/parallel_range.rs index 3da00b4..2d7eacf 100644 --- a/examples/parallel_range.rs +++ b/examples/parallel_range.rs @@ -30,18 +30,18 @@ impl ParallelProcessor for RangeProcessor { let count = self.counter.fetch_add(1, Ordering::Relaxed); // Print progress every 10,000 records - if count % 10_000 == 0 { - if let Some(tid) = self.tid { - println!( - "Thread {}: Processed {} records (Range: {}-{}, Index: {}, Len: {})", - tid, - count + 1, - self.range_start, - self.range_end, - record.index(), - record.sseq().len(), - ); - } + if count.is_multiple_of(10_000) + && let Some(tid) = self.tid + { + println!( + "Thread {}: Processed {} records (Range: {}-{}, Index: {}, Len: {})", + tid, + count + 1, + self.range_start, + self.range_end, + record.index(), + record.sseq().len(), + ); } Ok(()) diff --git a/examples/write.rs b/examples/write.rs index 979880d..d6aab5b 100644 --- a/examples/write.rs +++ b/examples/write.rs @@ -75,17 +75,15 @@ impl Args { fn format(&self) -> Format { if let Some(format) = self.format { format - } else { - if let Some(output) = &self.output { - match output.split(".").last() { - Some("bq") => Format::Bq, - Some("vbq") => Format::Vbq, - Some("cbq") => Format::Cbq, - _ => Format::default(), - } - } else { - Format::default() + } else if let Some(output) = &self.output { + match output.split('.').next_back() { + Some("bq") => Format::Bq, + Some("vbq") => Format::Vbq, + Some("cbq") => Format::Cbq, + _ => Format::default(), } + } else { + Format::default() } } fn bitsize(&self) -> BitSize { @@ -98,7 +96,7 @@ impl Args { /// Creates an output file handle fn ohandle(&self) -> Result { let path = if let Some(output) = &self.output { - output.to_string() + output.clone() } else { format!("{}{}", &self.prefix, self.format().extension()) }; @@ -225,7 +223,7 @@ impl PairedParallelProcessor for Encoder { fn encode_paired(args: &Args) -> Result<()> { let mut r1 = fastx::Reader::from_path(&args.input)?; - let mut r2 = fastx::Reader::from_path(&args.input2.as_ref().expect("Missing input2"))?; + let mut r2 = fastx::Reader::from_path(args.input2.as_ref().expect("Missing input2"))?; let ohandle = args.ohandle()?; // prepare writer From 8ff162a65b5f8ac0423402941bcdeb6a28ade014 Mon Sep 17 00:00:00 2001 From: noamteyssier <22600644+noamteyssier@users.noreply.github.com> Date: Wed, 15 Jul 2026 18:46:33 -0700 Subject: [PATCH 13/21] dep: update dependencies --- Cargo.toml | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 6e31503..a8b2634 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -11,27 +11,27 @@ categories = ["science::bioinformatics", "encoding", "data-structures"] keywords = ["binary", "nucleotide", "sequencing", "genomics", "fastq"] [dependencies] -anyhow = {version = "1.0.100", optional = true} +anyhow = {version = "1.0.103", optional = true} auto_impl = "1.3.0" -bitnuc = "0.4.0" -bytemuck = { version = "1.24.0", features = ["derive", "extern_crate_alloc"] } +bitnuc = "0.4.1" +bytemuck = { version = "1.25.1", features = ["derive", "extern_crate_alloc"] } byteorder = "1.5.0" -itoa = "1.0.17" -memchr = "2.7.6" -memmap2 = "0.9.9" +itoa = "1.0.18" +memchr = "2.8.3" +memmap2 = "0.9.11" num_cpus = "1.17.0" -paraseq = { version = "0.4.8", optional = true } +paraseq = { version = "0.4.14", optional = true } parking_lot = {version = "0.12.5", optional = true } -rand = { version = "0.9.2", features = ["small_rng"] } +rand = { version = "0.9.5", features = ["small_rng"] } sucds = "0.8.3" -thiserror = "2.0.17" +thiserror = "2.0.18" zstd = { version = "0.13.3", features = ["zstdmt"] } [dev-dependencies] -anyhow = "1.0.100" +anyhow = "1.0.103" parking_lot = "0.12.5" -clap = { version = "4.5.54", features = ["derive"] } -paraseq = "0.4.8" +clap = { version = "4.6.2", features = ["derive"] } +paraseq = "0.4.14" [features] default = ["paraseq", "anyhow"] From f877ba1177992ba5c9a4e3dd278e4471f040af32 Mon Sep 17 00:00:00 2001 From: noamteyssier <22600644+noamteyssier@users.noreply.github.com> Date: Wed, 15 Jul 2026 18:55:00 -0700 Subject: [PATCH 14/21] docs: keep a changelog --- CHANGELOG.md | 167 +++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 167 insertions(+) create mode 100644 CHANGELOG.md diff --git a/CHANGELOG.md b/CHANGELOG.md new file mode 100644 index 0000000..fd3e66e --- /dev/null +++ b/CHANGELOG.md @@ -0,0 +1,167 @@ +# Changelog + +All notable changes to this project will be documented in this file. + +The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), +and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). + +## [0.9.4] - 2026-07-15 + +### Fixed + +- Bug in the `bq` stream reader that used an incorrect record id for small buffers. + +### Changed + +- Added clippy checks and additional lint allowances to CI, plus general style fixes. +- Improved test coverage throughout the library. + +## [0.9.3] - 2026-07-01 + +### Changed + +- `Binseq` input is now accepted as a path rather than a pre-opened handle. + +### Fixed + +- Use `reserve` instead of `resize` to avoid unnecessary zeroing/copies (issue #94). +- Ensure `len_nef` is cleared correctly. + +### Tests + +- Added coverage for the sequential reader and N-containing sequences. + +## [0.9.1] - 2026-06-25 + +### Added + +- Support for writing only completed blocks, enabling on-thread compression and + reducing lock contention for parallel writers. +- `on_thread_complete` callback support. +- `ingest_completed` is now passed through, with behavior changes scoped to `cbq`. + +### Tests + +- Added coverage for `cbq` ingestion. + +## [0.9.0] - 2026-01-23 + +### Added + +- Integrated `cbq` (compressed BINSEQ) into the core library as a first-class format. +- Builder-based construction of a file from an existing header. +- Auto-encode a FASTX file (or a pair of FASTX files) directly into a BINSEQ format via + a builder method. +- A generic writer abstraction over BINSEQ files. +- Default quality score handling across all readers. +- Elias-Fano encoding for N-positions instead of raw `u64` indices, reducing index size. +- An iterator over block headers, and convenience accessors for index metadata. + +### Changed + +- **Breaking:** renamed `BINSEQ`/`VBINSEQ` naming throughout to `BQ`/`VBQ` + (e.g. `BinseqHeader` → `bq::FileHeader`), and renamed several `vbq`/`bq`-specific + headers and writer structs for clarity. +- Reworked the write API around a sequence record struct. +- Reworked the internal index to track record count and sequence count separately. +- Replaced `anyhow`-based errors throughout the crate with crate-native error types. +- Removed the external `vbq` index file in favor of writing the index into the file itself. + +### Fixed + +- Several small hotfixes to the write path. +- Incorrect binary file used in a test fixture. + +## [0.8.1] - 2025-12-10 + +### Fixed + +- Incorrect decoding of the extended buffer on batch decoding in `bq` when a full + chunk of primary data was present (issue #75). + +## [0.8.0] - 2025-12-10 + +### Added + +- `sseq`/`xseq` functions on the record trait for use during batch decoding, implemented + for both `bq` and `vbq`. +- Additional usage examples. + +### Changed + +- **Breaking:** improved `bq` decoding via batch decoding, keeping data in SIMD form longer. +- Improved header access logic for zero-copy reads. + +### Fixed + +- Correct index returned from decoding. +- Correct slicing when per-record flags are set in `bq`. + +## [0.7.8] - 2025-12-09 + +### Changed + +- `vbq` reader is now zero-copy or single-copy depending on compression status. +- Simplified the encoding API. +- Reusable decompression context for improved throughput. +- Updated all dependencies. + +### Fixed + +- Validate the size of bytes provided to the non-compressed reader. + +## [0.7.7] - 2025-12-08 + +### Changed + +- Split the sequencing context into multiple traits with distinct variants + (renamed `FullCtx` to `Ctx`). +- Removed unsafe code from the context implementation in favor of a resize-copy pattern. + +### Fixed + +- Logical error in quality-score padding (`ensure_quality`). + +### Docs + +- Documented the different context variants. + +## [0.7.6] - 2025-12-08 + +### Added + +- A basic sequencing context struct, with mutable references to its internals. +- Ability to fill in missing quality scores when required. +- A generic error-conversion trait, made public. + +## [0.7.5] - 2025-10-02 + +### Added + +- BINSEQ v2 format. +- Optional headers in `vbq` files, with the index moved into the file itself + (no more auxiliary index file). +- Support for 4-bit encodings, with a `bitsize` re-export so downstream users don't + need to depend on `bitnuc` directly. +- Header builder pattern for constructing `bq` and `vbq` files. + +### Changed + +- Per-record flags are now optional across `bq`, `vbq`, and the record API. +- `vbq` index now uses `u64` for cumulative record counts. + +## [0.6.5] - 2025-07-01 + +Baseline release for this changelog. Earlier history predates version tagging. + +[Unreleased]: https://github.com/arcinstitute/binseq/compare/v0.9.3...HEAD +[0.9.3]: https://github.com/arcinstitute/binseq/compare/v0.9.1...v0.9.3 +[0.9.1]: https://github.com/arcinstitute/binseq/compare/v0.9.0...v0.9.1 +[0.9.0]: https://github.com/arcinstitute/binseq/compare/v0.8.1...v0.9.0 +[0.8.1]: https://github.com/arcinstitute/binseq/compare/v0.8.0...v0.8.1 +[0.8.0]: https://github.com/arcinstitute/binseq/compare/v0.7.8...v0.8.0 +[0.7.8]: https://github.com/arcinstitute/binseq/compare/v0.7.7...v0.7.8 +[0.7.7]: https://github.com/arcinstitute/binseq/compare/v0.7.6...v0.7.7 +[0.7.6]: https://github.com/arcinstitute/binseq/compare/v0.7.5...v0.7.6 +[0.7.5]: https://github.com/arcinstitute/binseq/compare/v0.6.5...v0.7.5 +[0.6.5]: https://github.com/arcinstitute/binseq/releases/tag/v0.6.5 From 0a18840dbfbab883751b0d2221bd3366ba041162 Mon Sep 17 00:00:00 2001 From: noamteyssier <22600644+noamteyssier@users.noreply.github.com> Date: Wed, 15 Jul 2026 18:55:50 -0700 Subject: [PATCH 15/21] chore(semver): bump --- Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index b3f5710..01b01e1 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "binseq" -version = "0.9.3" +version = "0.9.4" edition = "2024" description = "A high efficiency binary format for sequencing data" license = "MIT" From a191e1f31537baeee533a2ea1f8a9eec993a5d49 Mon Sep 17 00:00:00 2001 From: noamteyssier <22600644+noamteyssier@users.noreply.github.com> Date: Wed, 15 Jul 2026 19:00:54 -0700 Subject: [PATCH 16/21] style: clippy fix suggestions --- examples/auto-write.rs | 2 +- examples/write.rs | 2 +- src/cbq/write.rs | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/examples/auto-write.rs b/examples/auto-write.rs index 8bcc88c..cffc16b 100644 --- a/examples/auto-write.rs +++ b/examples/auto-write.rs @@ -87,7 +87,7 @@ impl Args { let path = if let Some(output) = &self.output { output.clone() } else { - format!("{}{}", &self.prefix, self.format().extension()) + format!("{}{}", self.prefix, self.format().extension()) }; let ofile = File::create(path).map(BufWriter::new)?; Ok(Box::new(ofile)) diff --git a/examples/write.rs b/examples/write.rs index d6aab5b..c280998 100644 --- a/examples/write.rs +++ b/examples/write.rs @@ -98,7 +98,7 @@ impl Args { let path = if let Some(output) = &self.output { output.clone() } else { - format!("{}{}", &self.prefix, self.format().extension()) + format!("{}{}", self.prefix, self.format().extension()) }; let ofile = std::fs::File::create(path).map(BufWriter::new)?; Ok(Box::new(ofile)) diff --git a/src/cbq/write.rs b/src/cbq/write.rs index 68de3f7..e94df6c 100644 --- a/src/cbq/write.rs +++ b/src/cbq/write.rs @@ -270,7 +270,7 @@ mod tests { /// 64 distinct fixed-length sequences (each unique by index). fn sample_sequences() -> Vec> { - const BASES: [u8; 4] = [b'A', b'C', b'G', b'T']; + const BASES: [u8; 4] = *b"ACGT"; (0..64u32) .map(|i| { (0..40) @@ -431,7 +431,7 @@ mod tests { /// Blocks with no `N`s at all never populate `z_npos`, so a suite built /// only from `sample_sequences` (all ACGT) never exercises this path. fn sample_sequences_with_n(n_seq: usize, seq_len: usize) -> Vec> { - const BASES: [u8; 4] = [b'A', b'C', b'G', b'T']; + const BASES: [u8; 4] = *b"ACGT"; (0..n_seq) .map(|i| { (0..seq_len) From 3270b5e1772557ab76a41c439339df8f4bbee27d Mon Sep 17 00:00:00 2001 From: noamteyssier <22600644+noamteyssier@users.noreply.github.com> Date: Wed, 15 Jul 2026 19:12:44 -0700 Subject: [PATCH 17/21] refactor: expose magic through crate --- src/bq/header.rs | 5 +++++ src/bq/mod.rs | 2 +- src/vbq/header.rs | 5 +++++ src/vbq/mod.rs | 2 +- 4 files changed, 12 insertions(+), 2 deletions(-) diff --git a/src/bq/header.rs b/src/bq/header.rs index 61adf24..eed8c2f 100644 --- a/src/bq/header.rs +++ b/src/bq/header.rs @@ -16,6 +16,11 @@ use crate::error::{BuilderError, HeaderError, Result}; #[allow(clippy::unreadable_literal)] const MAGIC: u32 = 0x51455342; +/// The magic bytes as they appear at the start of a BQ file on disk. +/// +/// Used to identify BQ files by content rather than by file extension. +pub const FILE_MAGIC: [u8; 4] = MAGIC.to_le_bytes(); + /// Current format version of the binary sequence file format /// /// This version number allows for future format changes while maintaining backward compatibility. diff --git a/src/bq/mod.rs b/src/bq/mod.rs index fd194f6..47fa622 100644 --- a/src/bq/mod.rs +++ b/src/bq/mod.rs @@ -241,6 +241,6 @@ mod header; mod reader; mod writer; -pub use header::{FileHeader, FileHeaderBuilder, SIZE_HEADER}; +pub use header::{FILE_MAGIC, FileHeader, FileHeaderBuilder, SIZE_HEADER}; pub use reader::{MmapReader, RefRecord, StreamReader}; pub use writer::{Encoder, StreamWriter, StreamWriterBuilder, Writer, WriterBuilder}; diff --git a/src/vbq/header.rs b/src/vbq/header.rs index 29a88b3..6aa8ded 100644 --- a/src/vbq/header.rs +++ b/src/vbq/header.rs @@ -25,6 +25,11 @@ use crate::error::{HeaderError, ReadError, Result}; #[allow(clippy::unreadable_literal)] const MAGIC: u32 = 0x51455356; +/// The magic bytes as they appear at the start of a VBQ file on disk. +/// +/// Used to identify VBQ files by content rather than by file extension. +pub const FILE_MAGIC: [u8; 4] = MAGIC.to_le_bytes(); + /// Magic number for block identification: "BLOCKSEQ" in ASCII (0x5145534B434F4C42) /// /// This constant is used in block headers to validate block integrity. diff --git a/src/vbq/mod.rs b/src/vbq/mod.rs index 230cca0..12cc4bf 100644 --- a/src/vbq/mod.rs +++ b/src/vbq/mod.rs @@ -150,7 +150,7 @@ mod index; mod reader; mod writer; -pub use header::{BlockHeader, FileHeader, FileHeaderBuilder}; +pub use header::{BlockHeader, FILE_MAGIC, FileHeader, FileHeaderBuilder}; pub use index::{BlockIndex, BlockRange}; pub use reader::{MmapReader, RecordBlock, RecordBlockIter, RefRecord}; pub use writer::{Writer, WriterBuilder}; From 2dd17a59c52c7305f633f3d0e1488c4466a9abdf Mon Sep 17 00:00:00 2001 From: noamteyssier <22600644+noamteyssier@users.noreply.github.com> Date: Wed, 15 Jul 2026 19:12:55 -0700 Subject: [PATCH 18/21] feat: added format detection error type --- src/error.rs | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/error.rs b/src/error.rs index ddbde48..1c76ed9 100644 --- a/src/error.rs +++ b/src/error.rs @@ -40,8 +40,8 @@ pub enum Error { #[error("Error with UTF8: {0}")] Utf8Error(#[from] std::str::Utf8Error), - /// Errors related to missing extensions - ExtensionError(#[from] ExtensionError), + /// Errors related to determining the BINSEQ format of a file + FormatError(#[from] FormatError), /// Errors from the bitnuc dependency for nucleotide encoding/decoding #[error("Bitnuc error: {0}")] @@ -327,10 +327,10 @@ pub enum FastxEncodingError { } #[derive(thiserror::Error, Debug)] -pub enum ExtensionError { - /// When the extension is not supported - #[error("Unsupported extension in path: {0}")] - UnsupportedExtension(String), +pub enum FormatError { + /// When the BINSEQ format could not be determined from a file's magic bytes + #[error("Unable to determine BINSEQ format from magic bytes in file: {0}")] + UnrecognizedMagicBytes(String), } /// Trait for converting arbitrary errors into `Error` @@ -532,11 +532,11 @@ mod testing { assert!(error_str.contains("Missing sequence length")); } - // ==================== ExtensionError Tests ==================== + // ==================== FormatError Tests ==================== #[test] - fn test_extension_error_unsupported() { - let error = ExtensionError::UnsupportedExtension("test.xyz".to_string()); + fn test_format_error_unrecognized_magic_bytes() { + let error = FormatError::UnrecognizedMagicBytes("test.xyz".to_string()); let error_str = format!("{error}"); assert!(error_str.contains("test.xyz")); } From fce6fa79f026255f192f264fb0f518a654a841f3 Mon Sep 17 00:00:00 2001 From: noamteyssier <22600644+noamteyssier@users.noreply.github.com> Date: Wed, 15 Jul 2026 19:13:41 -0700 Subject: [PATCH 19/21] feat: determine format from magic bytes --- src/parallel.rs | 67 ++++++++++++++++++++++++++++++++++++++----------- src/write.rs | 34 +++++++++++++++++++++++++ 2 files changed, 86 insertions(+), 15 deletions(-) diff --git a/src/parallel.rs b/src/parallel.rs index 8d7c190..870a4eb 100644 --- a/src/parallel.rs +++ b/src/parallel.rs @@ -1,12 +1,30 @@ +use std::fs::File; +use std::io::Read as _; use std::ops::Range; use std::path::Path; use crate::{ BinseqRecord, Result, bq, cbq, - error::{ExtensionError, ReadError}, + error::{FormatError, ReadError}, vbq, + write::Format, }; +/// Number of leading bytes read from a file to identify its BINSEQ format. +/// +/// This must be at least as long as the longest format magic sequence (CBQ's, at 7 bytes). +const MAGIC_PEEK_LEN: usize = 7; + +/// Determines the BINSEQ format of a file by inspecting its leading magic bytes. +fn sniff_format>(path: P) -> Result { + let file = File::open(path.as_ref())?; + let mut buffer = Vec::with_capacity(MAGIC_PEEK_LEN); + file.take(MAGIC_PEEK_LEN as u64).read_to_end(&mut buffer)?; + Format::sniff(&buffer).ok_or_else(|| { + FormatError::UnrecognizedMagicBytes(path.as_ref().to_string_lossy().to_string()).into() + }) +} + /// An enum abstraction for BINSEQ readers that can process records in parallel /// /// This is a convenience enum that can be used for general workflows where the @@ -24,20 +42,10 @@ pub enum BinseqReader { } impl BinseqReader { pub fn new>(path: P) -> Result { - match path.as_ref().extension() { - Some(ext) => match ext.to_str() { - Some("bq") => Ok(Self::Bq(bq::MmapReader::new(path)?)), - Some("vbq") => Ok(Self::Vbq(vbq::MmapReader::new(path)?)), - Some("cbq") => Ok(Self::Cbq(cbq::MmapReader::new(path)?)), - _ => Err(ExtensionError::UnsupportedExtension( - path.as_ref().to_string_lossy().to_string(), - ) - .into()), - }, - None => Err(ExtensionError::UnsupportedExtension( - path.as_ref().to_string_lossy().to_string(), - ) - .into()), + match sniff_format(&path)? { + Format::Bq => Ok(Self::Bq(bq::MmapReader::new(path)?)), + Format::Vbq => Ok(Self::Vbq(vbq::MmapReader::new(path)?)), + Format::Cbq => Ok(Self::Cbq(cbq::MmapReader::new(path)?)), } } @@ -250,6 +258,35 @@ mod testing { use super::*; + #[test] + fn test_new_ignores_extension_uses_magic_bytes() { + let dir = std::env::temp_dir(); + + // A CBQ file saved with a .bq extension should still be read as CBQ. + let wrong_ext = dir.join("binseq_sniff_wrong_ext.bq"); + std::fs::copy("./data/subset.cbq", &wrong_ext).unwrap(); + let reader = BinseqReader::new(&wrong_ext).unwrap(); + assert!(matches!(reader, BinseqReader::Cbq(_))); + + // A BQ file with no extension at all should still be detected. + let no_ext = dir.join("binseq_sniff_no_ext"); + std::fs::copy("./data/subset.bq", &no_ext).unwrap(); + let reader = BinseqReader::new(&no_ext).unwrap(); + assert!(matches!(reader, BinseqReader::Bq(_))); + + std::fs::remove_file(&wrong_ext).unwrap(); + std::fs::remove_file(&no_ext).unwrap(); + } + + #[test] + fn test_new_unrecognized_file_errors() { + let dir = std::env::temp_dir(); + let junk = dir.join("binseq_sniff_junk.cbq"); + std::fs::write(&junk, b"not a binseq file at all").unwrap(); + assert!(BinseqReader::new(&junk).is_err()); + std::fs::remove_file(&junk).unwrap(); + } + #[derive(Clone, Default)] struct TestProcessor { pub n_records: Arc>, diff --git a/src/write.rs b/src/write.rs index fe23240..9d0ab17 100644 --- a/src/write.rs +++ b/src/write.rs @@ -96,6 +96,24 @@ impl Format { Self::Cbq => ".cbq", } } + + /// Determines the BINSEQ format by inspecting the magic bytes at the start of a buffer. + /// + /// This identifies the format from file content rather than a file extension, so it + /// works regardless of how the file is named. Returns `None` if `bytes` is too short + /// or does not start with a recognized magic sequence. + #[must_use] + pub fn sniff(bytes: &[u8]) -> Option { + if bytes.starts_with(cbq::FILE_MAGIC) { + Some(Self::Cbq) + } else if bytes.starts_with(&bq::FILE_MAGIC) { + Some(Self::Bq) + } else if bytes.starts_with(&vbq::FILE_MAGIC) { + Some(Self::Vbq) + } else { + None + } + } } /// Builder for creating [`BinseqWriter`] instances @@ -646,6 +664,22 @@ mod tests { assert_eq!(Format::Cbq.extension(), ".cbq"); } + #[test] + fn test_format_sniff() { + assert_eq!(Format::sniff(&bq::FILE_MAGIC), Some(Format::Bq)); + assert_eq!(Format::sniff(&vbq::FILE_MAGIC), Some(Format::Vbq)); + assert_eq!(Format::sniff(cbq::FILE_MAGIC), Some(Format::Cbq)); + + // Trailing bytes after the magic sequence are ignored. + let mut cbq_like = cbq::FILE_MAGIC.to_vec(); + cbq_like.extend_from_slice(b"trailing data"); + assert_eq!(Format::sniff(&cbq_like), Some(Format::Cbq)); + + assert_eq!(Format::sniff(b"not a binseq file"), None); + assert_eq!(Format::sniff(&[]), None); + assert_eq!(Format::sniff(b"BS"), None); + } + #[test] fn test_build_bq_writer() -> Result<()> { let writer = BinseqWriterBuilder::new(Format::Bq) From e65f4b3c9b32901c5eb0c7d3698ef52572d53a39 Mon Sep 17 00:00:00 2001 From: noamteyssier <22600644+noamteyssier@users.noreply.github.com> Date: Wed, 15 Jul 2026 19:13:48 -0700 Subject: [PATCH 20/21] docs: update changelog --- CHANGELOG.md | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index fd3e66e..0782fe2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,10 +11,20 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Bug in the `bq` stream reader that used an incorrect record id for small buffers. +### Added + +- `BinseqReader::new` now determines a file's BINSEQ format (BQ, VBQ, or CBQ) by sniffing its + magic bytes instead of relying on the file extension, so it works regardless of how the file + is named. +- `Format::sniff` for identifying a BINSEQ format from a byte buffer, plus public `FILE_MAGIC` + constants on `bq`, `vbq`, and `cbq`. + ### Changed - Added clippy checks and additional lint allowances to CI, plus general style fixes. - Improved test coverage throughout the library. +- Renamed `ExtensionError` to `FormatError` (`UnrecognizedMagicBytes` variant) to reflect + detection now being based on file content rather than the file extension. ## [0.9.3] - 2026-07-01 From b018101fb7e4a2acad786c19da91ac443f4f37fd Mon Sep 17 00:00:00 2001 From: noamteyssier <22600644+noamteyssier@users.noreply.github.com> Date: Wed, 15 Jul 2026 20:47:32 -0700 Subject: [PATCH 21/21] refactor: allocate on stack instead of heap --- src/parallel.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/parallel.rs b/src/parallel.rs index 870a4eb..eedd96c 100644 --- a/src/parallel.rs +++ b/src/parallel.rs @@ -18,8 +18,8 @@ const MAGIC_PEEK_LEN: usize = 7; /// Determines the BINSEQ format of a file by inspecting its leading magic bytes. fn sniff_format>(path: P) -> Result { let file = File::open(path.as_ref())?; - let mut buffer = Vec::with_capacity(MAGIC_PEEK_LEN); - file.take(MAGIC_PEEK_LEN as u64).read_to_end(&mut buffer)?; + let mut buffer = [0u8; MAGIC_PEEK_LEN]; + file.take(MAGIC_PEEK_LEN as u64).read_exact(&mut buffer)?; Format::sniff(&buffer).ok_or_else(|| { FormatError::UnrecognizedMagicBytes(path.as_ref().to_string_lossy().to_string()).into() })