From ef5639f64e846227f932b8d9b8bc8c53669c97a6 Mon Sep 17 00:00:00 2001 From: Pulkit Gupta <76155456+pulkit-30@users.noreply.github.com> Date: Fri, 6 Jan 2023 18:05:35 +0530 Subject: [PATCH 1/8] test: fix tap parser fails if a test logs a number --- lib/internal/test_runner/tap_parser.js | 10 ++++++++- .../parallel/test-runner-tap-parser-stream.js | 22 +++++++++++++++++++ 2 files changed, 31 insertions(+), 1 deletion(-) diff --git a/lib/internal/test_runner/tap_parser.js b/lib/internal/test_runner/tap_parser.js index c502c05eb8fc14..deef16ef1c86b1 100644 --- a/lib/internal/test_runner/tap_parser.js +++ b/lib/internal/test_runner/tap_parser.js @@ -642,8 +642,16 @@ class TapParser extends Transform { } const planEnd = this.#next(); - if (planEnd?.kind !== TokenKind.NUMERIC) { + if (!planEnd || planEnd.kind !== TokenKind.NUMERIC) { this.#error('Expected a plan end count'); + const node = { + kind: TokenKind.UNKNOWN, + node: { + value: this.#currentChunkAsString, + }, + }; + + return node; } const plan = { diff --git a/test/parallel/test-runner-tap-parser-stream.js b/test/parallel/test-runner-tap-parser-stream.js index bd10af29d88279..f07f88c46cb5c5 100644 --- a/test/parallel/test-runner-tap-parser-stream.js +++ b/test/parallel/test-runner-tap-parser-stream.js @@ -17,6 +17,28 @@ const cases = [ }, ], }, + { + input: '123', + expected: [ + { + kind: 'Unknown', + node: { value: '123' }, + nesting: 0, + lexeme: '123', + }, + ], + }, + { + input: '# 123', + expected: [ + { + kind: 'Comment', + node: { comment: '123' }, + nesting: 0, + lexeme: '# 123', + }, + ], + }, { input: 'invalid tap', expected: [ From c7bbb9a2470ec61c29a55c9fd22adb7d32c5504b Mon Sep 17 00:00:00 2001 From: Pulkit Gupta <76155456+pulkit-30@users.noreply.github.com> Date: Tue, 10 Jan 2023 18:52:38 +0530 Subject: [PATCH 2/8] enha: moving fix to upper-level logic of the parser --- lib/internal/test_runner/tap_parser.js | 48 ++++++++++---------------- 1 file changed, 18 insertions(+), 30 deletions(-) diff --git a/lib/internal/test_runner/tap_parser.js b/lib/internal/test_runner/tap_parser.js index deef16ef1c86b1..0d0276d8d0502b 100644 --- a/lib/internal/test_runner/tap_parser.js +++ b/lib/internal/test_runner/tap_parser.js @@ -542,12 +542,27 @@ class TapParser extends Transform { if (firstToken) { const { kind } = firstToken; - + let planStart, planToken, planEnd; switch (kind) { case TokenKind.TAP: return this.#Version(); case TokenKind.NUMERIC: - return this.#Plan(); + planStart = this.#next(); + if (planStart.kind !== TokenKind.NUMERIC) { + this.#error('Expected a plan start count'); + break; + } + planToken = this.#next(); + if (planToken?.kind !== TokenKind.TAP_PLAN) { + this.#error('Expected ".." symbol'); + break; + } + planEnd = this.#next(); + if (!planEnd || planEnd?.kind !== TokenKind.NUMERIC) { + this.#error('Expected a plan end count'); + break; + } + return this.#Plan(planStart, planEnd); case TokenKind.TAP_TEST_OK: case TokenKind.TAP_TEST_NOTOK: return this.#TestPoint(); @@ -627,38 +642,11 @@ class TapParser extends Transform { // ----------------Plan---------------- // Plan := "1.." (Number) (" # " Reason)? "\n" - #Plan() { - // Even if specs mention plan starts at 1, we need to make sure we read the plan start value - // in case of a missing or invalid plan start value - const planStart = this.#next(); - - if (planStart.kind !== TokenKind.NUMERIC) { - this.#error('Expected a plan start count'); - } - - const planToken = this.#next(); - if (planToken?.kind !== TokenKind.TAP_PLAN) { - this.#error('Expected ".." symbol'); - } - - const planEnd = this.#next(); - if (!planEnd || planEnd.kind !== TokenKind.NUMERIC) { - this.#error('Expected a plan end count'); - const node = { - kind: TokenKind.UNKNOWN, - node: { - value: this.#currentChunkAsString, - }, - }; - - return node; - } - + #Plan(planStart, planEnd) { const plan = { start: planStart.value, end: planEnd.value, }; - // Read optional reason const hashToken = this.#peek(); if (hashToken) { From 5079240bc9dff6285f9e06108c6b7f4c8d1b8766 Mon Sep 17 00:00:00 2001 From: Pulkit Gupta <76155456+pulkit-30@users.noreply.github.com> Date: Tue, 10 Jan 2023 20:12:50 +0530 Subject: [PATCH 3/8] fix: committing suggestions --- lib/internal/test_runner/tap_parser.js | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/lib/internal/test_runner/tap_parser.js b/lib/internal/test_runner/tap_parser.js index 0d0276d8d0502b..3678b2c4a93eee 100644 --- a/lib/internal/test_runner/tap_parser.js +++ b/lib/internal/test_runner/tap_parser.js @@ -542,27 +542,27 @@ class TapParser extends Transform { if (firstToken) { const { kind } = firstToken; - let planStart, planToken, planEnd; switch (kind) { case TokenKind.TAP: return this.#Version(); - case TokenKind.NUMERIC: - planStart = this.#next(); + case TokenKind.NUMERIC: { + const planStart = this.#next(); if (planStart.kind !== TokenKind.NUMERIC) { this.#error('Expected a plan start count'); break; } - planToken = this.#next(); + const planToken = this.#next(); if (planToken?.kind !== TokenKind.TAP_PLAN) { this.#error('Expected ".." symbol'); break; } - planEnd = this.#next(); + const planEnd = this.#next(); if (!planEnd || planEnd?.kind !== TokenKind.NUMERIC) { this.#error('Expected a plan end count'); break; } return this.#Plan(planStart, planEnd); + } case TokenKind.TAP_TEST_OK: case TokenKind.TAP_TEST_NOTOK: return this.#TestPoint(); From 1e646332c7b150a0e03db93b76334f7c0bd12800 Mon Sep 17 00:00:00 2001 From: Pulkit Gupta <76155456+pulkit-30@users.noreply.github.com> Date: Tue, 10 Jan 2023 21:05:20 +0530 Subject: [PATCH 4/8] fix: minor issues and unrelated changes --- lib/internal/test_runner/tap_parser.js | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/lib/internal/test_runner/tap_parser.js b/lib/internal/test_runner/tap_parser.js index 3678b2c4a93eee..35bb0cc5391506 100644 --- a/lib/internal/test_runner/tap_parser.js +++ b/lib/internal/test_runner/tap_parser.js @@ -542,6 +542,7 @@ class TapParser extends Transform { if (firstToken) { const { kind } = firstToken; + switch (kind) { case TokenKind.TAP: return this.#Version(); @@ -561,7 +562,7 @@ class TapParser extends Transform { this.#error('Expected a plan end count'); break; } - return this.#Plan(planStart, planEnd); + return this.#Plan({ start: planStart.value, end: planEnd.value }); } case TokenKind.TAP_TEST_OK: case TokenKind.TAP_TEST_NOTOK: @@ -642,11 +643,7 @@ class TapParser extends Transform { // ----------------Plan---------------- // Plan := "1.." (Number) (" # " Reason)? "\n" - #Plan(planStart, planEnd) { - const plan = { - start: planStart.value, - end: planEnd.value, - }; + #Plan(plan) { // Read optional reason const hashToken = this.#peek(); if (hashToken) { From 69ab9e31bb0548d41af604b9db6da75fdaa6e664 Mon Sep 17 00:00:00 2001 From: Pulkit Gupta <76155456+pulkit-30@users.noreply.github.com> Date: Tue, 31 Jan 2023 00:47:51 +0530 Subject: [PATCH 5/8] moved changes to #plan --- lib/internal/test_runner/tap_parser.js | 44 ++++++++++++++++---------- 1 file changed, 28 insertions(+), 16 deletions(-) diff --git a/lib/internal/test_runner/tap_parser.js b/lib/internal/test_runner/tap_parser.js index 35bb0cc5391506..58e1864719c4f8 100644 --- a/lib/internal/test_runner/tap_parser.js +++ b/lib/internal/test_runner/tap_parser.js @@ -547,22 +547,11 @@ class TapParser extends Transform { case TokenKind.TAP: return this.#Version(); case TokenKind.NUMERIC: { - const planStart = this.#next(); - if (planStart.kind !== TokenKind.NUMERIC) { - this.#error('Expected a plan start count'); - break; + const node = this.#Plan(); + if (node) { + return node; } - const planToken = this.#next(); - if (planToken?.kind !== TokenKind.TAP_PLAN) { - this.#error('Expected ".." symbol'); - break; - } - const planEnd = this.#next(); - if (!planEnd || planEnd?.kind !== TokenKind.NUMERIC) { - this.#error('Expected a plan end count'); - break; - } - return this.#Plan({ start: planStart.value, end: planEnd.value }); + break; } case TokenKind.TAP_TEST_OK: case TokenKind.TAP_TEST_NOTOK: @@ -643,7 +632,30 @@ class TapParser extends Transform { // ----------------Plan---------------- // Plan := "1.." (Number) (" # " Reason)? "\n" - #Plan(plan) { + #Plan() { + // Even if specs mention plan starts at 1, we need to make sure we read the plan start value + // in case of a missing or invalid plan start value + const planStart = this.#next(); + + if (planStart.kind !== TokenKind.NUMERIC) { + return this.#error('Expected a plan start count'); + } + + const planToken = this.#next(); + if (planToken?.kind !== TokenKind.TAP_PLAN) { + return this.#error('Expected ".." symbol'); + } + + const planEnd = this.#next(); + if (!planEnd || planEnd.kind !== TokenKind.NUMERIC) { + return this.#error('Expected a plan end count'); + } + + const plan = { + start: planStart.value, + end: planEnd.value, + }; + // Read optional reason const hashToken = this.#peek(); if (hashToken) { From e50df57044ba995c64e8a4816c05f2359c868f81 Mon Sep 17 00:00:00 2001 From: Pulkit Gupta <76155456+pulkit-30@users.noreply.github.com> Date: Thu, 2 Feb 2023 01:13:16 +0530 Subject: [PATCH 6/8] catch all possible bugs/errors by parser --- lib/internal/test_runner/tap_parser.js | 34 ++-- .../parallel/test-runner-tap-parser-stream.js | 165 ++++++++++++++++++ test/parallel/test-runner-tap-parser.js | 133 -------------- 3 files changed, 182 insertions(+), 150 deletions(-) diff --git a/lib/internal/test_runner/tap_parser.js b/lib/internal/test_runner/tap_parser.js index 58e1864719c4f8..f59a48e8709daa 100644 --- a/lib/internal/test_runner/tap_parser.js +++ b/lib/internal/test_runner/tap_parser.js @@ -270,8 +270,19 @@ class TapParser extends Transform { this.#yamlCurrentIndentationLevel = this.#subTestNestingLevel; } + let node = {}; + // Parse current chunk - const node = this.#TAPDocument(chunk); + try { + node = this.#TAPDocument(chunk); + } catch { + node = { + kind: TokenKind.UNKNOWN, + node: { + value: this.#currentChunkAsString, + }, + }; + } // Emit the parsed node to both the stream and the AST this.#emitOrBufferCurrentNode(node); @@ -282,12 +293,6 @@ class TapParser extends Transform { } #error(message) { - if (!this.#isSyncParsingEnabled) { - // When async parsing is enabled, don't throw. - // Unrecognized tokens would be ignored. - return; - } - const token = this.#currentToken || { value: '', kind: '' }; // Escape NewLine characters if (token.value === '\n') { @@ -546,13 +551,8 @@ class TapParser extends Transform { switch (kind) { case TokenKind.TAP: return this.#Version(); - case TokenKind.NUMERIC: { - const node = this.#Plan(); - if (node) { - return node; - } - break; - } + case TokenKind.NUMERIC: + return this.#Plan(); case TokenKind.TAP_TEST_OK: case TokenKind.TAP_TEST_NOTOK: return this.#TestPoint(); @@ -638,17 +638,17 @@ class TapParser extends Transform { const planStart = this.#next(); if (planStart.kind !== TokenKind.NUMERIC) { - return this.#error('Expected a plan start count'); + this.#error('Expected a plan start count'); } const planToken = this.#next(); if (planToken?.kind !== TokenKind.TAP_PLAN) { - return this.#error('Expected ".." symbol'); + this.#error('Expected ".." symbol'); } const planEnd = this.#next(); if (!planEnd || planEnd.kind !== TokenKind.NUMERIC) { - return this.#error('Expected a plan end count'); + this.#error('Expected a plan end count'); } const plan = { diff --git a/test/parallel/test-runner-tap-parser-stream.js b/test/parallel/test-runner-tap-parser-stream.js index f07f88c46cb5c5..80be92c121b73d 100644 --- a/test/parallel/test-runner-tap-parser-stream.js +++ b/test/parallel/test-runner-tap-parser-stream.js @@ -39,6 +39,171 @@ const cases = [ }, ], }, + { + input: '1..', + expected: [ + { + kind: 'Unknown', + node: { value: '1..' }, + nesting: 0, + lexeme: '1..', + }, + ], + }, + { + input: '1..abc', + expected: [ + { + kind: 'Unknown', + node: { value: '1..abc' }, + nesting: 0, + lexeme: '1..abc', + }, + ], + }, + { + input: '1..-1', + expected: [ + { + kind: 'Unknown', + node: { value: '1..-1' }, + nesting: 0, + lexeme: '1..-1', + }, + ], + }, + { + input: '1.1', + expected: [ + { + kind: 'Unknown', + node: { value: '1.1' }, + nesting: 0, + lexeme: '1.1', + }, + ], + }, + { + input: '1.....4', + expected: [ + { + kind: 'Unknown', + node: { value: '1.....4' }, + nesting: 0, + lexeme: '1.....4', + }, + ], + }, + { + input: 'TAP 12', + expected: [ + { + kind: 'Unknown', + node: { value: 'TAP 12' }, + nesting: 0, + lexeme: 'TAP 12', + }, + ], + }, + { + input: 'TAP version', + expected: [ + { + kind: 'Unknown', + node: { value: 'TAP version' }, + nesting: 0, + lexeme: 'TAP version', + }, + ], + }, + { + input: 'TAP version v14', + expected: [ + { + kind: 'Unknown', + node: { value: 'TAP version v14' }, + nesting: 0, + lexeme: 'TAP version v14', + }, + ], + }, + { + input: 'TAP TAP TAP', + expected: [ + { + kind: 'Unknown', + node: { value: 'TAP TAP TAP' }, + nesting: 0, + lexeme: 'TAP TAP TAP', + }, + ], + }, + { + input: '--- yaml', + expected: [ + { + kind: 'Unknown', + node: { value: '--- yaml' }, + nesting: 0, + lexeme: '--- yaml', + }, + ], + }, + { + input: '... ... yaml', + expected: [ + { + kind: 'Unknown', + node: { value: '... ... yaml' }, + nesting: 0, + lexeme: '... ... yaml', + }, + ], + }, + { + input: 'ook 1', + expected: [ + { + kind: 'Unknown', + node: { value: 'ook 1' }, + nesting: 0, + lexeme: 'ook 1', + }, + ], + }, + { + input: ' ok 98', + expected: [ + { + kind: 'Unknown', + node: { value: ' ok 98' }, + nesting: 0, + lexeme: ' ok 98', + }, + ], + }, + { + input: 'pragma ++++++', + expected: [ + { + kind: 'Unknown', + node: { value: 'pragma ++++++' }, + nesting: 0, + lexeme: 'pragma ++++++', + }, + ], + }, + { + input: 'Bailout!', + expected: [ + { + kind: 'Unknown', + node: { value: 'Bailout!' }, + nesting: 0, + lexeme: 'Bailout!', + }, + ], + }, { input: 'invalid tap', expected: [ diff --git a/test/parallel/test-runner-tap-parser.js b/test/parallel/test-runner-tap-parser.js index 530e56626a4c5b..b14f7a9a6b089b 100644 --- a/test/parallel/test-runner-tap-parser.js +++ b/test/parallel/test-runner-tap-parser.js @@ -73,24 +73,6 @@ function TAPParser(input) { ]); } -{ - assert.throws(() => TAPParser('TAP version'), { - name: 'SyntaxError', - code: 'ERR_TAP_PARSER_ERROR', - message: - 'Expected a version number, received "version" (VersionKeyword) at line 1, column 5 (start 4, end 10)', - }); -} - -{ - assert.throws(() => TAPParser('TAP'), { - name: 'SyntaxError', - code: 'ERR_TAP_PARSER_ERROR', - message: - 'Expected "version" keyword, received "TAP" (TAPKeyword) at line 1, column 1 (start 0, end 2)', - }); -} - // Test plan { @@ -123,42 +105,6 @@ function TAPParser(input) { ]); } -{ - assert.throws(() => TAPParser('1..'), { - name: 'SyntaxError', - code: 'ERR_TAP_PARSER_ERROR', - message: - 'Expected a plan end count, received "" (EOL) at line 1, column 4 (start 3, end 3)', - }); -} - -{ - assert.throws(() => TAPParser('1..abc'), { - name: 'SyntaxError', - code: 'ERR_TAP_PARSER_ERROR', - message: - 'Expected ".." symbol, received "..abc" (Literal) at line 1, column 2 (start 1, end 5)', - }); -} - -{ - assert.throws(() => TAPParser('1..-1'), { - name: 'SyntaxError', - code: 'ERR_TAP_PARSER_ERROR', - message: - 'Expected a plan end count, received "-" (Dash) at line 1, column 4 (start 3, end 3)', - }); -} - -{ - assert.throws(() => TAPParser('1.1'), { - name: 'SyntaxError', - code: 'ERR_TAP_PARSER_ERROR', - message: - 'Expected ".." symbol, received "." (Literal) at line 1, column 2 (start 1, end 1)', - }); -} - // Test point { @@ -914,24 +860,6 @@ ok 6 - nested1 ]); } -{ - assert.throws( - () => - TAPParser( - ` - message: 'description' - property: 'value' - ...` - ), - { - name: 'SyntaxError', - code: 'ERR_TAP_PARSER_ERROR', - message: - 'Unexpected YAML end marker, received "..." (YamlEndKeyword) at line 4, column 3 (start 48, end 50)', - } - ); -} - { assert.throws( () => @@ -950,26 +878,6 @@ ok 6 - nested1 ); } -{ - assert.throws( - () => - // Note the leading 3 spaces before --- - TAPParser( - ` - --- - message: 'description' - property: 'value' - ...` - ), - { - name: 'SyntaxError', - code: 'ERR_TAP_PARSER_ERROR', - message: - 'Expected valid YAML indentation (2 spaces), received " " (Whitespace) at line 2, column 3 (start 3, end 3)', - } - ); -} - { assert.throws( () => @@ -995,27 +903,6 @@ ok 6 - nested1 ); } -{ - assert.throws( - () => - // Note the leading 4 spaces before --- - TAPParser( - ` - --- - message: 'description' - property: 'value' - ... - ` - ), - { - name: 'SyntaxError', - code: 'ERR_TAP_PARSER_ERROR', - message: - 'Expected a valid token, received "---" (YamlStartKeyword) at line 2, column 5 (start 5, end 7)', - } - ); -} - { assert.throws( () => @@ -1067,26 +954,6 @@ ok 6 - nested1 ]); } -// Non-recognized - -{ - assert.throws(() => TAPParser('abc'), { - name: 'SyntaxError', - code: 'ERR_TAP_PARSER_ERROR', - message: - 'Expected a valid token, received "abc" (Literal) at line 1, column 1 (start 0, end 2)', - }); -} - -{ - assert.throws(() => TAPParser(' abc'), { - name: 'SyntaxError', - code: 'ERR_TAP_PARSER_ERROR', - message: - 'Expected a valid token, received "abc" (Literal) at line 1, column 5 (start 4, end 6)', - }); -} - // TAP document (with diagnostics) { From 11b5b79031bdafe511f3a759cf2af2bf4eba3a7c Mon Sep 17 00:00:00 2001 From: Pulkit Gupta <76155456+pulkit-30@users.noreply.github.com> Date: Thu, 2 Feb 2023 01:18:02 +0530 Subject: [PATCH 7/8] fix unwanted change --- lib/internal/test_runner/tap_parser.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/internal/test_runner/tap_parser.js b/lib/internal/test_runner/tap_parser.js index f59a48e8709daa..e5a22b0e733eb9 100644 --- a/lib/internal/test_runner/tap_parser.js +++ b/lib/internal/test_runner/tap_parser.js @@ -647,7 +647,7 @@ class TapParser extends Transform { } const planEnd = this.#next(); - if (!planEnd || planEnd.kind !== TokenKind.NUMERIC) { + if (planEnd?.kind !== TokenKind.NUMERIC) { this.#error('Expected a plan end count'); } From 93024dc1f9c1f80316854d106ca2da53e1f58d0c Mon Sep 17 00:00:00 2001 From: Pulkit Gupta <76155456+pulkit-30@users.noreply.github.com> Date: Sun, 5 Feb 2023 19:43:15 +0530 Subject: [PATCH 8/8] fix: minors --- lib/internal/test_runner/tap_parser.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/internal/test_runner/tap_parser.js b/lib/internal/test_runner/tap_parser.js index e5a22b0e733eb9..f4d73bb12b9082 100644 --- a/lib/internal/test_runner/tap_parser.js +++ b/lib/internal/test_runner/tap_parser.js @@ -270,7 +270,7 @@ class TapParser extends Transform { this.#yamlCurrentIndentationLevel = this.#subTestNestingLevel; } - let node = {}; + let node; // Parse current chunk try {