invalid location in tap reported when there is something that keep the tests from exiting and run global timeout reached

[rluvaton]

Version

20.7.0

Platform

Darwin Razs-MacBook-Pro.local 23.0.0 Darwin Kernel Version 23.0.0: Fri Sep 15 14:41:43 PDT 2023; root:xnu-10002.1.13~1/RELEASE_ARM64_T6000 arm64

Subsystem

test_runner

What steps will reproduce the bug?

1. clone https://github.com/rluvaton/tap-reporter-have-invalid-location-when-failed-on-before-hook-timeout
2. run npm test

How often does it reproduce? Is there a required condition?

the test runner should stop the tests when there is something that keeps it running

What is the expected behavior? Why is that the expected behavior?

to not see undefined:undefined:undefined in the location

What do you see instead?

$ node ./run-tests.js

TAP version 13
# Subtest: should work
ok 1 - should work
  ---
  duration_ms: 1.079375
  ...
# Subtest: ./a.test.js
not ok 1 - ./a.test.js
  ---
  duration_ms: 5001.997458
  location: 'undefined:undefined:undefined' <--------------- invalid
  failureType: 'testTimeoutFailure'
  error: 'test timed out after 5000ms'
  code: 'ERR_TEST_FAILURE'
  stack: |-
    async Promise.allSettled (index 0)
  ...
1..2
# tests 2
# suites 0
# pass 1
# fail 0
# cancelled 1
# skipped 0
# todo 0
# duration_ms 5006.3885

Process finished with exit code 0

Additional information

No response

[cjihrig]

I believe the fix for this issue and issue #49927 is:

diff --git a/lib/internal/test_runner/runner.js b/lib/internal/test_runner/runner.js
index 08f9b48dda..6e6faf5000 100644
--- a/lib/internal/test_runner/runner.js
+++ b/lib/internal/test_runner/runner.js
@@ -139,6 +139,17 @@ class FileTest extends Test {
   #rawBufferSize = 0;
   #reportedChildren = 0;
   failedSubtests = false;
+
+  constructor(options) {
+    super(options);
+    this.loc ??= {
+      __proto__: null,
+      line: 1,
+      column: 1,
+      file: this.name,
+    };
+  }
+
   #skipReporting() {
     return this.#reportedChildren > 0 && (!this.error || this.error.failureType === kSubtestsFailed);
   }

We should probably add path.resolve() around the location's file. In a semver major change, I think we should call path.resolve() on the input before calling super(options).

[MoLow]

@razvanbh @cjihrig will one of you open a PR?

[cjihrig]

I didn't have time to write a test the other day when I posted that comment and hoped someone would have by now 😄. I can do it today.