feat: ability to test rule errors and invalid schemas #16823

bmish · 2023-01-25T00:29:07Z

Prerequisites checklist

I have read the contributing guidelines.

What is the purpose of this pull request? (put an "X" next to an item)

[ ] Documentation update
[ ] Bug fix (template)
[ ] New rule (template)
[ ] Changes an existing rule (template)
[ ] Add autofix to a rule
[ ] Add a CLI option
[X] Add something to the core
[ ] Other, please explain:

What changes did you make? (Give an overview)

Implements this RFC:

feat: support for testing invalid rule schemas and runtime exceptions rfcs#103

Fixes #13434.

TODO:

Wait for RFC approval
Copy changes to flat-rule-tester.js
Add rule tester tests
More documentation
Add tests for fatal error when empty schema that's invalid, present schema that has one validation error, and present schema that has multiple validation errors (fix!: Update ajv options to align with aims for behaviour #17356)

Is there anything you'd like reviewers to focus on?

netlify · 2023-01-25T00:29:12Z

✅ Deploy Preview for docs-eslint canceled.

Name	Link
🔨 Latest commit	`2b39ad3`
🔍 Latest deploy log	https://app.netlify.com/sites/docs-eslint/deploys/64bf1aa78cd5c20008cb3540

github-actions · 2023-02-12T22:31:21Z

Hi everyone, it looks like we lost track of this pull request. Please review and see what the next steps are. This pull request will auto-close in 7 days without an update.

Rec0iL99 · 2023-02-13T05:20:46Z

WIP. The implementation proposal was approved recently. Don't close

bmish · 2023-05-22T02:25:41Z

lib/linter/linter.js

