test_runner: add time + comments to test nodes

nodejs · nodejs-github-bot · Nov 21, 2022 · Jun 11, 2022 · Jun 12, 2022 · Jun 13, 2022
commit 754289b7f3887ad73bab9e3f1bd82927c5995ff5
diff --git a/lib/internal/test_runner/tap_checker.js b/lib/internal/test_runner/tap_checker.js
@@ -46,7 +46,7 @@ class TAPValidationStrategy {
       throw new ERR_TAP_VALIDATION_ERROR('missing TAP plan');
     }
 
-    const { plan } = entry.node;
+    const plan = entry.node;
 
     if (!plan.start) {
       throw new ERR_TAP_VALIDATION_ERROR('missing plan start');
@@ -81,12 +81,10 @@ class TAPValidationStrategy {
     );
     const testPointEntries = ArrayPrototypeFilter(
       ast,
-      (node) =>
-        node.kind === TokenKind.TAP_TEST_OK ||
-        node.kind === TokenKind.TAP_TEST_NOTOK
+      (node) => node.kind === TokenKind.TAP_TEST_POINT
     );
 
-    const { plan } = planEntry.node;
+    const plan = planEntry.node;
 
     const planStart = NumberParseInt(plan.start, 10);
     const planEnd = NumberParseInt(plan.end, 10);
@@ -111,7 +109,7 @@ class TAPValidationStrategy {
       }
 
       for (let i = 0; i < testPointEntries.length; i++) {
-        const { test } = testPointEntries[i].node;
+        const test = testPointEntries[i].node;
         const testId = NumberParseInt(test.id, 10);
 
         if (testId < planStart || testId > planEnd) {

diff --git a/lib/internal/test_runner/tap_lexer.js b/lib/internal/test_runner/tap_lexer.js
@@ -23,6 +23,7 @@ const TokenKind = {
   TAP: 'TAPKeyword',
   TAP_VERSION: 'VersionKeyword',
   TAP_PLAN: 'PlanKeyword',
+  TAP_TEST_POINT: 'TestPointKeyword',
   TAP_SUBTEST_POINT: 'SubTestPointKeyword',
   TAP_TEST_OK: 'TestOkKeyword',
   TAP_TEST_NOTOK: 'TestNotOkKeyword',

diff --git a/lib/internal/test_runner/tap_parser.js b/lib/internal/test_runner/tap_parser.js
@@ -9,13 +9,16 @@ const {
 const { kEmptyObject } = require('internal/util');
 const {
   ArrayPrototypeFilter,
+  ArrayPrototypeForEach,
   ArrayPrototypeJoin,
   ArrayPrototypeMap,
   ArrayPrototypePush,
   ArrayPrototypeIncludes,
+  Number,
   RegExpPrototypeExec,
   RegExpPrototypeSymbolReplace,
   StringPrototypeTrim,
+  StringPrototypeSplit,
 } = primordials;
-const Transform = require('internal/streams/transform');
-const { TapLexer, TokenKind } = require('internal/test_runner/tap_lexer');
-const { TapChecker } = require('internal/test_runner/tap_checker');
-const {
-  codes: { ERR_TAP_VALIDATION_ERROR, ERR_TAP_PARSER_ERROR },
-} = require('internal/errors');
-const { kEmptyObject } = require('internal/util');
-const {
-  ArrayPrototypeFilter,
-  ArrayPrototypeForEach,
-  ArrayPrototypeJoin,
-  ArrayPrototypeMap,
-  ArrayPrototypePush,
-  ArrayPrototypeIncludes,
-  ArrayPrototypeSplice,
-  Boolean,
-  Number,
-  RegExpPrototypeExec,
-  RegExpPrototypeSymbolReplace,
-  String,
-  StringPrototypeTrim,
-  StringPrototypeSplit,
-} = primordials;
+const {
+  ArrayPrototypeFilter,
+  ArrayPrototypeForEach,
+  ArrayPrototypeIncludes,
+  ArrayPrototypeJoin,
+  ArrayPrototypeMap,
+  ArrayPrototypePush,
+  ArrayPrototypeSplice,
+  Boolean,
+  Number,
+  RegExpPrototypeExec,
+  RegExpPrototypeSymbolReplace,
+  String,
+  StringPrototypeSplit,
+  StringPrototypeTrim,
+  hardenRegExp,
+} = primordials;
+const Transform = require('internal/streams/transform');
+const { TapLexer, TokenKind } = require('internal/test_runner/tap_lexer');
+const { TapChecker } = require('internal/test_runner/tap_checker');
+const {
+  codes: { ERR_TAP_VALIDATION_ERROR, ERR_TAP_PARSER_ERROR },
+} = require('internal/errors');
+const { kEmptyObject } = require('internal/util');
-const Transform = require('internal/streams/transform');
-const { TapLexer, TokenKind } = require('internal/test_runner/tap_lexer');
-const { TapChecker } = require('internal/test_runner/tap_checker');
-const {
-  codes: { ERR_TAP_VALIDATION_ERROR, ERR_TAP_PARSER_ERROR },
-} = require('internal/errors');
-const { kEmptyObject } = require('internal/util');
-const {
-  ArrayPrototypeFilter,
-  ArrayPrototypeForEach,
-  ArrayPrototypeJoin,
-  ArrayPrototypeMap,
-  ArrayPrototypePush,
-  ArrayPrototypeIncludes,
-  ArrayPrototypeSplice,
-  Boolean,
-  Number,
-  RegExpPrototypeExec,
-  RegExpPrototypeSymbolReplace,
-  String,
-  StringPrototypeTrim,
-  StringPrototypeSplit,
-} = primordials;
+const {
+  ArrayPrototypeFilter,
+  ArrayPrototypeForEach,
+  ArrayPrototypeIncludes,
+  ArrayPrototypeJoin,
+  ArrayPrototypeMap,
+  ArrayPrototypePush,
+  ArrayPrototypeSplice,
+  Boolean,
+  Number,
+  RegExpPrototypeExec,
+  RegExpPrototypeSymbolReplace,
+  String,
+  StringPrototypeSplit,
+  StringPrototypeTrim,
+  hardenRegExp,
+} = primordials;
+const Transform = require('internal/streams/transform');
+const { TapLexer, TokenKind } = require('internal/test_runner/tap_lexer');
+const { TapChecker } = require('internal/test_runner/tap_checker');
+const {
+  codes: { ERR_TAP_VALIDATION_ERROR, ERR_TAP_PARSER_ERROR },
+} = require('internal/errors');
+const { kEmptyObject } = require('internal/util');
 /**
  *
@@ -55,8 +58,10 @@ class TapParser extends Transform {
   #currentTokenIndex = 0;
   #currentTokenChunk = 0;
   #currentToken = null;
-  #flatAst = [];
+  #commentsBuffer = [];
+  #flatAST = [];
   #subTestNestingLevel = 0;
+  #lastTestPointDuration = 0;
   #yamlBlockBuffer = [];
   #isYAMLBlock = false;
   #yamlCurrentIndentationLevel = 0;
@@ -103,14 +108,20 @@ class TapParser extends Transform {
       this.#error('Expected end of YAML block');
     }
 
-    return this.#flatAst;
+    if (this.#commentsBuffer.length > 0) {
+      ArrayPrototypeForEach(this.#commentsBuffer, (comment) => {
+        ArrayPrototypePush(this.#flatAST, comment);
+      });
+    }
+
+    return this.#flatAST;
   }
 
   // Check if the TAP content is semantically valid
   // Note: Validating the TAP content requires the whole AST to be available.
   check() {
     if (this.#isSyncParsingEnabled) {
-      return this.#checker.check(this.#flatAst);
+      return this.#checker.check(this.#flatAST);
     }
 
     // TODO(@manekinekko): when running in async mode, it doesn't make sense to
@@ -159,7 +170,6 @@ class TapParser extends Transform {
   // ----------------------------- Private API ----------------------------//
   // ----------------------------------------------------------------------//
 
-
   #parseTokens(callback = null) {
     for (let index = 0; index < this.#tokens.length; index++) {
       const chunk = this.#tokens[index];
@@ -303,20 +313,72 @@ class TapParser extends Transform {
     return (whitespaceCount / this.#kSubtestBlockIndentationFactor) | 0;
   }
 
+  // This method has 2 purposes:
+  // 1. It adds a new node to the (flat) AST
+  // 2. It returns that same node to the stream interface (for async parsing)
   #emit(value) {
     value = {
       ...value,
       nesting: this.#subTestNestingLevel,
     };
-    ArrayPrototypePush(this.#flatAst, value);
+
+    // If we are emitting a YAML block, we need to update the last node in the
+    // AST to include the YAML block as diagnostic.
+    // We only do this to produce an AST with fewer nodes.
+    if (value.kind === TokenKind.TAP_YAML_END) {
+      const lastNode = this.#flatAST.at(-1);
+
+      // Diagnostic nodes are only added to Test points of the same nesting level
+      if (
+        lastNode?.kind === TokenKind.TAP_TEST_POINT &&
+        lastNode?.nesting === value.nesting
+      ) {
+        lastNode.node.time = this.#lastTestPointDuration;
+        lastNode.node.diagnostics ||= [];
+
+        ArrayPrototypeForEach(value.node.diagnostics, (diagnostic) => {
+          ArrayPrototypePush(lastNode.node.diagnostics, diagnostic);
+        });
+
+        // eslint-disable-next-line brace-style
+      }
+
+      // Orphan YAML blocks? These are maybe general purpose YAML blocks
+      // So we add them to the AST as a YAML diagnostic.
+      else {
+        ArrayPrototypePush(this.#flatAST, value);
+      }
+      // eslint-disable-next-line brace-style
+    }
+
+    // We process any pending comments before emitting the current node
+    else if (value.kind === TokenKind.COMMENT) {
+      ArrayPrototypePush(this.#commentsBuffer, value);
+    } else {
+      // If we have pending comments, we add them to the current node
+      if (this.#commentsBuffer.length > 0) {
+        value.comments = ArrayPrototypeMap(
+          this.#commentsBuffer,
+          (c) => c.node.comment
+        );
+        this.#commentsBuffer = [];
+      }
+
+      ArrayPrototypePush(this.#flatAST, value);
+    }
+
     return value;
   }
 
   #serializeChunk(chunk) {
     return ArrayPrototypeJoin(
       ArrayPrototypeMap(
         // Exclude EOL and EOF tokens
-        ArrayPrototypeFilter(chunk, (token) => token.kind !== TokenKind.EOL && token.kind !== TokenKind.EOF),
+        ArrayPrototypeFilter(
+          chunk,
+          (token) =>
+            token.kind !== TokenKind.EOL && token.kind !== TokenKind.EOF
+        ),
         (token) => token.value
       ),
       ''
@@ -448,9 +510,7 @@ class TapParser extends Transform {
 
     const node = {
       kind: TokenKind.TAP_PLAN,
-      node: {
-        plan,
-      },
+      node: plan,
     };
 
     return this.#emit(node);
@@ -499,6 +559,7 @@ class TapParser extends Transform {
       id: numberToken,
       description: '',
       reason: '',
+      time: 0,
     };
 
     // Read optional description prefix " - "
@@ -543,10 +604,8 @@ class TapParser extends Transform {
     }
 
     const node = {
-      kind: isTestFailed ? TokenKind.TAP_TEST_NOTOK : TokenKind.TAP_TEST_OK,
-      node: {
-        test,
-      },
+      kind: TokenKind.TAP_TEST_POINT,
+      node: test,
     };
 
     return this.#emit(node);
@@ -687,6 +746,14 @@ class TapParser extends Transform {
   // YAMLLine := "  " (YAML)* "\n"
   #YAMLLine() {
     const yamlLiteral = this.#readNextLiterals();
+    const { 0: key, 1: value } = StringPrototypeSplit(yamlLiteral, ':');
-    const { 0: key, 1: value } = StringPrototypeSplit(yamlLiteral, ':');
+    const { 0: key, 1: value } = StringPrototypeSplit(yamlLiteral, ':', 2);
-    const { 0: key, 1: value } = StringPrototypeSplit(yamlLiteral, ':');
+    const { 0: key, 1: value } = StringPrototypeSplit(yamlLiteral, ':', 2);
+
+    switch (key) {
+      case 'duration_ms':
+        this.#lastTestPointDuration = Number(value);
+        break;
+    }
+
     ArrayPrototypePush(this.#yamlBlockBuffer, yamlLiteral);
   }