From 6a4b6edd3f55579bcda87ac8142a6167189f168f Mon Sep 17 00:00:00 2001 From: Yash Yadav Date: Thu, 27 May 2021 17:12:10 +0530 Subject: [PATCH 01/11] Emit timeout() from test.js to api.js --- lib/api.js | 7 +++++++ lib/runner.js | 28 +++++++++++++++++----------- lib/test.js | 2 ++ 3 files changed, 26 insertions(+), 11 deletions(-) diff --git a/lib/api.js b/lib/api.js index bb630d288..157562fc9 100644 --- a/lib/api.js +++ b/lib/api.js @@ -232,6 +232,13 @@ class Api extends Emittery { } const worker = fork(file, options, apiOptions.nodeArguments); + worker.onStateChange(data => { + if (data.Timeout && !apiOptions.debug) + { + // Console.log for now as debouce value is not getting updated. + console.log(`Timeout: ${data.Timeout}\n`); + } + }); runStatus.observeWorker(worker, file, {selectingLines: lineNumbers.length > 0}); deregisteredSharedWorkers.push(sharedWorkers.observeWorkerProcess(worker, runStatus)); diff --git a/lib/runner.js b/lib/runner.js index 5bca02c88..cadb68932 100644 --- a/lib/runner.js +++ b/lib/runner.js @@ -65,6 +65,10 @@ class Runner extends Emittery { return true; }; + this.emitStateChange = evt => { + this.emit('stateChange', Object.assign(evt)); + }; + let hasStarted = false; let scheduledStart = false; const meta = Object.freeze({ @@ -115,7 +119,7 @@ class Runner extends Emittery { } this.tasks.todo.push({title: rawTitle, metadata}); - this.emit('stateChange', { + this.emitStateChange({ type: 'declared-test', title: rawTitle, knownFailing: false, @@ -168,7 +172,7 @@ class Runner extends Emittery { this.snapshots.touch(title, metadata.taskIndex); - this.emit('stateChange', { + this.emitStateChange({ type: 'declared-test', title, knownFailing: metadata.failing, @@ -289,19 +293,20 @@ class Runner extends Emittery { powerAssert: this.powerAssert, title: `${task.title}${titleSuffix || ''}`, isHook: true, - testPassed + testPassed, + emitStateChange: this.emitStateChange })); const outcome = await this.runMultiple(hooks, this.serial); for (const result of outcome.storedResults) { if (result.passed) { - this.emit('stateChange', { + this.emitStateChange({ type: 'hook-finished', title: result.title, duration: result.duration, logs: result.logs }); } else { - this.emit('stateChange', { + this.emitStateChange({ type: 'hook-failed', title: result.title, err: serializeError('Hook failure', true, result.error), @@ -340,14 +345,15 @@ class Runner extends Emittery { metadata: task.metadata, powerAssert: this.powerAssert, title: task.title, - registerUniqueTitle: this.registerUniqueTitle + registerUniqueTitle: this.registerUniqueTitle, + emitStateChange: this.emitStateChange }); const result = await this.runSingle(test); testOk = result.passed; if (testOk) { - this.emit('stateChange', { + this.emitStateChange({ type: 'test-passed', title: result.title, duration: result.duration, @@ -363,7 +369,7 @@ class Runner extends Emittery { testPassed: testOk }); } else { - this.emit('stateChange', { + this.emitStateChange({ type: 'test-failed', title: result.title, err: serializeError('Test failure', true, result.error, this.file), @@ -399,7 +405,7 @@ class Runner extends Emittery { continue; } - this.emit('stateChange', { + this.emitStateChange({ type: 'selected-test', title: task.title, knownFailing: task.metadata.failing, @@ -425,7 +431,7 @@ class Runner extends Emittery { continue; } - this.emit('stateChange', { + this.emitStateChange({ type: 'selected-test', title: task.title, knownFailing: task.metadata.failing, @@ -451,7 +457,7 @@ class Runner extends Emittery { continue; } - this.emit('stateChange', { + this.emitStateChange({ type: 'selected-test', title: task.title, knownFailing: false, diff --git a/lib/test.js b/lib/test.js index e01c08d7a..53754a940 100644 --- a/lib/test.js +++ b/lib/test.js @@ -208,6 +208,7 @@ class Test { this.registerUniqueTitle = options.registerUniqueTitle; this.logs = []; this.teardowns = []; + this.emitStateChange = options.emitStateChange; const {snapshotBelongsTo = this.title, nextSnapshotIndex = 0} = options; this.snapshotBelongsTo = snapshotBelongsTo; @@ -431,6 +432,7 @@ class Test { this.finishDueToTimeout(); } }, ms); + this.emitStateChange({Timeout: this.timeoutMs}); } refreshTimeout() { From 1a099eae4f3edb277e5f2412270504843ce9dacb Mon Sep 17 00:00:00 2001 From: Yash Yadav Date: Wed, 2 Jun 2021 18:02:40 +0530 Subject: [PATCH 02/11] Skip global timeout if self-timeout takes stage --- lib/api.js | 21 ++++++++++++++------- lib/runner.js | 14 +++++++++++--- lib/test.js | 4 ++-- 3 files changed, 27 insertions(+), 12 deletions(-) diff --git a/lib/api.js b/lib/api.js index 157562fc9..1c2f7e7be 100644 --- a/lib/api.js +++ b/lib/api.js @@ -72,10 +72,17 @@ class Api extends Emittery { const pendingWorkers = new Set(); const timedOutWorkerFiles = new Set(); let restartTimer; + let customTimeout = 0; if (apiOptions.timeout && !apiOptions.debug) { const timeout = ms(apiOptions.timeout); restartTimer = debounce(() => { + if (customTimeout > timeout) + { + // Skip global timeout as the independent test timer will handle it. + customTimeout = 0; + return; + } // If failFast is active, prevent new test files from running after // the current ones are exited. if (failFast) { @@ -232,13 +239,7 @@ class Api extends Emittery { } const worker = fork(file, options, apiOptions.nodeArguments); - worker.onStateChange(data => { - if (data.Timeout && !apiOptions.debug) - { - // Console.log for now as debouce value is not getting updated. - console.log(`Timeout: ${data.Timeout}\n`); - } - }); + runStatus.observeWorker(worker, file, {selectingLines: lineNumbers.length > 0}); deregisteredSharedWorkers.push(sharedWorkers.observeWorkerProcess(worker, runStatus)); @@ -247,6 +248,12 @@ class Api extends Emittery { pendingWorkers.delete(worker); }); restartTimer(); + worker.onStateChange(data => { + if (data.type === 'test-timeout-configured' && !apiOptions.debug) + { + customTimeout = data.period; + } + }); await worker.promise; }, {concurrency, stopOnError: false}); diff --git a/lib/runner.js b/lib/runner.js index cadb68932..98873845b 100644 --- a/lib/runner.js +++ b/lib/runner.js @@ -66,9 +66,17 @@ class Runner extends Emittery { }; this.emitStateChange = evt => { - this.emit('stateChange', Object.assign(evt)); + this.emit('stateChange', evt); }; + this.notifyTimeoutUpdate = timeoutMs => { + this.emitStateChange({ + type: 'test-timeout-configured', + title: `Timeout ${timeoutMs} ms`, + period: timeoutMs + }) + } + let hasStarted = false; let scheduledStart = false; const meta = Object.freeze({ @@ -294,7 +302,7 @@ class Runner extends Emittery { title: `${task.title}${titleSuffix || ''}`, isHook: true, testPassed, - emitStateChange: this.emitStateChange + notifyTimeoutUpdate: this.notifyTimeoutUpdate })); const outcome = await this.runMultiple(hooks, this.serial); for (const result of outcome.storedResults) { @@ -346,7 +354,7 @@ class Runner extends Emittery { powerAssert: this.powerAssert, title: task.title, registerUniqueTitle: this.registerUniqueTitle, - emitStateChange: this.emitStateChange + notifyTimeoutUpdate: this.notifyTimeoutUpdate }); const result = await this.runSingle(test); diff --git a/lib/test.js b/lib/test.js index 53754a940..ae31578dd 100644 --- a/lib/test.js +++ b/lib/test.js @@ -208,7 +208,7 @@ class Test { this.registerUniqueTitle = options.registerUniqueTitle; this.logs = []; this.teardowns = []; - this.emitStateChange = options.emitStateChange; + this.notifyTimeoutUpdate = options.notifyTimeoutUpdate; const {snapshotBelongsTo = this.title, nextSnapshotIndex = 0} = options; this.snapshotBelongsTo = snapshotBelongsTo; @@ -432,7 +432,7 @@ class Test { this.finishDueToTimeout(); } }, ms); - this.emitStateChange({Timeout: this.timeoutMs}); + this.notifyTimeoutUpdate(this.timeoutMs); } refreshTimeout() { From 30ee578dd632cbcf3df6e5c2a98d4f23fcb9b431 Mon Sep 17 00:00:00 2001 From: Yash Yadav Date: Wed, 2 Jun 2021 21:24:48 +0530 Subject: [PATCH 03/11] fix existing tests --- lib/api.js | 16 +++++++--------- lib/runner.js | 4 ++-- lib/test.js | 5 ++++- 3 files changed, 13 insertions(+), 12 deletions(-) diff --git a/lib/api.js b/lib/api.js index 1c2f7e7be..623ffab1d 100644 --- a/lib/api.js +++ b/lib/api.js @@ -77,12 +77,12 @@ class Api extends Emittery { const timeout = ms(apiOptions.timeout); restartTimer = debounce(() => { - if (customTimeout > timeout) - { + if (customTimeout > timeout) { // Skip global timeout as the independent test timer will handle it. customTimeout = 0; return; } + // If failFast is active, prevent new test files from running after // the current ones are exited. if (failFast) { @@ -239,7 +239,11 @@ class Api extends Emittery { } const worker = fork(file, options, apiOptions.nodeArguments); - + worker.onStateChange(data => { + if (data.type === 'test-timeout-configured' && !apiOptions.debug) { + customTimeout = data.period; + } + }); runStatus.observeWorker(worker, file, {selectingLines: lineNumbers.length > 0}); deregisteredSharedWorkers.push(sharedWorkers.observeWorkerProcess(worker, runStatus)); @@ -248,12 +252,6 @@ class Api extends Emittery { pendingWorkers.delete(worker); }); restartTimer(); - worker.onStateChange(data => { - if (data.type === 'test-timeout-configured' && !apiOptions.debug) - { - customTimeout = data.period; - } - }); await worker.promise; }, {concurrency, stopOnError: false}); diff --git a/lib/runner.js b/lib/runner.js index 98873845b..12d23c46c 100644 --- a/lib/runner.js +++ b/lib/runner.js @@ -74,8 +74,8 @@ class Runner extends Emittery { type: 'test-timeout-configured', title: `Timeout ${timeoutMs} ms`, period: timeoutMs - }) - } + }); + }; let hasStarted = false; let scheduledStart = false; diff --git a/lib/test.js b/lib/test.js index ae31578dd..26905c7e6 100644 --- a/lib/test.js +++ b/lib/test.js @@ -432,7 +432,10 @@ class Test { this.finishDueToTimeout(); } }, ms); - this.notifyTimeoutUpdate(this.timeoutMs); + + if (this.notifyTimeoutUpdate) { + this.notifyTimeoutUpdate(this.timeoutMs); + } } refreshTimeout() { From 1449d507de2f47caf292529b9471fcba2cfed47d Mon Sep 17 00:00:00 2001 From: Mark Wubben Date: Sun, 6 Jun 2021 15:50:41 +0200 Subject: [PATCH 04/11] Undo unnecessary changes --- lib/runner.js | 24 ++++++++++-------------- 1 file changed, 10 insertions(+), 14 deletions(-) diff --git a/lib/runner.js b/lib/runner.js index 12d23c46c..8f19f6540 100644 --- a/lib/runner.js +++ b/lib/runner.js @@ -65,12 +65,8 @@ class Runner extends Emittery { return true; }; - this.emitStateChange = evt => { - this.emit('stateChange', evt); - }; - this.notifyTimeoutUpdate = timeoutMs => { - this.emitStateChange({ + this.emit('stateChange', { type: 'test-timeout-configured', title: `Timeout ${timeoutMs} ms`, period: timeoutMs @@ -127,7 +123,7 @@ class Runner extends Emittery { } this.tasks.todo.push({title: rawTitle, metadata}); - this.emitStateChange({ + this.emit('stateChange', { type: 'declared-test', title: rawTitle, knownFailing: false, @@ -180,7 +176,7 @@ class Runner extends Emittery { this.snapshots.touch(title, metadata.taskIndex); - this.emitStateChange({ + this.emit('stateChange', { type: 'declared-test', title, knownFailing: metadata.failing, @@ -307,14 +303,14 @@ class Runner extends Emittery { const outcome = await this.runMultiple(hooks, this.serial); for (const result of outcome.storedResults) { if (result.passed) { - this.emitStateChange({ + this.emit('stateChange', { type: 'hook-finished', title: result.title, duration: result.duration, logs: result.logs }); } else { - this.emitStateChange({ + this.emit('stateChange', { type: 'hook-failed', title: result.title, err: serializeError('Hook failure', true, result.error), @@ -361,7 +357,7 @@ class Runner extends Emittery { testOk = result.passed; if (testOk) { - this.emitStateChange({ + this.emit('stateChange', { type: 'test-passed', title: result.title, duration: result.duration, @@ -377,7 +373,7 @@ class Runner extends Emittery { testPassed: testOk }); } else { - this.emitStateChange({ + this.emit('stateChange', { type: 'test-failed', title: result.title, err: serializeError('Test failure', true, result.error, this.file), @@ -413,7 +409,7 @@ class Runner extends Emittery { continue; } - this.emitStateChange({ + this.emit('stateChange', { type: 'selected-test', title: task.title, knownFailing: task.metadata.failing, @@ -439,7 +435,7 @@ class Runner extends Emittery { continue; } - this.emitStateChange({ + this.emit('stateChange', { type: 'selected-test', title: task.title, knownFailing: task.metadata.failing, @@ -465,7 +461,7 @@ class Runner extends Emittery { continue; } - this.emitStateChange({ + this.emit('stateChange', { type: 'selected-test', title: task.title, knownFailing: false, From 89e63702078fbfaf4fda4605455899fba7f4d759 Mon Sep 17 00:00:00 2001 From: Mark Wubben Date: Sun, 6 Jun 2021 15:51:00 +0200 Subject: [PATCH 05/11] Event does not need a title --- lib/runner.js | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/runner.js b/lib/runner.js index 8f19f6540..c7e1d93a5 100644 --- a/lib/runner.js +++ b/lib/runner.js @@ -68,7 +68,6 @@ class Runner extends Emittery { this.notifyTimeoutUpdate = timeoutMs => { this.emit('stateChange', { type: 'test-timeout-configured', - title: `Timeout ${timeoutMs} ms`, period: timeoutMs }); }; From fa50d916faa9c8a7fbf8067de3169e86c6527fb3 Mon Sep 17 00:00:00 2001 From: Mark Wubben Date: Sun, 6 Jun 2021 15:53:02 +0200 Subject: [PATCH 06/11] Assume notifyTimeoutUpdate() is available --- lib/test.js | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/lib/test.js b/lib/test.js index 26905c7e6..d55869574 100644 --- a/lib/test.js +++ b/lib/test.js @@ -433,9 +433,7 @@ class Test { } }, ms); - if (this.notifyTimeoutUpdate) { - this.notifyTimeoutUpdate(this.timeoutMs); - } + this.notifyTimeoutUpdate(this.timeoutMs); } refreshTimeout() { From 4f0b7e3c407f9457024d006202b00b64cbfa1209 Mon Sep 17 00:00:00 2001 From: Yash Yadav Date: Sun, 13 Jun 2021 22:31:24 +0530 Subject: [PATCH 07/11] fixed ci and added an api test --- lib/api.js | 8 +++----- test-tap/api.js | 3 ++- test-tap/fixture/fail-fast/timeout/passes-slow.cjs | 9 +++++++++ test-tap/helper/ava-test.js | 6 ++++-- 4 files changed, 18 insertions(+), 8 deletions(-) create mode 100644 test-tap/fixture/fail-fast/timeout/passes-slow.cjs diff --git a/lib/api.js b/lib/api.js index 951eba20f..4f61c840c 100644 --- a/lib/api.js +++ b/lib/api.js @@ -73,14 +73,12 @@ export default class Api extends Emittery { const pendingWorkers = new Set(); const timedOutWorkerFiles = new Set(); let restartTimer; - let customTimeout = 0; + let ignoreTimeoutsUntil = 0; if (apiOptions.timeout && !apiOptions.debug) { const timeout = ms(apiOptions.timeout); restartTimer = debounce(() => { - if (customTimeout > timeout) { - // Skip global timeout as the independent test timer will handle it. - customTimeout = 0; + if (Date.now() < ignoreTimeoutsUntil) { return; } @@ -242,7 +240,7 @@ export default class Api extends Emittery { const worker = fork(file, options, apiOptions.nodeArguments); worker.onStateChange(data => { if (data.type === 'test-timeout-configured' && !apiOptions.debug) { - customTimeout = data.period; + ignoreTimeoutsUntil = Math.max(ignoreTimeoutsUntil, Date.now() + data.period); } }); runStatus.observeWorker(worker, file, {selectingLines: lineNumbers.length > 0}); diff --git a/test-tap/api.js b/test-tap/api.js index 2d4137faa..afcf36b54 100644 --- a/test-tap/api.js +++ b/test-tap/api.js @@ -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); diff --git a/test-tap/fixture/fail-fast/timeout/passes-slow.cjs b/test-tap/fixture/fail-fast/timeout/passes-slow.cjs new file mode 100644 index 000000000..e8eadb0c5 --- /dev/null +++ b/test-tap/fixture/fail-fast/timeout/passes-slow.cjs @@ -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(); +}); diff --git a/test-tap/helper/ava-test.js b/test-tap/helper/ava-test.js index 3e3ef13c1..ffd6993bb 100644 --- a/test-tap/helper/ava-test.js +++ b/test-tap/helper/ava-test.js @@ -20,7 +20,8 @@ export function withExperiments(experiments = {}) { fn, registerUniqueTitle, metadata: {type: 'test'}, - title + title, + notifyTimeoutUpdate: Object.assign(() => {}) }); } @@ -32,7 +33,8 @@ export function withExperiments(experiments = {}) { fn, registerUniqueTitle, metadata: {type: 'test', failing: true}, - title: 'test.failing' + title: 'test.failing', + notifyTimeoutUpdate: Object.assign(() => {}) }); }; From d5a724714ba5ebf1de16d0dfa9856896666e1e11 Mon Sep 17 00:00:00 2001 From: Yash Yadav Date: Sat, 19 Jun 2021 01:09:05 +0530 Subject: [PATCH 08/11] Added test for notifyTimeoutUpdate --- test-tap/runner.js | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/test-tap/runner.js b/test-tap/runner.js index 91a85f826..316111003 100644 --- a/test-tap/runner.js +++ b/test-tap/runner.js @@ -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 => { From 5ebb297567fe518be5eb5dae5a5dd4d2c9737a42 Mon Sep 17 00:00:00 2001 From: Mark Wubben Date: Sat, 19 Jun 2021 18:49:32 +0200 Subject: [PATCH 09/11] Make sure to restart the timer when it's ignored --- lib/api.js | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/api.js b/lib/api.js index 4f61c840c..03707eaf8 100644 --- a/lib/api.js +++ b/lib/api.js @@ -79,6 +79,7 @@ export default class Api extends Emittery { restartTimer = debounce(() => { if (Date.now() < ignoreTimeoutsUntil) { + restartTimer(); return; } From 46de7f99a6f1705e4281a32ab6c639b215882eac Mon Sep 17 00:00:00 2001 From: Mark Wubben Date: Sat, 19 Jun 2021 18:51:19 +0200 Subject: [PATCH 10/11] Simplify notifyTimeoutUpdate() stub --- test-tap/helper/ava-test.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test-tap/helper/ava-test.js b/test-tap/helper/ava-test.js index ffd6993bb..dc4ac17df 100644 --- a/test-tap/helper/ava-test.js +++ b/test-tap/helper/ava-test.js @@ -21,7 +21,7 @@ export function withExperiments(experiments = {}) { registerUniqueTitle, metadata: {type: 'test'}, title, - notifyTimeoutUpdate: Object.assign(() => {}) + notifyTimeoutUpdate() {} }); } @@ -34,7 +34,7 @@ export function withExperiments(experiments = {}) { registerUniqueTitle, metadata: {type: 'test', failing: true}, title: 'test.failing', - notifyTimeoutUpdate: Object.assign(() => {}) + notifyTimeoutUpdate() {} }); }; From d8dc072bd1f3057788bf2d45426aa2964187ace1 Mon Sep 17 00:00:00 2001 From: Mark Wubben Date: Sat, 19 Jun 2021 18:55:59 +0200 Subject: [PATCH 11/11] Fix stateChange test --- test-tap/runner.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test-tap/runner.js b/test-tap/runner.js index 316111003..f249158be 100644 --- a/test-tap/runner.js +++ b/test-tap/runner.js @@ -66,16 +66,16 @@ 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(); + t.end(); } }); + runner.notifyTimeoutUpdate(120); }); test('run serial tests before concurrent ones', t => {