Skip to content

Commit

Permalink
chore: follow up to runnable change (microsoft#27300)
Browse files Browse the repository at this point in the history
  • Loading branch information
pavelfeldman authored Sep 26, 2023
1 parent c0dc232 commit 07e794e
Show file tree
Hide file tree
Showing 3 changed files with 11 additions and 14 deletions.
8 changes: 1 addition & 7 deletions packages/playwright/src/worker/testInfo.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import type { TestInfoError, TestInfo, TestStatus, FullProject, FullConfig } fro
import type { AttachmentPayload, StepBeginPayload, StepEndPayload, WorkerInitParams } from '../common/ipc';
import type { TestCase } from '../common/test';
import { TimeoutManager } from './timeoutManager';
import type { RunnableType, TimeSlot, RunnableDescription } from './timeoutManager';
import type { RunnableType, TimeSlot } from './timeoutManager';
import type { Annotation, FullConfigInternal, FullProjectInternal } from '../common/config';
import type { Location } from '../../types/testReporter';
import { getContainedPath, normalizeAndSaveAttachment, serializeError, trimLongString } from '../util';
Expand Down Expand Up @@ -228,12 +228,6 @@ export class TestInfoImpl implements TestInfo {
this.duration = this._timeoutManager.defaultSlotTimings().elapsed | 0;
}

async _runWithRunnableAndFailOnError(runnable: RunnableDescription, cb: () => Promise<void>): Promise<TestInfoError | undefined> {
return await this._timeoutManager.withRunnable(runnable, async () => {
return await this._runAndFailOnError(cb);
});
}

async _runAndFailOnError(fn: () => Promise<void>, skips?: 'allowSkips'): Promise<TestInfoError | undefined> {
try {
await fn();
Expand Down
7 changes: 5 additions & 2 deletions packages/playwright/src/worker/timeoutManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ export type TimeSlot = {
elapsed: number;
};

export type RunnableType = 'test' | 'beforeAll' | 'afterAll' | 'beforeEach' | 'afterEach' | 'slow' | 'skip' | 'fail' | 'fixme' | 'teardown';
export type RunnableType = 'test' | 'beforeAll' | 'afterAll' | 'beforeEach' | 'afterEach' | 'afterHooks' | 'slow' | 'skip' | 'fail' | 'fixme' | 'teardown';

export type RunnableDescription = {
type: RunnableType;
Expand Down Expand Up @@ -59,7 +59,9 @@ export class TimeoutManager {

async withRunnable<R>(runnable: RunnableDescription, cb: () => Promise<R>): Promise<R> {
const existingRunnable = this._runnable;
const effectiveRunnable = { ...this._runnable, ...runnable };
const effectiveRunnable = { ...runnable };
if (!effectiveRunnable.slot)
effectiveRunnable.slot = this._runnable.slot;
this._updateRunnables(effectiveRunnable, undefined);
try {
return await cb();
Expand Down Expand Up @@ -129,6 +131,7 @@ export class TimeoutManager {
let message = '';
const timeout = this._currentSlot().timeout;
switch (this._runnable.type) {
case 'afterHooks':
case 'test': {
if (this._fixture) {
if (this._fixture.phase === 'setup') {
Expand Down
10 changes: 5 additions & 5 deletions packages/playwright/src/worker/workerMain.ts
Original file line number Diff line number Diff line change
Expand Up @@ -411,7 +411,7 @@ export class WorkerMain extends ProcessRunner {

// A timed-out test gets a full additional timeout to run after hooks.
const afterHooksSlot = testInfo._didTimeout ? { timeout: this._project.project.timeout, elapsed: 0 } : undefined;
await testInfo._runAsStepWithRunnable({ category: 'hook', title: 'After Hooks', runnableType: 'afterEach', runnableSlot: afterHooksSlot }, async step => {
await testInfo._runAsStepWithRunnable({ category: 'hook', title: 'After Hooks', runnableType: 'afterHooks', runnableSlot: afterHooksSlot }, async step => {
testInfo._afterHooksStep = step;
let firstAfterHooksError: TestInfoError | undefined;
await testInfo._runWithTimeout(async () => {
Expand All @@ -433,7 +433,7 @@ export class WorkerMain extends ProcessRunner {
// Teardown test-scoped fixtures. Attribute to 'test' so that users understand
// they should probably increase the test timeout to fix this issue.
debugTest(`tearing down test scope started`);
const testScopeError = await testInfo._runWithRunnableAndFailOnError({ type: 'test' }, () => {
const testScopeError = await testInfo._runAndFailOnError(() => {
return this._fixtureRunner.teardownScope('test', testInfo._timeoutManager);
});
debugTest(`tearing down test scope finished`);
Expand Down Expand Up @@ -463,10 +463,10 @@ export class WorkerMain extends ProcessRunner {
debugTest(`running full cleanup after the failure`);

const teardownSlot = { timeout: this._project.project.timeout, elapsed: 0 };
await testInfo._timeoutManager.withRunnable({ type: 'test', slot: teardownSlot }, async () => {
await testInfo._timeoutManager.withRunnable({ type: 'teardown', slot: teardownSlot }, async () => {
// Attribute to 'test' so that users understand they should probably increate the test timeout to fix this issue.
debugTest(`tearing down test scope started`);
const testScopeError = await testInfo._runWithRunnableAndFailOnError({ type: 'test' }, () => {
const testScopeError = await testInfo._runAndFailOnError(() => {
return this._fixtureRunner.teardownScope('test', testInfo._timeoutManager);
});
debugTest(`tearing down test scope finished`);
Expand All @@ -479,7 +479,7 @@ export class WorkerMain extends ProcessRunner {

// Attribute to 'teardown' because worker fixtures are not perceived as a part of a test.
debugTest(`tearing down worker scope started`);
const workerScopeError = await testInfo._runWithRunnableAndFailOnError({ type: 'teardown' }, () => {
const workerScopeError = await testInfo._runAndFailOnError(() => {
return this._fixtureRunner.teardownScope('worker', testInfo._timeoutManager);
});
debugTest(`tearing down worker scope finished`);
Expand Down

0 comments on commit 07e794e

Please sign in to comment.