Skip to content

Commit de23124

Browse files
GiggleLiuclaude
andcommitted
fix: cover all error branches of validate_variant_aliases for codecov
Refactor validation into a testable `validate_aliases_inner` that accepts mock data, so every conflict branch (empty alias, canonical-name collision, problem-alias collision, duplicate variant alias, multi-conflict) is exercised without depending on inventory state. Also test `variant_label` with both empty and non-empty variant dimensions. Patch coverage: 15 uncovered lines → ~2 (the inventory integration test panic branch, which is inherently unreachable on a healthy codebase). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent b55f927 commit de23124

2 files changed

Lines changed: 137 additions & 6 deletions

File tree

src/registry/variant.rs

Lines changed: 20 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,6 @@ pub fn find_variant_by_alias(
8888
/// problem names or problem-level aliases, and empty aliases for manually
8989
/// constructed [`VariantEntry`] values that bypass `declare_variants!`.
9090
pub fn validate_variant_aliases() -> Result<(), Vec<String>> {
91-
let mut conflicts = Vec::new();
9291
let mut problem_names: BTreeMap<String, Vec<String>> = BTreeMap::new();
9392

9493
for problem in super::problem_type::problem_types() {
@@ -111,10 +110,26 @@ pub fn validate_variant_aliases() -> Result<(), Vec<String>> {
111110
}
112111
}
113112

113+
let entries: Vec<_> = inventory::iter::<VariantEntry>()
114+
.map(|e| (variant_label(e), e.aliases))
115+
.collect();
116+
117+
validate_aliases_inner(&problem_names, &entries)
118+
}
119+
120+
/// Core validation logic, separated for testability with mock data.
121+
///
122+
/// - `problem_names`: lowercase key → list of human-readable sources (canonical names + problem-level aliases).
123+
/// - `entries`: `(variant_label, aliases_slice)` per variant entry.
124+
pub fn validate_aliases_inner(
125+
problem_names: &BTreeMap<String, Vec<String>>,
126+
entries: &[(String, &[&str])],
127+
) -> Result<(), Vec<String>> {
128+
let mut conflicts = Vec::new();
114129
let mut variant_aliases: BTreeMap<String, Vec<(String, String)>> = BTreeMap::new();
115-
for entry in inventory::iter::<VariantEntry> {
116-
let target = variant_label(entry);
117-
for alias in entry.aliases {
130+
131+
for (target, aliases) in entries {
132+
for alias in *aliases {
118133
if alias.trim().is_empty() {
119134
conflicts.push(format!(
120135
"variant-level alias on {target} is empty or whitespace-only"
@@ -159,7 +174,7 @@ pub fn validate_variant_aliases() -> Result<(), Vec<String>> {
159174
}
160175
}
161176

162-
fn variant_label(entry: &VariantEntry) -> String {
177+
pub fn variant_label(entry: &VariantEntry) -> String {
163178
let variant = entry.variant();
164179
if variant.is_empty() {
165180
return entry.name.to_string();

src/unit_tests/registry/variant.rs

Lines changed: 117 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,124 @@
1-
use crate::registry::variant::validate_variant_aliases;
1+
use crate::registry::variant::{validate_variant_aliases, variant_label};
2+
use std::collections::BTreeMap;
23

34
#[test]
45
fn variant_alias_inventory_is_valid() {
56
if let Err(conflicts) = validate_variant_aliases() {
67
panic!("variant alias validation failed:\n{}", conflicts.join("\n"));
78
}
89
}
10+
11+
// --- validate_aliases_inner unit tests ---
12+
13+
use crate::registry::variant::validate_aliases_inner;
14+
15+
fn empty_problem_names() -> BTreeMap<String, Vec<String>> {
16+
BTreeMap::new()
17+
}
18+
19+
#[test]
20+
fn validate_inner_accepts_valid_aliases() {
21+
let entries = vec![
22+
("Foo {k=K3}".to_string(), &["3FOO"][..]),
23+
("Foo {k=K2}".to_string(), &["2FOO"][..]),
24+
];
25+
assert!(validate_aliases_inner(&empty_problem_names(), &entries).is_ok());
26+
}
27+
28+
#[test]
29+
fn validate_inner_rejects_empty_alias() {
30+
let entries = vec![("Foo {k=K3}".to_string(), &[""][..])];
31+
let err = validate_aliases_inner(&empty_problem_names(), &entries).unwrap_err();
32+
assert_eq!(err.len(), 1);
33+
assert!(
34+
err[0].contains("empty or whitespace-only"),
35+
"expected empty alias error, got: {}",
36+
err[0]
37+
);
38+
}
39+
40+
#[test]
41+
fn validate_inner_rejects_whitespace_only_alias() {
42+
let entries = vec![("Foo".to_string(), &[" \t"][..])];
43+
let err = validate_aliases_inner(&empty_problem_names(), &entries).unwrap_err();
44+
assert!(err[0].contains("empty or whitespace-only"));
45+
}
46+
47+
#[test]
48+
fn validate_inner_rejects_collision_with_canonical_name() {
49+
let mut names = BTreeMap::new();
50+
names
51+
.entry("bar".to_string())
52+
.or_insert_with(Vec::new)
53+
.push("canonical problem name `Bar`".to_string());
54+
55+
let entries = vec![("Foo {k=K3}".to_string(), &["BAR"][..])];
56+
let err = validate_aliases_inner(&names, &entries).unwrap_err();
57+
assert_eq!(err.len(), 1);
58+
assert!(err[0].contains("conflicts with canonical problem name"));
59+
}
60+
61+
#[test]
62+
fn validate_inner_rejects_collision_with_problem_level_alias() {
63+
let mut names = BTreeMap::new();
64+
names
65+
.entry("baz".to_string())
66+
.or_insert_with(Vec::new)
67+
.push("problem-level alias `BAZ` for `Bazinga`".to_string());
68+
69+
let entries = vec![("Foo".to_string(), &["baz"][..])];
70+
let err = validate_aliases_inner(&names, &entries).unwrap_err();
71+
assert_eq!(err.len(), 1);
72+
assert!(err[0].contains("conflicts with problem-level alias"));
73+
}
74+
75+
#[test]
76+
fn validate_inner_rejects_duplicate_variant_aliases() {
77+
let entries = vec![
78+
("Foo {k=K3}".to_string(), &["DUP"][..]),
79+
("Bar {k=K2}".to_string(), &["dup"][..]),
80+
];
81+
let err = validate_aliases_inner(&empty_problem_names(), &entries).unwrap_err();
82+
assert_eq!(err.len(), 1);
83+
assert!(
84+
err[0].contains("duplicate variant-level alias"),
85+
"expected duplicate error, got: {}",
86+
err[0]
87+
);
88+
}
89+
90+
#[test]
91+
fn validate_inner_reports_multiple_conflicts() {
92+
let entries = vec![
93+
("A".to_string(), &[""][..]),
94+
("B".to_string(), &["X"][..]),
95+
("C".to_string(), &["x"][..]),
96+
];
97+
let err = validate_aliases_inner(&empty_problem_names(), &entries).unwrap_err();
98+
assert_eq!(err.len(), 2, "expected 2 conflicts, got: {err:?}");
99+
}
100+
101+
// --- variant_label unit tests ---
102+
103+
#[test]
104+
fn variant_label_bare_problem() {
105+
// Find a VariantEntry with no variant dimensions (empty variant list).
106+
// QUBO is a standalone problem with no variants.
107+
let entry = inventory::iter::<crate::registry::VariantEntry>()
108+
.find(|e| e.variant().is_empty())
109+
.expect("expected at least one VariantEntry with empty variant");
110+
let label = variant_label(entry);
111+
assert_eq!(label, entry.name);
112+
}
113+
114+
#[test]
115+
fn variant_label_with_variant_dimensions() {
116+
let entry = inventory::iter::<crate::registry::VariantEntry>()
117+
.find(|e| e.name == "KSatisfiability" && e.aliases.contains(&"3SAT"))
118+
.expect("expected KSatisfiability<K3> VariantEntry");
119+
let label = variant_label(entry);
120+
assert!(
121+
label.contains("k=K3"),
122+
"expected label to include k=K3, got: {label}"
123+
);
124+
}

0 commit comments

Comments
 (0)