feat(test runner): test.skip(title, testFunction) syntax (#7922)

docs/src/intro-js.md

@@ -133,20 +133,22 @@ test.only('focus this test', async ({ page }) => {
 
 ### Skip a test
 
-You can skip certain test based on the condition.
+Mark a test as skipped.
 
 ```js js-flavor=js
-test('skip this test', async ({ page, browserName }) => {
+test.skip('skip this test', async ({ page }) => {
-  test.skip(browserName === 'firefox', 'Still working on it');
+  // This test is not run
 });
 ```
 
 ```js js-flavor=ts
-test('skip this test', async ({ page, browserName }) => {
+test.skip('skip this test', async ({ page }) => {
-  test.skip(browserName === 'firefox', 'Still working on it');
+  // This test is not run
 });
 ```
 
+You can also skip a test when [some condition is met](./test-annotations.md#conditionally-skip-a-test).
+
 ### Group tests
 
 You can group tests to give them a logical name or to scope before/after hooks to the group.

docs/src/test-annotations-js.md

@@ -33,7 +33,23 @@ test.only('focus this test', async ({ page }) => {
 
 ## Skip a test
 
-You can skip certain tests based on the condition.
+Mark a test as skipped.
+
+```js js-flavor=js
+test.skip('skip this test', async ({ page }) => {
+  // This test is not run
+});
+```
+
+```js js-flavor=ts
+test.skip('skip this test', async ({ page }) => {
+  // This test is not run
+});
+```
+
+## Conditionally skip a test
+
+You can skip certain test based on the condition.
 
 ```js js-flavor=js
 test('skip this test', async ({ page, browserName }) => {

docs/src/test-api/class-test.md

@@ -528,13 +528,12 @@ Timeout in milliseconds.
 
 Skips a test or a group of tests.
 
-Unconditionally skip a test:
+Unconditionally skip a test, this is similar syntax to [`method: Test.(call)`]:
 
 ```js js-flavor=js
 const { test, expect } = require('@playwright/test');
 
-test('broken test', async ({ page }) => {
+test.skip('broken test', async ({ page }) => {
-  test.skip();
   // ...
 });
 ```
@@ -542,13 +541,12 @@ test('broken test', async ({ page }) => {
 ```js js-flavor=ts
 import { test, expect } from '@playwright/test';
 
-test('broken test', async ({ page }) => {
+test.skip('broken test', async ({ page }) => {
-  test.skip();
   // ...
 });
 ```
 
-Conditionally skip a test with an optional description:
+Conditionally skip a test with an optional description. In this case, call `test.skip()` inside the test function:
 
 ```js js-flavor=js
 const { test, expect } = require('@playwright/test');
@@ -617,15 +615,14 @@ test.beforeEach(async ({ page }) => {
 ```
 
 ### param: Test.skip.condition
-- `condition` <[void]|[boolean]|[function]\([Fixtures]\):[boolean]>
+- `titleOrCondition` <[string]|[void]|[boolean]|[function]\([Fixtures]\):[boolean]>
 
-Optional condition - either a boolean value, or a function that takes a fixtures object and returns a boolean. Test or tests are skipped when the condition is `true`.
+When used with `test.skip('test', () => {})` notation, first argument is a test title. Otherwise it is an optional skip condition - either a boolean value, or a function that takes a fixtures object and returns a boolean. Test or tests are skipped when the condition is `true`.
 
 ### param: Test.skip.description
-- `description` <[void]|[string]>
+- `testFunctionOrDescription` <[function]\([Fixtures], [TestInfo]\)|[void]|[string]>
-
-Optional description that will be reflected in a test report.
 
+When used with `test.skip('test', () => {})` notation, second argument is a test function. Otherwise it is an optional description that will be reflected in a test report.
 
 
 

src/test/test.ts

@@ -159,6 +159,7 @@ export class TestCase extends Base implements reporterTypes.TestCase {
     const test = new TestCase(this.title, this.fn, this._ordinalInFile, this._testType, this.location);
     test._only = this._only;
     test._requireFile = this._requireFile;
+    test.expectedStatus = this.expectedStatus;
     return test;
   }
 

src/test/testType.ts

@@ -55,7 +55,7 @@ export class TestTypeImpl {
     this.test = test;
   }
 
-  private _createTest(type: 'default' | 'only', location: Location, title: string, fn: Function) {
+  private _createTest(type: 'default' | 'only' | 'skip', location: Location, title: string, fn: Function) {
     throwIfRunningInsideJest();
     const suite = currentlyLoadingFileSuite();
     if (!suite)
@@ -70,6 +70,8 @@ export class TestTypeImpl {
 
     if (type === 'only')
       test._only = true;
+    if (type === 'skip')
+      test.expectedStatus = 'skipped';
   }
 
   private _describe(type: 'default' | 'only', location: Location, title: string, fn: Function) {
@@ -110,6 +112,12 @@ export class TestTypeImpl {
   private _modifier(type: 'skip' | 'fail' | 'fixme' | 'slow', location: Location, ...modifierArgs: [arg?: any | Function, description?: string]) {
     const suite = currentlyLoadingFileSuite();
     if (suite) {
+      if (typeof modifierArgs[0] === 'string' && typeof modifierArgs[1] === 'function') {
+        // Support for test.skip('title', () => {})
+        this._createTest('skip', location, modifierArgs[0], modifierArgs[1]);
+        return;
+      }
+
       if (typeof modifierArgs[0] === 'function') {
         suite._modifiers.push({ type, fn: modifierArgs[0], location, description: modifierArgs[1] });
       } else {

src/test/workerRunner.ts

@@ -227,7 +227,7 @@ export class WorkerRunner extends EventEmitter {
       fn: test.fn,
       repeatEachIndex: this._params.repeatEachIndex,
       retry: entry.retry,
-      expectedStatus: 'passed',
+      expectedStatus: test.expectedStatus,
       annotations: [],
       attachments: [],
       duration: 0,
@@ -493,6 +493,16 @@ function buildTestEndPayload(testId: string, testInfo: TestInfo): TestEndPayload
 }
 
 function modifier(testInfo: TestInfo, type: 'skip' | 'fail' | 'fixme' | 'slow', modifierArgs: [arg?: any, description?: string]) {
+  if (typeof modifierArgs[1] === 'function') {
+    throw new Error([
+      'It looks like you are calling test.skip() inside the test and pass a callback.',
+      'Pass a condition instead and optional description instead:',
+      `test('my test', async ({ page, isMobile }) => {`,
+      `  test.skip(isMobile, 'This test is not applicable on mobile');`,
+      `});`,
+    ].join('\n'));
+  }
+
   if (modifierArgs.length >= 1 && !modifierArgs[0])
     return;
 

tests/playwright-test/basic.spec.ts

@@ -366,3 +366,19 @@ test('should help with describe() misuse', async ({ runInlineTest }) => {
     `});`,
   ].join('\n'));
 });
+
+test('test.skip should define a skipped test', async ({ runInlineTest }) => {
+  const result = await runInlineTest({
+    'a.test.ts': `
+      const { test } = pwt;
+      const logs = [];
+      test.skip('foo', () => {
+        console.log('%%dontseethis');
+        throw new Error('foo');
+      });
+    `,
+  });
+  expect(result.exitCode).toBe(0);
+  expect(result.skipped).toBe(1);
+  expect(result.output).not.toContain('%%dontseethis');
+});

tests/playwright-test/test-modifiers.spec.ts

@@ -288,3 +288,21 @@ test('test.skip without a callback in describe block should skip hooks', async (
   expect(result.report.suites[0].suites[0].specs[0].tests[0].annotations).toEqual([{ type: 'skip', description: 'reason' }]);
   expect(result.output).not.toContain('%%');
 });
+
+test('test.skip should not define a skipped test inside another test', async ({ runInlineTest }) => {
+  const result = await runInlineTest({
+    'a.test.ts': `
+      const { test } = pwt;
+      const logs = [];
+      test('passes', () => {
+        test.skip('foo', () => {
+          console.log('%%dontseethis');
+          throw new Error('foo');
+        });
+      });
+    `,
+  });
+  expect(result.exitCode).toBe(1);
+  expect(result.failed).toBe(1);
+  expect(result.output).toContain('It looks like you are calling test.skip() inside the test and pass a callback');
+});

types/test.d.ts

@@ -1177,7 +1177,7 @@ export interface TestInfo {
 
   /**
    * Skips the currently running test. This is similar to
-   * [test.skip([condition, description])](https://playwright.dev/docs/api/class-test#test-skip).
+   * [test.skip(titleOrCondition, testFunctionOrDescription)](https://playwright.dev/docs/api/class-test#test-skip).
    * @param condition Optional condition - the test is skipped when the condition is `true`.
    * @param description Optional description that will be reflected in a test report.
    */
@@ -1245,7 +1245,7 @@ export interface TestInfo {
   /**
    * Expected status for the currently running test. This is usually `'passed'`, except for a few cases:
    * - `'skipped'` for skipped tests, e.g. with
-   *   [test.skip([condition, description])](https://playwright.dev/docs/api/class-test#test-skip);
+   *   [test.skip(titleOrCondition, testFunctionOrDescription)](https://playwright.dev/docs/api/class-test#test-skip);
    * - `'failed'` for tests marked as failed with
    *   [test.fail([condition, description])](https://playwright.dev/docs/api/class-test#test-fail).
    *
@@ -1527,13 +1527,13 @@ export interface TestType<TestArgs extends KeyValue, WorkerArgs extends KeyValue
   /**
    * Skips a test or a group of tests.
    *
-   * Unconditionally skip a test:
+   * Unconditionally skip a test, this is similar syntax to
+   * [test.(call)(title, testFunction)](https://playwright.dev/docs/api/class-test#test-call):
    *
    * ```js js-flavor=js
    * const { test, expect } = require('@playwright/test');
    *
-   * test('broken test', async ({ page }) => {
+   * test.skip('broken test', async ({ page }) => {
-   *   test.skip();
    *   // ...
    * });
    * ```
@@ -1541,13 +1541,12 @@ export interface TestType<TestArgs extends KeyValue, WorkerArgs extends KeyValue
    * ```js js-flavor=ts
    * import { test, expect } from '@playwright/test';
    *
-   * test('broken test', async ({ page }) => {
+   * test.skip('broken test', async ({ page }) => {
-   *   test.skip();
    *   // ...
    * });
    * ```
    *
-   * Conditionally skip a test with an optional description:
+   * Conditionally skip a test with an optional description. In this case, call `test.skip()` inside the test function:
    *
    * ```js js-flavor=js
    * const { test, expect } = require('@playwright/test');
@@ -1616,8 +1615,9 @@ export interface TestType<TestArgs extends KeyValue, WorkerArgs extends KeyValue
    * });
    * ```
    *
-   * @param condition Optional condition - either a boolean value, or a function that takes a fixtures object and returns a boolean. Test or tests are skipped when the condition is `true`.
+   * @param titleOrCondition When used with `test.skip('test', () => {})` notation, first argument is a test title. Otherwise it is an optional skip condition - either a boolean value, or a function that takes a fixtures object and returns a boolean. Test or tests are
-   * @param description Optional description that will be reflected in a test report.
+   * skipped when the condition is `true`.
+   * @param testFunctionOrDescription When used with `test.skip('test', () => {})` notation, second argument is a test function. Otherwise it is an optional description that will be reflected in a test report.
    */
   skip(): void;
   skip(condition: boolean): void;

types/testReporter.d.ts

@@ -113,7 +113,8 @@ export interface TestCase {
   titlePath(): string[];
   /**
    * Expected test status.
-   * - Tests marked as [test.skip([condition, description])](https://playwright.dev/docs/api/class-test#test-skip) or
+   * - Tests marked as
+   *   [test.skip(titleOrCondition, testFunctionOrDescription)](https://playwright.dev/docs/api/class-test#test-skip) or
    *   [test.fixme([condition, description])](https://playwright.dev/docs/api/class-test#test-fixme) are expected to be
    *   `'skipped'`.
    * - Tests marked as [test.fail([condition, description])](https://playwright.dev/docs/api/class-test#test-fail) are