Skip to content

Commit 342ec9e

Browse files
committed
test(formula): correct the stale #6133 comment in parse-cel-to-ast.test.ts (#6678)
Two comments in `parse-cel-to-ast.test.ts` described `classifyError`'s pre-#6202 behaviour as if it were current: - `:139-143` told the next reader that `((record.a)` is graded `runtime` because cel-js's `Expected RPAREN, got EOF` misses `classifyError`'s /parse|unexpected|syntax/i, and instructed them that asserting the correct verdict "would enshrine it". - `:116-118` gave "`classifyError` reads the phrasing" as the live reason for a test-design decision. Neither holds. PR #6202 (#6133) made the ParseError arm structural and PR #6677 (#6223) deleted the keyword table; the regex quoted in the first comment had no remaining occurrence in the package except that comment. Measured on this branch rather than taken from the changelog: "((record.a)" -> kind=parse | Expected RPAREN, got EOF "record.budget >" -> kind=parse | Unexpected token: EOF "record.a $$ 1" -> kind=parse | Unexpected character: $ "record.a ?? 3" -> kind=parse | Unexpected token: QUESTION The message wording is unchanged; only the grading moved. The comments are rewritten to that measurement, and `((record.a)` is promoted from "deliberately not asserted" to an ordinary assertion alongside `record.budget >`, since the verdict it was withheld over is now stable. No assertion in the file encoded the old grading, so nothing behavioural was pinned wrong; the omission was the only stale artefact besides prose.
1 parent 55da611 commit 342ec9e

1 file changed

Lines changed: 23 additions & 9 deletions

File tree

packages/formula/src/parse-cel-to-ast.test.ts

Lines changed: 23 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -114,8 +114,11 @@ describe('parseCelToAst — parity with celEngine.compile (#4812)', () => {
114114

115115
it.each(SYNTAX_REJECTED)('rejects exactly what compile() rejects at parse: %s', (source) => {
116116
// The parity claim is the accept/reject verdict itself. `compile`'s error
117-
// *classification* is asserted separately below — cel-js does not phrase
118-
// every syntax fault the same way, and `classifyError` reads the phrasing.
117+
// *classification* is asserted separately below, and is now independent of
118+
// wording: `classifyError` reads the thrown error's TYPE (`instanceof
119+
// ParseError`), not its phrasing — the keyword table it used to consult was
120+
// closed for parse faults by #6133 / PR #6202 and deleted outright by
121+
// #6223 / PR #6677.
119122
expect(parseCelToAst(source)).toBeNull();
120123
expect(celEngine.compile(source).ok).toBe(false);
121124
});
@@ -132,15 +135,26 @@ describe('parseCelToAst — parity with celEngine.compile (#4812)', () => {
132135
expect((parseCelToAst(source) as unknown as { checkedType?: unknown }).checkedType).toBeUndefined();
133136
});
134137

135-
it('classifies the common syntax fault as `parse`', () => {
136-
const compiled = celEngine.compile('record.budget >');
138+
// Both wordings, because the pair used to disagree and the disagreement was
139+
// the whole point of this test. cel-js phrases a dangling operator as
140+
// `Unexpected token: EOF` and an unbalanced delimiter as `Expected RPAREN,
141+
// got EOF` (both measured on cel-js 8.0.0, unchanged); when `classifyError`
142+
// matched /parse|unexpected|syntax/i the second wording missed every keyword
143+
// and a genuine syntax fault reached the author graded `runtime`. #6133 /
144+
// PR #6202 replaced that arm with `err instanceof ParseError`, so the grade no
145+
// longer depends on which of the two an author happens to write.
146+
//
147+
// The exhaustive per-wording matrix lives in `cel-error-classification.test.ts`
148+
// (#6133); these two are pinned HERE because this suite's `SYNTAX_REJECTED`
149+
// list is what claims they are parse faults, and a list that says `parse`
150+
// while the engine says otherwise is the drift #6678 was filed for.
151+
it.each([
152+
['dangling operator', 'record.budget >'],
153+
['unbalanced delimiter', '((record.a)'],
154+
])('classifies a %s as `parse`', (_label, source) => {
155+
const compiled = celEngine.compile(source);
137156
expect(compiled.ok).toBe(false);
138157
if (!compiled.ok) expect(compiled.error.kind).toBe('parse');
139-
// NOT asserted for `((record.a)`: cel-js phrases an unbalanced delimiter as
140-
// `Expected RPAREN, got EOF`, which `classifyError`'s
141-
// /parse|unexpected|syntax/i does not match, so a genuine syntax fault is
142-
// reported to the author as `runtime`. Pre-existing, out of scope for #4812,
143-
// filed separately — asserting it here would enshrine it.
144158
});
145159

146160
it.each(PARSES_BUT_FAILS_CHECK)(

0 commit comments

Comments
 (0)