Skip to content

Commit 40a0775

Browse files
ST-DDTfasttime
ST-DDT
and
fasttime
authored
feat: add assertion options to RuleTester (#137)
* feat: add assertion options to RuleTester * docs: address soem feedback * docs: address some feedback * docs: improve requiredScenarios documentation * docs: add implementation hint * chore: apply suggestions * chore: apply suggestions * chore: apply suggestions * chore: strike out requiredScenarios * chore: cleanup * chore: add PR link * chore: merge assertionOptions into test parameter * chore: format file * chore: apply review suggestions * docs: apply suggestion * docs: apply suggestion * docs: apply suggestion * docs: apply suggestion * docs: apply parameter description suggestion * chore: move variant to alternatives * chore: list 3rd alternative * Update designs/2025-rule-tester-assertion-options/README.md Co-authored-by: Francesco Trotta <[email protected]> --------- Co-authored-by: Francesco Trotta <[email protected]>
1 parent c6f199f commit 40a0775

File tree

1 file changed

+365
-0
lines changed
  • designs/2025-rule-tester-assertion-options

1 file changed

+365
-0
lines changed

‎designs/2025-rule-tester-assertion-options/README.md‎

Lines changed: 365 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,365 @@
1+
- Repo: https://github.com/eslint/eslint
2+
- Start Date: 2025-07-10
3+
- RFC PR: https://github.com/eslint/rfcs/pull/137
4+
- Authors: ST-DDT
5+
6+
# Rule Tester Assertions Options
7+
8+
## Summary
9+
10+
Add options that control which assertions are required for each `RuleTester` test case.
11+
12+
## Motivation
13+
14+
In most eslint(-plugin)s' rules the error assertions are different from each other.
15+
Adding options that could be set/shared when configuring the rule tester would ensure a common base level of assertions is met throughout the (plugin's) project.
16+
The options could be defined on two levels. On `RuleTester`'s `constructor` effectively impacting all tests, or on the test `run` method itself, so only that set is affected.
17+
18+
## Detailed Design
19+
20+
```ts
21+
ruleTester.run("rule-name", rule, {
22+
assertionOptions?: {
23+
/**
24+
* Require message assertion for each invalid test case.
25+
* If `true`, each error must extend `string | RegExp | { message: string | RegExp } | { messageId: string }`.
26+
* If `'message'`, each error must extend `string | RegExp | { message: string | RegExp }`.
27+
* If `'messageId'`, each error must extend `{ messageId: string }`.
28+
*
29+
* @default false
30+
*/
31+
requireMessage?: boolean | 'message' | 'messageId';
32+
/**
33+
* Require full location assertions for each invalid test case.
34+
* If `true`, each error must extend `{ line: number, column: number, endLine?: number | undefined, endColumn?: number | undefined }`.
35+
* `endLine` or `endColumn` may be absent or undefined, if the observed error does not contain these properties.
36+
*
37+
* @default false
38+
*/
39+
requireLocation?: boolean;
40+
},
41+
valid: [ /* ... */ ],
42+
invalid: [ /* ... */ ],
43+
});
44+
```
45+
46+
### requireMessage
47+
48+
If `requireMessage` is set to `true`, the invalid test case cannot consist of an error count assertion only, but must also include a message assertion. (See below)
49+
This can be done either by providing only a `string`/`RegExp` message, or by using the `message`/`messageId` property of the error object in the `TestCaseError` (Same as the current behavior).
50+
If `true`, each error must extend `string | RegExp | { message: string | RegExp } | { messageId: string }`.
51+
If `'message'`, each error must extend `string | RegExp | { message: string | RegExp }`.
52+
If `'messageId'`, each error must extend `{ messageId: string }`.
53+
54+
```ts
55+
ruleTester.run("rule-name", rule, {
56+
assertionOptions: {
57+
requireMessage: true,
58+
},
59+
invalid: [
60+
{
61+
code: "const a = 1;",
62+
errors: 1, // ❌ Message isn't being checked here
63+
},
64+
{
65+
code: "const a = 2;",
66+
errors: [
67+
"Error message here.", // ✅ we only check the error message
68+
],
69+
},
70+
{
71+
code: "const a = 3;",
72+
errors: [
73+
{
74+
message: "Error message here.", // ✅ we check the error message and potentially other properties
75+
},
76+
],
77+
},
78+
],
79+
});
80+
```
81+
82+
### requireLocation
83+
84+
If `requireLocation` is set to `true`, the invalid test case's error validation must include all location assertions such as `line`, `column`, `endLine`, and `endColumn`.
85+
`endLine` or `endColumn` may be absent or `undefined`, if the observed error does not contain these properties.
86+
87+
```ts
88+
ruleTester.run("rule-name", rule, {
89+
assertionOptions: {
90+
requireLocation: true,
91+
},
92+
invalid: [
93+
{
94+
code: "const a = 1;",
95+
errors: 1, // ❌ Location isn't checked here
96+
},
97+
{
98+
code: "const a = 2;",
99+
errors: [
100+
"Error message here.", // ❌ Location isn't checked here
101+
],
102+
},
103+
{
104+
code: "const a = 3;",
105+
errors: [
106+
{
107+
line: 1, // ❌ Location isn't fully checked here
108+
column: 1,
109+
},
110+
],
111+
},
112+
{
113+
code: "const a = 4;",
114+
errors: [
115+
{
116+
line: 1, // ✅ All location properties have been checked
117+
column: 1,
118+
endLine: 1,
119+
endColumn: 12,
120+
},
121+
],
122+
},
123+
],
124+
});
125+
```
126+
127+
## Implementation Hint
128+
129+
```patch
130+
diff --git a/lib/rule-tester/rule-tester.js b/lib/rule-tester/rule-tester.js
131+
index dbd8c274f..b39631faa 100644
132+
--- a/lib/rule-tester/rule-tester.js
133+
+++ b/lib/rule-tester/rule-tester.js
134+
@@ -555,6 +555,10 @@ class RuleTester {
135+
* @param {string} ruleName The name of the rule to run.
136+
* @param {RuleDefinition} rule The rule to test.
137+
* @param {{
138+
+ * assertionOptions?: {
139+
+ * requireMessage?: boolean | "message" | "messageId",
140+
+ * requireLocation?: boolean
141+
+ * },
142+
* valid: (ValidTestCase | string)[],
143+
* invalid: InvalidTestCase[]
144+
* }} test The collection of tests to run.
145+
@@ -563,6 +567,14 @@ class RuleTester {
146+
* @returns {void}
147+
*/
148+
run(ruleName, rule, test) {
149+
+ if (!test || typeof test !== "object") {
150+
+ throw new TypeError(
151+
+ `Test Scenarios for rule ${ruleName} : Could not find test scenario object`,
152+
+ );
153+
+ }
154+
+
155+
+ const { requireMessage = false, requireLocation = false } =
156+
+ test.assertionOptions ?? {};
157+
const testerConfig = this.testerConfig,
158+
requiredScenarios = ["valid", "invalid"],
159+
scenarioErrors = [],
160+
@@ -582,12 +594,6 @@ class RuleTester {
161+
);
162+
}
163+
164+
- if (!test || typeof test !== "object") {
165+
- throw new TypeError(
166+
- `Test Scenarios for rule ${ruleName} : Could not find test scenario object`,
167+
- );
168+
- }
169+
-
170+
requiredScenarios.forEach(scenarioType => {
171+
if (!test[scenarioType]) {
172+
scenarioErrors.push(
173+
@@ -1092,6 +1098,10 @@ class RuleTester {
174+
}
175+
176+
if (typeof item.errors === "number") {
177+
+ assert.ok(
178+
+ !requireMessage && !requireLocation,
179+
+ "Invalid cases must have 'errors' value as an array",
180+
+ );
181+
if (item.errors === 0) {
182+
assert.fail(
183+
"Invalid cases must have 'error' value greater than 0",
184+
@@ -1137,6 +1147,10 @@ class RuleTester {
185+
186+
if (typeof error === "string" || error instanceof RegExp) {
187+
// Just an error message.
188+
+ assert.ok(
189+
+ requireMessage !== "messageId" && !requireLocation,
190+
+ "Invalid cases must have 'errors' value as an array of objects e.g. { message: 'error message' }",
191+
+ );
192+
assertMessageMatches(message.message, error);
193+
assert.ok(
194+
message.suggestions === void 0,
195+
@@ -1157,6 +1171,10 @@ class RuleTester {
196+
});
197+
198+
if (hasOwnProperty(error, "message")) {
199+
+ assert.ok(
200+
+ requireMessage !== "messageId",
201+
+ "The test run requires 'messageId' but the error (also) uses 'message'.",
202+
+ );
203+
assert.ok(
204+
!hasOwnProperty(error, "messageId"),
205+
"Error should not specify both 'message' and a 'messageId'.",
206+
@@ -1170,6 +1188,10 @@ class RuleTester {
207+
error.message,
208+
);
209+
} else if (hasOwnProperty(error, "messageId")) {
210+
+ assert.ok(
211+
+ requireMessage !== "message",
212+
+ "The test run requires 'message' but the error (also) uses 'messageId'.",
213+
+ );
214+
assert.ok(
215+
ruleHasMetaMessages,
216+
"Error can not use 'messageId' if rule under test doesn't define 'meta.messages'.",
217+
@@ -1242,21 +1264,43 @@ class RuleTester {
218+
if (hasOwnProperty(error, "line")) {
219+
actualLocation.line = message.line;
220+
expectedLocation.line = error.line;
221+
+ } else if (requireLocation) {
222+
+ assert.fail(
223+
+ "Test error must specify a 'line' property.",
224+
+ );
225+
}
226+
227+
if (hasOwnProperty(error, "column")) {
228+
actualLocation.column = message.column;
229+
expectedLocation.column = error.column;
230+
+ } else if (requireLocation) {
231+
+ assert.fail(
232+
+ "Test error must specify a 'column' property.",
233+
+ );
234+
}
235+
236+
if (hasOwnProperty(error, "endLine")) {
237+
actualLocation.endLine = message.endLine;
238+
expectedLocation.endLine = error.endLine;
239+
+ } else if (
240+
+ requireLocation &&
241+
+ hasOwnProperty(message, "endLine")
242+
+ ) {
243+
+ assert.fail(
244+
+ "Test error must specify an 'endLine' property.",
245+
+ );
246+
}
247+
248+
if (hasOwnProperty(error, "endColumn")) {
249+
actualLocation.endColumn = message.endColumn;
250+
expectedLocation.endColumn = error.endColumn;
251+
+ } else if (
252+
+ requireLocation &&
253+
+ hasOwnProperty(message, "endColumn")
254+
+ ) {
255+
+ assert.fail(
256+
+ "Test error must specify an 'endColumn' property.",
257+
+ );
258+
}
259+
260+
if (Object.keys(expectedLocation).length > 0) {
261+
```
262+
263+
## Documentation
264+
265+
This RFC will be documented in the RuleTester documentation, explaining the new options and how to use them.
266+
So mainly here: https://eslint.org/docs/latest/integrate/nodejs-api#ruletester
267+
Additionally, we should write a short blog post to announce for plugin maintainers, to raise awareness of the new options and encourage them to use them in their tests.
268+
269+
## Drawbacks
270+
271+
This proposal adds slightly more complexity to the RuleTester logic, as it needs to handle the new options and enforce the assertions based on them.
272+
Currently, the RuleTester logic is already deeply nested, so adding more options may make it harder to read and maintain.
273+
274+
Due to limitations of the `RuleTester`, the error message does not point to the correct location of the test case.
275+
While this is true already, it gets slightly more relevant, since now it might report missing properties, that cannot be searched for in the test file.
276+
277+
See also: https://github.com/eslint/eslint/issues/19936
278+
279+
If we enable the `requireMessage` and `requireLocation` options by default, it would be a breaking change for existing tests that do not follow these assertion requirements yet.
280+
It is not planned to enable `requireMessage` or `requireLocation` by default.
281+
282+
## Backwards Compatibility Analysis
283+
284+
This change should not affect existing ESLint users or plugin developers, as it only adds new options to the RuleTester and does not change any existing behavior.
285+
286+
## Alternatives
287+
288+
### Constructor based options
289+
290+
```ts
291+
new RuleTester(testerConfig: {...}, assertionOptions: {
292+
/**
293+
* Require message assertion for each invalid test case.
294+
* If `true`, each error must extend `string | RegExp | { message: string | RegExp } | { messageId: string }`.
295+
* If `'message'`, each error must extend `string | RegExp | { message: string | RegExp }`.
296+
* If `'messageId'`, each error must extend `{ messageId: string }`.
297+
*
298+
* @default false
299+
*/
300+
requireMessage: boolean | 'message' | 'messageId';
301+
/**
302+
* Require full location assertions for each invalid test case.
303+
* If `true`, each error must extend `{ line: number, column: number, endLine?: number | undefined, endColumn?: number | undefined }`.
304+
* `endLine` or `endColumn` may be absent, if the observed error does not contain these properties.
305+
*
306+
* @default false
307+
*/
308+
requireLocation: boolean;
309+
} = {});
310+
```
311+
312+
### Options Setter Method
313+
314+
Alternatively add an `setAssertionOptions()` method independently of the constructor and test method.
315+
316+
### ESLint RuleTester Lint-Rule
317+
318+
As an alternative to this proposal, we could add a eslint rule that applies the same assertions, but uses the central eslint config.
319+
While this would apply the same assertions for all rule testers, it would be a lot more complex to implement and maintain,
320+
it requires identifying the RuleTester calls in the codebase and might run into issues if the assertions aren't specified inline but via a variable or transformation.
321+
322+
### RuleTester Estimated Error Location
323+
324+
The following issue makes it easier to identify the exact/estimated error location (outside of/more precise than `RuleTester#run`):
325+
326+
- https://github.com/eslint/eslint/issues/19936
327+
328+
## Open Questions
329+
330+
1. ~~Is there a need for disabling scenarios like `valid` or `invalid`?~~ No, unused scenarios can be omitted using empty arrays. If needed, this option can be added later on.
331+
2. ~~Should we use constructor-based options or test method-based options? Do we support both? Or global options so it applies to all test files?~~ Currently, the method level options (variant 2) is the prefered one.
332+
3. ~~Should we enable the `requireMessage` and `requireLocation` options by default? (Breaking change)~~ No
333+
4. ~~Do we add a `requireMessageId` option or should we alter the `requireMessage` option to support both message and messageId assertions?~~ Just `requireMessage: boolean | 'message' | 'messageid'`
334+
5. ~~Should we add a `strict` option that enables all assertion options by default?~~ No, currently not planned.
335+
6. ~~Should we inline the options?~~ No, this might cause issues with alphabetical object properties.
336+
7. ~~What should the otpions be called?~~ We name it `assertionOptions` to avoid issues with alphabetical object properties. e.g. `invalid, options, valid`
337+
338+
## Help Needed
339+
340+
English is not my first language, so I would appreciate help with the wording and grammar of this RFC.
341+
I'm able to implement this RFC, if we decide to go with options instead of a new rule.
342+
343+
## Frequently Asked Questions
344+
345+
### Why
346+
347+
Because it is easy to miss a missing assertion in RuleTester test cases, especially when many new invalid test cases are added.
348+
349+
## Related Discussions
350+
351+
The idea was initially sparked by this comment: vuejs/eslint-plugin-vue#2773 (comment)
352+
353+
- https://github.com/vuejs/eslint-plugin-vue/pull/2773#discussion_r2176359714
354+
355+
> It might be helpful to include more detailed error information, such as line, column, endLine, and endColumn...
356+
> [...]
357+
> Let’s make the new cases more detailed first. 😊
358+
359+
The first steps have been taken in: eslint/eslint#19904 - feat: output full actual location in rule tester if different
360+
361+
- https://github.com/eslint/eslint/pull/19904
362+
363+
This lead to the issue that this RFC is based on: eslint/eslint#19921 - Change Request: Add options to rule-tester requiring certain assertions
364+
365+
- https://github.com/eslint/eslint/issues/19921

0 commit comments

Comments
 (0)