chore: nest test steps based on zones (#22108)

Fixes: #21423
This commit is contained in:
Pavel Feldman 2023-03-30 21:05:07 -07:00 committed by GitHub
parent eacaf5fc89
commit b1fdf0bcb6
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
11 changed files with 325 additions and 150 deletions

View file

@ -16,7 +16,7 @@
import type { RawStack } from './stackTrace';
export type ZoneType = 'apiZone' | 'expectZone';
export type ZoneType = 'apiZone' | 'expectZone' | 'stepZone';
class ZoneManager {
lastZoneId = 0;

View file

@ -82,10 +82,9 @@ export type TestEndPayload = {
export type StepBeginPayload = {
testId: string;
stepId: string;
parentStepId: string | undefined;
title: string;
category: string;
canHaveChildren: boolean;
forceNoParent: boolean;
wallTime: number; // milliseconds since unix epoch
location?: { file: string, line: number, column: number };
};

View file

@ -19,7 +19,6 @@ import { currentlyLoadingFileSuite, currentTestInfo, setCurrentlyLoadingFileSuit
import { TestCase, Suite } from './test';
import { wrapFunctionWithLocation } from './transform';
import type { Fixtures, FixturesWithLocation, Location, TestType } from './types';
import { serializeError } from '../util';
const testTypeSymbol = Symbol('testType');
@ -212,22 +211,11 @@ export class TestTypeImpl {
const testInfo = currentTestInfo();
if (!testInfo)
throw new Error(`test.step() can only be called from a test`);
const step = testInfo._addStep({
return testInfo._runAsStep(body, {
category: 'test.step',
title,
location,
canHaveChildren: true,
forceNoParent: false,
wallTime: Date.now(),
});
try {
const result = await body();
step.complete({});
return result;
} catch (e) {
step.complete({ error: serializeError(e) });
throw e;
}
}
private _extend(location: Location, fixtures: Fixtures) {

View file

@ -272,8 +272,6 @@ const playwrightFixtures: Fixtures<TestFixtures, WorkerFixtures> = ({
location: stackTrace?.frames[0] as any,
category: 'pw:api',
title: apiCall,
canHaveChildren: false,
forceNoParent: false,
wallTime,
});
userData.userObject = step;

View file

@ -216,8 +216,6 @@ class ExpectMetaInfoProxyHandler implements ProxyHandler<any> {
location: stackFrames[0],
category: 'expect',
title: trimLongString(customMessage || defaultTitle, 1024),
canHaveChildren: true,
forceNoParent: false,
wallTime
});
testInfo.currentStep = step;

View file

@ -28,7 +28,6 @@ import type { FullConfigInternal } from '../common/types';
type TestResultData = {
result: TestResult;
steps: Map<string, TestStep>;
stepStack: Set<TestStep>;
};
type TestData = {
test: TestCase;
@ -218,7 +217,7 @@ export class Dispatcher {
const onTestBegin = (params: TestBeginPayload) => {
const data = this._testById.get(params.testId)!;
const result = data.test._appendTestResult();
data.resultByWorkerIndex.set(worker.workerIndex, { result, stepStack: new Set(), steps: new Map() });
data.resultByWorkerIndex.set(worker.workerIndex, { result, steps: new Map() });
result.parallelIndex = worker.parallelIndex;
result.workerIndex = worker.workerIndex;
result.startTime = new Date(params.startWallTime);
@ -267,8 +266,8 @@ export class Dispatcher {
// The test has finished, but steps are still coming. Just ignore them.
return;
}
const { result, steps, stepStack } = runData;
const parentStep = params.forceNoParent ? undefined : [...stepStack].pop();
const { result, steps } = runData;
const parentStep = params.parentStepId ? steps.get(params.parentStepId) : undefined;
const step: TestStep = {
title: params.title,
titlePath: () => {
@ -284,8 +283,6 @@ export class Dispatcher {
};
steps.set(params.stepId, step);
(parentStep || result).steps.push(step);
if (params.canHaveChildren)
stepStack.add(step);
this._reporter.onStepBegin?.(data.test, result, step);
};
worker.on('stepBegin', onStepBegin);
@ -297,7 +294,7 @@ export class Dispatcher {
// The test has finished, but steps are still coming. Just ignore them.
return;
}
const { result, steps, stepStack } = runData;
const { result, steps } = runData;
const step = steps.get(params.stepId);
if (!step) {
this._reporter.onStdErr?.('Internal error: step end without step begin: ' + params.stepId, data.test, result);
@ -308,7 +305,6 @@ export class Dispatcher {
step.duration = params.wallTime - step.startTime.getTime();
if (params.error)
step.error = params.error;
stepStack.delete(step);
steps.delete(params.stepId);
this._reporter.onStepEnd?.(data.test, result, step);
};

View file

@ -88,13 +88,23 @@ class Fixture {
const workerInfo: WorkerInfo = { config: testInfo.config, parallelIndex: testInfo.parallelIndex, workerIndex: testInfo.workerIndex, project: testInfo.project };
const info = this.registration.scope === 'worker' ? workerInfo : testInfo;
testInfo._timeoutManager.setCurrentFixture(this._runnableDescription);
this._selfTeardownComplete = Promise.resolve().then(() => this.registration.fn(params, useFunc, info)).catch((e: any) => {
const handleError = (e: any) => {
this.failed = true;
if (!useFuncStarted.isDone())
useFuncStarted.reject(e);
else
throw e;
});
};
try {
const result = this.registration.fn(params, useFunc, info);
if (result instanceof Promise)
this._selfTeardownComplete = result.catch(handleError);
else
this._selfTeardownComplete = Promise.resolve();
} catch (e) {
handleError(e);
}
await useFuncStarted;
testInfo._timeoutManager.setCurrentFixture(undefined);
}

View file

@ -16,7 +16,7 @@
import fs from 'fs';
import path from 'path';
import { monotonicTime } from 'playwright-core/lib/utils';
import { captureRawStack, monotonicTime, zones } from 'playwright-core/lib/utils';
import type { TestInfoError, TestInfo, TestStatus } from '../../types/test';
import type { StepBeginPayload, StepEndPayload, WorkerInitParams } from '../common/ipc';
import type { TestCase } from '../common/test';
@ -33,10 +33,9 @@ export type TestInfoErrorState = {
interface TestStepInternal {
complete(result: { error?: Error | TestInfoError }): void;
stepId: string;
title: string;
category: string;
canHaveChildren: boolean;
forceNoParent: boolean;
wallTime: number;
location?: Location;
refinedTitle?: string;
@ -195,26 +194,34 @@ export class TestInfoImpl implements TestInfo {
this.duration = this._timeoutManager.defaultSlotTimings().elapsed | 0;
}
async _runFn(fn: Function, skips?: 'allowSkips'): Promise<TestInfoError | undefined> {
async _runFn(fn: () => Promise<void>, skips?: 'allowSkips', stepInfo?: Omit<TestStepInternal, 'complete' | 'wallTime' | 'parentStepId' | 'stepId'>): Promise<TestInfoError | undefined> {
const step = stepInfo ? this._addStep({ ...stepInfo, wallTime: Date.now() }) : undefined;
try {
await fn();
if (step)
await zones.run('stepZone', step, fn);
else
await fn();
step?.complete({});
} catch (error) {
if (skips === 'allowSkips' && error instanceof SkipError) {
if (this.status === 'passed')
this.status = 'skipped';
} else {
const serialized = serializeError(error);
step?.complete({ error: serialized });
this._failWithError(serialized, true /* isHardError */);
return serialized;
}
}
}
_addStep(data: Omit<TestStepInternal, 'complete'>): TestStepInternal {
_addStep(data: Omit<TestStepInternal, 'complete' | 'stepId'>): TestStepInternal {
const stepId = `${data.category}@${data.title}@${++this._lastStepId}`;
const parentStep = zones.zoneData<TestStepInternal>('stepZone', captureRawStack());
let callbackHandled = false;
const firstErrorIndex = this.errors.length;
const step: TestStepInternal = {
stepId,
...data,
complete: result => {
if (callbackHandled)
@ -248,6 +255,7 @@ export class TestInfoImpl implements TestInfo {
const payload: StepBeginPayload = {
testId: this._test.id,
stepId,
parentStepId: parentStep ? parentStep.stepId : undefined,
...data,
location,
};
@ -292,16 +300,18 @@ export class TestInfoImpl implements TestInfo {
this._hasHardError = state.hasHardError;
}
async _runAsStep<T>(cb: () => Promise<T>, stepInfo: Omit<TestStepInternal, 'complete' | 'wallTime'>): Promise<T> {
async _runAsStep<T>(cb: (step: TestStepInternal) => Promise<T>, stepInfo: Omit<TestStepInternal, 'complete' | 'wallTime' | 'parentStepId' | 'stepId'>): Promise<T> {
const step = this._addStep({ ...stepInfo, wallTime: Date.now() });
try {
const result = await cb();
step.complete({});
return result;
} catch (e) {
step.complete({ error: e instanceof SkipError ? undefined : serializeError(e) });
throw e;
}
return await zones.run('stepZone', step, async () => {
try {
const result = await cb(step);
step.complete({});
return result;
} catch (e) {
step.complete({ error: e instanceof SkipError ? undefined : serializeError(e) });
throw e;
}
});
}
_isFailure() {

View file

@ -320,17 +320,10 @@ export class WorkerMain extends ProcessRunner {
return;
}
const beforeHooksStep = testInfo._addStep({
category: 'hook',
title: 'Before Hooks',
canHaveChildren: true,
forceNoParent: true,
wallTime: Date.now(),
});
let params: object | null = null;
// Note: wrap all preparation steps together, because failure/skip in any of them
// prevents further setup and/or test from running.
const maybeError = await testInfo._runFn(async () => {
await testInfo._runFn(async () => {
// Run "beforeAll" modifiers on parent suites, unless already run during previous tests.
for (const suite of suites) {
if (this._extraSuiteAnnotations.has(suite))
@ -362,21 +355,24 @@ export class WorkerMain extends ProcessRunner {
// Setup fixtures required by the test.
testInfo._timeoutManager.setCurrentRunnable({ type: 'test' });
const params = await this._fixtureRunner.resolveParametersForFunction(test.fn, testInfo, 'test');
beforeHooksStep.complete({}); // Report fixture hooks step as completed.
if (params === null) {
// Fixture setup failed, we should not run the test now.
return;
}
params = await this._fixtureRunner.resolveParametersForFunction(test.fn, testInfo, 'test');
}, 'allowSkips', {
category: 'hook',
title: 'Before Hooks',
});
if (params === null) {
// Fixture setup failed, we should not run the test now.
return;
}
await testInfo._runFn(async () => {
// Now run the test itself.
debugTest(`test function started`);
const fn = test.fn; // Extract a variable to get a better stack trace ("myTest" vs "TestCase.myTest [as fn]").
await fn(params, testInfo);
debugTest(`test function finished`);
}, 'allowSkips');
beforeHooksStep.complete({ error: maybeError }); // Second complete is a no-op.
});
if (didFailBeforeAllForSuite) {
@ -386,100 +382,96 @@ export class WorkerMain extends ProcessRunner {
this._skipRemainingTestsInSuite = didFailBeforeAllForSuite;
}
const afterHooksStep = testInfo._addStep({
category: 'hook',
title: 'After Hooks',
canHaveChildren: true,
forceNoParent: true,
wallTime: Date.now(),
});
let firstAfterHooksError: TestInfoError | undefined;
let afterHooksSlot: TimeSlot | undefined;
if (testInfo._didTimeout) {
// A timed-out test gets a full additional timeout to run after hooks.
afterHooksSlot = { timeout: this._project.timeout, elapsed: 0 };
testInfo._timeoutManager.setCurrentRunnable({ type: 'afterEach', slot: afterHooksSlot });
}
await testInfo._runWithTimeout(async () => {
// Note: do not wrap all teardown steps together, because failure in any of them
// does not prevent further teardown steps from running.
// Run "immediately upon test failure" callbacks.
if (testInfo._isFailure()) {
const onFailureError = await testInfo._runFn(async () => {
testInfo._timeoutManager.setCurrentRunnable({ type: 'test', slot: afterHooksSlot });
for (const [fn, title] of testInfo._onTestFailureImmediateCallbacks) {
debugTest(`on-failure callback started`);
await testInfo._runAsStep(fn, {
category: 'hook',
title,
canHaveChildren: true,
forceNoParent: false,
});
debugTest(`on-failure callback finished`);
}
});
firstAfterHooksError = firstAfterHooksError || onFailureError;
}
// Run "afterEach" hooks, unless we failed at beforeAll stage.
if (shouldRunAfterEachHooks) {
const afterEachError = await testInfo._runFn(() => this._runEachHooksForSuites(reversedSuites, 'afterEach', testInfo, afterHooksSlot));
firstAfterHooksError = firstAfterHooksError || afterEachError;
}
// Run "afterAll" hooks for suites that are not shared with the next test.
// In case of failure the worker will be stopped and we have to make sure that afterAll
// hooks run before test fixtures teardown.
for (const suite of reversedSuites) {
if (!nextSuites.has(suite) || testInfo._isFailure()) {
const afterAllError = await this._runAfterAllHooksForSuite(suite, testInfo);
firstAfterHooksError = firstAfterHooksError || afterAllError;
}
}
// Teardown test-scoped fixtures. Attribute to 'test' so that users understand
// they should probably increate the test timeout to fix this issue.
testInfo._timeoutManager.setCurrentRunnable({ type: 'test', slot: afterHooksSlot });
debugTest(`tearing down test scope started`);
const testScopeError = await testInfo._runFn(() => this._fixtureRunner.teardownScope('test', testInfo._timeoutManager));
debugTest(`tearing down test scope finished`);
firstAfterHooksError = firstAfterHooksError || testScopeError;
});
if (testInfo._isFailure())
this._isStopped = true;
if (this._isStopped) {
// Run all remaining "afterAll" hooks and teardown all fixtures when worker is shutting down.
// Mark as "cleaned up" early to avoid running cleanup twice.
this._didRunFullCleanup = true;
// Give it more time for the full cleanup.
await testInfo._runAsStep(async step => {
let firstAfterHooksError: TestInfoError | undefined;
await testInfo._runWithTimeout(async () => {
debugTest(`running full cleanup after the failure`);
for (const suite of reversedSuites) {
const afterAllError = await this._runAfterAllHooksForSuite(suite, testInfo);
firstAfterHooksError = firstAfterHooksError || afterAllError;
// Note: do not wrap all teardown steps together, because failure in any of them
// does not prevent further teardown steps from running.
// Run "immediately upon test failure" callbacks.
if (testInfo._isFailure()) {
const onFailureError = await testInfo._runFn(async () => {
testInfo._timeoutManager.setCurrentRunnable({ type: 'test', slot: afterHooksSlot });
for (const [fn, title] of testInfo._onTestFailureImmediateCallbacks) {
debugTest(`on-failure callback started`);
await testInfo._runAsStep(fn, {
category: 'hook',
title,
});
debugTest(`on-failure callback finished`);
}
});
firstAfterHooksError = firstAfterHooksError || onFailureError;
}
const teardownSlot = { timeout: this._project.timeout, elapsed: 0 };
// Attribute to 'test' so that users understand they should probably increate the test timeout to fix this issue.
testInfo._timeoutManager.setCurrentRunnable({ type: 'test', slot: teardownSlot });
// Run "afterEach" hooks, unless we failed at beforeAll stage.
if (shouldRunAfterEachHooks) {
const afterEachError = await testInfo._runFn(() => this._runEachHooksForSuites(reversedSuites, 'afterEach', testInfo, afterHooksSlot));
firstAfterHooksError = firstAfterHooksError || afterEachError;
}
// Run "afterAll" hooks for suites that are not shared with the next test.
// In case of failure the worker will be stopped and we have to make sure that afterAll
// hooks run before test fixtures teardown.
for (const suite of reversedSuites) {
if (!nextSuites.has(suite) || testInfo._isFailure()) {
const afterAllError = await this._runAfterAllHooksForSuite(suite, testInfo);
firstAfterHooksError = firstAfterHooksError || afterAllError;
}
}
// Teardown test-scoped fixtures. Attribute to 'test' so that users understand
// they should probably increate the test timeout to fix this issue.
testInfo._timeoutManager.setCurrentRunnable({ type: 'test', slot: afterHooksSlot });
debugTest(`tearing down test scope started`);
const testScopeError = await testInfo._runFn(() => this._fixtureRunner.teardownScope('test', testInfo._timeoutManager));
debugTest(`tearing down test scope finished`);
firstAfterHooksError = firstAfterHooksError || testScopeError;
// Attribute to 'teardown' because worker fixtures are not perceived as a part of a test.
testInfo._timeoutManager.setCurrentRunnable({ type: 'teardown', slot: teardownSlot });
debugTest(`tearing down worker scope started`);
const workerScopeError = await testInfo._runFn(() => this._fixtureRunner.teardownScope('worker', testInfo._timeoutManager));
debugTest(`tearing down worker scope finished`);
firstAfterHooksError = firstAfterHooksError || workerScopeError;
});
}
afterHooksStep.complete({ error: firstAfterHooksError });
if (testInfo._isFailure())
this._isStopped = true;
if (this._isStopped) {
// Run all remaining "afterAll" hooks and teardown all fixtures when worker is shutting down.
// Mark as "cleaned up" early to avoid running cleanup twice.
this._didRunFullCleanup = true;
// Give it more time for the full cleanup.
await testInfo._runWithTimeout(async () => {
debugTest(`running full cleanup after the failure`);
for (const suite of reversedSuites) {
const afterAllError = await this._runAfterAllHooksForSuite(suite, testInfo);
firstAfterHooksError = firstAfterHooksError || afterAllError;
}
const teardownSlot = { timeout: this._project.timeout, elapsed: 0 };
// Attribute to 'test' so that users understand they should probably increate the test timeout to fix this issue.
testInfo._timeoutManager.setCurrentRunnable({ type: 'test', slot: teardownSlot });
debugTest(`tearing down test scope started`);
const testScopeError = await testInfo._runFn(() => this._fixtureRunner.teardownScope('test', testInfo._timeoutManager));
debugTest(`tearing down test scope finished`);
firstAfterHooksError = firstAfterHooksError || testScopeError;
// Attribute to 'teardown' because worker fixtures are not perceived as a part of a test.
testInfo._timeoutManager.setCurrentRunnable({ type: 'teardown', slot: teardownSlot });
debugTest(`tearing down worker scope started`);
const workerScopeError = await testInfo._runFn(() => this._fixtureRunner.teardownScope('worker', testInfo._timeoutManager));
debugTest(`tearing down worker scope finished`);
firstAfterHooksError = firstAfterHooksError || workerScopeError;
});
}
if (firstAfterHooksError)
step.complete({ error: firstAfterHooksError });
}, {
category: 'hook',
title: 'After Hooks',
});
this._currentTest = null;
setCurrentTestInfo(null);
this.dispatchEvent('testEnd', buildTestEndPayload(testInfo));
@ -500,8 +492,6 @@ export class WorkerMain extends ProcessRunner {
const result = await testInfo._runAsStep(() => this._fixtureRunner.resolveParametersAndRunFunction(modifier.fn, testInfo, scope), {
category: 'hook',
title: `${modifier.type} modifier`,
canHaveChildren: true,
forceNoParent: false,
location: modifier.location,
});
debugTest(`modifier at "${formatLocation(modifier.location)}" finished`);
@ -527,8 +517,6 @@ export class WorkerMain extends ProcessRunner {
await testInfo._runAsStep(() => this._fixtureRunner.resolveParametersAndRunFunction(hook.fn, testInfo, 'all-hooks-only'), {
category: 'hook',
title: `${hook.type} hook`,
canHaveChildren: true,
forceNoParent: false,
location: hook.location,
});
} catch (e) {
@ -557,8 +545,6 @@ export class WorkerMain extends ProcessRunner {
await testInfo._runAsStep(() => this._fixtureRunner.resolveParametersAndRunFunction(hook.fn, testInfo, 'all-hooks-only'), {
category: 'hook',
title: `${hook.type} hook`,
canHaveChildren: true,
forceNoParent: false,
location: hook.location,
});
});
@ -577,8 +563,6 @@ export class WorkerMain extends ProcessRunner {
await testInfo._runAsStep(() => this._fixtureRunner.resolveParametersAndRunFunction(hook.fn, testInfo, 'test'), {
category: 'hook',
title: `${hook.type} hook`,
canHaveChildren: true,
forceNoParent: false,
location: hook.location,
});
} catch (e) {

View file

@ -599,3 +599,23 @@ test('should report skipped tests in-order with correct properties', async ({ ru
'retries-3',
]);
});
test('should skip tests if beforeEach has skip', async ({ runInlineTest }) => {
const result = await runInlineTest({
'a.test.ts': `
import { test, expect } from '@playwright/test';
test.beforeEach(() => {
test.skip();
});
test('no marker', () => {
console.log('skip-me');
});
`,
});
const expectTest = expectTestHelper(result);
expect(result.exitCode).toBe(0);
expect(result.passed).toBe(0);
expect(result.skipped).toBe(1);
expectTest('no marker', 'skipped', 'skipped', ['skip']);
expect(result.output).not.toContain('skip-me');
});

View file

@ -457,3 +457,175 @@ test('should mark step as failed when soft expect fails', async ({ runInlineTest
{ title: 'After Hooks', category: 'hook' }
]);
});
test('should nest steps based on zones', async ({ runInlineTest }) => {
const result = await runInlineTest({
'reporter.ts': stepHierarchyReporter,
'playwright.config.ts': `
module.exports = {
reporter: './reporter',
};
`,
'a.test.ts': `
import { test, expect } from '@playwright/test';
test.beforeAll(async () => {
await test.step('in beforeAll', () => {});
});
test.afterAll(async () => {
await test.step('in afterAll', () => {});
});
test.beforeEach(async () => {
await test.step('in beforeEach', () => {});
});
test.afterEach(async () => {
await test.step('in afterEach', () => {});
});
test.only('foo', async ({ page }) => {
await test.step('grand', async () => {
await Promise.all([
test.step('parent1', async () => {
await test.step('child1', async () => {
await page.click('body');
});
}),
test.step('parent2', async () => {
await test.step('child2', async () => {
await expect(page.locator('body')).toBeVisible();
});
}),
]);
});
});
`
}, { reporter: '', workers: 1 });
expect(result.exitCode).toBe(0);
const objects = result.outputLines.map(line => JSON.parse(line));
expect(objects).toEqual([
{
title: 'Before Hooks',
category: 'hook',
steps: [
{
title: 'beforeAll hook',
category: 'hook',
steps: [
{
title: 'in beforeAll',
category: 'test.step',
location: { file: 'a.test.ts', line: 'number', column: 'number' }
}
],
location: { file: 'a.test.ts', line: 'number', column: 'number' }
},
{
title: 'beforeEach hook',
category: 'hook',
steps: [
{
title: 'in beforeEach',
category: 'test.step',
location: { file: 'a.test.ts', line: 'number', column: 'number' }
}
],
location: { file: 'a.test.ts', line: 'number', column: 'number' }
},
{
title: 'browserContext.newPage',
category: 'pw:api'
}
]
},
{
title: 'grand',
category: 'test.step',
steps: [
{
title: 'parent1',
category: 'test.step',
steps: [
{
title: 'child1',
category: 'test.step',
location: { file: 'a.test.ts', line: 'number', column: 'number' },
steps: [
{
title: 'page.click(body)',
category: 'pw:api',
location: { file: 'a.test.ts', line: 'number', column: 'number' }
}
]
}
],
location: {
file: 'a.test.ts',
line: 'number',
column: 'number'
}
},
{
title: 'parent2',
category: 'test.step',
steps: [
{
title: 'child2',
category: 'test.step',
location: { file: 'a.test.ts', line: 'number', column: 'number' },
steps: [
{
title: 'expect.toBeVisible',
category: 'expect',
location: { file: 'a.test.ts', line: 'number', column: 'number' }
}
]
}
],
location: { file: 'a.test.ts', line: 'number', column: 'number' }
}
],
location: {
file: 'a.test.ts',
line: 'number',
column: 'number'
}
},
{
title: 'After Hooks',
category: 'hook',
steps: [
{
title: 'afterEach hook',
category: 'hook',
steps: [
{
title: 'in afterEach',
category: 'test.step',
location: { file: 'a.test.ts', line: 'number', column: 'number' }
}
],
location: { file: 'a.test.ts', line: 'number', column: 'number' }
},
{
title: 'afterAll hook',
category: 'hook',
steps: [
{
title: 'in afterAll',
category: 'test.step',
location: { file: 'a.test.ts', line: 'number', column: 'number' }
}
],
location: { file: 'a.test.ts', line: 'number', column: 'number' }
},
{
title: 'browserContext.close',
category: 'pw:api'
}
]
}
]);
});