Sitelet https://github.com/nodejs/node/issues/49926
Skip to content

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

Description

@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

Activity

  1. added
    confirmed-bugIssues and PRs for confirmed bugs.
    test_runnerIssues and PRs related to the test runner subsystem.
    on Sep 28, 2023
  2. cjihrig commented on Sep 29, 2023

    @cjihrig
    Contributor

    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).

  3. MoLow commented on Oct 1, 2023

    @MoLow
    Member

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

  4. cjihrig commented on Oct 1, 2023

    @cjihrig
    Contributor

    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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    confirmed-bugIssues and PRs for confirmed bugs.test_runnerIssues and PRs related to the test runner subsystem.

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions