Skip to content

Commit 54fc50c

Browse files
rob-pclaude
andcommitted
test: build option structs by parsing, not by struct literal
`MultiplexQuantOpts` and ATAC `ProcessOpts` were constructed in tests with struct literals naming every field. A literal must name all of them, so each new CLI option is an E0063 in that file -- and `cargo build` does not compile tests, so it only surfaces under `cargo test` / `--all-targets`, typically after the change looks like it compiles. The same literal has now broken three times: 53c7f31 (--sample-bc-ori, arriving broken in PR #199), b3192a3 (the flattened decoder options), and --small-thresh today. Meanwhile the equivalent tests for MapQuantOpts never broke, because quant.rs builds its opts with `Cli::parse_from`. Adopt that pattern in both places: new options pick up their clap defaults, and an option that is genuinely required fails loudly at parse time naming itself. `atac process` requires a read-input form at parse time, so the helper supplies one and then clears it, keeping the exact baseline the assertions were written against. The AtacChemistry/Macs3GenomeSize imports existed only to name types in the literal and are now unused. Four small literals remain in chemistry.rs, where each test exercises specific field combinations and the structs are stable; those are left alone. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VhyNvEWUgY8ALVacdJHxVJ
1 parent a72fd59 commit 54fc50c

2 files changed

Lines changed: 53 additions & 69 deletions

File tree

src/atac/process.rs

Lines changed: 36 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -621,46 +621,45 @@ fn af_gpl(af_home_path: &Path, opts: &ProcessOpts) -> anyhow::Result<GplStageOut
621621
mod tests {
622622
use std::fs;
623623

624-
use crate::atac::commands::{AtacChemistry, Macs3GenomeSize};
625-
626624
use super::*;
627625

626+
/// Build `ProcessOpts` the way a user would: by parsing a command line.
627+
/// A struct literal has to name every field, so adding an option to the
628+
/// CLI breaks this file with an E0063 that only `--all-targets` surfaces
629+
/// -- `cargo build` does not compile tests. Parsing instead lets new
630+
/// options pick up their clap defaults, and anything genuinely required
631+
/// fails loudly at parse time.
628632
fn base_process_opts() -> ProcessOpts {
629-
ProcessOpts {
630-
decode: crate::simpleaf_commands::PiscemDecoderOpts {
631-
decoder: String::from("auto"),
632-
thread_policy: None,
633-
},
634-
index: PathBuf::from("/tmp/index"),
635-
reads1: None,
636-
reads2: None,
637-
reads: None,
638-
barcode_reads: vec![],
639-
chemistry: AtacChemistry::TenxV2,
640-
barcode_length: 16,
641-
output: PathBuf::from("/tmp/out"),
642-
threads: 1,
643-
call_peaks: false,
644-
permit_barcode_ori: None,
645-
unfiltered_pl: None,
646-
min_reads: 10,
647-
compress: false,
648-
ignore_ambig_hits: false,
649-
no_poison: false,
650-
use_chr: false,
651-
thr: 0.8,
652-
bin_size: 50,
653-
bin_overlap: 2,
654-
no_tn5_shift: false,
655-
check_kmer_orphan: false,
656-
max_ec_card: 4096,
657-
max_hit_occ: 64,
658-
max_hit_occ_recover: 1024,
659-
max_read_occ: 250,
660-
gsize: Macs3GenomeSize::KnownOpt("hs"),
661-
qvalue: 0.1,
662-
extsize: 50,
663-
}
633+
use clap::Parser;
634+
let cli_args = vec![
635+
"simpleaf",
636+
"atac",
637+
"process",
638+
"--index",
639+
"/tmp/index",
640+
"--output",
641+
"/tmp/out",
642+
"--chemistry",
643+
"10x-v2",
644+
"--barcode-reads",
645+
"/tmp/bc.fastq",
646+
"--threads",
647+
"1",
648+
// one of the read-input forms is required at parse time; the
649+
// tests below supply their own, so clear it back out afterwards
650+
// to keep the baseline these assertions were written against.
651+
"--reads",
652+
"/tmp/r.fastq",
653+
];
654+
let mut opts = match crate::Cli::parse_from(cli_args).command {
655+
crate::simpleaf_commands::Commands::Atac(
656+
crate::atac::commands::AtacCommand::Process(opts),
657+
) => opts,
658+
cmd => panic!("expected atac process command, found {:?}", cmd),
659+
};
660+
opts.reads = None;
661+
opts.barcode_reads = vec![];
662+
opts
664663
}
665664

666665
#[test]

src/simpleaf_commands/multiplex_quant.rs

Lines changed: 17 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -783,6 +783,7 @@ mod tests {
783783
use super::{resolve_user_supplied_index, t2g_mode};
784784
use crate::simpleaf_commands::MultiplexQuantOpts;
785785
use crate::utils::probe_utils::ProbeT2gMode;
786+
use clap::Parser;
786787
use serde_json::json;
787788
use std::fs;
788789
use std::path::Path;
@@ -857,40 +858,24 @@ mod tests {
857858
assert!(gene_id_to_name.is_none());
858859
}
859860

861+
/// Build `MultiplexQuantOpts` the way a user would: by parsing a command
862+
/// line. A struct literal has to name every field, so adding an option to
863+
/// the CLI breaks this file with an E0063 that only `--all-targets`
864+
/// surfaces -- `cargo build` does not compile tests. Parsing instead lets
865+
/// new options pick up their clap defaults, and anything genuinely
866+
/// required fails loudly at parse time.
867+
fn parse_multiplex_quant_opts(args: &[&str]) -> MultiplexQuantOpts {
868+
let mut cli_args = vec!["simpleaf", "multiplex-quant"];
869+
cli_args.extend_from_slice(args);
870+
match crate::Cli::parse_from(cli_args).command {
871+
crate::simpleaf_commands::Commands::MultiplexQuant(opts) => opts,
872+
cmd => panic!("expected multiplex-quant command, found {:?}", cmd),
873+
}
874+
}
875+
860876
#[test]
861877
fn usa_flag_maps_to_usa_mode() {
862-
let opts = MultiplexQuantOpts {
863-
chemistry: None,
864-
geometry: None,
865-
organism: None,
866-
cell_bc_list: None,
867-
expected_ori: String::from("both"),
868-
sample_bc_ori: None,
869-
decode: crate::simpleaf_commands::PiscemDecoderOpts {
870-
decoder: String::from("auto"),
871-
thread_policy: None,
872-
},
873-
sample_correction_mode: String::from("exact"),
874-
output: Path::new(".").to_path_buf(),
875-
threads: 1,
876-
index: None,
877-
probe_set: None,
878-
t2g_map: None,
879-
usa: true,
880-
sample_bc_list: None,
881-
reads1: Vec::new(),
882-
reads2: Vec::new(),
883-
resolution: String::from("cr-like"),
884-
small_thresh: None,
885-
kmer_length: 23,
886-
skipping_strategy: String::from("permissive"),
887-
struct_constraints: false,
888-
max_ec_card: 4096,
889-
dict: crate::simpleaf_commands::PiscemDict::Auto,
890-
min_reads: 10,
891-
anndata_out: false,
892-
};
893-
878+
let opts = parse_multiplex_quant_opts(&["-o", ".", "--usa"]);
894879
assert_eq!(t2g_mode(&opts), ProbeT2gMode::Usa);
895880
}
896881

0 commit comments

Comments
 (0)