From ce2b12e812d0d14c1cca79ca594f695c7b9ab7ab Mon Sep 17 00:00:00 2001 From: Kevin Date: Mon, 24 Oct 2022 18:25:45 -0700 Subject: [PATCH 1/4] adding notes before making change unecessary dif temp add logs for pending tests fix linting issues undo unecessary change --- lib/reporters/default.js | 3 +++ lib/run-status.js | 17 +++++++++++++++++ lib/runner.js | 6 ++++++ 3 files changed, 26 insertions(+) diff --git a/lib/reporters/default.js b/lib/reporters/default.js index 6bb14fc94..dd4543df6 100644 --- a/lib/reporters/default.js +++ b/lib/reporters/default.js @@ -370,8 +370,11 @@ export default class Reporter { } this.lineWriter.writeLine(`${testsInFile.size} tests were pending in ${this.relativeFile(file)}\n`); + const testTitleToLogs = evt.pendingTestsLogReference.get(file) ?? new Map(); for (const title of testsInFile) { + const logs = testTitleToLogs.get(title) ?? []; this.lineWriter.writeLine(`${figures.circleDotted} ${this.prefixTitle(file, title)}`); + this.writeLogs({logs}); } this.lineWriter.writeLine(''); diff --git a/lib/run-status.js b/lib/run-status.js index ea1149fa4..efd4d5961 100644 --- a/lib/run-status.js +++ b/lib/run-status.js @@ -9,6 +9,7 @@ export default class RunStatus extends Emittery { super(); this.pendingTests = new Map(); + this.pendingTestsLogReference = new Map(); this.emptyParallelRun = parallelRuns && parallelRuns.currentFileCount === 0 @@ -60,6 +61,7 @@ export default class RunStatus extends Emittery { }); this.pendingTests.set(testFile, new Set()); + this.pendingTestsLogReference.set(testFile, new Map()); worker.onStateChange(data => this.emitStateChange(data)); } @@ -124,10 +126,15 @@ export default class RunStatus extends Emittery { fileStats.remainingTests--; this.removePendingTest(event); break; + case 'test-register-log-reference': + this.addLogReference(event); + break; case 'timeout': stats.timeouts++; event.pendingTests = this.pendingTests; + event.pendingTestsLogReference = this.pendingTestsLogReference; this.pendingTests = new Map(); + this.pendingTestsLogReference = new Map(); for (const testsInFile of event.pendingTests.values()) { stats.timedOutTests += testsInFile.size; } @@ -135,11 +142,15 @@ export default class RunStatus extends Emittery { break; case 'interrupt': event.pendingTests = this.pendingTests; + event.pendingTestsLogReference = this.pendingTestsLogReference; this.pendingTests = new Map(); + this.pendingTestsLogReference = new Map(); break; case 'process-exit': event.pendingTests = this.pendingTests; + event.pendingTestsLogReference = this.pendingTestsLogReference; this.pendingTests = new Map(); + this.pendingTestsLogReference = new Map(); break; case 'uncaught-exception': stats.uncaughtExceptions++; @@ -198,6 +209,12 @@ export default class RunStatus extends Emittery { return 0; } + addLogReference(event) { + if (this.pendingTestsLogReference.has(event.testFile)) { + this.pendingTestsLogReference.get(event.testFile).set(event.title, event.logs); + } + } + addPendingTest(event) { if (this.pendingTests.has(event.testFile)) { this.pendingTests.get(event.testFile).add(event.title); diff --git a/lib/runner.js b/lib/runner.js index 181e28758..bb5dd647e 100644 --- a/lib/runner.js +++ b/lib/runner.js @@ -358,6 +358,12 @@ export default class Runner extends Emittery { notifyTimeoutUpdate: this.notifyTimeoutUpdate, }); + this.emit('stateChange', { + type: 'test-register-log-reference', + title: task.title, + logs: test.logs, + }); + const result = await this.runSingle(test); testOk = result.passed; From 1937048a124aa3d903f88c506205dd31f642f979 Mon Sep 17 00:00:00 2001 From: Kevin Date: Tue, 1 Nov 2022 11:34:31 -0700 Subject: [PATCH 2/4] add tests --- test-tap/fixture/long-running.cjs | 1 + .../fixture/report/timeoutcontextlogs/a.cjs | 20 +++++++++++ .../fixture/report/timeoutcontextlogs/b.cjs | 17 +++++++++ .../report/timeoutcontextlogs/package.json | 1 + test-tap/helper/report.js | 1 + test-tap/integration/assorted.js | 1 + test-tap/reporters/default.js | 1 + .../default.timeoutcontextlogs.v14.log | 36 +++++++++++++++++++ .../default.timeoutcontextlogs.v16.log | 36 +++++++++++++++++++ .../default.timeoutcontextlogs.v18.log | 36 +++++++++++++++++++ 10 files changed, 150 insertions(+) create mode 100644 test-tap/fixture/report/timeoutcontextlogs/a.cjs create mode 100644 test-tap/fixture/report/timeoutcontextlogs/b.cjs create mode 100644 test-tap/fixture/report/timeoutcontextlogs/package.json create mode 100644 test-tap/reporters/default.timeoutcontextlogs.v14.log create mode 100644 test-tap/reporters/default.timeoutcontextlogs.v16.log create mode 100644 test-tap/reporters/default.timeoutcontextlogs.v18.log diff --git a/test-tap/fixture/long-running.cjs b/test-tap/fixture/long-running.cjs index 8fadb2184..0bf486e90 100644 --- a/test-tap/fixture/long-running.cjs +++ b/test-tap/fixture/long-running.cjs @@ -3,6 +3,7 @@ const delay = require('delay'); const test = require('../../entrypoints/main.cjs'); test('slow', async t => { + t.log('helpful log of a pending test') await delay(5000); t.pass(); }); diff --git a/test-tap/fixture/report/timeoutcontextlogs/a.cjs b/test-tap/fixture/report/timeoutcontextlogs/a.cjs new file mode 100644 index 000000000..b00bc0c17 --- /dev/null +++ b/test-tap/fixture/report/timeoutcontextlogs/a.cjs @@ -0,0 +1,20 @@ +const delay = require('delay'); + +const test = require('../../../../entrypoints/main.cjs'); + +test('a passes', t => t.pass()); + +test('a slow', async t => { + t.log('this slow test prints useful debug message'); + await delay(15_000); + t.log('another useful debug message', {x: 5}); + t.pass(); +}); +test('a slow two', async t => { + t.log('another useful debug message just text'); + await delay(15_000); + t.log('another useful debug message', {x: 5}); + t.pass(); +}); + +test('a passes two', t => t.pass()); diff --git a/test-tap/fixture/report/timeoutcontextlogs/b.cjs b/test-tap/fixture/report/timeoutcontextlogs/b.cjs new file mode 100644 index 000000000..dfb0a8dac --- /dev/null +++ b/test-tap/fixture/report/timeoutcontextlogs/b.cjs @@ -0,0 +1,17 @@ +const delay = require('delay'); + +const test = require('../../../../entrypoints/main.cjs'); + +test('a passes', t => t.pass()); + +test('a slow', async t => { + t.log('hello world'); + await delay(15_000); + t.pass(); +}); +test('a slow two', async t => { + await delay(15_000); + t.pass(); +}); + +test('a passes two', t => t.pass()); diff --git a/test-tap/fixture/report/timeoutcontextlogs/package.json b/test-tap/fixture/report/timeoutcontextlogs/package.json new file mode 100644 index 000000000..0967ef424 --- /dev/null +++ b/test-tap/fixture/report/timeoutcontextlogs/package.json @@ -0,0 +1 @@ +{} diff --git a/test-tap/helper/report.js b/test-tap/helper/report.js index 3da08b777..0dde8b04c 100644 --- a/test-tap/helper/report.js +++ b/test-tap/helper/report.js @@ -126,6 +126,7 @@ exports.only = reporter => run('only', reporter); exports.timeoutInSingleFile = reporter => run('timeoutInSingleFile', reporter); exports.timeoutInMultipleFiles = reporter => run('timeoutInMultipleFiles', reporter); exports.timeoutWithMatch = reporter => run('timeoutWithMatch', reporter, {match: ['*needle*']}); +exports.timeoutContextLogs = reporter => run('timeoutContextLogs', reporter); exports.watch = reporter => run('watch', reporter); exports.edgeCases = reporter => run('edgeCases', reporter, { filter: [ diff --git a/test-tap/integration/assorted.js b/test-tap/integration/assorted.js index db292e8ac..d2b8d79fe 100644 --- a/test-tap/integration/assorted.js +++ b/test-tap/integration/assorted.js @@ -21,6 +21,7 @@ test('timeout', {skip: ciInfo.isCI}, t => { test('interrupt', {skip: ciInfo.isCI}, t => { const proc = execCli(['long-running.cjs'], (_, stdout) => { + t.match(stdout, 'helpful log of a pending test'); t.match(stdout, /SIGINT/); t.end(); }); diff --git a/test-tap/reporters/default.js b/test-tap/reporters/default.js index 3ab81d01d..fc51bb348 100644 --- a/test-tap/reporters/default.js +++ b/test-tap/reporters/default.js @@ -51,6 +51,7 @@ test(async t => { t.test('single file run', run('timeoutInSingleFile')); t.test('multiple files run', run('timeoutInMultipleFiles')); t.test('single file with only certain tests matched run', run('timeoutWithMatch')); + t.test('logs provided during a pending test logged at the end', run('timeoutContextLogs')) t.end(); }); }); diff --git a/test-tap/reporters/default.timeoutcontextlogs.v14.log b/test-tap/reporters/default.timeoutcontextlogs.v14.log new file mode 100644 index 000000000..e501aa87a --- /dev/null +++ b/test-tap/reporters/default.timeoutcontextlogs.v14.log @@ -0,0 +1,36 @@ + +---tty-stream-chunk-separator + ✔ a › a passes +---tty-stream-chunk-separator + ✔ a › a passes two +---tty-stream-chunk-separator +  + ✘ Timed out while running tests + + 2 tests were pending in a.cjs + + ◌ a › a slow + ℹ this slow test prints useful debug message + ◌ a › a slow two + ℹ another useful debug message just text + +---tty-stream-chunk-separator + ✔ b › a passes +---tty-stream-chunk-separator + ✔ b › a passes two +---tty-stream-chunk-separator +  + ✘ Timed out while running tests + + 2 tests were pending in b.cjs + + ◌ b › a slow + ℹ hello world + ◌ b › a slow two + +---tty-stream-chunk-separator + ─ + + 4 tests passed + 4 tests remained pending after a timeout +---tty-stream-chunk-separator diff --git a/test-tap/reporters/default.timeoutcontextlogs.v16.log b/test-tap/reporters/default.timeoutcontextlogs.v16.log new file mode 100644 index 000000000..e501aa87a --- /dev/null +++ b/test-tap/reporters/default.timeoutcontextlogs.v16.log @@ -0,0 +1,36 @@ + +---tty-stream-chunk-separator + ✔ a › a passes +---tty-stream-chunk-separator + ✔ a › a passes two +---tty-stream-chunk-separator +  + ✘ Timed out while running tests + + 2 tests were pending in a.cjs + + ◌ a › a slow + ℹ this slow test prints useful debug message + ◌ a › a slow two + ℹ another useful debug message just text + +---tty-stream-chunk-separator + ✔ b › a passes +---tty-stream-chunk-separator + ✔ b › a passes two +---tty-stream-chunk-separator +  + ✘ Timed out while running tests + + 2 tests were pending in b.cjs + + ◌ b › a slow + ℹ hello world + ◌ b › a slow two + +---tty-stream-chunk-separator + ─ + + 4 tests passed + 4 tests remained pending after a timeout +---tty-stream-chunk-separator diff --git a/test-tap/reporters/default.timeoutcontextlogs.v18.log b/test-tap/reporters/default.timeoutcontextlogs.v18.log new file mode 100644 index 000000000..e501aa87a --- /dev/null +++ b/test-tap/reporters/default.timeoutcontextlogs.v18.log @@ -0,0 +1,36 @@ + +---tty-stream-chunk-separator + ✔ a › a passes +---tty-stream-chunk-separator + ✔ a › a passes two +---tty-stream-chunk-separator +  + ✘ Timed out while running tests + + 2 tests were pending in a.cjs + + ◌ a › a slow + ℹ this slow test prints useful debug message + ◌ a › a slow two + ℹ another useful debug message just text + +---tty-stream-chunk-separator + ✔ b › a passes +---tty-stream-chunk-separator + ✔ b › a passes two +---tty-stream-chunk-separator +  + ✘ Timed out while running tests + + 2 tests were pending in b.cjs + + ◌ b › a slow + ℹ hello world + ◌ b › a slow two + +---tty-stream-chunk-separator + ─ + + 4 tests passed + 4 tests remained pending after a timeout +---tty-stream-chunk-separator From 98bbc25e75b44a033f716f098731cfa2e400a696 Mon Sep 17 00:00:00 2001 From: Kevin Date: Tue, 1 Nov 2022 11:50:29 -0700 Subject: [PATCH 3/4] add tests for sigint printing pending tests --- test-tap/fixture/long-running.cjs | 2 +- test-tap/fixture/report/timeoutcontextlogs/a.cjs | 8 ++++---- test-tap/fixture/report/timeoutcontextlogs/b.cjs | 2 ++ test-tap/integration/assorted.js | 1 + test-tap/reporters/default.js | 2 +- test-tap/reporters/default.timeoutcontextlogs.v14.log | 6 ++++-- test-tap/reporters/default.timeoutcontextlogs.v16.log | 6 ++++-- test-tap/reporters/default.timeoutcontextlogs.v18.log | 6 ++++-- 8 files changed, 21 insertions(+), 12 deletions(-) diff --git a/test-tap/fixture/long-running.cjs b/test-tap/fixture/long-running.cjs index 0bf486e90..f2871ff46 100644 --- a/test-tap/fixture/long-running.cjs +++ b/test-tap/fixture/long-running.cjs @@ -3,7 +3,7 @@ const delay = require('delay'); const test = require('../../entrypoints/main.cjs'); test('slow', async t => { - t.log('helpful log of a pending test') + t.log('helpful log of a pending test'); await delay(5000); t.pass(); }); diff --git a/test-tap/fixture/report/timeoutcontextlogs/a.cjs b/test-tap/fixture/report/timeoutcontextlogs/a.cjs index b00bc0c17..d353b4e84 100644 --- a/test-tap/fixture/report/timeoutcontextlogs/a.cjs +++ b/test-tap/fixture/report/timeoutcontextlogs/a.cjs @@ -5,15 +5,15 @@ const test = require('../../../../entrypoints/main.cjs'); test('a passes', t => t.pass()); test('a slow', async t => { - t.log('this slow test prints useful debug message'); + t.log('this slow test prints useful debug message just text'); await delay(15_000); - t.log('another useful debug message', {x: 5}); + t.log('this never logs because test times out'); t.pass(); }); test('a slow two', async t => { - t.log('another useful debug message just text'); - await delay(15_000); t.log('another useful debug message', {x: 5}); + await delay(15_000); + t.log('this never logs because test times out'); t.pass(); }); diff --git a/test-tap/fixture/report/timeoutcontextlogs/b.cjs b/test-tap/fixture/report/timeoutcontextlogs/b.cjs index dfb0a8dac..8789c9c6c 100644 --- a/test-tap/fixture/report/timeoutcontextlogs/b.cjs +++ b/test-tap/fixture/report/timeoutcontextlogs/b.cjs @@ -7,10 +7,12 @@ test('a passes', t => t.pass()); test('a slow', async t => { t.log('hello world'); await delay(15_000); + t.log('this never prints due to test time out'); t.pass(); }); test('a slow two', async t => { await delay(15_000); + t.log('this never prints due to test time out'); t.pass(); }); diff --git a/test-tap/integration/assorted.js b/test-tap/integration/assorted.js index d2b8d79fe..643d7e75b 100644 --- a/test-tap/integration/assorted.js +++ b/test-tap/integration/assorted.js @@ -14,6 +14,7 @@ const __dirname = fileURLToPath(new URL('.', import.meta.url)); test('timeout', {skip: ciInfo.isCI}, t => { execCli(['long-running.cjs', '-T', '1s'], (error, stdout) => { t.ok(error); + t.match(stdout, 'helpful log of a pending test'); t.match(stdout, /Timed out/); t.end(); }); diff --git a/test-tap/reporters/default.js b/test-tap/reporters/default.js index fc51bb348..62781ed23 100644 --- a/test-tap/reporters/default.js +++ b/test-tap/reporters/default.js @@ -51,7 +51,7 @@ test(async t => { t.test('single file run', run('timeoutInSingleFile')); t.test('multiple files run', run('timeoutInMultipleFiles')); t.test('single file with only certain tests matched run', run('timeoutWithMatch')); - t.test('logs provided during a pending test logged at the end', run('timeoutContextLogs')) + t.test('logs provided during a pending test logged at the end', run('timeoutContextLogs')); t.end(); }); }); diff --git a/test-tap/reporters/default.timeoutcontextlogs.v14.log b/test-tap/reporters/default.timeoutcontextlogs.v14.log index e501aa87a..04018664f 100644 --- a/test-tap/reporters/default.timeoutcontextlogs.v14.log +++ b/test-tap/reporters/default.timeoutcontextlogs.v14.log @@ -10,9 +10,11 @@ 2 tests were pending in a.cjs ◌ a › a slow - ℹ this slow test prints useful debug message + ℹ this slow test prints useful debug message just text ◌ a › a slow two - ℹ another useful debug message just text + ℹ another useful debug message { +  x: 5, + } ---tty-stream-chunk-separator ✔ b › a passes diff --git a/test-tap/reporters/default.timeoutcontextlogs.v16.log b/test-tap/reporters/default.timeoutcontextlogs.v16.log index e501aa87a..04018664f 100644 --- a/test-tap/reporters/default.timeoutcontextlogs.v16.log +++ b/test-tap/reporters/default.timeoutcontextlogs.v16.log @@ -10,9 +10,11 @@ 2 tests were pending in a.cjs ◌ a › a slow - ℹ this slow test prints useful debug message + ℹ this slow test prints useful debug message just text ◌ a › a slow two - ℹ another useful debug message just text + ℹ another useful debug message { +  x: 5, + } ---tty-stream-chunk-separator ✔ b › a passes diff --git a/test-tap/reporters/default.timeoutcontextlogs.v18.log b/test-tap/reporters/default.timeoutcontextlogs.v18.log index e501aa87a..04018664f 100644 --- a/test-tap/reporters/default.timeoutcontextlogs.v18.log +++ b/test-tap/reporters/default.timeoutcontextlogs.v18.log @@ -10,9 +10,11 @@ 2 tests were pending in a.cjs ◌ a › a slow - ℹ this slow test prints useful debug message + ℹ this slow test prints useful debug message just text ◌ a › a slow two - ℹ another useful debug message just text + ℹ another useful debug message { +  x: 5, + } ---tty-stream-chunk-separator ✔ b › a passes From 29ed7201eb622edcd0a8f397bba8b3a1ccc210dc Mon Sep 17 00:00:00 2001 From: Kevin Date: Fri, 4 Nov 2022 09:21:26 -0700 Subject: [PATCH 4/4] chore: address feedback --- lib/reporters/default.js | 4 ++-- lib/run-status.js | 24 +++++++++++------------- 2 files changed, 13 insertions(+), 15 deletions(-) diff --git a/lib/reporters/default.js b/lib/reporters/default.js index dd4543df6..804e285cc 100644 --- a/lib/reporters/default.js +++ b/lib/reporters/default.js @@ -370,9 +370,9 @@ export default class Reporter { } this.lineWriter.writeLine(`${testsInFile.size} tests were pending in ${this.relativeFile(file)}\n`); - const testTitleToLogs = evt.pendingTestsLogReference.get(file) ?? new Map(); + const testTitleToLogs = evt.pendingTestsLogs.get(file); for (const title of testsInFile) { - const logs = testTitleToLogs.get(title) ?? []; + const logs = testTitleToLogs?.get(title); this.lineWriter.writeLine(`${figures.circleDotted} ${this.prefixTitle(file, title)}`); this.writeLogs({logs}); } diff --git a/lib/run-status.js b/lib/run-status.js index efd4d5961..eb01caa78 100644 --- a/lib/run-status.js +++ b/lib/run-status.js @@ -9,7 +9,7 @@ export default class RunStatus extends Emittery { super(); this.pendingTests = new Map(); - this.pendingTestsLogReference = new Map(); + this.pendingTestsLogs = new Map(); this.emptyParallelRun = parallelRuns && parallelRuns.currentFileCount === 0 @@ -61,7 +61,7 @@ export default class RunStatus extends Emittery { }); this.pendingTests.set(testFile, new Set()); - this.pendingTestsLogReference.set(testFile, new Map()); + this.pendingTestsLogs.set(testFile, new Map()); worker.onStateChange(data => this.emitStateChange(data)); } @@ -127,14 +127,14 @@ export default class RunStatus extends Emittery { this.removePendingTest(event); break; case 'test-register-log-reference': - this.addLogReference(event); + this.addPendingTestLogs(event); break; case 'timeout': stats.timeouts++; event.pendingTests = this.pendingTests; - event.pendingTestsLogReference = this.pendingTestsLogReference; + event.pendingTestsLogs = this.pendingTestsLogs; this.pendingTests = new Map(); - this.pendingTestsLogReference = new Map(); + this.pendingTestsLogs = new Map(); for (const testsInFile of event.pendingTests.values()) { stats.timedOutTests += testsInFile.size; } @@ -142,15 +142,15 @@ export default class RunStatus extends Emittery { break; case 'interrupt': event.pendingTests = this.pendingTests; - event.pendingTestsLogReference = this.pendingTestsLogReference; + event.pendingTestsLogs = this.pendingTestsLogs; this.pendingTests = new Map(); - this.pendingTestsLogReference = new Map(); + this.pendingTestsLogs = new Map(); break; case 'process-exit': event.pendingTests = this.pendingTests; - event.pendingTestsLogReference = this.pendingTestsLogReference; + event.pendingTestsLogs = this.pendingTestsLogs; this.pendingTests = new Map(); - this.pendingTestsLogReference = new Map(); + this.pendingTestsLogs = new Map(); break; case 'uncaught-exception': stats.uncaughtExceptions++; @@ -209,10 +209,8 @@ export default class RunStatus extends Emittery { return 0; } - addLogReference(event) { - if (this.pendingTestsLogReference.has(event.testFile)) { - this.pendingTestsLogReference.get(event.testFile).set(event.title, event.logs); - } + addPendingTestLogs(event) { + this.pendingTestsLogs.get(event.testFile)?.set(event.title, event.logs); } addPendingTest(event) {