Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Have the global timeout take into account t.timeout() durations #2758

Merged
merged 12 commits into from
Jun 19, 2021
10 changes: 10 additions & 0 deletions lib/api.js
Original file line number Diff line number Diff line change
Expand Up @@ -73,10 +73,15 @@ export default class Api extends Emittery {
const pendingWorkers = new Set();
const timedOutWorkerFiles = new Set();
let restartTimer;
let ignoreTimeoutsUntil = 0;
if (apiOptions.timeout && !apiOptions.debug) {
const timeout = ms(apiOptions.timeout);

restartTimer = debounce(() => {
if (Date.now() < ignoreTimeoutsUntil) {
return;
}

// If failFast is active, prevent new test files from running after
// the current ones are exited.
if (failFast) {
Expand Down Expand Up @@ -233,6 +238,11 @@ export default class Api extends Emittery {
}

const worker = fork(file, options, apiOptions.nodeArguments);
worker.onStateChange(data => {
if (data.type === 'test-timeout-configured' && !apiOptions.debug) {
ignoreTimeoutsUntil = Math.max(ignoreTimeoutsUntil, Date.now() + data.period);
}
});
runStatus.observeWorker(worker, file, {selectingLines: lineNumbers.length > 0});
deregisteredSharedWorkers.push(observeWorkerProcess(worker, runStatus));

Expand Down
13 changes: 11 additions & 2 deletions lib/runner.js
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,13 @@ export default class Runner extends Emittery {
return true;
};

this.notifyTimeoutUpdate = timeoutMs => {
this.emit('stateChange', {
type: 'test-timeout-configured',
period: timeoutMs
});
};

let hasStarted = false;
let scheduledStart = false;
const meta = Object.freeze({
Expand Down Expand Up @@ -290,7 +297,8 @@ export default class Runner extends Emittery {
powerAssert: this.powerAssert,
title: `${task.title}${titleSuffix || ''}`,
isHook: true,
testPassed
testPassed,
notifyTimeoutUpdate: this.notifyTimeoutUpdate
}));
const outcome = await this.runMultiple(hooks, this.serial);
for (const result of outcome.storedResults) {
Expand Down Expand Up @@ -341,7 +349,8 @@ export default class Runner extends Emittery {
metadata: task.metadata,
powerAssert: this.powerAssert,
title: task.title,
registerUniqueTitle: this.registerUniqueTitle
registerUniqueTitle: this.registerUniqueTitle,
notifyTimeoutUpdate: this.notifyTimeoutUpdate
});

const result = await this.runSingle(test);
Expand Down
3 changes: 3 additions & 0 deletions lib/test.js
Original file line number Diff line number Diff line change
Expand Up @@ -208,6 +208,7 @@ export default class Test {
this.registerUniqueTitle = options.registerUniqueTitle;
this.logs = [];
this.teardowns = [];
this.notifyTimeoutUpdate = options.notifyTimeoutUpdate;

const {snapshotBelongsTo = this.title, nextSnapshotIndex = 0} = options;
this.snapshotBelongsTo = snapshotBelongsTo;
Expand Down Expand Up @@ -431,6 +432,8 @@ export default class Test {
this.finishDueToTimeout();
}
}, ms);

this.notifyTimeoutUpdate(this.timeoutMs);
}

refreshTimeout() {
Expand Down
3 changes: 2 additions & 1 deletion test-tap/api.js
Original file line number Diff line number Diff line change
Expand Up @@ -291,7 +291,8 @@ for (const opt of opts) {

return api.run({files: [
path.join(__dirname, 'fixture/fail-fast/timeout/fails.cjs'),
path.join(__dirname, 'fixture/fail-fast/timeout/passes.cjs')
path.join(__dirname, 'fixture/fail-fast/timeout/passes.cjs'),
path.join(__dirname, 'fixture/fail-fast/timeout/passes-slow.cjs')
]})
.then(runStatus => {
t.ok(api.options.failFast);
Expand Down
9 changes: 9 additions & 0 deletions test-tap/fixture/fail-fast/timeout/passes-slow.cjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
const delay = require('delay');

const test = require('../../../../entrypoints/main.cjs');

test('slow pass with timeout', async t => {
t.timeout(120);
await delay(110);
t.pass();
});
6 changes: 4 additions & 2 deletions test-tap/helper/ava-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,8 @@ export function withExperiments(experiments = {}) {
fn,
registerUniqueTitle,
metadata: {type: 'test'},
title
title,
notifyTimeoutUpdate: Object.assign(() => {})
});
}

Expand All @@ -32,7 +33,8 @@ export function withExperiments(experiments = {}) {
fn,
registerUniqueTitle,
metadata: {type: 'test', failing: true},
title: 'test.failing'
title: 'test.failing',
notifyTimeoutUpdate: Object.assign(() => {})
});
};

Expand Down
15 changes: 15 additions & 0 deletions test-tap/runner.js
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,21 @@ test('runner emits "stateChange" events', t => {
});
});

test('notifyTimeoutUpdate emits "stateChange" event', t => {
const runner = new Runner();

runner.notifyTimeoutUpdate(120);
runner.on('stateChange', evt => {
if (evt.type === 'test-timeout-configured') {
t.same(evt, {
type: 'test-timeout-configured',
period: 120
});
t.pass();
}
});
});

test('run serial tests before concurrent ones', t => {
const array = [];
return promiseEnd(new Runner(), runner => {
Expand Down