Skip to content

Commit ceb07dc

Browse files
authored
feat: add new rule require-test-case-name (#553)
* feat(test-case-name-property): add rule to require `name` for test cases This change adds a new rule to require `name` be present in test cases under certain conditions. This rule aims to ensure test suites are producing logs in a form that make it easy to identify failing test, when they happen. For thoroughly tested rules, it's not uncommon to have the same `code` across multiple test cases, with only `options` or `settings` differing between them. Requiring these test cases to have a `name` helps ensure the test output is meaningful and distinct. * Address copilot feedbac * address some of the lint violations * fix more lint violations * rename rule * update meta.docs.url * fix more lint violations * fix more lint violations * fix more lint violations * added additional tests
1 parent 6a2f25d commit ceb07dc

16 files changed

+769
-0
lines changed

README.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,7 @@ export default [
105105
| [no-identical-tests](docs/rules/no-identical-tests.md) | disallow identical tests || 🔧 | | |
106106
| [no-only-tests](docs/rules/no-only-tests.md) | disallow the test case property `only` || | 💡 | |
107107
| [prefer-output-null](docs/rules/prefer-output-null.md) | disallow invalid RuleTester test cases where the `output` matches the `code` || 🔧 | | |
108+
| [require-test-case-name](docs/rules/require-test-case-name.md) | require test cases to have a `name` property under certain conditions | | | | |
108109
| [test-case-property-ordering](docs/rules/test-case-property-ordering.md) | require the properties of a test case to be placed in a consistent order | | 🔧 | | |
109110
| [test-case-shorthand-strings](docs/rules/test-case-shorthand-strings.md) | enforce consistent usage of shorthand strings for test cases with no options | | 🔧 | | |
110111

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
# Require test cases to have a `name` property under certain conditions (`eslint-plugin/require-test-case-name`)
2+
3+
<!-- end auto-generated rule header -->
4+
5+
This rule enforces that test cases include a `name` property, under certain circumstances based on the configuration.
6+
7+
## Rule Details
8+
9+
This rule aims to ensure test suites are producing logs in a form that make it easy to identify failing test, when they happen.
10+
For thoroughly tested rules, it's not uncommon to have the same `code` across multiple test cases, with only `options` or `settings` differing between them.
11+
Requiring these test cases to have a `name` helps ensure the test output is meaningful and distinct.
12+
13+
### Options
14+
15+
This rule has one option.
16+
17+
#### `require: 'always' | 'objects' | 'objects-with-config'`
18+
19+
- `always`: all test cases should have a `name` property (this means that no shorthand string test cases are allowed as a side effect)
20+
- `objects`: requires that a `name` property is present in all `object`-based test cases.
21+
- `objects-with-config` (default): requires that test cases that have `options` or `settings` defined, should also have a `name` property.
22+
23+
Examples of **incorrect** code for this rule:
24+
25+
```js
26+
// invalid; require: objects-with-config (default)
27+
const testCase1 = {
28+
code: 'foo',
29+
options: ['baz'],
30+
};
31+
32+
// invalid; require: objects
33+
const testCase2 = {
34+
code: 'foo',
35+
};
36+
37+
// invalid; require: always
38+
const testCase3 = 'foo';
39+
```
40+
41+
Examples of **correct** code for this rule:
42+
43+
```js
44+
// require: objects-with-config, objects
45+
const testCase1 = 'foo';
46+
47+
// require: objects-with-config, objects, always
48+
const testCase2 = {
49+
code: 'foo',
50+
options: ['baz'],
51+
name: "foo (option: ['baz'])",
52+
};
53+
54+
// require: objects-with-config, objects, always
55+
const testCase4 = {
56+
code: 'foo',
57+
name: 'foo without options',
58+
};
59+
```
60+
61+
## When Not to Use It
62+
63+
If you aren't concerned with the nature of the test logs or don't want to require `name` on test cases.

lib/index.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ import requireMetaHasSuggestions from './rules/require-meta-has-suggestions.ts';
3636
import requireMetaSchemaDescription from './rules/require-meta-schema-description.ts';
3737
import requireMetaSchema from './rules/require-meta-schema.ts';
3838
import requireMetaType from './rules/require-meta-type.ts';
39+
import requireTestCaseName from './rules/require-test-case-name.ts';
3940
import testCasePropertyOrdering from './rules/test-case-property-ordering.ts';
4041
import testCaseShorthandStrings from './rules/test-case-shorthand-strings.ts';
4142

@@ -115,6 +116,7 @@ const allRules = {
115116
'require-meta-schema-description': requireMetaSchemaDescription,
116117
'require-meta-schema': requireMetaSchema,
117118
'require-meta-type': requireMetaType,
119+
'require-test-case-name': requireTestCaseName,
118120
'test-case-property-ordering': testCasePropertyOrdering,
119121
'test-case-shorthand-strings': testCaseShorthandStrings,
120122
} satisfies Record<string, Rule.RuleModule>;
Lines changed: 141 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,141 @@
1+
import type { Rule } from 'eslint';
2+
3+
import { evaluateObjectProperties, getKeyName, getTestInfo } from '../utils.ts';
4+
import type { TestInfo } from '../types.ts';
5+
6+
type TestCaseData = {
7+
node: NonNullable<TestInfo['valid'][number]>;
8+
isObject: boolean;
9+
hasName: boolean;
10+
hasConfig: boolean;
11+
};
12+
13+
const violationFilters = {
14+
always: (testCase: TestCaseData) => !testCase.hasName,
15+
objects: (testCase: TestCaseData) => testCase.isObject && !testCase.hasName,
16+
'objects-with-config': (testCase: TestCaseData) =>
17+
testCase.isObject && testCase.hasConfig && !testCase.hasName,
18+
} satisfies Record<Options['require'], (testCase: TestCaseData) => boolean>;
19+
20+
const violationMessages = {
21+
always: 'nameRequiredAlways',
22+
objects: 'nameRequiredObjects',
23+
'objects-with-config': 'nameRequiredObjectsWithConfig',
24+
} satisfies Record<Options['require'], string>;
25+
26+
type Options = {
27+
require: 'always' | 'objects' | 'objects-with-config';
28+
};
29+
30+
const rule: Rule.RuleModule = {
31+
meta: {
32+
type: 'suggestion',
33+
docs: {
34+
description:
35+
'require test cases to have a `name` property under certain conditions',
36+
category: 'Tests',
37+
recommended: false,
38+
url: 'https://github.com/eslint-community/eslint-plugin-eslint-plugin/tree/HEAD/docs/rules/require-test-case-name.md',
39+
},
40+
schema: [
41+
{
42+
additionalProperties: false,
43+
properties: {
44+
require: {
45+
description:
46+
'When should the name property be required on a test case object.',
47+
enum: ['always', 'objects', 'objects-with-config'],
48+
},
49+
},
50+
type: 'object',
51+
},
52+
],
53+
defaultOptions: [{ require: 'objects-with-config' }],
54+
messages: {
55+
nameRequiredAlways:
56+
'This test case is missing the `name` property. All test cases should have a name property.',
57+
nameRequiredObjects:
58+
'This test case is missing the `name` property. Test cases defined as objects should have a name property.',
59+
nameRequiredObjectsWithConfig:
60+
'This test case is missing the `name` property. Test cases defined as objects with additional configuration should have a name property.',
61+
},
62+
},
63+
64+
create(context) {
65+
const { require: requireOption = 'objects-with-config' }: Options =
66+
context.options[0] || {};
67+
const sourceCode = context.sourceCode;
68+
69+
/**
70+
* Validates test cases and reports them if found in violation
71+
* @param cases A list of test case nodes
72+
*/
73+
function validateTestCases(cases: TestInfo['valid']): void {
74+
// Gather all of the information from each test case
75+
const testCaseData: TestCaseData[] = cases
76+
.filter((testCase) => !!testCase)
77+
.map((testCase) => {
78+
if (
79+
testCase.type === 'Literal' ||
80+
testCase.type === 'TemplateLiteral'
81+
) {
82+
return {
83+
node: testCase,
84+
isObject: false,
85+
hasName: false,
86+
hasConfig: false,
87+
};
88+
}
89+
if (testCase.type === 'ObjectExpression') {
90+
let hasName = false;
91+
let hasConfig = false;
92+
93+
// evaluateObjectProperties is used here to expand spread elements
94+
for (const property of evaluateObjectProperties(
95+
testCase,
96+
sourceCode.scopeManager,
97+
)) {
98+
if (property.type === 'Property') {
99+
const keyName = getKeyName(
100+
property,
101+
sourceCode.getScope(testCase),
102+
);
103+
if (keyName === 'name') {
104+
hasName = true;
105+
} else if (keyName === 'options' || keyName === 'settings') {
106+
hasConfig = true;
107+
}
108+
}
109+
}
110+
111+
return {
112+
node: testCase,
113+
isObject: true,
114+
hasName,
115+
hasConfig,
116+
};
117+
}
118+
return null;
119+
})
120+
.filter((testCase) => !!testCase);
121+
122+
const violations = testCaseData.filter(violationFilters[requireOption]);
123+
for (const violation of violations) {
124+
context.report({
125+
node: violation.node,
126+
messageId: violationMessages[requireOption],
127+
});
128+
}
129+
}
130+
131+
return {
132+
Program(ast) {
133+
getTestInfo(context, ast)
134+
.map((testRun) => [...testRun.valid, ...testRun.invalid])
135+
.forEach(validateTestCases);
136+
},
137+
};
138+
},
139+
};
140+
141+
export default rule;

tests/lib/rules/consistent-output.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,7 @@ ruleTester.run('consistent-output', rule, {
6767
});
6868
`,
6969
options: ['always'],
70+
name: 'test case with code, output, and errors (options: always)',
7071
},
7172
`
7273
new NotRuleTester().run('foo', bar, {
@@ -118,6 +119,7 @@ ruleTester.run('consistent-output', rule, {
118119
`,
119120
options: ['always'],
120121
errors: [ERROR],
122+
name: 'invalid test case missing output (options: always)',
121123
},
122124
],
123125
});

tests/lib/rules/meta-property-ordering.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,7 @@ ruleTester.run('test-case-property-ordering', rule, {
6565
create() {},
6666
};`,
6767
options: [['schema', 'docs']],
68+
name: 'custom order (options: [schema, docs])',
6869
},
6970
`
7071
module.exports = {
@@ -179,6 +180,7 @@ ruleTester.run('test-case-property-ordering', rule, {
179180
data: { order: ['type', 'docs', 'fixable'].join(', ') },
180181
},
181182
],
183+
name: 'custom order with extra prop (options: [type, docs, fixable])',
182184
},
183185
],
184186
});

tests/lib/rules/no-property-in-node.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -110,6 +110,7 @@ ruleTester.run('no-property-in-node', rule, {
110110
additionalNodeTypeFiles: [/not-found/],
111111
},
112112
],
113+
name: 'additionalNodeTypeFiles with no matches',
113114
},
114115
],
115116
invalid: [
@@ -204,6 +205,7 @@ ruleTester.run('no-property-in-node', rule, {
204205
messageId: 'in',
205206
},
206207
],
208+
name: 'additionalNodeTypeFiles with matches',
207209
},
208210
],
209211
});

0 commit comments

Comments
 (0)