feat(test runner): run modifier functions once if they do not depend on test fixtures (#7436)

src/test/fixtures.ts

@@ -261,11 +261,16 @@ export class FixtureRunner {
     await fixture.setup(info);
     return fixture;
   }
-}
 
-export function inheritFixtureParameterNames(from: Function, to: Function, location: Location) {
+  dependsOnWorkerFixturesOnly(fn: Function, location: Location): boolean {
-  if (!(to as any)[signatureSymbol])
+    const names = fixtureParameterNames(fn, location);
-    (to as any)[signatureSymbol] = innerFixtureParameterNames(from, location);
+    for (const name of names) {
+      const registration = this.pool!.registrations.get(name)!;
+      if (registration.scope !== 'worker')
+        return false;
+    }
+    return true;
+  }
 }
 
 const signatureSymbol = Symbol('signature');

src/test/project.ts

@@ -73,6 +73,8 @@ export class ProjectImpl {
       for (let parent = spec.parent; parent; parent = parent.parent) {
         for (const hook of parent._hooks)
           pool.validateFunction(hook.fn, hook.type + ' hook', hook.type === 'beforeEach' || hook.type === 'afterEach', hook.location);
+        for (const modifier of parent._modifiers)
+          pool.validateFunction(modifier.fn, modifier.type + ' modifier', true, modifier.location);
       }
     }
     return this.specPools.get(spec)!;

src/test/test.ts

@@ -16,7 +16,7 @@
 
 import * as reporterTypes from './reporter';
 import type { TestTypeImpl } from './testType';
-import { Location } from './types';
+import { Annotations, Location } from './types';
 
 class Base {
   title: string;
@@ -68,6 +68,13 @@ export class Spec extends Base implements reporterTypes.Spec {
   }
 }
 
