chore(test runner): make 'debug' an explicit option internally (#32154)

This allows any time slot that has a legitimate timeout of zero to be
updated later on. See test for an example.

Previously, setting timeout to zero at any moment was considered a
"debug mode" and any subsequent timeouts were ignored.
This commit is contained in:
Dmitry Gozman 2024-08-16 01:44:37 -07:00 committed by GitHub
parent b2ccfc3d01
commit 1537d3c2de
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
7 changed files with 42 additions and 23 deletions

View File

@ -79,7 +79,7 @@ export class FullConfigInternal {
globalTimeout: takeFirst(configCLIOverrides.globalTimeout, userConfig.globalTimeout, 0),
grep: takeFirst(userConfig.grep, defaultGrep),
grepInvert: takeFirst(userConfig.grepInvert, null),
maxFailures: takeFirst(configCLIOverrides.maxFailures, userConfig.maxFailures, 0),
maxFailures: takeFirst(configCLIOverrides.debug ? 1 : undefined, configCLIOverrides.maxFailures, userConfig.maxFailures, 0),
metadata: takeFirst(userConfig.metadata, {}),
preserveOutput: takeFirst(userConfig.preserveOutput, 'always'),
reporter: takeFirst(configCLIOverrides.reporter, resolveReporters(userConfig.reporter, configDir), [[defaultReporter]]),
@ -99,7 +99,7 @@ export class FullConfigInternal {
(this.config as any)[configInternalSymbol] = this;
const workers = takeFirst(configCLIOverrides.workers, userConfig.workers, '50%');
const workers = takeFirst(configCLIOverrides.debug ? 1 : undefined, configCLIOverrides.workers, userConfig.workers, '50%');
if (typeof workers === 'string') {
if (workers.endsWith('%')) {
const cpus = os.cpus().length;
@ -179,7 +179,7 @@ export class FullProjectInternal {
snapshotDir: takeFirst(pathResolve(configDir, projectConfig.snapshotDir), pathResolve(configDir, config.snapshotDir), testDir),
testIgnore: takeFirst(projectConfig.testIgnore, config.testIgnore, []),
testMatch: takeFirst(projectConfig.testMatch, config.testMatch, '**/*.@(spec|test).?(c|m)[jt]s?(x)'),
timeout: takeFirst(configCLIOverrides.timeout, projectConfig.timeout, config.timeout, defaultTimeout),
timeout: takeFirst(configCLIOverrides.debug ? 0 : undefined, configCLIOverrides.timeout, projectConfig.timeout, config.timeout, defaultTimeout),
use: mergeObjects(config.use, projectConfig.use, configCLIOverrides.use),
dependencies: projectConfig.dependencies || [],
teardown: projectConfig.teardown,

View File

@ -20,6 +20,7 @@ import type { ConfigLocation, FullConfigInternal } from './config';
import type { ReporterDescription, TestInfoError, TestStatus } from '../../types/test';
export type ConfigCLIOverrides = {
debug?: boolean;
forbidOnly?: boolean;
fullyParallel?: boolean;
globalTimeout?: number;

View File

@ -229,7 +229,7 @@ const playwrightFixtures: Fixtures<TestFixtures, WorkerFixtures> = ({
playwrightLibrary.selectors.setTestIdAttribute(testIdAttribute);
testInfo.snapshotSuffix = process.platform;
if (debugMode())
testInfo.setTimeout(0);
(testInfo as TestInfoImpl)._setDebugMode();
for (const browserType of [playwright.chromium, playwright.firefox, playwright.webkit]) {
(browserType as any)._defaultContextOptions = _combinedContextOptions;
(browserType as any)._defaultContextTimeout = actionTimeout || 0;
@ -270,7 +270,7 @@ const playwrightFixtures: Fixtures<TestFixtures, WorkerFixtures> = ({
step?.complete({ error });
},
onWillPause: () => {
currentTestInfo()?.setTimeout(0);
currentTestInfo()?._setDebugMode();
},
runAfterCreateBrowserContext: async (context: BrowserContext) => {
await artifactsRecorder?.didCreateBrowserContext(context);

View File

@ -308,9 +308,7 @@ function overridesFromOptions(options: { [key: string]: any }): ConfigCLIOverrid
if (options.headed || options.debug)
overrides.use = { headless: false };
if (!options.ui && options.debug) {
overrides.maxFailures = 1;
overrides.timeout = 0;
overrides.workers = 1;
overrides.debug = true;
process.env.PWDEBUG = '1';
}
if (!options.ui && options.trace) {

View File

@ -167,6 +167,8 @@ export class TestInfoImpl implements TestInfo {
this.expectedStatus = test?.expectedStatus ?? 'skipped';
this._timeoutManager = new TimeoutManager(this.project.timeout);
if (configInternal.configCLIOverrides.debug)
this._setDebugMode();
this.outputDir = (() => {
const relativeTestFilePath = path.relative(this.project.testDir, this._requireFile.replace(/\.(spec|test)\.(js|ts|jsx|tsx|mjs|mts|cjs|cts)$/, ''));
@ -225,17 +227,6 @@ export class TestInfoImpl implements TestInfo {
}
}
private _findLastNonFinishedStep(filter: (step: TestStepInternal) => boolean) {
let result: TestStepInternal | undefined;
const visit = (step: TestStepInternal) => {
if (!step.endWallTime && filter(step))
result = step;
step.steps.forEach(visit);
};
this._steps.forEach(visit);
return result;
}
private _findLastStageStep() {
for (let i = this._stages.length - 1; i >= 0; i--) {
if (this._stages[i].step)
@ -404,6 +395,10 @@ export class TestInfoImpl implements TestInfo {
}
}
_setDebugMode() {
this._timeoutManager.setIgnoreTimeouts();
}
// ------------ TestInfo methods ------------
async attach(name: string, options: { path?: string, body?: string | Buffer, contentType?: string } = {}) {

View File

@ -52,11 +52,16 @@ export const kMaxDeadline = 2147483647; // 2^31-1
export class TimeoutManager {
private _defaultSlot: TimeSlot;
private _running?: Running;
private _ignoreTimeouts = false;
constructor(timeout: number) {
this._defaultSlot = { timeout, elapsed: 0 };
}
setIgnoreTimeouts() {
this._ignoreTimeouts = true;
}
interrupt() {
if (this._running)
this._running.timeoutPromise.reject(this._createTimeoutError(this._running));
@ -94,7 +99,7 @@ export class TimeoutManager {
if (running.timer)
clearTimeout(running.timer);
running.timer = undefined;
if (!running.slot.timeout) {
if (this._ignoreTimeouts || !running.slot.timeout) {
running.deadline = kMaxDeadline;
return;
}
@ -119,8 +124,6 @@ export class TimeoutManager {
setTimeout(timeout: number) {
const slot = this._running ? this._running.slot : this._defaultSlot;
if (!slot.timeout)
return; // Zero timeout means some debug mode - do not set a timeout.
slot.timeout = timeout;
if (this._running)
this._updateTimeout(this._running);

View File

@ -159,7 +159,7 @@ test('should ignore test.setTimeout when debugging', async ({ runInlineTest }) =
await new Promise(f => setTimeout(f, 2000));
});
`
}, { timeout: 0 });
}, { debug: true });
expect(result.exitCode).toBe(0);
expect(result.passed).toBe(1);
});
@ -558,3 +558,25 @@ test('should allow custom worker fixture timeout longer than force exit cap', as
expect(result.output).toContain(`Error: Oh my!`);
expect(result.output).toContain(`1 error was not a part of any test, see above for details`);
});
test('test.setTimeout should be able to change custom fixture timeout', async ({ runInlineTest }) => {
const result = await runInlineTest({
'a.spec.ts': `
import { test as base, expect } from '@playwright/test';
const test = base.extend({
foo: [async ({}, use) => {
console.log('\\n%%foo setup');
test.setTimeout(100);
await new Promise(f => setTimeout(f, 3000));
await use('foo');
console.log('\\n%%foo teardown');
}, { timeout: 0 }],
});
test('times out', async ({ foo }) => {
});
`
});
expect(result.exitCode).toBe(1);
expect(result.failed).toBe(1);
expect(result.output).toContain(`Fixture "foo" timeout of 100ms exceeded during setup`);
});