[RuleTester] Test case validation

[DMartens]

The version of ESLint you are using.
Node version: v15.3.0
npm version: v6.14.2
Local ESLint version: v7.15.0 (Currently used)
Global ESLint version: Not found

The problem you want to solve.
The rule tester throws a type error and does not run any test case if the test case property is not a string:

import { RuleTester } from 'eslint';
const ruleTester = new RuleTester();
ruleTester.run("rule-id", rule, {
	valid: [{
		code: [
			'...',
		] // <-- missing .join('\n')
	}],
	invalid: [],
});

which results in an ugly error:

text.replace is not a function 
  at ​​​sanitize​​​ ​./node_modules/eslint/lib/rule-tester/rule-tester.js:207

Relevant code is here.
I categorized this as a change as there is only little test case validation but this issue may also be seen as a bug report.

Your take on the correct solution to problem.
The rule tester only fails the impacted test case with an instruction on how to correct the error.
This may be expanded to other properties than the code property.
Currently the rule tester only checks the structure of the scenarios (eg. valid and invalid are present) and for test cases that options are an array and that parser is an absolute path.

Are you willing to submit a pull request to implement this change?
Yes

[anikethsaha]

Thanks for the issue

so your proposal is to allow objects, arrays too along with a string for code ?
if this is so, IMO, I wouldn't prefer that as it seems just an extra operation for the Eslint to handle with little usability. It's always better to handle this by the developer itself.

Let me know if I am getting this wrong.

[DMartens]

No I don't propose allowing other types for the code property.
The described problem happens if the code property is not present or is not a string.
The problem is that because the type error is thrown, no test case is executed and you don't know why.
Especially if you have many test cases, it is hard to track down why the error happened.
My proposal is simply to validate the test case properties (in this case the code property) that they have the appropriate type. This would happen inside the test case, so the developer knows which test case has the error.

[anikethsaha]

Okay, so to add a check for code and throwing appropriate error right?

Make sense to me.

[mdjermanovic]

Makes sense to me, it would be useful to provide more helpful info. I'll champion this.

Currently the rule tester only checks the structure of the scenarios (eg. valid and invalid are present) and for test cases that options are an array and that parser is an absolute path.

Some properties are passed through as ESLint config properties and validated as such. For example, try to specify globals: [] or env: [].

These are all RuleTester-specific properties:

eslint/lib/rule-tester/rule-tester.js

Lines 118 to 124 in 06b5809

	const RuleTesterParameters = [
	"code",
	"filename",
	"options",
	"errors",
	"output"
	];

options are validated in runRuleForItem, errors is validated in testInvalidTemplate.

Aside from code, we can consider explicit validations for the remaining two properties, but I think we should limit the scope to only those that certainly cause throwing or failing the test if the type is invalid. Other changes would probably need a RFC.

[bmish]

I have opened up a fix for this in #15425.

Note that separately, I also have an RFC open for some additional test validations that are breaking changes: eslint/rfcs#84. Please comment on the RFC if there are additional breaking changes you would like to suggest.

[mdjermanovic]

Marked as accepted since this is more of a bug than an enhancement.