test: expand insta snapshot coverage across pre-existing tests - #171
Merged
Conversation
Convert 39 value-asserting test sites (across 10 modules) from hand-transcribed struct literals and substring-probe clusters to `insta` snapshots, growing snapshot coverage from 8 sites to 47. Snapshots are more observable regression targets: they capture whole structures (so counts are implied), and subsume negative `!contains(...)` guards by rendering the full output. Scope was limited to genuine *value* checks — multi-field struct/enum equality, collection contents, formatted diagnostics/error messages, generated-code strings, and token/ATN dumps. Property checks (boolean predicates, bounds, round-trip/algebraic invariants, ordering) are kept as explicit assertions, with a snapshot layered alongside where both the value and the invariant matter. Notes: - Each converted test module (or bare `#[test]` fn) carries `#[allow(clippy::disallowed_methods)]` because `.clippy.toml` bans `.unwrap()` and the insta macros unwrap internal I/O — matching the existing `semantics.rs` site. - Snapshot targets are deterministic: generator data is `BTreeMap`/`BTreeSet` backed, and the lexer byte-span tests snapshot an explicit tuple because `TokenView`'s `Debug` omits `byte_span`. - Net -235 source lines; all 888 tests pass and clippy is clean with `--all-targets --all-features -- -D warnings`.
Add a "Snapshot tests (insta)" section to CLAUDE.md and AGENTS.md (kept in sync) directing contributors to reach for snapshots on value checks and keep explicit assertions for properties. Documents the project-specific traps: the mandatory `#[allow(clippy::disallowed_methods)]` on test modules, `default-features = false` (no serde macros), the HashMap-order vs BTreeMap-safe determinism rule and the TokenView/byte_span gotcha, and the `cargo insta test`/`accept` workflow.
Copy/Paste DetectionFound 19 duplication(s) across 10 changed Rust file(s) (threshold: 100 tokens). Show duplicationsFound a 14 line (150 tokens) duplication in the following files:
let mut atn = ParserAtnBuilder::new(3);
add_state(&mut atn, 0, AtnStateKind::RuleStart);
add_state(&mut atn, 1, AtnStateKind::BlockStart);
add_state(&mut atn, 2, AtnStateKind::Basic);
add_state(&mut atn, 3, AtnStateKind::Basic);
add_state(&mut atn, 4, AtnStateKind::Basic);
add_state(&mut atn, 5, AtnStateKind::Basic);
add_state(&mut atn, 6, AtnStateKind::BlockEnd);
add_state(&mut atn, 7, AtnStateKind::RuleStop);
atn.set_rule_to_start_state(vec![0])
.expect("rule start states");
atn.set_rule_to_stop_state(vec![7])
.expect("rule stop states");
atn.add_decision_state(1).expect("decision state");
```rust
---
Found a 27 line (145 tokens) duplication in the following files:
* Starting at line 15152 of src/parser.rs
* Starting at line 15285 of src/parser.rs
```rust
fn generated_match_token_recovers_missing_token_from_context_follow() {
let atn = generated_match_recovery_atn();
let data = RecognizerData::new(
"Mini.g4",
Vocabulary::new(
[None, Some("'X'"), Some("'Y'")],
[None, Some("X"), Some("Y")],
[None::<&str>, None, None],
),
);
let mut parser = BaseParser::new(
CommonTokenStream::new(Source {
tokens: vec![TestToken::eof("parser-test", 3, 1, 3)],
index: 0,
}),
data,
);
parser.rule_context_stack = vec![
RuleContextFrame {
rule_index: 0,
invoking_state: 0,
},
RuleContextFrame {
rule_index: 1,
invoking_state: 1,
},
];Found a 34 line (134 tokens) duplication in the following files:
if self.fast_parser_predicate_matches(predicate_context, transition, index) {
let boundary = left_recursive_boundary(atn, state, target);
outcomes.extend(
self.recognize_state_fast(
atn,
FastRecognizeRequest {
state_number: target,
stop_state,
index,
rule_start_index,
decision_start_index: next_decision_start_index,
precedence,
depth: depth + 1,
recovery_symbols: Rc::clone(&epsilon_recovery_symbols),
recovery_state: epsilon_recovery_state,
},
FastRecognizeScratch {
predicate_context,
visiting,
memo,
expected,
native_depth: native_depth + 1,
},
)
.into_iter()
.map(|mut outcome| {
if let Some(rule_index) = boundary {
let boundary = self.arena_boundary_node(rule_index, 0);
self.defer_fast_outcome_node(&mut outcome, boundary);
}
outcome
}),
);
} else {
```rust
---
Found a 33 line (133 tokens) duplication in the following files:
* Starting at line 8593 of src/parser.rs
* Starting at line 8630 of src/parser.rs
* Starting at line 8669 of src/parser.rs
```rust
let boundary = left_recursive_boundary(atn, state, target);
outcomes.extend(
self.recognize_state_fast(
atn,
FastRecognizeRequest {
state_number: target,
stop_state,
index,
rule_start_index,
decision_start_index: next_decision_start_index,
precedence,
depth: depth + 1,
recovery_symbols: Rc::clone(&epsilon_recovery_symbols),
recovery_state: epsilon_recovery_state,
},
FastRecognizeScratch {
predicate_context,
visiting,
memo,
expected,
native_depth: native_depth + 1,
},
)
.into_iter()
.map(|mut outcome| {
if let Some(rule_index) = boundary {
let boundary = self.arena_boundary_node(rule_index, 0);
self.defer_fast_outcome_node(&mut outcome, boundary);
}
outcome
}),
);
}Found a 27 line (127 tokens) duplication in the following files:
let mut atn = ParserAtnBuilder::new(2);
assert_eq!(
atn.add_state(AtnStateKind::RuleStart, Some(0))
.expect("state")
.index(),
0
);
assert_eq!(
atn.add_state(AtnStateKind::BlockStart, Some(0))
.expect("state")
.index(),
1
);
assert_eq!(
atn.add_state(AtnStateKind::Basic, Some(0))
.expect("state")
.index(),
2
);
assert_eq!(
atn.add_state(AtnStateKind::Basic, Some(0))
.expect("state")
.index(),
3
);
assert_eq!(
atn.add_state(AtnStateKind::BlockEnd, Some(0))
```rust
---
Found a 22 line (125 tokens) duplication in the following files:
* Starting at line 14136 of src/parser.rs
* Starting at line 14208 of src/parser.rs
```rust
atn.add_state(AtnStateKind::BlockEnd, Some(0))
.expect("state")
.index(),
4
);
assert_eq!(
atn.add_state(AtnStateKind::RuleStop, Some(0))
.expect("state")
.index(),
5
);
atn.set_rule_to_start_state(vec![0])
.expect("rule start states");
atn.set_rule_to_stop_state(vec![5])
.expect("rule stop states");
atn.add_decision_state(1).expect("decision state");
atn.add_transition(0, ParserTransitionSpec::Epsilon { target: 1 })
.expect("transition");
atn.add_transition(
1,
ParserTransitionSpec::Atom {
target: 2,Found a 18 line (122 tokens) duplication in the following files:
atn.set_rule_to_stop_state(vec![7])
.expect("rule stop states");
atn.add_decision_state(1).expect("decision state");
atn.add_transition(0, ParserTransitionSpec::Epsilon { target: 1 })
.expect("transition");
atn.add_transition(1, ParserTransitionSpec::Epsilon { target: 2 })
.expect("transition");
atn.add_transition(1, ParserTransitionSpec::Epsilon { target: 4 })
.expect("transition");
atn.add_transition(
2,
ParserTransitionSpec::Atom {
target: 3,
label: 1,
},
)
.expect("transition");
atn.add_transition(3, ParserTransitionSpec::Epsilon { target: 6 })
```rust
---
Found a 34 line (119 tokens) duplication in the following files:
* Starting at line 9585 of src/parser.rs
* Starting at line 9660 of src/parser.rs
```rust
outcomes.extend(
self.recognize_state(
atn,
RecognizeRequest {
state_number: *target,
stop_state,
index,
rule_start_index,
decision_start_index: next_decision_start_index,
init_action_rules,
predicates,
semantics,
rule_args,
member_actions,
return_actions,
local_int_arg,
member_values: member_values.clone(),
return_values: return_values.clone(),
rule_alt_number: next_alt_number,
track_alt_numbers,
consumed_eof,
committed_decision: transition_committed,
precedence,
depth: depth + 1,
recovery_symbols: epsilon_recovery_symbols.clone(),
recovery_state: epsilon_recovery_state,
},
visiting,
memo,
expected,
)
.into_iter()
.map(|mut outcome| {
prepend_decision(&mut outcome, decision);Found a 24 line (113 tokens) duplication in the following files:
impl IntStream for LookaheadIntStream {
fn consume(&mut self) {
if self.la(1) != TOKEN_EOF {
self.index += 1;
}
}
fn la(&mut self, offset: isize) -> i32 {
if offset <= 0 {
return 0;
}
let offset = offset.cast_unsigned() - 1;
self.symbols
.get(self.index + offset)
.copied()
.unwrap_or(TOKEN_EOF)
}
fn index(&self) -> usize {
self.index
}
fn seek(&mut self, index: usize) {
self.index = index.min(self.symbols.len());
```rust
---
Found a 15 line (113 tokens) duplication in the following files:
* Starting at line 15210 of src/parser.rs
* Starting at line 15486 of src/parser.rs
```rust
fn generated_match_token_counts_single_token_deletion_recovery() {
let atn = generated_match_recovery_atn();
let data = RecognizerData::new(
"Mini.g4",
Vocabulary::new(
[None, Some("'X'"), Some("'Y'"), Some("'Z'")],
[None, Some("X"), Some("Y"), Some("Z")],
[None::<&str>, None, None, None],
),
);
let mut parser = BaseParser::new(
CommonTokenStream::new(Source {
tokens: vec![
TestToken::new(3).with_text("z"),
TestToken::new(2).with_text("y"),Found a 12 line (112 tokens) duplication in the following files:
let mut atn = ParserAtnBuilder::new(1);
for (state, kind, rule) in [
(0, AtnStateKind::RuleStart, 0),
(1, AtnStateKind::StarLoopEntry, 0),
(2, AtnStateKind::Basic, 0), // ops hub
(3, AtnStateKind::Basic, 0), // shift prec
(4, AtnStateKind::Basic, 0), // shift first >
(5, AtnStateKind::Basic, 0), // shift second >
(6, AtnStateKind::Basic, 0), // rel prec
(7, AtnStateKind::Basic, 0), // rel >
(8, AtnStateKind::LoopEnd, 0),
(9, AtnStateKind::RuleStop, 0),
```rust
---
Found a 22 line (112 tokens) duplication in the following files:
* Starting at line 14547 of src/parser.rs
* Starting at line 14748 of src/parser.rs
```rust
fn predicate_after_token_atn() -> Atn {
let mut atn = ParserAtnBuilder::new(2);
assert_eq!(
atn.add_state(AtnStateKind::RuleStart, Some(0))
.expect("state")
.index(),
0
);
assert_eq!(
atn.add_state(AtnStateKind::Basic, Some(0))
.expect("state")
.index(),
1
);
assert_eq!(
atn.add_state(AtnStateKind::Basic, Some(0))
.expect("state")
.index(),
2
);
assert_eq!(
atn.add_state(AtnStateKind::Basic, Some(0))Found a 22 line (111 tokens) duplication in the following files:
fn left_recursive_loop_with_caller_follow_atn(caller_symbol: i32) -> Atn {
let mut atn = ParserAtnBuilder::new(2);
assert_eq!(
atn.add_state(AtnStateKind::RuleStart, Some(0))
.expect("state")
.index(),
0
);
assert_eq!(
atn.add_state(AtnStateKind::Basic, Some(0))
.expect("state")
.index(),
1
);
assert_eq!(
atn.add_state(AtnStateKind::Basic, Some(0))
.expect("state")
.index(),
2
);
assert_eq!(
atn.add_state(AtnStateKind::RuleStart, Some(1))
```rust
---
Found a 14 line (110 tokens) duplication in the following files:
* Starting at line 14881 of src/parser.rs
* Starting at line 17523 of src/parser.rs
```rust
fn parser_matches_token_and_reports_mismatch() {
let source = Source {
tokens: vec![
TestToken::new(1).with_text("x"),
TestToken::eof("parser-test", 1, 1, 1),
],
index: 0,
};
let data = RecognizerData::new(
"Mini.g4",
Vocabulary::new([None, Some("'x'")], [None, Some("X")], [None::<&str>, None]),
);
let mut parser = BaseParser::new(CommonTokenStream::new(source), data);
let matched = parser.match_token(1).expect("token 1 should match");Found a 13 line (109 tokens) duplication in the following files:
fn parser_matches_token_and_reports_mismatch() {
let source = Source {
tokens: vec![
TestToken::new(1).with_text("x"),
TestToken::eof("parser-test", 1, 1, 1),
],
index: 0,
};
let data = RecognizerData::new(
"Mini.g4",
Vocabulary::new([None, Some("'x'")], [None, Some("X")], [None::<&str>, None]),
);
let mut parser = BaseParser::new(CommonTokenStream::new(source), data);
```rust
---
Found a 22 line (108 tokens) duplication in the following files:
* Starting at line 7462 of src/parser.rs
* Starting at line 7853 of src/parser.rs
```rust
) -> Option<RecognizeOutcome> {
let (error_index, message) = self.expected_error_message(rule_index, start_index, expected);
let diagnostic = diagnostic_for_token(self.token_at(error_index), message);
let mut next_index = error_index;
loop {
let symbol = self.token_type_at(next_index);
if sync_symbols.contains(&symbol) {
if next_index == error_index {
return None;
}
break;
}
if symbol == TOKEN_EOF {
break;
}
let after = self.consume_index(next_index, symbol);
if after == next_index {
break;
}
next_index = after;
}
let mut nodes = NodeSeqId::EMPTY;Found a 15 line (108 tokens) duplication in the following files:
fn outcome_ties_keep_later_non_recursive_alternative() {
let arena = RecognitionArena::default();
let first = RecognizeOutcome {
index: 1,
consumed_eof: false,
alt_number: 0,
member_values: BTreeMap::new(),
return_values: BTreeMap::new(),
diagnostics: DiagnosticSeqId::EMPTY,
decisions: Vec::new(),
actions: vec![ParserAction::new(1, 0, 0, None)],
nodes: NodeSeqId::EMPTY,
};
let second = RecognizeOutcome {
actions: vec![ParserAction::new(2, 0, 0, None)],
```rust
---
Found a 16 line (105 tokens) duplication in the following files:
* Starting at line 6637 of src/parser.rs
* Starting at line 7221 of src/parser.rs
```rust
let start_state = atn.rule_to_start_state().get(rule_index).ok_or_else(|| {
AntlrError::Unsupported(format!("rule {rule_index} has no start state"))
})?;
let stop_state = atn
.rule_to_stop_state()
.get(rule_index)
.filter(|state| *state != usize::MAX)
.ok_or_else(|| {
AntlrError::Unsupported(format!("rule {rule_index} has no stop state"))
})?;
let start_index = self.current_visible_index();
self.clear_prediction_diagnostics();
self.reset_per_parse_caches();
self.reset_recognition_arena();
let caller_follow_state = self.pending_invoking_follow_state(atn);Found a 13 line (100 tokens) duplication in the following files:
let mut expected = BTreeSet::new();
for index in (1..self.rule_context_stack.len()).rev() {
let invoking_state = self.rule_context_stack[index].invoking_state;
let Ok(state_number) = usize::try_from(invoking_state) else {
continue;
};
let Some(Transition::Rule { follow_state, .. }) = atn
.state(state_number)
.and_then(|state| state.transitions().first())
.map(ParserTransition::data)
else {
continue;
};
```rust
</details> |
|
Claude finished @tinovyatkin's task in 14m 50s —— View job Code review — no blocking issues ✅This is a clean, well-scoped test refactor (assertions → What I verifiedDeterminism (the flakiness risk) — clean. The four whole-struct
Invariant preservation — the two load-bearing claims check out:
CLAUDE.md compliance — clean. Mechanical consistency — clean. Removed helpers Optional (non-blocking) — one observationIn Note on the duplication botThe 19 flagged duplications are in pre-existing production recursion ( Checks completed:
Verdict: LGTM. No bugs, no CLAUDE.md violations, no determinism hazards found. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (33)
📒 Files selected for processing (12)
📝 WalkthroughWalkthroughThe PR documents insta snapshot conventions and converts runtime, grammar, parser, lexer, vocabulary, recognizer, and code-generator tests from manual assertions to structured or rendered snapshot assertions. No public declarations or production behavior are changed. ChangesSnapshot testing migration
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
📊 Source Code Metrics (this PR vs
|
| File | Cyclomatic | Cognitive | Functions | LLOC | MI |
|---|---|---|---|---|---|
| src/parser.rs | 2067 ⚪ | 1367 ⚪ | 645 ⚪ | 4482 (main: 4486) 🟢 | 0 ⚪ |
| src/bin/antlr4-rust-gen.rs | 2226 ⚪ | 1409 ⚪ | 489 (main: 491) 🟢 | 3889 (main: 3936) 🟢 | 0 ⚪ |
| src/lexer.rs | 272 ⚪ | 66 ⚪ | 162 ⚪ | 430 (main: 440) 🟢 | 0 ⚪ |
| src/bin_support/grammar/syntax.rs | 226 ⚪ | 105 ⚪ | 59 ⚪ | 322 (main: 350) 🟢 | 0 ⚪ |
| src/atn/serialized.rs | 298 ⚪ | 217 ⚪ | 36 ⚪ | 304 (main: 309) 🟢 | 0 ⚪ |
| src/recognizer.rs | 41 ⚪ | 1 ⚪ | 33 ⚪ | 47 ⚪ | 12.04 (main: 11.92) 🟢 |
| src/bin_support/grammar/escape_sequence.rs | 52 ⚪ | 16 ⚪ | 25 ⚪ | 71 (main: 73) 🟢 | 12.76 (main: 13.39) 🔴 |
| src/vocabulary.rs | 27 (main: 29) 🟢 | 1 (main: 6) 🟢 | 10 ⚪ | 28 (main: 34) 🟢 | 22.17 (main: 22.32) 🔴 |
| src/bin_support/grammar/ported_tests.rs | 5 ⚪ | 2 ⚪ | 1 ⚪ | 10 (main: 7) 🔴 | 29.72 (main: 30.93) 🔴 |
Generated by mehen v1.6.0 — the code quality watcher.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
instawas introduced as a dev-dependency in #141 but only used in new codegen tests. This PR expands snapshot coverage into pre-existing tests where a snapshot is a better regression target than manual assertions — 8 snapshot sites → 47 (39 conversions across 10 modules).Snapshots are more observable: they capture whole structures (so counts are implied) and subsume negative
!contains(...)guards by rendering the full output. Net −235 source lines.Scope & method
Candidates were surfaced by a survey pass over every test-bearing module, then each was adversarially verified for determinism and invariant-loss before conversion. Only genuine value checks were converted:
ParserAtnPrediction, compiledGeneratedParserSteptrees,PortableLocalData)Property checks are deliberately left as explicit assertions — boolean predicates, bounds, round-trip/algebraic invariants, ordering — with a snapshot layered alongside where both the value and the invariant matter (e.g. the predicate-hoisting ordering check).
Notable correctness guards
#[allow(clippy::disallowed_methods)]on each converted test module /#[test]fn:.clippy.tomlbans.unwrap()and the insta macros unwrap internal I/O, so CI clippy fails without it (matches the existingsemantics.rssite).BTreeMap/BTreeSet-backed (generator data) or explicitly ordered; noPredictionFxHasher/HashMapiteration order is snapshotted. The lexer byte-span tests snapshot an explicit(start, stop, text, byte_span)tuple becauseTokenView'sDebugomitsbyte_span— a naivetokensnapshot would silently drop the field those tests exist to pin.Docs
Adds a "Snapshot tests (insta)" section to
CLAUDE.mdandAGENTS.md(kept in sync) directing contributors to prefer snapshots for value checks, keep assertions for properties, and documenting the project-specific traps above plus thecargo insta test/acceptworkflow.Verification
cargo test --locked --all-features— 888 pass, 0 failcargo clippy --locked --all-targets --all-features -- -D warnings— cleancargo fmt --checkclean on all touched files (a pre-existingsrc/prediction.rsdrift is left untouched, per repo convention)🤖 Generated with Claude Code
Summary by CodeRabbit
Tests
Documentation