Skip to content

Commit c0ce87a

Browse files
mskeltonmskelton
authored
feat: Add valid-title rule (#177)
* feat: Add `valid-title` rule * Cleanup
1 parent 34b3f7b commit c0ce87a

13 files changed

+1559
-22
lines changed

‎README.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -173,3 +173,4 @@ command line option.\
173173
| | | | [require-top-level-describe](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/require-top-level-describe.md) | Require test cases and hooks to be inside a `test.describe` block |
174174
| | 🔧 | | [require-soft-assertions](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/require-soft-assertions.md) | Require assertions to use `expect.soft()` |
175175
| ✔ | | | [valid-expect](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/valid-expect.md) | Enforce valid `expect()` usage |
176+
| ✔ | 🔧 | | [valid-title](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/valid-title.md) | Enforce valid titles |

‎docs/rules/require-top-level-describe.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ using the `maxTopLevelDescribes` option:
6262

6363
Examples of **incorrect** code with the above config:
6464

65-
```js
65+
```javascript
6666
test.describe('test suite', () => {
6767
test('test', () => {});
6868
});

‎docs/rules/valid-title.md

Lines changed: 250 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,250 @@
1+
# Enforce valid titles (`valid-title`)
2+
3+
Checks that the title of test blocks are valid by ensuring that titles are:
4+
5+
- not empty,
6+
- is a string,
7+
- not prefixed with their block name,
8+
- have no leading or trailing spaces
9+
10+
## Rule details
11+
12+
**emptyTitle**
13+
14+
An empty title is not informative, and serves little purpose.
15+
16+
Examples of **incorrect** code for this rule:
17+
18+
```javascript
19+
test.describe('', () => {});
20+
test.describe('foo', () => {
21+
test('', () => {});
22+
});
23+
test('', () => {});
24+
```
25+
26+
Examples of **correct** code for this rule:
27+
28+
```javascript
29+
test.describe('foo', () => {});
30+
test.describe('foo', () => {
31+
test('bar', () => {});
32+
});
33+
test('foo', () => {});
34+
```
35+
36+
**titleMustBeString**
37+
38+
Titles for `describe` and `test` blocks should always be a string; you can
39+
disable this with the `ignoreTypeOfDescribeName` and `ignoreTypeOfTestName`
40+
options.
41+
42+
Examples of **incorrect** code for this rule:
43+
44+
```javascript
45+
test(123, () => {});
46+
test.describe(String(/.+/), () => {});
47+
test.describe(myFunction, () => {});
48+
test.describe(6, function () {});
49+
```
50+
51+
Examples of **correct** code for this rule:
52+
53+
```javascript
54+
test('is a string', () => {});
55+
test.describe('is a string', () => {});
56+
```
57+
58+
Examples of **correct** code when `ignoreTypeOfDescribeName` is `true`:
59+
60+
```javascript
61+
test('is a string', () => {});
62+
test.describe('is a string', () => {});
63+
64+
test.describe(String(/.+/), () => {});
65+
test.describe(myFunction, () => {});
66+
test.describe(6, function () {});
67+
```
68+
69+
Examples of **correct** code when `ignoreTypeOfTestName` is `true`:
70+
71+
```javascript
72+
const myTestName = 'is a string';
73+
74+
test(String(/.+/), () => {});
75+
test(myFunction, () => {});
76+
test(myTestName, () => {});
77+
test(6, function () {});
78+
```
79+
80+
**duplicatePrefix**
81+
82+
A `describe` / `test` block should not start with `duplicatePrefix`
83+
84+
Examples of **incorrect** code for this rule
85+
86+
```javascript
87+
test('test foo', () => {});
88+
89+
test.describe('foo', () => {
90+
test('test bar', () => {});
91+
});
92+
93+
test.describe('describe foo', () => {
94+
test('bar', () => {});
95+
});
96+
```
97+
98+
Examples of **correct** code for this rule
99+
100+
```javascript
101+
test('foo', () => {});
102+
103+
test.describe('foo', () => {
104+
test('bar', () => {});
105+
});
106+
```
107+
108+
**accidentalSpace**
109+
110+
A `describe` / `test` block should not contain accidentalSpace, but can be
111+
turned off via the `ignoreSpaces` option:
112+
113+
Examples of **incorrect** code for this rule
114+
115+
```javascript
116+
test(' foo', () => {});
117+
118+
test.describe('foo', () => {
119+
test(' bar', () => {});
120+
});
121+
122+
test.describe(' foo', () => {
123+
test('bar', () => {});
124+
});
125+
126+
test.describe('foo ', () => {
127+
test('bar', () => {});
128+
});
129+
```
130+
131+
Examples of **correct** code for this rule
132+
133+
```javascript
134+
test('foo', () => {});
135+
136+
test.describe('foo', () => {
137+
test('bar', () => {});
138+
});
139+
```
140+
141+
## Options
142+
143+
```ts
144+
interface Options {
145+
ignoreSpaces?: boolean;
146+
ignoreTypeOfDescribeName?: boolean;
147+
disallowedWords?: string[];
148+
mustNotMatch?: Partial<Record<'describe' | 'test', string>> | string;
149+
mustMatch?: Partial<Record<'describe' | 'test', string>> | string;
150+
}
151+
```
152+
153+
#### `ignoreSpaces`
154+
155+
Default: `false`
156+
157+
When enabled, the leading and trailing spaces won't be checked.
158+
159+
#### `ignoreTypeOfDescribeName`
160+
161+
Default: `false`
162+
163+
When enabled, the type of the first argument to `describe` blocks won't be
164+
checked.
165+
166+
#### `disallowedWords`
167+
168+
Default: `[]`
169+
170+
A string array of words that are not allowed to be used in test titles. Matching
171+
is not case-sensitive, and looks for complete words:
172+
173+
Examples of **incorrect** code when using `disallowedWords`:
174+
175+
```javascript
176+
// with disallowedWords: ['correct', 'all', 'every', 'properly']
177+
test.describe('the correct way to do things', () => {});
178+
test.describe('every single one of them', () => {});
179+
test('has ALL the things', () => {});
180+
test(`that the value is set properly`, () => {});
181+
```
182+
183+
Examples of **correct** code when using `disallowedWords`:
184+
185+
```javascript
186+
// with disallowedWords: ['correct', 'all', 'every', 'properly']
187+
test('correctly sets the value', () => {});
188+
test('that everything is as it should be', () => {});
189+
test.describe('the proper way to handle things', () => {});
190+
```
191+
192+
#### `mustMatch` & `mustNotMatch`
193+
194+
Defaults: `{}`
195+
196+
Allows enforcing that titles must match or must not match a given Regular
197+
Expression, with an optional message. An object can be provided to apply
198+
different Regular Expressions (with optional messages) to specific Playwright
199+
test function groups (`describe`, `test`).
200+
201+
Examples of **incorrect** code when using `mustMatch`:
202+
203+
```javascript
204+
// with mustMatch: '^that'
205+
test.describe('the correct way to do things', () => {});
206+
test('this there!', () => {});
207+
208+
// with mustMatch: { test: '^that' }
209+
test.describe('the tests that will be run', () => {});
210+
test('the stuff works', () => {});
211+
test('errors that are thrown have messages', () => {});
212+
```
213+
214+
Examples of **correct** code when using `mustMatch`:
215+
216+
```javascript
217+
// with mustMatch: '^that'
218+
test.describe('that thing that needs to be done', () => {});
219+
test('that this there!', () => {});
220+
221+
// with mustMatch: { test: '^that' }
222+
test.describe('the tests will be run', () => {});
223+
test('that the stuff works', () => {});
224+
```
225+
226+
Optionally you can provide a custom message to show for a particular matcher by
227+
using a tuple at any level where you can provide a matcher:
228+
229+
```javascript
230+
const prefixes = ['when', 'with', 'without', 'if', 'unless', 'for'];
231+
const prefixesList = prefixes.join(' - \n');
232+
233+
module.exports = {
234+
rules: {
235+
'playwright/valid-title': [
236+
'error',
237+
{
238+
mustNotMatch: ['\\.$', 'Titles should not end with a full-stop'],
239+
mustMatch: {
240+
describe: [
241+
new RegExp(`^(?:[A-Z]|\\b(${prefixes.join('|')})\\b`, 'u').source,
242+
`Describe titles should either start with a capital letter or one of the following prefixes: ${prefixesList}`,
243+
],
244+
test: /[^A-Z]/u.source,
245+
},
246+
},
247+
],
248+
},
249+
};
250+
```

‎src/index.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ import preferWebFirstAssertions from './rules/prefer-web-first-assertions';
2727
import requireSoftAssertions from './rules/require-soft-assertions';
2828
import requireTopLevelDescribe from './rules/require-top-level-describe';
2929
import validExpect from './rules/valid-expect';
30+
import validTitle from './rules/valid-title';
3031

3132
const index = {
3233
configs: {},
@@ -59,6 +60,7 @@ const index = {
5960
'require-soft-assertions': requireSoftAssertions,
6061
'require-top-level-describe': requireTopLevelDescribe,
6162
'valid-expect': validExpect,
63+
'valid-title': validTitle,
6264
},
6365
};
6466

@@ -82,6 +84,7 @@ const sharedConfig = {
8284
'playwright/no-wait-for-timeout': 'warn',
8385
'playwright/prefer-web-first-assertions': 'error',
8486
'playwright/valid-expect': 'error',
87+
'playwright/valid-title': 'error',
8588
},
8689
};
8790

‎src/rules/expect-expect.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { Rule } from 'eslint';
22
import * as ESTree from 'estree';
3-
import { dig, isExpectCall, isTest } from '../utils/ast';
3+
import { dig, isExpectCall, isTestCall } from '../utils/ast';
44
import { getAdditionalAssertFunctionNames } from '../utils/misc';
55

66
function isAssertionCall(
@@ -33,7 +33,7 @@ export default {
3333

3434
return {
3535
CallExpression(node) {
36-
if (isTest(node, ['fixme', 'only', 'skip'])) {
36+
if (isTestCall(node, ['fixme', 'only', 'skip'])) {
3737
unchecked.push(node);
3838
} else if (isAssertionCall(node, additionalAssertFunctionNames)) {
3939
checkExpressions(context.sourceCode.getAncestors(node));

‎src/rules/no-conditional-in-test.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,12 @@
11
import { Rule } from 'eslint';
2-
import { findParent, isTest } from '../utils/ast';
2+
import { findParent, isTestCall } from '../utils/ast';
33

44
export default {
55
create(context) {
66
function checkConditional(node: Rule.Node & Rule.NodeParentExtension) {
77
const call = findParent(node, 'CallExpression');
88

9-
if (call && isTest(call)) {
9+
if (call && isTestCall(call)) {
1010
context.report({ messageId: 'conditionalInTest', node });
1111
}
1212
}

‎src/rules/no-focused-test.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,12 @@
11
import { Rule } from 'eslint';
2-
import { isDescribeCall, isPropertyAccessor, isTest } from '../utils/ast';
2+
import { isDescribeCall, isPropertyAccessor, isTestCall } from '../utils/ast';
33

44
export default {
55
create(context) {
66
return {
77
CallExpression(node) {
88
if (
9-
(isTest(node) || isDescribeCall(node)) &&
9+
(isTestCall(node) || isDescribeCall(node)) &&
1010
node.callee.type === 'MemberExpression' &&
1111
isPropertyAccessor(node.callee, 'only')
1212
) {

‎src/rules/no-skipped-test.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import { Rule } from 'eslint';
22
import {
33
isDescribeCall,
44
isPropertyAccessor,
5-
isTest,
5+
isTestCall,
66
isTestIdentifier,
77
} from '../utils/ast';
88

@@ -19,7 +19,7 @@ export default {
1919
callee.type === 'MemberExpression' &&
2020
isPropertyAccessor(callee, 'skip')
2121
) {
22-
const isHook = isTest(node) || isDescribeCall(node);
22+
const isHook = isTestCall(node) || isDescribeCall(node);
2323

2424
// If allowConditional is enabled and it's not a test/describe hook,
2525
// we ignore any `test.skip` calls that have no arguments.

‎src/rules/prefer-lowercase-title.ts

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -3,18 +3,12 @@ import * as ESTree from 'estree';
33
import {
44
getStringValue,
55
isDescribeCall,
6-
isStringLiteral,
7-
isTest,
6+
isStringNode,
7+
isTestCall,
88
} from '../utils/ast';
99

1010
type Method = 'test' | 'test.describe';
1111

12-
function isString(
13-
node: ESTree.Expression | ESTree.SpreadElement,
14-
): node is ESTree.Literal | ESTree.TemplateLiteral {
15-
return node && (isStringLiteral(node) || node.type === 'TemplateLiteral');
16-
}
17-
1812
export default {
1913
create(context) {
2014
const { allowedPrefixes, ignore, ignoreTopLevelDescribe } = {
@@ -30,7 +24,7 @@ export default {
3024
CallExpression(node) {
3125
const method = isDescribeCall(node)
3226
? 'test.describe'
33-
: isTest(node)
27+
: isTestCall(node)
3428
? 'test'
3529
: null;
3630

@@ -45,7 +39,7 @@ export default {
4539
}
4640

4741
const [title] = node.arguments;
48-
if (!isString(title)) {
42+
if (!isStringNode(title)) {
4943
return;
5044
}
5145

0 commit comments

Comments
 (0)