tapjs / stack-utils

Captures and cleans stack traces.
https://www.npmjs.com/package/stack-utils
MIT License
190 stars 35 forks source link

return empty string instead of null if all internals #13

Closed SamVerschueren closed 8 years ago

SamVerschueren commented 8 years ago

PR based upon discussion at https://github.com/sindresorhus/ava/pull/510

I tested this change against node-tap and all test succeed except for the coverage-checks.js file.

test/coverage-checks.js ............................. 17/23 12s

I'm not familiar with node-tap at all so maybe other people can join the discussion for this tiny change. For example, why does it return null in the first place?

// @sindresorhus @jamestalmage @isaacs

isaacs commented 8 years ago

@SamVerschueren it works for me. Can you share the error that is printed out?

SamVerschueren commented 8 years ago

@isaacs It's quite a lot. Is this ok?

test/coverage-checks.js ............................. 17/23 13s
  fails > --branches 1
  not ok should match pattern provided
    found: >+
      -----------------|----------|----------|----------|----------|----------------|

      File             |  % Stmts | % Branch |  % Funcs |  % Lines |Uncovered Lines
      |

      -----------------|----------|----------|----------|----------|----------------|

       ../stack-utils/ |     63.7 |    45.98 |    91.67 |     63.7

      |                |

        index.js       |     63.7 |    45.98 |    91.67 |     63.7 |... 273,274,277

      |

       bin/            |    39.26 |    24.41 |     37.5 |    39.26

      |                |

        run.js         |    39.26 |    24.41 |     37.5 |    39.26 |... 628,629,630

      |

       lib/            |    61.97 |    55.41 |    59.35 |    62.08

      |                |

        assert.js      |    90.74 |       84 |      100 |    90.74 |... 255,263,264

      |

        mocha.js       |    21.21 |        0 |        0 |    22.58 |... 47,48,49,51

      |

        root.js        |     71.7 |       60 |    55.56 |     71.7 |... 62,66,67,99

      |

        stack.js       |      100 |      100 |      100 |      100

      |                |

        synonyms.js    |      100 |      100 |      100 |      100

      |                |

        test.js        |    56.35 |    50.28 |    55.29 |    56.35 |... 0,1394,1398

      |

      -----------------|----------|----------|----------|----------|----------------|

      All files        |    57.35 |    48.07 |    57.49 |    57.43
      |                |

      -----------------|----------|----------|----------|----------|----------------|

    pattern: |
      --------------|----------|----------|----------|----------|----------------|
      File          |  % Stmts | % Branch |  % Funcs |  % Lines |Uncovered Lines |
      --------------|----------|----------|----------|----------|----------------|
    at:
      line: 53
      column: 11
      file: test/coverage-checks.js
    stack: |
      test/coverage-checks.js:53:11
      ChildProcess.exithandler (child_process.js:210:5)
      Pipe._onclose (net.js:469:12)
    source: |
      t.match(stdout, banner)

  fails > --branches=100 --lines=0
  not ok should match pattern provided
    found: >+
      -----------------|----------|----------|----------|----------|----------------|

      File             |  % Stmts | % Branch |  % Funcs |  % Lines |Uncovered Lines
      |

      -----------------|----------|----------|----------|----------|----------------|

       ../stack-utils/ |     63.7 |    45.98 |    91.67 |     63.7

      |                |

        index.js       |     63.7 |    45.98 |    91.67 |     63.7 |... 273,274,277

      |

       bin/            |    48.77 |    41.78 |    53.13 |    48.77

      |                |

        run.js         |    48.77 |    41.78 |    53.13 |    48.77 |... 628,629,630

      |

       lib/            |    61.97 |    55.41 |    59.35 |    62.08

      |                |

        assert.js      |    90.74 |       84 |      100 |    90.74 |... 255,263,264

      |

        mocha.js       |    21.21 |        0 |        0 |    22.58 |... 47,48,49,51

      |

        root.js        |     71.7 |       60 |    55.56 |     71.7 |... 62,66,67,99

      |

        stack.js       |      100 |      100 |      100 |      100

      |                |

        synonyms.js    |      100 |      100 |      100 |      100

      |                |

        test.js        |    56.35 |    50.28 |    55.29 |    56.35 |... 0,1394,1398

      |

      -----------------|----------|----------|----------|----------|----------------|

      All files        |    59.35 |    51.73 |    60.48 |    59.43
      |                |

      -----------------|----------|----------|----------|----------|----------------|

    pattern: |
      --------------|----------|----------|----------|----------|----------------|
      File          |  % Stmts | % Branch |  % Funcs |  % Lines |Uncovered Lines |
      --------------|----------|----------|----------|----------|----------------|
    at:
      line: 53
      column: 11
      file: test/coverage-checks.js
    stack: |
      test/coverage-checks.js:53:11
      ChildProcess.exithandler (child_process.js:210:5)
      Pipe._onclose (net.js:469:12)
    source: |
      t.match(stdout, banner)

  fails > --check-coverage
  not ok should match pattern provided
    found: >+
      -----------------|----------|----------|----------|----------|----------------|

      File             |  % Stmts | % Branch |  % Funcs |  % Lines |Uncovered Lines
      |

      -----------------|----------|----------|----------|----------|----------------|

       ../stack-utils/ |     63.7 |    45.98 |    91.67 |     63.7

      |                |

        index.js       |     63.7 |    45.98 |    91.67 |     63.7 |... 273,274,277

      |

       bin/            |    49.69 |    42.25 |    53.13 |    49.69

      |                |

        run.js         |    49.69 |    42.25 |    53.13 |    49.69 |... 628,629,630

      |

       lib/            |    61.97 |    55.41 |    59.35 |    62.08

      |                |

        assert.js      |    90.74 |       84 |      100 |    90.74 |... 255,263,264

      |

        mocha.js       |    21.21 |        0 |        0 |    22.58 |... 47,48,49,51

      |

        root.js        |     71.7 |       60 |    55.56 |     71.7 |... 62,66,67,99

      |

        stack.js       |      100 |      100 |      100 |      100

      |                |

        synonyms.js    |      100 |      100 |      100 |      100

      |                |

        test.js        |    56.35 |    50.28 |    55.29 |    56.35 |... 0,1394,1398

      |

      -----------------|----------|----------|----------|----------|----------------|

      All files        |    59.55 |    51.83 |    60.48 |    59.63
      |                |

      -----------------|----------|----------|----------|----------|----------------|

    pattern: |
      --------------|----------|----------|----------|----------|----------------|
      File          |  % Stmts | % Branch |  % Funcs |  % Lines |Uncovered Lines |
      --------------|----------|----------|----------|----------|----------------|
    at:
      line: 53
      column: 11
      file: test/coverage-checks.js
    stack: |
      test/coverage-checks.js:53:11
      ChildProcess.exithandler (child_process.js:210:5)
      Pipe._onclose (net.js:469:12)
    source: |
      t.match(stdout, banner)

  passes > --lines=1
  not ok should match pattern provided
    found: >+
      -----------------|----------|----------|----------|----------|----------------|

      File             |  % Stmts | % Branch |  % Funcs |  % Lines |Uncovered Lines
      |

      -----------------|----------|----------|----------|----------|----------------|

       ../stack-utils/ |     63.7 |    45.98 |    91.67 |     63.7

      |                |

        index.js       |     63.7 |    45.98 |    91.67 |     63.7 |... 273,274,277

      |

       bin/            |    50.61 |    43.66 |    53.13 |    50.61

      |                |

        run.js         |    50.61 |    43.66 |    53.13 |    50.61 |... 628,629,630

      |

       lib/            |    61.97 |    55.41 |    59.35 |    62.08

      |                |

        assert.js      |    90.74 |       84 |      100 |    90.74 |... 255,263,264

      |

        mocha.js       |    21.21 |        0 |        0 |    22.58 |... 47,48,49,51

      |

        root.js        |     71.7 |       60 |    55.56 |     71.7 |... 62,66,67,99

      |

        stack.js       |      100 |      100 |      100 |      100

      |                |

        synonyms.js    |      100 |      100 |      100 |      100

      |                |

        test.js        |    56.35 |    50.28 |    55.29 |    56.35 |... 0,1394,1398

      |

      -----------------|----------|----------|----------|----------|----------------|

      All files        |    59.74 |    52.13 |    60.48 |    59.82
      |                |

      -----------------|----------|----------|----------|----------|----------------|

    pattern: |
      --------------|----------|----------|----------|----------|----------------|
      File          |  % Stmts | % Branch |  % Funcs |  % Lines |Uncovered Lines |
      --------------|----------|----------|----------|----------|----------------|
    at:
      line: 68
      column: 11
      file: test/coverage-checks.js
    stack: |
      test/coverage-checks.js:68:11
      ChildProcess.exithandler (child_process.js:194:7)
      Pipe._onclose (net.js:469:12)
    source: |
      t.match(stdout, banner)

  passes > --lines=1 --check-coverage
  not ok should match pattern provided
    found: >+
      -----------------|----------|----------|----------|----------|----------------|

      File             |  % Stmts | % Branch |  % Funcs |  % Lines |Uncovered Lines
      |

      -----------------|----------|----------|----------|----------|----------------|

       ../stack-utils/ |     63.7 |    45.98 |    91.67 |     63.7

      |                |

        index.js       |     63.7 |    45.98 |    91.67 |     63.7 |... 273,274,277

      |

       bin/            |    50.61 |     44.6 |    53.13 |    50.61

      |                |

        run.js         |    50.61 |     44.6 |    53.13 |    50.61 |... 628,629,630

      |

       lib/            |    61.97 |    55.41 |    59.35 |    62.08

      |                |

        assert.js      |    90.74 |       84 |      100 |    90.74 |... 255,263,264

      |

        mocha.js       |    21.21 |        0 |        0 |    22.58 |... 47,48,49,51

      |

        root.js        |     71.7 |       60 |    55.56 |     71.7 |... 62,66,67,99

      |

        stack.js       |      100 |      100 |      100 |      100

      |                |

        synonyms.js    |      100 |      100 |      100 |      100

      |                |

        test.js        |    56.35 |    50.28 |    55.29 |    56.35 |... 0,1394,1398

      |

      -----------------|----------|----------|----------|----------|----------------|

      All files        |    59.74 |    52.32 |    60.48 |    59.82
      |                |

      -----------------|----------|----------|----------|----------|----------------|

    pattern: |
      --------------|----------|----------|----------|----------|----------------|
      File          |  % Stmts | % Branch |  % Funcs |  % Lines |Uncovered Lines |
      --------------|----------|----------|----------|----------|----------------|
    at:
      line: 68
      column: 11
      file: test/coverage-checks.js
    stack: |
      test/coverage-checks.js:68:11
      ChildProcess.exithandler (child_process.js:194:7)
      Pipe._onclose (net.js:469:12)
    source: |
      t.match(stdout, banner)

  passes > --lines 1 --statements 1 --functions 1 --branches 1
  not ok should match pattern provided
    found: >+
      -----------------|----------|----------|----------|----------|----------------|

      File             |  % Stmts | % Branch |  % Funcs |  % Lines |Uncovered Lines
      |

      -----------------|----------|----------|----------|----------|----------------|

       ../stack-utils/ |     63.7 |    45.98 |    91.67 |     63.7

      |                |

        index.js       |     63.7 |    45.98 |    91.67 |     63.7 |... 273,274,277

      |

       bin/            |    50.61 |     44.6 |    53.13 |    50.61

      |                |

        run.js         |    50.61 |     44.6 |    53.13 |    50.61 |... 628,629,630

      |

       lib/            |    61.97 |    55.41 |    59.35 |    62.08

      |                |

        assert.js      |    90.74 |       84 |      100 |    90.74 |... 255,263,264

      |

        mocha.js       |    21.21 |        0 |        0 |    22.58 |... 47,48,49,51

      |

        root.js        |     71.7 |       60 |    55.56 |     71.7 |... 62,66,67,99

      |

        stack.js       |      100 |      100 |      100 |      100

      |                |

        synonyms.js    |      100 |      100 |      100 |      100

      |                |

        test.js        |    56.35 |    50.28 |    55.29 |    56.35 |... 0,1394,1398

      |

      -----------------|----------|----------|----------|----------|----------------|

      All files        |    59.74 |    52.32 |    60.48 |    59.82
      |                |

      -----------------|----------|----------|----------|----------|----------------|

    pattern: |
      --------------|----------|----------|----------|----------|----------------|
      File          |  % Stmts | % Branch |  % Funcs |  % Lines |Uncovered Lines |
      --------------|----------|----------|----------|----------|----------------|
    at:
      line: 68
      column: 11
      file: test/coverage-checks.js
    stack: |
      test/coverage-checks.js:68:11
      ChildProcess.exithandler (child_process.js:194:7)
      Pipe._onclose (net.js:469:12)
    source: |
      t.match(stdout, banner)
isaacs commented 8 years ago

Ah, @SamVerschueren, that's because nyc is showing linked packages in the output, with their relative paths, which causes the output to be stretched further than the test is expecting.

If you npm install ../stack-utils instead of linking it, it'll work fine.

This LGTM if you're cool with it, @jamestalmage.

jamestalmage commented 8 years ago

that's because nyc is showing linked packages in the output

Should be fixed in https://github.com/bcoe/nyc/pull/160

LGTM

jamestalmage commented 8 years ago

0.4.0 is published. @SamVerschueren - Your AVA PR should just be a dependency bump now.

SamVerschueren commented 8 years ago

Thanks @jamestalmage and @isaacs :)