test_runner: add timestamp to JUnit reporter testsuites

Emit the standard JUnit timestamp (ISO 8601) on <testsuite> elements,
which the reporter was omitting. The suite start time is reconstructed
as end-minus-duration, because the runner reports a suite's test:start
lazily (when its first subtest reports), which would otherwise record a
time close to the suite's end.
Fixes: #64028
Signed-off-by: sangwook <rewq5991@gmail.com>
PR-URL: #64029
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Jacob Smith <jacob@frende.me>

Author: Han5991

lib/internal/test_runner/reporter/junit.js

@@ -5,6 +5,9 @@ const {
  ArrayPrototypeMap,
  ArrayPrototypePush,
  ArrayPrototypeSome,
+ Date,
+ DateNow,
+ DatePrototypeToISOString,
  NumberPrototypeToFixed,
  ObjectEntries,
  RegExpPrototypeSymbolReplace,
@@ -112,6 +115,11 @@ module.exports = async function* junitReporter(source) {
 currentTest.attrs.tests=nonCommentChildren.length;
 currentTest.attrs.failures=ArrayPrototypeFilter(currentTest.children,isFailure).length;
 currentTest.attrs.skipped=ArrayPrototypeFilter(currentTest.children,isSkipped).length;
+// A suite's `test:start` is emitted lazily (when its first subtest
+// reports), so derive the start time from the end minus the measured
+// duration rather than stamping the (late) test:start moment.
+currentTest.attrs.timestamp=
+DatePrototypeToISOString(newDate(DateNow()-event.data.details.duration_ms));
 currentTest.attrs.hostname=HOSTNAME;
 }else{
 currentTest.tag='testcase';

test/common/assertSnapshot.js

@@ -226,6 +226,7 @@ function replaceJunitDuration(str) {
 .replaceAll(/time="[0-9.]+"/g,'time="*"')
 .replaceAll(/duration_ms[0-9.]+/g,'duration_ms *')
 .replaceAll(`hostname="${hostname()}"`,'hostname="HOSTNAME"')
+.replaceAll(/timestamp="[^"]*"/g,'timestamp="*"')
 .replaceAll(/file="[^"]*"/g,'file="*"');
 }
 

test/fixtures/test-runner/output/junit_reporter.snapshot

@@ -128,7 +128,7 @@ true !== false
 	<testcasename="immediate throw - passes but warns"time="*"classname="test"file="*"/>
 	<testcasename="immediate reject - passes but warns"time="*"classname="test"file="*"/>
 	<testcasename="immediate resolve pass"time="*"classname="test"file="*"/>
-	<testsuitename="subtest sync throw fail"time="*"disabled="0"errors="0"tests="1"failures="1"skipped="0"hostname="HOSTNAME">
+	<testsuitename="subtest sync throw fail"time="*"disabled="0"errors="0"tests="1"failures="1"skipped="0"timestamp="*"hostname="HOSTNAME">
  <testcasename="+sync throw fail"time="*"classname="test"file="*"failure="thrown from subtest sync throw fail">
  <failuretype="testCodeFailure"message="thrown from subtest sync throw fail">
 Error [ERR_TEST_FAILURE]: thrown from subtest sync throw fail
@@ -151,15 +151,15 @@ Error [ERR_TEST_FAILURE]: thrown from subtest sync throw fail
 [Error [ERR_TEST_FAILURE]: Symbol(thrown symbol from sync throw non-error fail)] { code: 'ERR_TEST_FAILURE', failureType: 'testCodeFailure', cause: Symbol(thrown symbol from sync throw non-error fail) }
  </failure>
 	</testcase>
-	<testsuitename="level 0a"time="*"disabled="0"errors="0"tests="4"failures="0"skipped="0"hostname="HOSTNAME">
+	<testsuitename="level 0a"time="*"disabled="0"errors="0"tests="4"failures="0"skipped="0"timestamp="*"hostname="HOSTNAME">
  <testcasename="level 1a"time="*"classname="test"file="*"/>
  <testcasename="level 1b"time="*"classname="test"file="*"/>
  <testcasename="level 1c"time="*"classname="test"file="*"/>
  <testcasename="level 1d"time="*"classname="test"file="*"/>
 	</testsuite>
-	<testsuitename="top level"time="*"disabled="0"errors="0"tests="2"failures="0"skipped="0"hostname="HOSTNAME">
+	<testsuitename="top level"time="*"disabled="0"errors="0"tests="2"failures="0"skipped="0"timestamp="*"hostname="HOSTNAME">
  <testcasename="+long running"time="*"classname="test"file="*"/>
- <testsuitename="+short running"time="*"disabled="0"errors="0"tests="1"failures="0"skipped="0"hostname="HOSTNAME">
+ <testsuitename="+short running"time="*"disabled="0"errors="0"tests="1"failures="0"skipped="0"timestamp="*"hostname="HOSTNAME">
  <testcasename="++short running"time="*"classname="test"file="*"/>
  </testsuite>
 	</testsuite>
@@ -266,7 +266,7 @@ Error [ERR_TEST_FAILURE]: thrown from callback async throw
  </failure>
 	</testcase>
 	<testcasename="callback async throw after done"time="*"classname="test"file="*"/>
-	<testsuitename="only is set on subtests but not in only mode"time="*"disabled="0"errors="0"tests="3"failures="0"skipped="0"hostname="HOSTNAME">
+	<testsuitename="only is set on subtests but not in only mode"time="*"disabled="0"errors="0"tests="3"failures="0"skipped="0"timestamp="*"hostname="HOSTNAME">
  <testcasename="running subtest 1"time="*"classname="test"file="*"/>
  <testcasename="running subtest 3"time="*"classname="test"file="*"/>
  <testcasename="running subtest 4"time="*"classname="test"file="*"/>
@@ -288,7 +288,7 @@ Error [ERR_TEST_FAILURE]: thrown from callback async throw
 }
  </failure>
 	</testcase>
-	<testsuitename="subtest sync throw fails"time="*"disabled="0"errors="0"tests="2"failures="2"skipped="0"hostname="HOSTNAME">
+	<testsuitename="subtest sync throw fails"time="*"disabled="0"errors="0"tests="2"failures="2"skipped="0"timestamp="*"hostname="HOSTNAME">
  <testcasename="sync throw fails at first"time="*"classname="test"file="*"failure="thrown from subtest sync throw fails at first">
  <failuretype="testCodeFailure"message="thrown from subtest sync throw fails at first">
 Error [ERR_TEST_FAILURE]: thrown from subtest sync throw fails at first

test/parallel/test-runner-reporters.js

@@ -200,6 +200,12 @@ describe('node:test reporters', { concurrency: true }, () => {
 assert.strictEqual(child.stdout.toString(),'');
 constfileContents=fs.readFileSync(file,'utf8');
 assert.match(fileContents,/<testsuite.*name="nested".*tests="2".*failures="1".*skipped="0".*>/);
+// The exact timestamp format is intentionally not pinned here (still under
+// discussion); assert only that the value is present and a real date.
+const{1: timestamp}=fileContents.match(/<testsuite[^>]*timestamp="([^"]+)"/)??[];
+assert.ok(timestamp,'testsuite should have a timestamp attribute');
+assert.match(timestamp,/^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}/);
+assert.ok(!Number.isNaN(Date.parse(timestamp)),`expected a valid date, got ${timestamp}`);
 assert.match(fileContents,/<testcase.*name="failing".*>\s*<failure.*type="testCodeFailure".*message="error".*>/);
 assert.match(fileContents,/<testcase.*name="ok".*classname="test".*\/>/);
 assert.match(fileContents,/<testcase.*name="toplevel".*classname="test".*\/>/);