-
-
Notifications
You must be signed in to change notification settings - Fork 88
feat: add assertion options to RuleTester #137
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
fasttime
merged 22 commits into
eslint:main
from
ST-DDT:2025-rule-tester-assertion-options
Oct 24, 2025
Merged
Changes from 1 commit
Commits
Show all changes
22 commits
Select commit
Hold shift + click to select a range
3d8c0ba
feat: add assertion options to RuleTester
ST-DDT e25685f
docs: address soem feedback
ST-DDT 641dabb
docs: address some feedback
ST-DDT 05a971e
docs: improve requiredScenarios documentation
ST-DDT 19762be
docs: add implementation hint
ST-DDT d27f172
chore: apply suggestions
ST-DDT 69af370
chore: apply suggestions
ST-DDT cd33bfc
chore: apply suggestions
ST-DDT 9a07ded
chore: strike out requiredScenarios
ST-DDT ad0293b
chore: cleanup
ST-DDT b720c9d
chore: add PR link
ST-DDT eef7306
chore: merge assertionOptions into test parameter
ST-DDT 429e0a3
chore: format file
ST-DDT aa12816
chore: apply review suggestions
ST-DDT 55b1ec7
docs: apply suggestion
ST-DDT ae0c11d
docs: apply suggestion
ST-DDT ab96b31
docs: apply suggestion
ST-DDT e378647
docs: apply suggestion
ST-DDT 5f137b0
docs: apply parameter description suggestion
ST-DDT c3a09c4
chore: move variant to alternatives
ST-DDT e3aff7b
chore: list 3rd alternative
ST-DDT 50b7323
Update designs/2025-rule-tester-assertion-options/README.md
ST-DDT File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Some comments aren't visible on the classic Files Changed page.
There are no files selected for viewing
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,241 @@ | ||
| - Repo: https://github.com/eslint/eslint | ||
| - Start Date: 2025-07-10 | ||
| - RFC PR: (leave this empty, to be filled in later) | ||
| - Authors: ST-DDT | ||
|
|
||
| # Rule Tester Assertions Options | ||
|
|
||
| ## Summary | ||
|
|
||
| Add options that control which assertions are required for each `RuleTester` test case. | ||
|
|
||
| ## Motivation | ||
|
|
||
| In most eslint(-plugin)s' rules the error assertions are different from each other. | ||
| 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. | ||
| 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. | ||
|
|
||
| ## Detailed Design | ||
|
ST-DDT marked this conversation as resolved.
|
||
|
|
||
| ### Variant 1 - Constructor based options | ||
|
ST-DDT marked this conversation as resolved.
Outdated
|
||
|
|
||
| ````ts | ||
| new RuleTester(testerConfig: {...}, assertionOptions: { | ||
| /** | ||
| * Require message assertion for each invalid test case. | ||
| * | ||
| * @default false | ||
| */ | ||
| requireMessage: boolean; | ||
| /** | ||
| * Require full location assertions for each invalid test case. | ||
| * | ||
| * @default false | ||
| */ | ||
| requireLocation: boolean; | ||
| } = {}); | ||
| ```` | ||
|
|
||
| ### Variant 2 - Test method based options | ||
|
ST-DDT marked this conversation as resolved.
Outdated
|
||
|
|
||
| ````ts | ||
| ruleTester.run("rule-name", rule, tests, assertionOptions: { | ||
| /** | ||
| * Require message assertion for each invalid test case. | ||
| * | ||
| * @default false | ||
| */ | ||
| requireMessage: boolean; | ||
| /** | ||
| * Require full location assertions for each invalid test case. | ||
| * | ||
| * @default false | ||
| */ | ||
| requireLocation: boolean; | ||
| /** | ||
| * Require and expect only the given test scenarios. | ||
| * This allows omitting certain scenarios from this run with the current options. | ||
| * | ||
| * @default ["valid","invalid"] | ||
| */ | ||
| requiredScenarios: ReadonlyArray<'valid' | 'invalid'>; | ||
| }); | ||
| ```` | ||
|
|
||
| ### Shared Logic | ||
|
|
||
| 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. | ||
| This can be done either by providing a only message, or by using the `message` property of the error object in the assertion (Same as the current behavior). | ||
|
ST-DDT marked this conversation as resolved.
Outdated
|
||
| We could enable this property by default, but it would be a breaking change. | ||
|
ST-DDT marked this conversation as resolved.
Outdated
|
||
| If we add a `requireMessageId` option, it would be mutually exclusive with `requireMessage`, and the invalid test case cannot consist of an error count or message assertion only, but must also include a messageId assertion. | ||
| Alternatively, we could alter the `requireMessage` option to `false | true | "message" | "messageId"` (`true` => `"message"`). | ||
|
ST-DDT marked this conversation as resolved.
Outdated
|
||
|
|
||
| ````ts | ||
| ruleTester.run("rule-name", rule, { | ||
| invalid: [ | ||
| { | ||
| code: "const a = 1;", | ||
| errors: 1, // ❌ | ||
| }, | ||
| { | ||
| code: "const a = 2;", | ||
| errors: [ | ||
| "Error message here.", // ✅ | ||
| ] | ||
| }, | ||
| { | ||
| code: "const a = 3;", | ||
| errors: [ | ||
| { | ||
| message: "Error message here.", // ✅ | ||
| } | ||
| ] | ||
| } | ||
| ] | ||
| }, { | ||
| requireMessage: true | ||
| }); | ||
| ```` | ||
|
|
||
| If `requireLocation` is set to `true`, the invalid test case cannot consist of an error count or errorMessage assertion only, but must also include a full location assertion. | ||
|
ST-DDT marked this conversation as resolved.
Outdated
|
||
| We could enable this property by default, but it would be a breaking change. | ||
|
|
||
| ````ts | ||
| ruleTester.run("rule-name", rule, { | ||
| invalid: [ | ||
| { | ||
| code: "const a = 1;", | ||
| errors: 1, // ❌ | ||
| }, | ||
| { | ||
| code: "const a = 2;", | ||
| errors: [ | ||
| "Error message here.", // ❌ | ||
| ] | ||
| }, | ||
| { | ||
| code: "const a = 3;", | ||
| errors: [ | ||
| { | ||
| line: 1, // ❌ | ||
| column: 1, | ||
|
|
||
| } | ||
| ] | ||
| }, | ||
| { | ||
| code: "const a = 4;", | ||
| errors: [ | ||
| { | ||
| line: 1, // ✅ | ||
| column: 1, | ||
| endLine: 1, | ||
| endColumn: 12, | ||
| } | ||
| ] | ||
| } | ||
| ] | ||
| }, { | ||
| requireLocation: true | ||
| }); | ||
| ```` | ||
|
|
||
| If `requiredScenarios` is set, the `run` will only require and expect the given scenarios. | ||
| This can only be used for the `run` method, not the constructor, because there should always be at least one valid and one invalid test case. | ||
| The `requiredScenarios` option can be used to omit certain scenarios from the run, e.g. if the user wants to import a set of tests from a different source, that may have other assertion requirements or haven't achieved the quality needed to require the same assertion strictness. | ||
|
ST-DDT marked this conversation as resolved.
Outdated
|
||
|
|
||
| ````ts | ||
| ruleTester.run("rule-name", rule, { | ||
| valid: [...], // ✅ | ||
| invalid: [...], | ||
| }, { | ||
| // Default is ["valid", "invalid"] | ||
| }); | ||
| ruleTester.run("rule-name", rule, { | ||
| valid: [...], // ❌ | ||
| }, { | ||
| // Default is ["valid", "invalid"] | ||
| }); | ||
| ruleTester.run("rule-name", rule, { | ||
| valid: [...], // ✅ | ||
| invalid: [...], | ||
| }, { | ||
| requiredScenarios: ["valid", "invalid"] | ||
| }); | ||
| ruleTester.run("rule-name", rule, { | ||
| valid: [...], // ✅ | ||
| }, { | ||
| requiredScenarios: ["valid"] | ||
| }); | ||
| ruleTester.run("rule-name", rule, { | ||
| valid: [...], // ❌ | ||
| invalid: [...], | ||
| }, { | ||
| requiredScenarios: ["invalid"] | ||
| }); | ||
| ```` | ||
|
|
||
| ## Documentation | ||
|
|
||
| This RFC will be documented in the RuleTester documentation, explaining the new options and how to use them. | ||
| So mainly here: https://eslint.org/docs/latest/integrate/nodejs-api#ruletester | ||
| 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. | ||
|
|
||
| ## Drawbacks | ||
|
|
||
| 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. | ||
| Currently, the RuleTester logic is already deeply nested, so adding more options may make it harder to read and maintain. | ||
|
|
||
| Additionally, since we add the options as a second parameter it might interfere with future additions to the parameters. | ||
| This could by eleviated by renaming the parameter from `assertionOptions` to `options` (either from the start or when the need for different type of options arises). | ||
|
|
||
|
ST-DDT marked this conversation as resolved.
Outdated
|
||
| 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. | ||
|
|
||
| ## Backwards Compatibility Analysis | ||
|
|
||
| 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. | ||
| 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. | ||
| If we want to enable them by default, we should do so in a major release and communicate the upcoming change to the users early via blog post. | ||
|
ST-DDT marked this conversation as resolved.
Outdated
|
||
|
|
||
| ## Alternatives | ||
|
|
||
| As an alternative to this proposal, we could add a eslint rule that applies the same assertions, but uses the central eslint config. | ||
| While this would apply the same assertions for all rule testers, it would be a lot more complex to implement and maintain, | ||
| 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. | ||
|
ST-DDT marked this conversation as resolved.
|
||
|
|
||
| ## Open Questions | ||
|
|
||
| 1. Is there a need for disabling scenarios like `valid` or `invalid`? | ||
|
ST-DDT marked this conversation as resolved.
Outdated
|
||
| 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? | ||
|
ST-DDT marked this conversation as resolved.
Outdated
ST-DDT marked this conversation as resolved.
Outdated
|
||
| 3. Should we enable the `requireMessage` and `requireLocation` options by default? (Breaking change) | ||
|
ST-DDT marked this conversation as resolved.
Outdated
|
||
| 4. Do we add a `requireMessageId` option or should we alter the `requireMessage` option to support both message and messageId assertions? | ||
|
ST-DDT marked this conversation as resolved.
Outdated
|
||
| 5. Should we add a `strict` option that enables all assertion options by default? | ||
|
ST-DDT marked this conversation as resolved.
Outdated
|
||
|
|
||
| ## Help Needed | ||
|
|
||
| English is not my first language, so I would appreciate help with the wording and grammar of this RFC. | ||
|
ST-DDT marked this conversation as resolved.
|
||
| I'm able to implement this RFC, if we decide to go with options instead of a new rule. | ||
|
|
||
| ## Frequently Asked Questions | ||
|
|
||
| ### Why | ||
|
|
||
| Because it is easy to miss a missing assertion in RuleTester test cases, especially when many new invalid test cases are added. | ||
|
|
||
| ## Related Discussions | ||
|
|
||
| The idea was initially sparked by this comment: vuejs/eslint-plugin-vue#2773 (comment) | ||
|
|
||
| - https://github.com/vuejs/eslint-plugin-vue/pull/2773#discussion_r2176359714 | ||
|
|
||
| > It might be helpful to include more detailed error information, such as line, column, endLine, and endColumn... | ||
| > [...] | ||
| > Let’s make the new cases more detailed first. 😊 | ||
|
|
||
| The first steps have been taken in: eslint/eslint#19904 - feat: output full actual location in rule tester if different | ||
|
|
||
| - https://github.com/eslint/eslint/pull/19904 | ||
|
|
||
| This lead to the issue that this RFC is based on: eslint/eslint#19921 - Change Request: Add options to rule-tester requiring certain assertions | ||
|
|
||
| - https://github.com/eslint/eslint/issues/19921 | ||
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.
Uh oh!
There was an error while loading. Please reload this page.