@@ -1340,6 +1340,7 @@ class Linter {
 providedOptions.physicalFilename
 );
 } catch (err) {
+ err.messageForTest = err.message; // Original exception message without extra helper text for asserting in fatal test cases.


I'm not sure how to hit this codepath in the tests. This is in _verifyWithoutProcessors().

I would assume creating a rule that throws an error would hit this?

bmish · 2023-05-22T02:50:16Z

This is ready for review.

fasttime · 2023-06-16T06:11:40Z

Marking as accepted, as this implements a merged RFC.

nzakas

LGTM. Seems pretty straightforward. I just left a couple of cleanup suggestions but otherwise, nice work!

Would like @mdjermanovic to review before merging.

docs/src/integrate/nodejs-api.md

nzakas · 2023-06-16T15:15:00Z

lib/linter/linter.js

@@ -1340,6 +1340,7 @@ class Linter {
 providedOptions.physicalFilename
 );
 } catch (err) {
+ err.messageForTest = err.message; // Original exception message without extra helper text for asserting in fatal test cases.


I would assume creating a rule that throws an error would hit this?

lib/shared/schema-validation-error.js

mdjermanovic · 2023-06-19T10:57:32Z

docs/src/integrate/nodejs-api.md

@@ -836,6 +859,8 @@ In addition to the properties above, invalid test cases can also have the follow
 If a string is provided as an error instead of an object, the string is used to assert the `message` of the error.
 * `output` (string, required if the rule fixes code): Asserts the output that will be produced when using this rule for a single pass of autofixing (e.g. with the `--fix` command line flag). If this is `null`, asserts that none of the reported problems suggest autofixes.

+Fatal test cases (which are optional) are the same as invalid test cases, except that `code` is optional (it may be irrelevant when testing rule options), and there's an `error` object instead of an `errors` array. The `error` object should include one or both of the `message` of the error and the error (exception) class `name`. Fatal test cases can be used to test custom errors thrown by the rule, or invalid rule options (in which case the error `name` will be `SchemaValidationError`, and the `message` will be come from JSON Schema -- note that strings from JSON Schema are subject to change with future upgrades).


"are the same as invalid test cases" would be confusing because fatal and invalid test cases do not share any specific properties (invalid test cases have errors and output; fatal test cases have error).

Reworded this paragraph to be more clear.

mdjermanovic · 2023-06-19T11:45:14Z

lib/rule-tester/rule-tester.js

@@ -130,6 +153,7 @@ const RuleTesterParameters = [
 "code",
 "filename",
 "options",
+ "error",


It would be good to ensure that valid and invalid test cases don't have an error property and that fatal test cases don't have errors or output.

(the same for flat-rule-tester)

mdjermanovic · 2023-06-19T11:59:53Z

lib/rule-tester/rule-tester.js

@@ -688,6 +746,22 @@ class RuleTester {
 try {
 SourceCode.prototype.getComments = getCommentsDeprecation;
 messages = linter.verify(code, config, filename);


If code wasn't specified in the test case, this will fail with TypeError: Cannot read properties of undefined (reading 'text').

(the same for flat-rule-tester)

I think I've already tested this scenario with test cases prefixed with:

// omit `code`

Isn't that the scenario you're talking about?

lib/rule-tester/rule-tester.js

nzakas · 2023-07-14T15:37:36Z

@bmish can you circle back around to finish this up based on @mdjermanovic's feedback?

Co-authored-by: Nicholas C. Zakas <[email protected]>

Co-authored-by: Milos Djermanovic <[email protected]>

* main: (101 commits) docs: Migrating `eslint-env` configuration comments (eslint#17390) Merge pull request from GHSA-qwh7-v8hg-w8rh feat: adds option for allowing empty object patterns as parameter (eslint#17365) fix: FlatESLint#getRulesMetaForResults shouldn't throw on unknown rules (eslint#17393) docs: fix Ignoring Files section in config migration guide (eslint#17392) docs: Update README feat: Improve config error messages (eslint#17385) fix: Update no-loop-func to not overlap with no-undef (eslint#17358) docs: Update README docs: Update README 8.45.0 Build: changelog update for 8.45.0 chore: package.json update for @eslint/js release docs: add playground links to correct and incorrect code blocks (eslint#17306) docs: Expand rule option schema docs (eslint#17198) docs: Config Migration Guide (eslint#17230) docs: Update README fix: Fix suggestion message in `no-useless-escape` (eslint#17339) docs: Update README chore: update eslint-config-eslint exports (eslint#17336) ...

Co-authored-by: Milos Djermanovic <[email protected]>

nzakas · 2023-07-28T15:39:39Z

@mdjermanovic this one is ready for you to look over again

bmish · 2023-08-01T10:05:37Z

tests/lib/rule-tester/rule-tester.js

@@ -2232,6 +2232,268 @@ describe("RuleTester", () => {
 });
 });

+ describe("fatal test cases", () => {


Moved to draft since it looks like I need to add more test cases to ensure all these scenarios work correctly:

empty schema, validation error when options are passed

present schema that has one validation error

present schema that has multiple validation errors

fasttime · 2023-08-20T20:36:47Z

In #17475 we have changed the logic in RuleTester and FlatRuleTester to avoid creating empty suites for valid or invalid tests. In other words, describe will be only called if there are any its to call inside of it.

I was thinking that the same change should be applied to fatal tests: describe("fatal", ... should be only called if any fatal tests were specified.

nzakas · 2024-01-17T18:02:49Z

@bmish what's the status on this? Looks like it hasn't been updated in a while. Are you looking to get this into v9?

bmish · 2024-01-17T18:19:22Z

@nzakas this isn't a breaking change so I don't think it needs to be included in the initial version of ESLint v9. Will aim to get it into a later version.

nzakas · 2024-01-17T20:20:06Z

Sounds good. I'll take it off my radar for now.

eslint-github-bot bot added triage An ESLint team member will look at this issue soon feature This change adds a new feature to ESLint labels Jan 25, 2023

bmish mentioned this pull request Jan 25, 2023

feat: support for testing invalid rule schemas and runtime exceptions eslint/rfcs#103

Merged

github-actions bot added the Stale label Feb 12, 2023

Rec0iL99 removed the Stale label Feb 13, 2023

bmish self-assigned this Feb 13, 2023

bmish force-pushed the rfc-103 branch from 4123314 to 6fddd47 Compare February 13, 2023 20:51

bmish force-pushed the rfc-103 branch 2 times, most recently from a33a598 to fbd87fd Compare May 22, 2023 01:21

feat: ability to test rule errors and invalid schemas per RFC-103

acc77ba

bmish force-pushed the rfc-103 branch from fbd87fd to acc77ba Compare May 22, 2023 02:22

bmish commented May 22, 2023

View reviewed changes

bmish marked this pull request as ready for review May 22, 2023 02:50

bmish requested a review from a team as a code owner May 22, 2023 02:50

fasttime added accepted There is consensus among the team that this change meets the criteria for inclusion and removed triage An ESLint team member will look at this issue soon labels Jun 16, 2023

nzakas previously approved these changes Jun 16, 2023

View reviewed changes

mdjermanovic added the core Relates to ESLint's core APIs and features label Jun 19, 2023

mdjermanovic reviewed Jun 19, 2023

View reviewed changes

bradzacher mentioned this pull request Jun 24, 2023

fix(eslint-plugin): fix schemas across several rules and add schema tests typescript-eslint/typescript-eslint#6947

Merged

3 tasks

bmish and others added 4 commits July 23, 2023 13:46

Update docs/src/integrate/nodejs-api.md

a61487b

Co-authored-by: Nicholas C. Zakas <[email protected]>

Update lib/shared/schema-validation-error.js

bd22fa5

Co-authored-by: Nicholas C. Zakas <[email protected]>

Update lib/rule-tester/rule-tester.js

0f606e3

Co-authored-by: Milos Djermanovic <[email protected]>

null check

f015c1b

bmish and others added 5 commits July 23, 2023 14:09

Update lib/rule-tester/rule-tester.js

f03dbb2

Co-authored-by: Milos Djermanovic <[email protected]>

fix jsdoc

109fab0

reword node api doc

de93fe6

test null check

2b39ad3

bmish mentioned this pull request Jul 25, 2023

fix!: Update ajv options to align with aims for behaviour #17356

Closed

1 task

bmish marked this pull request as draft August 1, 2023 09:42

bmish commented Aug 1, 2023

View reviewed changes

bmish dismissed nzakas’s stale review via 2b39ad3 March 12, 2024 12:05

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: ability to test rule errors and invalid schemas #16823

feat: ability to test rule errors and invalid schemas #16823

bmish commented Jan 25, 2023 •

edited

netlify bot commented Jan 25, 2023 •

edited

github-actions bot commented Feb 12, 2023

Rec0iL99 commented Feb 13, 2023 •

edited

bmish May 22, 2023

nzakas Jun 16, 2023

bmish commented May 22, 2023

fasttime commented Jun 16, 2023

nzakas left a comment

nzakas Jun 16, 2023

mdjermanovic Jun 19, 2023

bmish Jul 23, 2023

mdjermanovic Jun 19, 2023

mdjermanovic Jun 19, 2023

bmish Jul 24, 2023

nzakas commented Jul 14, 2023

nzakas commented Jul 28, 2023

bmish Aug 1, 2023

fasttime commented Aug 20, 2023

nzakas commented Jan 17, 2024

bmish commented Jan 17, 2024

nzakas commented Jan 17, 2024

feat: ability to test rule errors and invalid schemas #16823

Are you sure you want to change the base?

feat: ability to test rule errors and invalid schemas #16823

Conversation

bmish commented Jan 25, 2023 • edited

Prerequisites checklist

What is the purpose of this pull request? (put an "X" next to an item)

What changes did you make? (Give an overview)

Is there anything you'd like reviewers to focus on?

netlify bot commented Jan 25, 2023 • edited

✅ Deploy Preview for docs-eslint canceled.

github-actions bot commented Feb 12, 2023

Rec0iL99 commented Feb 13, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bmish commented May 22, 2023

fasttime commented Jun 16, 2023

nzakas left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nzakas commented Jul 14, 2023

nzakas commented Jul 28, 2023

Choose a reason for hiding this comment

fasttime commented Aug 20, 2023

nzakas commented Jan 17, 2024

bmish commented Jan 17, 2024

nzakas commented Jan 17, 2024

bmish commented Jan 25, 2023 •

edited

netlify bot commented Jan 25, 2023 •

edited

Rec0iL99 commented Feb 13, 2023 •

edited