Skip to content

Running tests with --test-coverage-branches (same for lines|functions) doesn't emit test:fail when threshold isn't met #54812

Description

@rozzilla

Version

22.8.0

Platform

Darwin N4V4PGFGPT 23.6.0 Darwin Kernel Version 23.6.0: Mon Jul 29 21:16:46 PDT 2024; root:xnu-10063.141.2~1/RELEASE_ARM64_T8112 arm64

Subsystem

No response

What steps will reproduce the bug?

I'm running Node with the following options:

 --experimental-test-coverage --test-coverage-exclude='**/__tests__/**' --test-coverage-branches=95 --env-file=.env.test

and calling an executable that calls the run method as by doc.

If a test fails, a test:fail event is properly emitted.
If the coverage threshold is not met, I get a message from the test:diagnostic event like Error: 82.35% function coverage does not meet a threshold of 95%. (I get the same on the stdout). But no test:fail event is emitted, which makes it harder to detect it.

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

Always

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

test:fail event should be emitted once coverage is not meet

What do you see instead?

no test:fail event emitted

Additional information

same thing apply when using --test-coverage-functions or --test-coverage-lines options

Activity

  1. self-assigned this
    on Sep 6, 2024
  2. added
    coverageIssues and PRs related to Node.js code coverage support.
    test_runnerIssues and PRs related to the test runner subsystem.
    on Sep 6, 2024
  3. removed their assignment
    on Sep 6, 2024
  4. avivkeller commented on Sep 6, 2024

    @avivkeller
    Member

    I'm able to reproduce. I'll have a look at this later today.

    FWIW the process does exit with code 1, however all the tests do pass.

    CC @nodejs/test_runner

  5. cjihrig commented on Sep 6, 2024

    @cjihrig
    Contributor

    A test:fail event should not be emitted for coverage issues. There is a test:coverage event that would be more appropriate.

  6. rozzilla commented on Sep 6, 2024

    @rozzilla
    Author

    A test:fail event should not be emitted for coverage issues. There is a test:coverage event that would be more appropriate.

    Mmm, well, the CHANGELOG (not the docs, since they're not updated yet) says:

    If the code coverage fails to meet the specified thresholds for any category, the process will exit with code 1.

    Considering that, it feels more like a test:fail. Anyway, if there is a clear indication of coverage check failures somewhere, it'd be enough 👍🏼

  7. cjihrig commented on Sep 6, 2024

    @cjihrig
    Contributor

    test:fail means a test failed. It's already possible to end up with a failing exit code independent of coverage. For example, if a test passes, but creates a setTimeout() or other async activity that generates an error. The test has already finished and reported itself as being successful. So the error gets surfaced through a diagnostic and the exit code is set to 1. That's basically what is happening in the code coverage case as well. I agree that it should be signaled in the coverage event though.

  8. avivkeller commented on Sep 6, 2024

    @avivkeller
    Member

    See #54813

  9. moved this from Awaiting Triage to In Progress in Node.js feature requestson Sep 10, 2024
  10. moved this from In Progress to Done in Node.js feature requestson Sep 13, 2024
  11. added a commit that references this issue on Oct 4, 2024
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

    coverageIssues and PRs related to Node.js code coverage support.feature requestIssues requesting new Node.js features.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