Repository: playwright-community/eslint-plugin-playwright
Pull request #309 (2024-08-19 14:41:12)

Apply title rules to test.step
Updated `valid-title` rule to apply to `test.step` commands.

Albinutte: @mskelton Thanks for the review! I addressed your comment (it was only about the first test case, right?) and pushed a tiny update to rule description.

Patch:
diff --git a/docs/rules/valid-title.md b/docs/rules/valid-title.md
index 5e69637..e8393e6 100644
--- a/docs/rules/valid-title.md
+++ b/docs/rules/valid-title.md
@@ -143,6 +143,7 @@ test.describe('foo', () => {
 ```ts
 interface Options {
   ignoreSpaces?: boolean
+  ignoreTypeOfStepName?: boolean
   ignoreTypeOfTestName?: boolean
   ignoreTypeOfDescribeName?: boolean
   disallowedWords?: string[]
@@ -157,6 +158,13 @@ Default: `false`
 
 When enabled, the leading and trailing spaces won't be checked.
 
+#### `ignoreTypeOfStepName`
+
+Default: `true`
+
+When enabled, the type of the first argument to `test.step` blocks won't be
+checked.
+
 #### `ignoreTypeOfDescribeName`
 
 Default: `false`
diff --git a/src/rules/valid-title.ts b/src/rules/valid-title.ts
index 004b1e7..30d9f6d 100644
--- a/src/rules/valid-title.ts
+++ b/src/rules/valid-title.ts
@@ -44,6 +44,7 @@ const compileMatcherPatterns = (
 
     return {
       describe: compiledMatcher,
+      step: compiledMatcher,
       test: compiledMatcher,
     }
   }
@@ -52,6 +53,7 @@ const compileMatcherPatterns = (
     describe: matchers.describe
       ? compileMatcherPattern(matchers.describe)
       : null,
+    step: matchers.step ? compileMatcherPattern(matchers.step) : null,
     test: matchers.test ? compileMatcherPattern(matchers.test) : null,
   }
 }
@@ -67,12 +69,13 @@ const MatcherAndMessageSchema = {
   type: 'array',
 } as const
 
-type MatcherGroups = 'describe' | 'test'
+type MatcherGroups = 'describe' | 'step' | 'test'
 
 interface Options {
   disallowedWords?: string[]
   ignoreSpaces?: boolean
   ignoreTypeOfDescribeName?: boolean
+  ignoreTypeOfStepName?: boolean
   ignoreTypeOfTestName?: boolean
   mustMatch?:
     | Partial<Record<MatcherGroups, string | MatcherAndMessage>>
@@ -91,6 +94,7 @@ export default createRule({
       disallowedWords = [],
       ignoreSpaces = false,
       ignoreTypeOfDescribeName = false,
+      ignoreTypeOfStepName = true,
       ignoreTypeOfTestName = false,
       mustMatch,
       mustNotMatch,
@@ -106,7 +110,11 @@ export default createRule({
     return {
       CallExpression(node) {
         const call = parseFnCall(context, node)
-        if (call?.type !== 'test' && call?.type !== 'describe') {
+        if (
+          call?.type !== 'test' &&
+          call?.type !== 'describe' &&
+          call?.type !== 'step'
+        ) {
           return
         }
 
@@ -124,7 +132,8 @@ export default createRule({
           if (
             !(
               (call.type === 'describe' && ignoreTypeOfDescribeName) ||
-              (call.type === 'test' && ignoreTypeOfTestName)
+              (call.type === 'test' && ignoreTypeOfTestName) ||
+              (call.type === 'step' && ignoreTypeOfStepName)
             ) &&
             (argument as ESTree.Node).type !== 'TemplateLiteral'
           ) {
@@ -262,7 +271,7 @@ export default createRule({
                 additionalProperties: {
                   oneOf: [{ type: 'string' }, MatcherAndMessageSchema],
                 },
-                propertyNames: { enum: ['describe', 'test'] },
+                propertyNames: { enum: ['describe', 'test', 'step'] },
                 type: 'object',
               },
             ],
@@ -281,6 +290,10 @@ export default createRule({
             default: false,
             type: 'boolean',
           },
+          ignoreTypeOfStepName: {
+            default: true,
+            type: 'boolean',
+          },
           ignoreTypeOfTestName: {
             default: false,
             type: 'boolean',


Test patch:
diff --git a/src/rules/valid-title.test.ts b/src/rules/valid-title.test.ts
index 9a11ffb..8263a51 100644
--- a/src/rules/valid-title.test.ts
+++ b/src/rules/valid-title.test.ts
@@ -63,6 +63,18 @@ runRuleTester('valid-title', rule, {
       ],
       options: [{ disallowedWords: ['properly'] }],
     },
+    {
+      code: 'test.step(`that the value is set properly`, function () {})',
+      errors: [
+        {
+          column: 11,
+          data: { word: 'properly' },
+          line: 1,
+          messageId: 'disallowedWord',
+        },
+      ],
+      options: [{ disallowedWords: ['properly'] }],
+    },
     // Global aliases
     {
       code: 'it("the correct way to properly handle all things", () => {});',
@@ -142,7 +154,9 @@ runRuleTester('mustMatch & mustNotMatch options', rule, {
           });
 
           test.describe('e2e tests #e4e', () => {
-            test('is another test #e2e #playwright4life', () => {});
+            test('is another test #e2e #playwright4life', () => {
+              test.step('#wow', () => {});
+            });
           });
         });
       `,
@@ -165,6 +179,15 @@ runRuleTester('mustMatch & mustNotMatch options', rule, {
           line: 9,
           messageId: 'mustNotMatch',
         },
+        {
+          column: 17,
+          data: {
+            functionName: 'step',
+            pattern: /(?:#(?!unit|e2e))\w+/u,
+          },
+          line: 10,
+          messageId: 'mustNotMatch',
+        },
       ],
       options: [
         {
@@ -183,7 +206,9 @@ runRuleTester('mustMatch & mustNotMatch options', rule, {
           });
 
           test.describe('e2e tests #e4e', () => {
-            test('is another test #e2e #playwright4life', () => {});
+            test('is another test #e2e #playwright4life', () => {
+              test.step('#wow', () => {});
+            });
           });
         });
       `,
@@ -208,6 +233,15 @@ runRuleTester('mustMatch & mustNotMatch options', rule, {
           line: 9,
           messageId: 'mustNotMatchCustom',
         },
+        {
+          column: 17,
+          data: {
+            message: 'Please include "#unit" or "#e2e" in titles',
+            pattern: /(?:#(?!unit|e2e))\w+/u,
+          },
+          line: 10,
+          messageId: 'mustNotMatchCustom',
+        },
       ],
       options: [
         {
@@ -232,7 +266,9 @@ runRuleTester('mustMatch & mustNotMatch options', rule, {
           });
 
           test.describe('e2e tests #e4e', () => {
-            test('is another test #e2e #playwright4life', () => {});
+            test('is another test #e2e #playwright4life', () => {
+              test.step('#wow', () => {});
+            });
           });
         });
       `,
@@ -264,7 +300,9 @@ runRuleTester('mustMatch & mustNotMatch options', rule, {
           });
 
           test.describe('e2e tests #e4e', () => {
-            test('is another test #e2e #playwright4life', () => {});
+            test('is another test #e2e #playwright4life', () => {
+              test.step('#wow', () => {});
+            });
           });
         });
       `,
@@ -302,7 +340,9 @@ runRuleTester('mustMatch & mustNotMatch options', rule, {
           });
 
           test.describe('e2e tests #e4e', () => {
-            test('is another test #e2e #playwright4life', () => {});
+            test('is another test #e2e #playwright4life', () => {
+              test.step('#wow', () => {});
+            });
           });
         });
       `,
@@ -334,7 +374,9 @@ runRuleTester('mustMatch & mustNotMatch options', rule, {
           });
 
           test.describe('e2e tests #e4e', () => {
-            test('is another test #e2e #playwright4life', () => {});
+            test('is another test #e2e #playwright4life', () => {
+              test.step('#wow', () => {});
+            });
           });
         });
       `,
@@ -409,6 +451,21 @@ runRuleTester('mustMatch & mustNotMatch options', rule, {
         { mustMatch: { describe: /#(?:unit|integration|e2e)/u.source } },
       ],
     },
+    {
+      code: 'test.step("the test", () => {});',
+      errors: [
+        {
+          column: 11,
+          data: {
+            functionName: 'step',
+            pattern: /#(?:unit|integration|e2e)/u,
+          },
+          line: 1,
+          messageId: 'mustMatch',
+        },
+      ],
+      options: [{ mustMatch: { step: /#(?:unit|integration|e2e)/u.source } }],
+    },
     {
       code: 'test.describe.skip("the test", () => {});',
       errors: [
@@ -503,6 +560,12 @@ runRuleTester('mustMatch & mustNotMatch options', rule, {
         { mustMatch: { describe: /#(?:unit|integration|e2e)/u.source } },
       ],
     },
+    {
+      code: 'test.step("correctly sets the value", () => {});',
+      options: [
+        { mustMatch: { describe: /#(?:unit|integration|e2e)/u.source } },
+      ],
+    },
     {
       code: javascript`
         test.describe('things to test', () => {
@@ -657,6 +720,17 @@ runRuleTester('title-must-be-string', rule, {
         },
       ],
     },
+    {
+      code: 'test.step(123, () => {});',
+      errors: [
+        {
+          column: 11,
+          line: 1,
+          messageId: 'titleMustBeString',
+        },
+      ],
+      options: [{ ignoreTypeOfStepName: false }],
+    },
     // Global aliases
     {
       code: 'it(String(/.+/), () => {});',
@@ -685,6 +759,7 @@ runRuleTester('title-must-be-string', rule, {
     'test.describe.skip("is a string", () => {});',
     'test.describe.skip(`${myFunc} is a string`, () => {});',
     'test.describe("is a string", () => {});',
+    'test.step(123, () => {});',
     {
       code: 'test.describe(String(/.+/), () => {});',
       options: [{ ignoreTypeOfDescribeName: true }],
@@ -811,6 +886,17 @@ runRuleTester('no-empty-title', rule, {
         },
       ],
     },
+    {
+      code: 'test.step(``, function () {})',
+      errors: [
+        {
+          column: 1,
+          data: { functionName: 'step' },
+          line: 1,
+          messageId: 'emptyTitle',
+        },
+      ],
+    },
     // Global aliases
     {
       code: 'it.describe("", function () {})',
@@ -840,6 +926,7 @@ runRuleTester('no-empty-title', rule, {
     'test.skip(`foo`, function () {})',
     'test(`${foo}`, function () {})',
     'test.fixme(`${foo}`, function () {})',
+    'test.step(`${foo}`, function () {})',
     // Global aliases
     {
       code: 'test.describe()',
@@ -969,6 +1056,21 @@ runRuleTester('no-accidental-space', rule, {
       errors: [{ column: 6, line: 1, messageId: 'accidentalSpace' }],
       output: 'test("foo", function () {})',
     },
+    {
+      code: 'test.step(` foo bar bang  `, function () {})',
+      errors: [{ column: 11, line: 1, messageId: 'accidentalSpace' }],
+      output: 'test.step(`foo bar bang`, function () {})',
+    },
+    {
+      code: 'test.step(" foo", function () {})',
+      errors: [{ column: 11, line: 1, messageId: 'accidentalSpace' }],
+      output: 'test.step("foo", function () {})',
+    },
+    {
+      code: 'test.step(" foo  ", function () {})',
+      errors: [{ column: 11, line: 1, messageId: 'accidentalSpace' }],
+      output: 'test.step("foo", function () {})',
+    },
     {
       code: javascript`
         test.describe(' foo', () => {
@@ -1013,6 +1115,8 @@ runRuleTester('no-accidental-space', rule, {
     'test("foo", function () {})',
     'test.describe()',
     'test.describe("foo", function () {})',
+    'test.step()',
+    'test.step("foo", function () {})',
     'test.only()',
     'test.only("foo", function () {})',
     javascript`
@@ -1135,6 +1239,37 @@ runRuleTester('no-duplicate-prefix test', rule, {
   ],
 })
 
+runRuleTester('no-duplicate-prefix step', rule, {
+  invalid: [
+    {
+      code: 'test.step("step foo", function () {})',
+      errors: [{ column: 11, line: 1, messageId: 'duplicatePrefix' }],
+      output: 'test.step("foo", function () {})',
+    },
+    {
+      code: 'test.step("step foo", function () {})',
+      errors: [{ column: 11, line: 1, messageId: 'duplicatePrefix' }],
+      output: 'test.step("foo", function () {})',
+    },
+    {
+      code: 'test.step("step foo", function () {})',
+      errors: [{ column: 11, line: 1, messageId: 'duplicatePrefix' }],
+      output: 'test.step("foo", function () {})',
+    },
+    {
+      code: "test.step('step foo', function () {})",
+      errors: [{ column: 11, line: 1, messageId: 'duplicatePrefix' }],
+      output: "test.step('foo', function () {})",
+    },
+    {
+      code: 'test.step(`step foo`, function () {})',
+      errors: [{ column: 11, line: 1, messageId: 'duplicatePrefix' }],
+      output: 'test.step(`foo`, function () {})',
+    },
+  ],
+  valid: ['test.step("foo", function () {})'],
+})
+
 runRuleTester('no-duplicate-prefix nested', rule, {
   invalid: [
     {
@@ -1176,6 +1311,23 @@ runRuleTester('no-duplicate-prefix nested', rule, {
         })
       `,
     },
+    {
+      code: javascript`
+        test.describe('foo', () => {
+          test('bar', () => {
+            test.step('step foobar', () => {})
+          })
+        })
+      `,
+      errors: [{ column: 15, line: 3, messageId: 'duplicatePrefix' }],
+      output: javascript`
+        test.describe('foo', () => {
+          test('bar', () => {
+            test.step('foobar', () => {})
+          })
+        })
+      `,
+    },
     // Global aliases
     {
       code: javascript`