Skip to content

Possible error with coverage report #12136

Closed
Closed
Possible error with coverage report#12136
Labels
fsIssues and PRs related to file-system APIs and the fs module.questionIssues asking questions about Node.js.testIssues and PRs related to Node.js core tests and test infrastructure.

Description

@lucamaraschi
lucamaraschi
opened on Mar 30, 2017
  • Version: master
  • Platform: all
  • Subsystem: test, fs

In the test coverage report line 131 is marked as not covered but a test for this exception/line already exists (https://github.com/nodejs/node/blob/master/test/parallel/test-fs-make-callback.js).

cc: @addaleax && @nodejs/testing

Activity

  1. addaleax commented on Mar 30, 2017

    @addaleax
    addaleax
    on Mar 30, 2017
    Member

    @lucamaraschi That test uses fs.stat which uses makeStatsCallback, not makeCallback for special callback handling… I think the line is really not covered?

  2. addaleax
    added
    fsIssues and PRs related to file-system APIs and the fs module.
    questionIssues asking questions about Node.js.
    testIssues and PRs related to Node.js core tests and test infrastructure.
    on Mar 30, 2017

  3. lucamaraschi commented on Mar 30, 2017

    @lucamaraschi
    lucamaraschi
    on Mar 30, 2017
    ContributorAuthor

    @addaleax you are indeed right!Gonna open a PR on the test to extend the case also to makeCallback.

  4. lucamaraschi
    closed this as completedon Mar 30, 2017

  5. gibfahn commented on Mar 30, 2017

    @gibfahn
    gibfahn
    on Mar 30, 2017
    Member

    @lucamaraschi in that case I'll reopen this, and you can close it in your PR!

    In case you didn't know, you can add:

    Fixes: https://github.com/nodejs/node/issues/12136

    to your commit description to auto-close this when your PR lands.

  6. gibfahn
    reopened this on Mar 30, 2017

  7. cjihrig commented on Mar 30, 2017

    @cjihrig
    cjihrig
    on Mar 30, 2017
    Contributor

    @gibfahn I'm not sure that adding coverage for that line of code is related to this issue. This issue was opened for a possible bug in the coverage reporting, but that turned out not to be the case.

  8. gibfahn commented on Mar 30, 2017

    @gibfahn
    gibfahn
    on Mar 30, 2017
    Member

    @cjihrig the way I see it, the issue was raised to ask why that line wasn't being reported as covered. The answer is because the test doesn't cover it, so a PR fixing the test would resolve the issue.

  9. mhdawson
    closed this as completedin 53828e8on Apr 3, 2017
  10. gibfahn
    added 3 commits that reference this issue on Jun 19, 2017

    test: extended test to makeCallback cb type check

    b690537

    test: extended test to makeCallback cb type check

    f255541
  11. MylesBorins
    added a commit that references this issue on Jul 11, 2017

    test: extended test to makeCallback cb type check

    e6d6a41
  12. abhishekumar-tyagi
    added a commit that references this issue on May 5, 2024

    test: extended test to makeCallback cb type check

    0d27e35
  13. sjarmak
    added a commit that references this issue on Jul 27, 2026

    test: extended test to makeCallback cb type check

    53828e8
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

    fsIssues and PRs related to file-system APIs and the fs module.questionIssues asking questions about Node.js.testIssues and PRs related to Node.js core tests and test infrastructure.

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions