Request: Print console coverage report #6

Closed
davglass opened this Issue Oct 12, 2012 · 13 comments

Comments

Projects
None yet
2 participants
Collaborator

davglass commented Oct 12, 2012

When using istanbul from your package.json, it would be nice if it could print a small report at the end.

This way, when using Travis, we have a record of the coverage numbers after the tests are run. Since Travis doesn't track build assets and it only tracks the log output. Putting a simple report in the output would be handy.

Also, you could add some flags to set the values of the warnings. Then if the code coverage report doesn't meat these numbers you can process.exit(1) to fail the build.

Owner

gotwarlost commented Oct 12, 2012

How about we split this into two tickets, one for the small text output and another to enforce coverage. The latter needs some thought on interface whereas the former is more of typing work.

Also, Istanbul refrains from explicitly exiting from the process since stdout/ stderr may not fully flush etc. if it gets into this business and could interfere with other exit handlers that the app under test could have set up. So some thinking on that is required before we do the process.exit(1) thing.

I will create a new issue for the exit(1) thing

Owner

gotwarlost commented Oct 14, 2012

Created a textreport branch on which this change is committed. @davglass I'm looking for your feedback on command line interface and report behavior. Please check out this branch, try it out and let me know.

cover and test commands have the following new option:

  --console <type>
          type of report to print to console, one of summary (default),
          detail, both or none

summary produces something like this:

=============================== Coverage summary ===============================
Statements   : 71.43% ( 10/14 )
Branches     : 66.67% ( 4/6 )
Functions    : 60% ( 3/5 )
Lines        : 71.43% ( 10/14 )
================================================================================

detail produces something like this

--------------------------+-----------+-----------+-----------+-----------+
File                      |   % Stmts |% Branches |   % Funcs |   % Lines |
--------------------------+-----------+-----------+-----------+-----------+
   lib/                   |     66.67 |        50 |        50 |     66.67 |
      bar.js              |        40 |         0 |         0 |        40 |
      foo.js              |       100 |       100 |       100 |       100 |
   lib/util/              |       100 |       100 |       100 |       100 |
      generate-names.js   |       100 |       100 |       100 |       100 |
   vendor/                |     66.67 |       100 |        50 |     66.67 |
      dummy_vendor_lib.js |     66.67 |       100 |        50 |     66.67 |
--------------------------+-----------+-----------+-----------+-----------+
All files                 |     71.43 |     66.67 |        60 |     71.43 |
--------------------------+-----------+-----------+-----------+-----------+
Collaborator

davglass commented Oct 15, 2012

I love this:
https://travis-ci.org/#!/yui/node-cssmin/jobs/2797430

My "only" recommendation is to not use full system paths when printing, use relative to this directory.

These can get pretty long if the directory tree (think a hudson workspace) is deeply nested.

Owner

gotwarlost commented Oct 15, 2012

That full filesystem path appears when you have a single file under test. When there are multiple files the common prefix is stripped. I need to add handling so that the entire directory path is nuked when a single file is involved.

I will fix this as a separate issue.

Collaborator

davglass commented Oct 15, 2012

Thanks, other than that I love it. I've already put this branch into several of my CLI tools so I can see a coverage report via Travis :)

Owner

gotwarlost commented Oct 15, 2012

The only thing that is stopping me from merging this branch is the name of the --console option. Could you suggest an alternative? Maybe --print ?

Collaborator

davglass commented Oct 15, 2012

I'm fine with --print

Collaborator

davglass commented Oct 15, 2012

Can you make this a method that I can use when istanbul is required? This way my CLI tools that gather coverage can print the exact same report.

Owner

gotwarlost commented Oct 15, 2012

On the browser? It's already available for node using the Report mechanism. (i.e. istanbul.Report.create('text-summary') etc.)

Browser is a harder problem since the report infrastructure is not set up to run on a browser.

Collaborator

davglass commented Oct 15, 2012

Nice, that's what I needed..

Owner

gotwarlost commented Oct 17, 2012

Calling done. @davglass I'm going to nuke the textreport branch once you confirm you are not using it in any way. Note that the branch also has the backwards-incompatible --print change.

@gotwarlost gotwarlost closed this Oct 17, 2012

Collaborator

davglass commented Oct 19, 2012

All of my modules have been switched over to the lastest istanbul on npm, so you can nuke the branch all you want.

FYI, I ❤️ this report:
https://travis-ci.org/yui/shifter/jobs/2847225

Owner

gotwarlost commented Oct 19, 2012

Nice!

BTW, v0.1.8 also has the absolute path fix where it won't print the full directory name if there is only one file being covered. You should already have it if you are using the latest.

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