+export type Modifier = {
+  type: 'slow' | 'fixme' | 'skip' | 'fail',
+  fn: Function,
+  location: Location,
+  description: string | undefined
+};
+
 export class Suite extends Base implements reporterTypes.Suite {
   suites: Suite[] = [];
   specs: Spec[] = [];
@@ -79,6 +86,8 @@ export class Suite extends Base implements reporterTypes.Suite {
     location: Location,
   }[] = [];
   _timeout: number | undefined;
+  _annotations: Annotations = [];
+  _modifiers: Modifier[] = [];
 
   _addSpec(spec: Spec) {
     spec.parent = this;
@@ -168,7 +177,7 @@ export class Test implements reporterTypes.Test {
   skipped = false;
   expectedStatus: reporterTypes.TestStatus = 'passed';
   timeout = 0;
-  annotations: { type: string, description?: string }[] = [];
+  annotations: Annotations = [];
   projectName = '';
   retries = 0;
 

src/test/testType.ts

@@ -18,8 +18,7 @@ import { expect } from './expect';
 import { currentlyLoadingFileSuite, currentTestInfo, setCurrentlyLoadingFileSuite } from './globals';
 import { Spec, Suite } from './test';
 import { wrapFunctionWithLocation } from './transform';
-import { Fixtures, FixturesWithLocation, Location, TestInfo, TestType } from './types';
+import { Fixtures, FixturesWithLocation, Location, TestType } from './types';
-import { inheritFixtureParameterNames } from './fixtures';
 
 const countByFile = new Map<string, number>();
 
@@ -101,17 +100,16 @@ export class TestTypeImpl {
     suite._hooks.push({ type: name, fn, location });
   }
 
-  private _modifier(type: 'skip' | 'fail' | 'fixme' | 'slow', location: Location, ...modiferAgs: [arg?: any | Function, description?: string]) {
+  private _modifier(type: 'skip' | 'fail' | 'fixme' | 'slow', location: Location, ...modifierArgs: [arg?: any | Function, description?: string]) {
     const suite = currentlyLoadingFileSuite();
     if (suite) {
-      if (typeof modiferAgs[0] === 'function') {
+      if (typeof modifierArgs[0] === 'function') {
-        const [conditionFn, description] = modiferAgs;
+        suite._modifiers.push({ type, fn: modifierArgs[0], location, description: modifierArgs[1] });
-        const fn = (args: any, testInfo: TestInfo) => testInfo[type](conditionFn(args), description!);
-        inheritFixtureParameterNames(conditionFn, fn, location);
-        suite._hooks.unshift({ type: 'beforeEach', fn, location });
       } else {
-        const fn = ({}: any, testInfo: TestInfo) => testInfo[type](...modiferAgs as [any, any]);
+        if (modifierArgs.length >= 1 && !modifierArgs[0])
-        suite._hooks.unshift({ type: 'beforeEach', fn, location });
+          return;
+        const description = modifierArgs[1];
+        suite._annotations.push({ type, description });
       }
       return;
     }
@@ -119,9 +117,9 @@ export class TestTypeImpl {
     const testInfo = currentTestInfo();
     if (!testInfo)
       throw new Error(`test.${type}() can only be called inside test, describe block or fixture`);
-    if (typeof modiferAgs[0] === 'function')
+    if (typeof modifierArgs[0] === 'function')
       throw new Error(`test.${type}() with a function can only be called inside describe block`);
-    testInfo[type](...modiferAgs as [any, any]);
+    testInfo[type](...modifierArgs as [any, any]);
   }
 
   private _setTimeout(timeout: number) {

src/test/types.ts

@@ -22,3 +22,4 @@ export type FixturesWithLocation = {
   fixtures: Fixtures;
   location: Location;
 };
+export type Annotations = { type: string, description?: string }[];

src/test/workerRunner.ts

@@ -23,8 +23,8 @@ import { monotonicTime, DeadlineRunner, raceAgainstDeadline, serializeError } fr
 import { TestBeginPayload, TestEndPayload, RunPayload, TestEntry, DonePayload, WorkerInitParams } from './ipc';
 import { setCurrentTestInfo } from './globals';
 import { Loader } from './loader';
-import { Spec, Suite, Test } from './test';
+import { Modifier, Spec, Suite, Test } from './test';
-import { TestInfo, WorkerInfo } from './types';
+import { Annotations, TestInfo, WorkerInfo } from './types';
 import { ProjectImpl } from './project';
 import { FixtureRunner } from './fixtures';
 
@@ -127,17 +127,34 @@ export class WorkerRunner extends EventEmitter {
     }
 
     this._fixtureRunner.setPool(this._project.buildPool(anySpec));
-    await this._runSuite(fileSuite);
+    await this._runSuite(fileSuite, []);
     if (this._isStopped)
       return;
 
     this._reportDone();
   }
 
-  private async _runSuite(suite: Suite) {
+  private async _runSuite(suite: Suite, annotations: Annotations) {
     if (this._isStopped)
       return;
-    const skipHooks = !this._hasTestsToRun(suite);
+    annotations = annotations.concat(suite._annotations);
+
+    for (const beforeAllModifier of suite._modifiers) {
+      if (this._isStopped)
+        return;
+      if (!this._fixtureRunner.dependsOnWorkerFixturesOnly(beforeAllModifier.fn, beforeAllModifier.location))
+        continue;
+      // TODO: separate timeout for beforeAll modifiers?
+      const result = await raceAgainstDeadline(this._fixtureRunner.resolveParametersAndRunHookOrTest(beforeAllModifier.fn, 'worker', undefined), this._deadline());
+      if (result.timedOut) {
+        this._fatalError = serializeError(new Error(`Timeout of ${this._project.config.timeout}ms exceeded while running ${beforeAllModifier.type} modifier`));
+        this._reportDoneAndStop();
+      }
+      if (!!result.result)
+        annotations.push({ type: beforeAllModifier.type, description: beforeAllModifier.description });
+    }
+
+    const skipHooks = !this._hasTestsToRun(suite) || annotations.some(a => a.type === 'fixme' || a.type === 'skip');
     for (const hook of suite._hooks) {
       if (hook.type !== 'beforeAll' || skipHooks)
         continue;
@@ -152,9 +169,9 @@ export class WorkerRunner extends EventEmitter {
     }
     for (const entry of suite._entries) {
       if (entry instanceof Suite)
-        await this._runSuite(entry);
+        await this._runSuite(entry, annotations);
       else
-        await this._runSpec(entry);
+        await this._runSpec(entry, annotations);
     }
     for (const hook of suite._hooks) {
       if (hook.type !== 'afterAll' || skipHooks)
@@ -170,7 +187,7 @@ export class WorkerRunner extends EventEmitter {
     }
   }
 
-  private async _runSpec(spec: Spec) {
+  private async _runSpec(spec: Spec, annotations: Annotations) {
     if (this._isStopped)
       return;
     const test = spec.tests[0];
@@ -252,6 +269,24 @@ export class WorkerRunner extends EventEmitter {
       }
     }
 
+    // Process annotations defined on parent suites.
+    for (const annotation of annotations) {
+      testInfo.annotations.push(annotation);
+      switch (annotation.type) {
+        case 'fixme':
+        case 'skip':
+          testInfo.expectedStatus = 'skipped';
+          break;
+        case 'fail':
+          if (testInfo.expectedStatus !== 'skipped')
+            testInfo.expectedStatus = 'failed';
+          break;
+        case 'slow':
+          testInfo.setTimeout(testInfo.timeout * 3);
+          break;
+      }
+    }
+
     this._setCurrentTest({ testInfo, testId });
     const deadline = () => {
       return testInfo.timeout ? startTime + testInfo.timeout : undefined;
@@ -316,6 +351,18 @@ export class WorkerRunner extends EventEmitter {
 
   private async _runTestWithBeforeHooks(test: Test, testInfo: TestInfo) {
     try {
+      const beforeEachModifiers: Modifier[] = [];
+      for (let s = test.spec.parent; s; s = s.parent) {
+        const modifiers = s._modifiers.filter(modifier => !this._fixtureRunner.dependsOnWorkerFixturesOnly(modifier.fn, modifier.location));
+        beforeEachModifiers.push(...modifiers.reverse());
+      }
+      beforeEachModifiers.reverse();
+      for (const modifier of beforeEachModifiers) {
+        if (this._isStopped)
+          return;
+        const result = await this._fixtureRunner.resolveParametersAndRunHookOrTest(modifier.fn, 'test', testInfo);
+        testInfo[modifier.type](!!result, modifier.description!);
+      }
       await this._runHooks(test.spec.parent!, 'beforeEach', testInfo);
     } catch (error) {
       if (error instanceof SkipError) {

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

@@ -214,3 +214,77 @@ test('modifier with a function should throw in the test', async ({ runInlineTest
   expect(result.exitCode).toBe(1);
   expect(result.output).toContain('test.skip() with a function can only be called inside describe block');
 });
+
+test('test.skip with worker fixtures only should skip before hooks and tests', async ({ runInlineTest }) => {
+  const result = await runInlineTest({
+    'a.test.ts': `
+      const test = pwt.test.extend({
+        foo: [ 'foo', { scope: 'worker' }],
+      });
+      const logs = [];
+      test.beforeEach(() => {
+        console.log('\\n%%beforeEach');
+      });
+      test('passed', () => {
+        console.log('\\n%%passed');
+      });
+      test.describe('suite1', () => {
+        test.skip(({ foo }) => {
+          console.log('\\n%%skip');
+          return foo === 'foo';
+        }, 'reason');
+        test.beforeAll(() => {
+          console.log('\\n%%beforeAll');
+        });
+        test('skipped1', () => {
+          console.log('\\n%%skipped1');
+        });
+        test.describe('suite2', () => {
+          test('skipped2', () => {
+            console.log('\\n%%skipped2');
+          });
+        });
+      });
+    `,
+  });
+  expect(result.exitCode).toBe(0);
+  expect(result.passed).toBe(1);
+  expect(result.skipped).toBe(2);
+  expect(result.report.suites[0].specs[0].tests[0].annotations).toEqual([]);
+  expect(result.report.suites[0].suites[0].specs[0].tests[0].annotations).toEqual([{ type: 'skip', description: 'reason' }]);
+  expect(result.report.suites[0].suites[0].suites[0].specs[0].tests[0].annotations).toEqual([{ type: 'skip', description: 'reason' }]);
+  expect(result.output.split('\n').filter(line => line.startsWith('%%'))).toEqual([
+    '%%beforeEach',
+    '%%passed',
+    '%%skip',
+  ]);
+});
+
+test('test.skip without a callback in describe block should skip hooks', async ({ runInlineTest }) => {
+  const result = await runInlineTest({
+    'a.test.ts': `
+      const { test } = pwt;
+      const logs = [];
+      test.beforeAll(() => {
+        console.log('%%beforeAll');
+      });
+      test.beforeEach(() => {
+        console.log('%%beforeEach');
+      });
+      test.skip(true, 'reason');
+      test('skipped1', () => {
+        console.log('%%skipped1');
+      });
+      test.describe('suite1', () => {
+        test('skipped2', () => {
+          console.log('%%skipped2');
+        });
+      });
+    `,
+  });
+  expect(result.exitCode).toBe(0);
+  expect(result.skipped).toBe(2);
+  expect(result.report.suites[0].specs[0].tests[0].annotations).toEqual([{ type: 'skip', description: 'reason' }]);
+  expect(result.report.suites[0].suites[0].specs[0].tests[0].annotations).toEqual([{ type: 'skip', description: 'reason' }]);
+  expect(result.output).not.toContain('%%');
+});