From de70efa97fc59722a0e61f285828643a6efca3cf Mon Sep 17 00:00:00 2001 From: Pulkit Gupta <76155456+pulkit-30@users.noreply.github.com> Date: Tue, 31 Jan 2023 15:59:25 +0530 Subject: [PATCH 1/6] test_runner: top-level diagnostics not ommited when running with --test --- lib/internal/test_runner/runner.js | 6 +++++- test/message/test_runner_output.js | 6 ++++++ test/message/test_runner_output.out | 14 ++++++++++---- test/message/test_runner_output_cli.out | 16 ++++++++++++++-- test/message/test_runner_output_dot_reporter.out | 2 +- .../message/test_runner_output_spec_reporter.out | 10 ++++++---- 6 files changed, 42 insertions(+), 12 deletions(-) diff --git a/lib/internal/test_runner/runner.js b/lib/internal/test_runner/runner.js index 742d43a51d4e6d..e66bb0f6382fe2 100644 --- a/lib/internal/test_runner/runner.js +++ b/lib/internal/test_runner/runner.js @@ -130,6 +130,10 @@ function getRunArgs({ path, inspectPort }) { class FileTest extends Test { #buffer = []; + #checkNestedComment(node) { + return ArrayPrototypeIncludes(['tests', 'pass', 'fail', 'cancelled', 'skipped', 'todo', 'duration_ms'], + node.comment.split(' ')[0]); + } #handleReportItem({ kind, node, nesting = 0 }) { nesting += 1; @@ -183,7 +187,7 @@ class FileTest extends Test { break; case TokenKind.COMMENT: - if (nesting === 1) { + if (nesting === 1 && this.#checkNestedComment(node)) { // Ignore file top level diagnostics break; } diff --git a/test/message/test_runner_output.js b/test/message/test_runner_output.js index 91caca3038e7b8..5648811c66dfc7 100644 --- a/test/message/test_runner_output.js +++ b/test/message/test_runner_output.js @@ -383,3 +383,9 @@ test('unfinished test with unhandledRejection', async () => { setTimeout(() => Promise.reject(new Error('bar'))); }); }); + +test('should not omit top-level diagnostics', () => { + setImmediate(() => { + done(); + }); +}); diff --git a/test/message/test_runner_output.out b/test/message/test_runner_output.out index 15d2009816a961..807bd01ba3ce6b 100644 --- a/test/message/test_runner_output.out +++ b/test/message/test_runner_output.out @@ -620,8 +620,13 @@ not ok 64 - unfinished test with unhandledRejection * * ... +# Subtest: should not omit top-level diagnostics +ok 65 - should not omit top-level diagnostics + --- + duration_ms: * + ... # Subtest: invalid subtest fail -not ok 65 - invalid subtest fail +not ok 66 - invalid subtest fail --- duration_ms: * failureType: 'parentAlreadyFinished' @@ -630,15 +635,16 @@ not ok 65 - invalid subtest fail stack: |- * ... -1..65 +1..66 # Warning: Test "unhandled rejection - passes but warns" generated asynchronous activity after the test ended. This activity created the error "Error: rejected from unhandled rejection fail" and would have caused the test to fail, but instead triggered an unhandledRejection event. # Warning: Test "async unhandled rejection - passes but warns" generated asynchronous activity after the test ended. This activity created the error "Error: rejected from async unhandled rejection fail" and would have caused the test to fail, but instead triggered an unhandledRejection event. # Warning: Test "immediate throw - passes but warns" generated asynchronous activity after the test ended. This activity created the error "Error: thrown from immediate throw fail" and would have caused the test to fail, but instead triggered an uncaughtException event. # Warning: Test "immediate reject - passes but warns" generated asynchronous activity after the test ended. This activity created the error "Error: rejected from immediate reject fail" and would have caused the test to fail, but instead triggered an unhandledRejection event. # Warning: Test "callback called twice in different ticks" generated asynchronous activity after the test ended. This activity created the error "Error [ERR_TEST_FAILURE]: callback invoked multiple times" and would have caused the test to fail, but instead triggered an uncaughtException event. # Warning: Test "callback async throw after done" generated asynchronous activity after the test ended. This activity created the error "Error: thrown from callback async throw after done" and would have caused the test to fail, but instead triggered an uncaughtException event. -# tests 65 -# pass 27 +# Warning: Test "should not omit top-level diagnostics" generated asynchronous activity after the test ended. This activity created the error "ReferenceError: done is not defined" and would have caused the test to fail, but instead triggered an uncaughtException event. +# tests 66 +# pass 28 # fail 21 # cancelled 2 # skipped 10 diff --git a/test/message/test_runner_output_cli.out b/test/message/test_runner_output_cli.out index a8e2f2c5b586f4..5e8899e83990d8 100644 --- a/test/message/test_runner_output_cli.out +++ b/test/message/test_runner_output_cli.out @@ -618,8 +618,13 @@ TAP version 13 * * ... + # Subtest: should not omit top-level diagnostics + ok 65 - should not omit top-level diagnostics + --- + duration_ms: * + ... # Subtest: invalid subtest fail - not ok 65 - invalid subtest fail + not ok 66 - invalid subtest fail --- duration_ms: * failureType: 'parentAlreadyFinished' @@ -628,7 +633,14 @@ TAP version 13 stack: |- * ... - 1..65 + 1..66 + # Warning: Test "unhandled rejection - passes but warns" generated asynchronous activity after the test ended. This activity created the error "Error: rejected from unhandled rejection fail" and would have caused the test to fail, but instead triggered an unhandledRejection event. + # Warning: Test "async unhandled rejection - passes but warns" generated asynchronous activity after the test ended. This activity created the error "Error: rejected from async unhandled rejection fail" and would have caused the test to fail, but instead triggered an unhandledRejection event. + # Warning: Test "immediate throw - passes but warns" generated asynchronous activity after the test ended. This activity created the error "Error: thrown from immediate throw fail" and would have caused the test to fail, but instead triggered an uncaughtException event. + # Warning: Test "immediate reject - passes but warns" generated asynchronous activity after the test ended. This activity created the error "Error: rejected from immediate reject fail" and would have caused the test to fail, but instead triggered an unhandledRejection event. + # Warning: Test "callback called twice in different ticks" generated asynchronous activity after the test ended. This activity created the error "Error [ERR_TEST_FAILURE]: callback invoked multiple times" and would have caused the test to fail, but instead triggered an uncaughtException event. + # Warning: Test "callback async throw after done" generated asynchronous activity after the test ended. This activity created the error "Error: thrown from callback async throw after done" and would have caused the test to fail, but instead triggered an uncaughtException event. + # Warning: Test "should not omit top-level diagnostics" generated asynchronous activity after the test ended. This activity created the error "ReferenceError: done is not defined" and would have caused the test to fail, but instead triggered an uncaughtException event. not ok 1 - *test_runner_output.js --- duration_ms: * diff --git a/test/message/test_runner_output_dot_reporter.out b/test/message/test_runner_output_dot_reporter.out index 823ecfb146b991..b17043aa08334c 100644 --- a/test/message/test_runner_output_dot_reporter.out +++ b/test/message/test_runner_output_dot_reporter.out @@ -1,4 +1,4 @@ ..XX...X..XXX.X..... XXX.....X..X...X.... .........X...XXX.XX. -.....XXXXXXX...XXXX +.....XXXXXXX...XXX.X diff --git a/test/message/test_runner_output_spec_reporter.out b/test/message/test_runner_output_spec_reporter.out index f7e2b7e66d800a..e8a8a770df00c5 100644 --- a/test/message/test_runner_output_spec_reporter.out +++ b/test/message/test_runner_output_spec_reporter.out @@ -58,9 +58,9 @@ async assertion fail (*ms) AssertionError [ERR_ASSERTION]: Expected values to be strictly equal: - + true !== false - + * * * @@ -262,6 +262,7 @@ * * + should not omit top-level diagnostics (*ms) invalid subtest fail (*ms) 'test could not be started because its parent finished' @@ -271,8 +272,9 @@ Warning: Test "immediate reject - passes but warns" generated asynchronous activity after the test ended. This activity created the error "Error: rejected from immediate reject fail" and would have caused the test to fail, but instead triggered an unhandledRejection event. Warning: Test "callback called twice in different ticks" generated asynchronous activity after the test ended. This activity created the error "Error [ERR_TEST_FAILURE]: callback invoked multiple times" and would have caused the test to fail, but instead triggered an uncaughtException event. Warning: Test "callback async throw after done" generated asynchronous activity after the test ended. This activity created the error "Error: thrown from callback async throw after done" and would have caused the test to fail, but instead triggered an uncaughtException event. - tests 65 - pass 27 + Warning: Test "should not omit top-level diagnostics" generated asynchronous activity after the test ended. This activity created the error "ReferenceError: done is not defined" and would have caused the test to fail, but instead triggered an uncaughtException event. + tests 66 + pass 28 fail 21 cancelled 2 skipped 10 From bb83008d3ce1f03518b1673f55f4848bc432eddb Mon Sep 17 00:00:00 2001 From: Pulkit Gupta <76155456+pulkit-30@users.noreply.github.com> Date: Tue, 31 Jan 2023 16:38:07 +0530 Subject: [PATCH 2/6] remove redundant test --- lib/internal/test_runner/runner.js | 4 ++-- test/message/test_runner_output.js | 6 ------ test/message/test_runner_output.out | 14 ++++---------- test/message/test_runner_output_cli.out | 10 ++-------- test/message/test_runner_output_dot_reporter.out | 2 +- test/message/test_runner_output_spec_reporter.out | 6 ++---- 6 files changed, 11 insertions(+), 31 deletions(-) diff --git a/lib/internal/test_runner/runner.js b/lib/internal/test_runner/runner.js index e66bb0f6382fe2..de6b8bac66a8d5 100644 --- a/lib/internal/test_runner/runner.js +++ b/lib/internal/test_runner/runner.js @@ -51,6 +51,7 @@ const { const kFilterArgs = ['--test', '--experimental-test-coverage', '--watch']; const kFilterArgValues = ['--test-reporter', '--test-reporter-destination']; +const diagnosticsFilterArgs = ['tests', 'pass', 'fail', 'cancelled', 'skipped', 'todo', 'duration_ms']; // TODO(cjihrig): Replace this with recursive readdir once it lands. function processPath(path, testFiles, options) { @@ -131,8 +132,7 @@ function getRunArgs({ path, inspectPort }) { class FileTest extends Test { #buffer = []; #checkNestedComment(node) { - return ArrayPrototypeIncludes(['tests', 'pass', 'fail', 'cancelled', 'skipped', 'todo', 'duration_ms'], - node.comment.split(' ')[0]); + return ArrayPrototypeIncludes(diagnosticsFilterArgs, node.comment.split(' ')[0]); } #handleReportItem({ kind, node, nesting = 0 }) { nesting += 1; diff --git a/test/message/test_runner_output.js b/test/message/test_runner_output.js index 5648811c66dfc7..91caca3038e7b8 100644 --- a/test/message/test_runner_output.js +++ b/test/message/test_runner_output.js @@ -383,9 +383,3 @@ test('unfinished test with unhandledRejection', async () => { setTimeout(() => Promise.reject(new Error('bar'))); }); }); - -test('should not omit top-level diagnostics', () => { - setImmediate(() => { - done(); - }); -}); diff --git a/test/message/test_runner_output.out b/test/message/test_runner_output.out index 807bd01ba3ce6b..15d2009816a961 100644 --- a/test/message/test_runner_output.out +++ b/test/message/test_runner_output.out @@ -620,13 +620,8 @@ not ok 64 - unfinished test with unhandledRejection * * ... -# Subtest: should not omit top-level diagnostics -ok 65 - should not omit top-level diagnostics - --- - duration_ms: * - ... # Subtest: invalid subtest fail -not ok 66 - invalid subtest fail +not ok 65 - invalid subtest fail --- duration_ms: * failureType: 'parentAlreadyFinished' @@ -635,16 +630,15 @@ not ok 66 - invalid subtest fail stack: |- * ... -1..66 +1..65 # Warning: Test "unhandled rejection - passes but warns" generated asynchronous activity after the test ended. This activity created the error "Error: rejected from unhandled rejection fail" and would have caused the test to fail, but instead triggered an unhandledRejection event. # Warning: Test "async unhandled rejection - passes but warns" generated asynchronous activity after the test ended. This activity created the error "Error: rejected from async unhandled rejection fail" and would have caused the test to fail, but instead triggered an unhandledRejection event. # Warning: Test "immediate throw - passes but warns" generated asynchronous activity after the test ended. This activity created the error "Error: thrown from immediate throw fail" and would have caused the test to fail, but instead triggered an uncaughtException event. # Warning: Test "immediate reject - passes but warns" generated asynchronous activity after the test ended. This activity created the error "Error: rejected from immediate reject fail" and would have caused the test to fail, but instead triggered an unhandledRejection event. # Warning: Test "callback called twice in different ticks" generated asynchronous activity after the test ended. This activity created the error "Error [ERR_TEST_FAILURE]: callback invoked multiple times" and would have caused the test to fail, but instead triggered an uncaughtException event. # Warning: Test "callback async throw after done" generated asynchronous activity after the test ended. This activity created the error "Error: thrown from callback async throw after done" and would have caused the test to fail, but instead triggered an uncaughtException event. -# Warning: Test "should not omit top-level diagnostics" generated asynchronous activity after the test ended. This activity created the error "ReferenceError: done is not defined" and would have caused the test to fail, but instead triggered an uncaughtException event. -# tests 66 -# pass 28 +# tests 65 +# pass 27 # fail 21 # cancelled 2 # skipped 10 diff --git a/test/message/test_runner_output_cli.out b/test/message/test_runner_output_cli.out index 5e8899e83990d8..525f8db3c885e3 100644 --- a/test/message/test_runner_output_cli.out +++ b/test/message/test_runner_output_cli.out @@ -618,13 +618,8 @@ TAP version 13 * * ... - # Subtest: should not omit top-level diagnostics - ok 65 - should not omit top-level diagnostics - --- - duration_ms: * - ... # Subtest: invalid subtest fail - not ok 66 - invalid subtest fail + not ok 65 - invalid subtest fail --- duration_ms: * failureType: 'parentAlreadyFinished' @@ -633,14 +628,13 @@ TAP version 13 stack: |- * ... - 1..66 + 1..65 # Warning: Test "unhandled rejection - passes but warns" generated asynchronous activity after the test ended. This activity created the error "Error: rejected from unhandled rejection fail" and would have caused the test to fail, but instead triggered an unhandledRejection event. # Warning: Test "async unhandled rejection - passes but warns" generated asynchronous activity after the test ended. This activity created the error "Error: rejected from async unhandled rejection fail" and would have caused the test to fail, but instead triggered an unhandledRejection event. # Warning: Test "immediate throw - passes but warns" generated asynchronous activity after the test ended. This activity created the error "Error: thrown from immediate throw fail" and would have caused the test to fail, but instead triggered an uncaughtException event. # Warning: Test "immediate reject - passes but warns" generated asynchronous activity after the test ended. This activity created the error "Error: rejected from immediate reject fail" and would have caused the test to fail, but instead triggered an unhandledRejection event. # Warning: Test "callback called twice in different ticks" generated asynchronous activity after the test ended. This activity created the error "Error [ERR_TEST_FAILURE]: callback invoked multiple times" and would have caused the test to fail, but instead triggered an uncaughtException event. # Warning: Test "callback async throw after done" generated asynchronous activity after the test ended. This activity created the error "Error: thrown from callback async throw after done" and would have caused the test to fail, but instead triggered an uncaughtException event. - # Warning: Test "should not omit top-level diagnostics" generated asynchronous activity after the test ended. This activity created the error "ReferenceError: done is not defined" and would have caused the test to fail, but instead triggered an uncaughtException event. not ok 1 - *test_runner_output.js --- duration_ms: * diff --git a/test/message/test_runner_output_dot_reporter.out b/test/message/test_runner_output_dot_reporter.out index b17043aa08334c..823ecfb146b991 100644 --- a/test/message/test_runner_output_dot_reporter.out +++ b/test/message/test_runner_output_dot_reporter.out @@ -1,4 +1,4 @@ ..XX...X..XXX.X..... XXX.....X..X...X.... .........X...XXX.XX. -.....XXXXXXX...XXX.X +.....XXXXXXX...XXXX diff --git a/test/message/test_runner_output_spec_reporter.out b/test/message/test_runner_output_spec_reporter.out index e8a8a770df00c5..cf47c7ac7e33ff 100644 --- a/test/message/test_runner_output_spec_reporter.out +++ b/test/message/test_runner_output_spec_reporter.out @@ -262,7 +262,6 @@ * * - should not omit top-level diagnostics (*ms) invalid subtest fail (*ms) 'test could not be started because its parent finished' @@ -272,9 +271,8 @@ Warning: Test "immediate reject - passes but warns" generated asynchronous activity after the test ended. This activity created the error "Error: rejected from immediate reject fail" and would have caused the test to fail, but instead triggered an unhandledRejection event. Warning: Test "callback called twice in different ticks" generated asynchronous activity after the test ended. This activity created the error "Error [ERR_TEST_FAILURE]: callback invoked multiple times" and would have caused the test to fail, but instead triggered an uncaughtException event. Warning: Test "callback async throw after done" generated asynchronous activity after the test ended. This activity created the error "Error: thrown from callback async throw after done" and would have caused the test to fail, but instead triggered an uncaughtException event. - Warning: Test "should not omit top-level diagnostics" generated asynchronous activity after the test ended. This activity created the error "ReferenceError: done is not defined" and would have caused the test to fail, but instead triggered an uncaughtException event. - tests 66 - pass 28 + tests 65 + pass 27 fail 21 cancelled 2 skipped 10 From d3ab0fcd9a8b32d4e0ec267797ae08987b50d36d Mon Sep 17 00:00:00 2001 From: Pulkit Gupta <76155456+pulkit-30@users.noreply.github.com> Date: Tue, 31 Jan 2023 17:02:36 +0530 Subject: [PATCH 3/6] used StringPrototypeSplit primordials and added more conditions --- lib/internal/test_runner/runner.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/internal/test_runner/runner.js b/lib/internal/test_runner/runner.js index de6b8bac66a8d5..8721caea960bd4 100644 --- a/lib/internal/test_runner/runner.js +++ b/lib/internal/test_runner/runner.js @@ -16,6 +16,7 @@ const { SafeMap, SafeSet, StringPrototypeStartsWith, + StringPrototypeSplit, } = primordials; const { spawn } = require('child_process'); @@ -132,7 +133,8 @@ function getRunArgs({ path, inspectPort }) { class FileTest extends Test { #buffer = []; #checkNestedComment(node) { - return ArrayPrototypeIncludes(diagnosticsFilterArgs, node.comment.split(' ')[0]); + const commentArgs = StringPrototypeSplit(node.comment, ' '); + return commentArgs.length === 2 && ArrayPrototypeIncludes(diagnosticsFilterArgs, commentArgs[0]); } #handleReportItem({ kind, node, nesting = 0 }) { nesting += 1; From 754e8d389158b94e867bbc5f190f8600ce2d1d35 Mon Sep 17 00:00:00 2001 From: Pulkit Gupta <76155456+pulkit-30@users.noreply.github.com> Date: Tue, 31 Jan 2023 20:32:57 +0530 Subject: [PATCH 4/6] move StringPrototypeSplit up a line and rename variable --- lib/internal/test_runner/runner.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/internal/test_runner/runner.js b/lib/internal/test_runner/runner.js index 8721caea960bd4..7390b83d2ce9cc 100644 --- a/lib/internal/test_runner/runner.js +++ b/lib/internal/test_runner/runner.js @@ -15,8 +15,8 @@ const { SafePromiseAllSettledReturnVoid, SafeMap, SafeSet, - StringPrototypeStartsWith, StringPrototypeSplit, + StringPrototypeStartsWith, } = primordials; const { spawn } = require('child_process'); @@ -52,7 +52,7 @@ const { const kFilterArgs = ['--test', '--experimental-test-coverage', '--watch']; const kFilterArgValues = ['--test-reporter', '--test-reporter-destination']; -const diagnosticsFilterArgs = ['tests', 'pass', 'fail', 'cancelled', 'skipped', 'todo', 'duration_ms']; +const kDiagnosticsFilterArgs = ['tests', 'pass', 'fail', 'cancelled', 'skipped', 'todo', 'duration_ms']; // TODO(cjihrig): Replace this with recursive readdir once it lands. function processPath(path, testFiles, options) { @@ -134,7 +134,7 @@ class FileTest extends Test { #buffer = []; #checkNestedComment(node) { const commentArgs = StringPrototypeSplit(node.comment, ' '); - return commentArgs.length === 2 && ArrayPrototypeIncludes(diagnosticsFilterArgs, commentArgs[0]); + return commentArgs.length === 2 && ArrayPrototypeIncludes(kDiagnosticsFilterArgs, commentArgs[0]); } #handleReportItem({ kind, node, nesting = 0 }) { nesting += 1; From 7719a4b7b373b07766044027242239cb796037b9 Mon Sep 17 00:00:00 2001 From: Pulkit Gupta <76155456+pulkit-30@users.noreply.github.com> Date: Tue, 31 Jan 2023 22:11:32 +0530 Subject: [PATCH 5/6] used StringPrototypeIndexOf in place of StringPrototypeSplit primordials --- lib/internal/test_runner/runner.js | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/lib/internal/test_runner/runner.js b/lib/internal/test_runner/runner.js index 7390b83d2ce9cc..319787dfa03b02 100644 --- a/lib/internal/test_runner/runner.js +++ b/lib/internal/test_runner/runner.js @@ -15,7 +15,8 @@ const { SafePromiseAllSettledReturnVoid, SafeMap, SafeSet, - StringPrototypeSplit, + StringPrototypeIndexOf, + StringPrototypeSlice, StringPrototypeStartsWith, } = primordials; @@ -132,9 +133,11 @@ function getRunArgs({ path, inspectPort }) { class FileTest extends Test { #buffer = []; - #checkNestedComment(node) { - const commentArgs = StringPrototypeSplit(node.comment, ' '); - return commentArgs.length === 2 && ArrayPrototypeIncludes(kDiagnosticsFilterArgs, commentArgs[0]); + #checkNestedComment({ comment }) { + const firstSpaceIndex = StringPrototypeIndexOf(comment, ' '); + const secondSpaceIndex = StringPrototypeIndexOf(comment, ' ', firstSpaceIndex + 1); + return secondSpaceIndex === -1 && + ArrayPrototypeIncludes(kDiagnosticsFilterArgs, StringPrototypeSlice(comment, 0, firstSpaceIndex)); } #handleReportItem({ kind, node, nesting = 0 }) { nesting += 1; From 6671519e8fdf8db0686ee8f91065a2bcc793f8f3 Mon Sep 17 00:00:00 2001 From: Pulkit Gupta <76155456+pulkit-30@users.noreply.github.com> Date: Wed, 1 Feb 2023 14:46:52 +0530 Subject: [PATCH 6/6] added condition to firstSpaceIndex --- lib/internal/test_runner/runner.js | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/internal/test_runner/runner.js b/lib/internal/test_runner/runner.js index 319787dfa03b02..7ef91cf036a94f 100644 --- a/lib/internal/test_runner/runner.js +++ b/lib/internal/test_runner/runner.js @@ -135,6 +135,7 @@ class FileTest extends Test { #buffer = []; #checkNestedComment({ comment }) { const firstSpaceIndex = StringPrototypeIndexOf(comment, ' '); + if (firstSpaceIndex === -1) return false; const secondSpaceIndex = StringPrototypeIndexOf(comment, ' ', firstSpaceIndex + 1); return secondSpaceIndex === -1 && ArrayPrototypeIncludes(kDiagnosticsFilterArgs, StringPrototypeSlice(comment, 0, firstSpaceIndex));