Skip to content

Commit 1537d3c

Browse files
authored
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.
1 parent b2ccfc3 commit 1537d3c

File tree

7 files changed

+42
-23
lines changed

7 files changed

+42
-23
lines changed

packages/playwright/src/common/config.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,7 @@ export class FullConfigInternal {
7979
globalTimeout: takeFirst(configCLIOverrides.globalTimeout, userConfig.globalTimeout, 0),
8080
grep: takeFirst(userConfig.grep, defaultGrep),
8181
grepInvert: takeFirst(userConfig.grepInvert, null),
82-
maxFailures: takeFirst(configCLIOverrides.maxFailures, userConfig.maxFailures, 0),
82+
maxFailures: takeFirst(configCLIOverrides.debug ? 1 : undefined, configCLIOverrides.maxFailures, userConfig.maxFailures, 0),
8383
metadata: takeFirst(userConfig.metadata, {}),
8484
preserveOutput: takeFirst(userConfig.preserveOutput, 'always'),
8585
reporter: takeFirst(configCLIOverrides.reporter, resolveReporters(userConfig.reporter, configDir), [[defaultReporter]]),
@@ -99,7 +99,7 @@ export class FullConfigInternal {
9999

100100
(this.config as any)[configInternalSymbol] = this;
101101

102-
const workers = takeFirst(configCLIOverrides.workers, userConfig.workers, '50%');
102+
const workers = takeFirst(configCLIOverrides.debug ? 1 : undefined, configCLIOverrides.workers, userConfig.workers, '50%');
103103
if (typeof workers === 'string') {
104104
if (workers.endsWith('%')) {
105105
const cpus = os.cpus().length;
@@ -179,7 +179,7 @@ export class FullProjectInternal {
179179
snapshotDir: takeFirst(pathResolve(configDir, projectConfig.snapshotDir), pathResolve(configDir, config.snapshotDir), testDir),
180180
testIgnore: takeFirst(projectConfig.testIgnore, config.testIgnore, []),
181181
testMatch: takeFirst(projectConfig.testMatch, config.testMatch, '**/*.@(spec|test).?(c|m)[jt]s?(x)'),
182-
timeout: takeFirst(configCLIOverrides.timeout, projectConfig.timeout, config.timeout, defaultTimeout),
182+
timeout: takeFirst(configCLIOverrides.debug ? 0 : undefined, configCLIOverrides.timeout, projectConfig.timeout, config.timeout, defaultTimeout),
183183
use: mergeObjects(config.use, projectConfig.use, configCLIOverrides.use),
184184
dependencies: projectConfig.dependencies || [],
185185
teardown: projectConfig.teardown,

packages/playwright/src/common/ipc.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import type { ConfigLocation, FullConfigInternal } from './config';
2020
import type { ReporterDescription, TestInfoError, TestStatus } from '../../types/test';
2121

2222
export type ConfigCLIOverrides = {
23+
debug?: boolean;
2324
forbidOnly?: boolean;
2425
fullyParallel?: boolean;
2526
globalTimeout?: number;

packages/playwright/src/index.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -229,7 +229,7 @@ const playwrightFixtures: Fixtures<TestFixtures, WorkerFixtures> = ({
229229
playwrightLibrary.selectors.setTestIdAttribute(testIdAttribute);
230230
testInfo.snapshotSuffix = process.platform;
231231
if (debugMode())
232-
testInfo.setTimeout(0);
232+
(testInfo as TestInfoImpl)._setDebugMode();
233233
for (const browserType of [playwright.chromium, playwright.firefox, playwright.webkit]) {
234234
(browserType as any)._defaultContextOptions = _combinedContextOptions;
235235
(browserType as any)._defaultContextTimeout = actionTimeout || 0;
@@ -270,7 +270,7 @@ const playwrightFixtures: Fixtures<TestFixtures, WorkerFixtures> = ({
270270
step?.complete({ error });
271271
},
272272
onWillPause: () => {
273-
currentTestInfo()?.setTimeout(0);
273+
currentTestInfo()?._setDebugMode();
274274
},
275275
runAfterCreateBrowserContext: async (context: BrowserContext) => {
276276
await artifactsRecorder?.didCreateBrowserContext(context);

packages/playwright/src/program.ts

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -308,9 +308,7 @@ function overridesFromOptions(options: { [key: string]: any }): ConfigCLIOverrid
308308
if (options.headed || options.debug)
309309
overrides.use = { headless: false };
310310
if (!options.ui && options.debug) {
311-
overrides.maxFailures = 1;
312-
overrides.timeout = 0;
313-
overrides.workers = 1;
311+
overrides.debug = true;
314312
process.env.PWDEBUG = '1';
315313
}
316314
if (!options.ui && options.trace) {

packages/playwright/src/worker/testInfo.ts

Lines changed: 6 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -167,6 +167,8 @@ export class TestInfoImpl implements TestInfo {
167167
this.expectedStatus = test?.expectedStatus ?? 'skipped';
168168

169169
this._timeoutManager = new TimeoutManager(this.project.timeout);
170+
if (configInternal.configCLIOverrides.debug)
171+
this._setDebugMode();
170172

171173
this.outputDir = (() => {
172174
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 {
225227
}
226228
}
227229

228-
private _findLastNonFinishedStep(filter: (step: TestStepInternal) => boolean) {
229-
let result: TestStepInternal | undefined;
230-
const visit = (step: TestStepInternal) => {
231-
if (!step.endWallTime && filter(step))
232-
result = step;
233-
step.steps.forEach(visit);
234-
};
235-
this._steps.forEach(visit);
236-
return result;
237-
}
238-
239230
private _findLastStageStep() {
240231
for (let i = this._stages.length - 1; i >= 0; i--) {
241232
if (this._stages[i].step)
@@ -404,6 +395,10 @@ export class TestInfoImpl implements TestInfo {
404395
}
405396
}
406397

398+
_setDebugMode() {
399+
this._timeoutManager.setIgnoreTimeouts();
400+
}
401+
407402
// ------------ TestInfo methods ------------
408403

409404
async attach(name: string, options: { path?: string, body?: string | Buffer, contentType?: string } = {}) {

packages/playwright/src/worker/timeoutManager.ts

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -52,11 +52,16 @@ export const kMaxDeadline = 2147483647; // 2^31-1
5252
export class TimeoutManager {
5353
private _defaultSlot: TimeSlot;
5454
private _running?: Running;
55+
private _ignoreTimeouts = false;
5556

5657
constructor(timeout: number) {
5758
this._defaultSlot = { timeout, elapsed: 0 };
5859
}
5960

61+
setIgnoreTimeouts() {
62+
this._ignoreTimeouts = true;
63+
}
64+
6065
interrupt() {
6166
if (this._running)
6267
this._running.timeoutPromise.reject(this._createTimeoutError(this._running));
@@ -94,7 +99,7 @@ export class TimeoutManager {
9499
if (running.timer)
95100
clearTimeout(running.timer);
96101
running.timer = undefined;
97-
if (!running.slot.timeout) {
102+
if (this._ignoreTimeouts || !running.slot.timeout) {
98103
running.deadline = kMaxDeadline;
99104
return;
100105
}
@@ -119,8 +124,6 @@ export class TimeoutManager {
119124

120125
setTimeout(timeout: number) {
121126
const slot = this._running ? this._running.slot : this._defaultSlot;
122-
if (!slot.timeout)
123-
return; // Zero timeout means some debug mode - do not set a timeout.
124127
slot.timeout = timeout;
125128
if (this._running)
126129
this._updateTimeout(this._running);

tests/playwright-test/timeout.spec.ts

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -159,7 +159,7 @@ test('should ignore test.setTimeout when debugging', async ({ runInlineTest }) =
159159
await new Promise(f => setTimeout(f, 2000));
160160
});
161161
`
162-
}, { timeout: 0 });
162+
}, { debug: true });
163163
expect(result.exitCode).toBe(0);
164164
expect(result.passed).toBe(1);
165165
});
@@ -558,3 +558,25 @@ test('should allow custom worker fixture timeout longer than force exit cap', as
558558
expect(result.output).toContain(`Error: Oh my!`);
559559
expect(result.output).toContain(`1 error was not a part of any test, see above for details`);
560560
});
561+
562+
test('test.setTimeout should be able to change custom fixture timeout', async ({ runInlineTest }) => {
563+
const result = await runInlineTest({
564+
'a.spec.ts': `
565+
import { test as base, expect } from '@playwright/test';
566+
const test = base.extend({
567+
foo: [async ({}, use) => {
568+
console.log('\\n%%foo setup');
569+
test.setTimeout(100);
570+
await new Promise(f => setTimeout(f, 3000));
571+
await use('foo');
572+
console.log('\\n%%foo teardown');
573+
}, { timeout: 0 }],
574+
});
575+
test('times out', async ({ foo }) => {
576+
});
577+
`
578+
});
579+
expect(result.exitCode).toBe(1);
580+
expect(result.failed).toBe(1);
581+
expect(result.output).toContain(`Fixture "foo" timeout of 100ms exceeded during setup`);
582+
});

0 commit comments

Comments
 (0)