Change Request: Stricter rule test validation

[bmish]

ESLint version

8.0.0-rc.0

What problem do you want to solve?

• Currently, it's easy to forget to assert what suggestions a test case yields, or that a test case yields no assertions. As a result, this can be an area of poor test coverage, leading to bugs around suggestions.
• It's also inconvenient to have to add suggestions: [] to every single invalid test case for a rule or tests that do not produce suggestions, in order to achieve the desired test coverage.

What do you think is the correct solution?

Invalid test cases that yield suggestions should require a suggestions assertion.

This would be the same as how invalid test cases that produce an autofix require an output assertion, and fail with this error:

AssertionError [ERR_ASSERTION]: The rule fixed the code. Please add 'output' property.

This behavior was added as a breaking change in ESLint v7.

We should add a similar assertion for suggestions.

AssertionError [ERR_ASSERTION]: The rule violation produced a suggestion. Please add a 'suggestions' property to the error object.

Example of invalid test case with suggestions assertion:

new RuleTester().run('example-rule', rule, {
  valid: [],
  invalid: [
    {
      code: `foo();`,
      output: null,
      errors: [
        {
          messageId: 'bar',
          type: 'Property',
          suggestions: [], // or array of suggestions if rule provides them
        },
      ],
    },
  ]
});

Participation

• I am willing to submit a pull request for this change.

Additional comments

No response

[nzakas]

This seems reasonable to me, however, it is a breaking change and would need to wait until ESLint v9.0.0, which won’t be until next year. That’s a really long time to wait, so not sure the best path forward here.

[bmish]

Would this require an RFC? If so, I can put that together so that it can be lined up for v9.

Also, have we begun tracking breaking changes for v9 yet? Even though it's a year away, I'd like to make sure this gets on the list (assuming it gets accepted).

[nzakas]

Yes, we would need an RFC.

and yes, we are tracking features for v9 on the v9 project.

[github-actions]

Oops! It looks like we lost track of this issue. What do we want to do here? This issue will auto-close in 7 days without an update.

[bmish]

Still planning to do this, assigned myself.

[bmish]

I have opened an RFC for this issue, along with some related rule test validation improvements: eslint/rfcs#84

[fisker]

👎 for this.

In some rule, I only add one test for the suggestions, maybe because it's simple and good enough, then focus other tests on the auto-fix or something else, eg "message", "report loaction", why I need assert suggestions in all those?

The tester should not force anything, test one thing in one case is good practice!

[nzakas]

Please review the RFC and leave your comments there.

[bmish]

Reopening since the RFC hasn't been implemented yet.

[mdjermanovic]

Since the related eslint/rfcs#84 includes more changes than just requiring suggestion assertions, I changed the title of this issue so that it tracks all the changes, i.e. the full implementation of eslint/rfcs#84:

We will add additional assertions to the rule test runner to address several potential points of poor test coverage, especially related to rule suggestions.

Assertions added to invalid test case error objects:

• Must contain suggestions if the test case produces suggestions
• Must contain the violation message (message or messageId, but not both)

Assertions added to invalid test case suggestion objects:

• Must contain the suggestion code output
• Must contain the suggestion message (messageId or desc, but not both)
• The suggestion code output must differ from the test case code

Assertions added to all test cases:

• Any autofix output must differ from the test case code
• Cannot have identical test cases
• The optional filename property must be a string
• The optional only property must be a boolean