From b746f2a8399d9fbcd91637a85d2702c66010c998 Mon Sep 17 00:00:00 2001 From: Teakowa Date: Fri, 16 Oct 2026 13:00:56 +0800 Subject: [PATCH 1/2] test(runner): drop comments that restate the test name --- conformance/runner/src/session.rs | 4 ---- conformance/runner/tests/cli.rs | 1 - 2 files changed, 5 deletions(-) diff --git a/conformance/runner/src/session.rs b/conformance/runner/src/session.rs index 6b7a6fc..67fa92d 100644 --- a/conformance/runner/src/session.rs +++ b/conformance/runner/src/session.rs @@ -238,8 +238,6 @@ mod tests { #[test] fn mismatched_response_kills_provider() { - // A response that fails the matcher must not leave the provider - // running. assert_provider_reaped( "reaped-mismatch", "#!/bin/sh\necho $$ > \"$1\"\necho '{\"jsonrpc\":\"2.0\",\"id\":1,\"result\":{}}'\nexec sleep 30\n", @@ -248,7 +246,6 @@ mod tests { #[test] fn unparseable_response_kills_provider() { - // A non-JSON line must not leave the provider running either. assert_provider_reaped( "reaped-unparseable", "#!/bin/sh\necho $$ > \"$1\"\necho 'not-json'\nexec sleep 30\n", @@ -286,7 +283,6 @@ mod tests { #[test] fn output_past_the_last_step_fails() { - // A provider that emits an extra line after answering must not pass. let dir = scratch("extra"); let script = dir.join("provider.sh"); fs::write( diff --git a/conformance/runner/tests/cli.rs b/conformance/runner/tests/cli.rs index e42593e..71f20b4 100644 --- a/conformance/runner/tests/cli.rs +++ b/conformance/runner/tests/cli.rs @@ -21,7 +21,6 @@ fn scratch(name: &str) -> PathBuf { #[test] fn flag_without_value_is_an_error_not_the_next_flag() { - // `--provider` followed by another flag must not swallow it as a value. let out = runner() .args(["--provider", "--validate-only"]) .output() From d8ce545d02c4b9462d35ec65081984aa202c5da4 Mon Sep 17 00:00:00 2001 From: Teakowa Date: Fri, 16 Oct 2026 13:01:06 +0800 Subject: [PATCH 2/2] refactor(mock-provider): name simulation failure modes simulate() now returns Result instead of Option, so the parse-time failure arm names itself without a prose justification. OpKind::Div uses checked_div, so i64::MIN / -1 surfaces as the declared x-demo/simulation-failed diagnostic instead of panicking. Also drop comments that restated test names, duplicated the expect_* recovery contract already documented on those functions, or decorated the test module with a section banner. --- conformance/mock-provider/src/methods.rs | 4 +++- conformance/mock-provider/src/puzzle.rs | 16 +++++++++++----- conformance/mock-provider/src/puzzle/parse.rs | 14 ++++---------- conformance/mock-provider/src/puzzle/tests.rs | 7 +------ 4 files changed, 19 insertions(+), 22 deletions(-) diff --git a/conformance/mock-provider/src/methods.rs b/conformance/mock-provider/src/methods.rs index 9007631..38d040c 100644 --- a/conformance/mock-provider/src/methods.rs +++ b/conformance/mock-provider/src/methods.rs @@ -125,7 +125,9 @@ impl Server { let puzzle = parse_document(&entry.text) .puzzle .expect("no errors implies a puzzle"); - let value = puzzle.simulate().expect("no errors implies resolvable ops"); + let value = puzzle + .simulate() + .expect("no errors implies a clean simulation"); let artifact = if format == SUMMARY_ARTIFACT_FORMAT { summary_artifact(&puzzle, value) } else { diff --git a/conformance/mock-provider/src/puzzle.rs b/conformance/mock-provider/src/puzzle.rs index f59c98c..41dee30 100644 --- a/conformance/mock-provider/src/puzzle.rs +++ b/conformance/mock-provider/src/puzzle.rs @@ -75,7 +75,7 @@ impl OpKind { OpKind::Add => lhs.checked_add(rhs), OpKind::Sub => lhs.checked_sub(rhs), OpKind::Mul => lhs.checked_mul(rhs), - OpKind::Div => (rhs != 0).then(|| lhs / rhs), + OpKind::Div => lhs.checked_div(rhs), } } } @@ -105,18 +105,24 @@ pub(crate) struct Puzzle { solution_range: Range, } +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub(crate) enum SimError { + UnknownOp, + Arithmetic, +} + impl Puzzle { fn op(&self, name: &str) -> Option<&Op> { self.ops.iter().find(|op| op.name == name) } - pub(crate) fn simulate(&self) -> Option { + pub(crate) fn simulate(&self) -> Result { let mut value = self.start; for entry in &self.solution { - let op = self.op(&entry.name)?; - value = op.kind.apply(value, op.arg)?; + let op = self.op(&entry.name).ok_or(SimError::UnknownOp)?; + value = op.kind.apply(value, op.arg).ok_or(SimError::Arithmetic)?; } - Some(value) + Ok(value) } } diff --git a/conformance/mock-provider/src/puzzle/parse.rs b/conformance/mock-provider/src/puzzle/parse.rs index 5eb94aa..46010f9 100644 --- a/conformance/mock-provider/src/puzzle/parse.rs +++ b/conformance/mock-provider/src/puzzle/parse.rs @@ -7,7 +7,7 @@ use std::collections::HashSet; use super::lex::{Token, TokenKind, describe, tokenize}; use super::text::{Position, Range, SourceText}; -use super::{Diagnostic, Op, ParseOutput, Puzzle, SolutionEntry, diagnostic}; +use super::{Diagnostic, Op, ParseOutput, Puzzle, SimError, SolutionEntry, diagnostic}; pub(crate) fn parse_document(text: &str) -> ParseOutput { let src = SourceText::new(text); @@ -298,19 +298,15 @@ fn parse_top(parser: &mut Parser) -> Option { Some(("x-demo/empty-solution", "solution is empty".to_string())) } else { match puzzle.simulate() { - Some(value) if value != puzzle.target => Some(( + Ok(value) if value != puzzle.target => Some(( "x-demo/target-not-reached", format!( "solution does not reach target: expected {}, reached {}", puzzle.target, value ), )), - Some(_) => None, - // All ops resolved here (unresolved references are already - // errors), so a failed simulation is an arithmetic failure: - // division by zero or integer overflow. It must be an error — - // compile cannot produce an artifact without a value. - None => { + Ok(_) | Err(SimError::UnknownOp) => None, + Err(SimError::Arithmetic) => { parser.diagnostics.push(diagnostic( puzzle.solution_range, "error", @@ -364,8 +360,6 @@ fn parse_op(parser: &mut Parser, puzzle: &mut Puzzle, name_tok: &Token) { return; }; let name = name.clone(); - // A failed expect_* has already reported and recovered to the next line; - // continuing would cascade the same diagnostic into the following op. if !parser.expect_punct(TokenKind::Colon, "':'") { return; } diff --git a/conformance/mock-provider/src/puzzle/tests.rs b/conformance/mock-provider/src/puzzle/tests.rs index 3f94197..009256a 100644 --- a/conformance/mock-provider/src/puzzle/tests.rs +++ b/conformance/mock-provider/src/puzzle/tests.rs @@ -42,7 +42,7 @@ fn parses_clean_puzzle() { assert_eq!(puzzle.name, "clean"); assert_eq!(puzzle.target, 40); assert_eq!(puzzle.start, 10); - assert_eq!(puzzle.simulate(), Some(40)); + assert_eq!(puzzle.simulate(), Ok(40)); } #[test] @@ -150,9 +150,6 @@ fn diagnostics_are_sorted_by_range_start() { #[test] fn arithmetic_failure_is_an_error_not_silence() { - // Division by zero and i64 overflow make simulate() return None; that - // must surface as an error diagnostic or compile's "no errors implies - // resolvable ops" invariant breaks. for src in [ "puzzle p {\n target = 1\n start = 1\n ops {\n d: x => x / 0\n }\n solution = [ d ]\n}", "puzzle p {\n target = 1\n start = 9223372036854775807\n ops {\n d: x => x + 1\n }\n solution = [ d ]\n}", @@ -357,8 +354,6 @@ fn lookup_unknown_within_is_none() { } } -// -- Symbol queries (lpp/symbols, definition, references, rename) ---------- - /// Parses `text` into a puzzle and returns it with its source text. fn puzzle_of(text: &str) -> (SourceText<'_>, Puzzle) { let src = SourceText::new(text);