ferlores / mochify-istanbul

Instrument + create reports using istanbul with the mochify stream
Other
20 stars 11 forks source link

Default setup reports no coverage #2

Closed mantoni closed 9 years ago

mantoni commented 9 years ago

I'm using a simple default setup to check code coverage with these results:

$ node_modules/.bin/mochify --plugin [ mochify-istanbul --report text ]
# phantomjs:

  ............

  12 passing (12ms)

---------------------|-----------|-----------|-----------|-----------|
File                 |   % Stmts |% Branches |   % Funcs |   % Lines |
---------------------|-----------|-----------|-----------|-----------|
   lib/              |     16.67 |         0 |         0 |     16.67 |
      sorter.js      |     16.67 |         0 |         0 |     16.67 |
   test/             |      3.23 |       100 |         0 |      3.23 |
      create-test.js |      3.23 |       100 |         0 |      3.23 |
---------------------|-----------|-----------|-----------|-----------|
All files            |      9.09 |         0 |         0 |      9.09 |
---------------------|-----------|-----------|-----------|-----------|

Although this project has 99.76% coverage reported by coverify:

$ node_modules/.bin/mochify --cover
# phantomjs:

  ............                                                                     

  12 passing (34ms)

# .../lib/sorter.js: line 12, column -68-41

      throw new TypeError("Sorter cannot resolve '" + keys.slice(0, i).join(".")
        + "'' from " + JSON.stringify(obj));
        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

# coverage: 427/428 (99.76 %)

Error: Exit 1
ferlores commented 9 years ago

I'm not sure if I understand the issue. Are you pointing that with istanbul you get only 16.67% of coverage on the file while coverify reports 99,76%?

If so, I would suggest you to first exclude the test file and all the modules that you are not testing (--exclude '**/+(test|node_modules)/**/*'). I will improve the exclude parameter so you can pass an array of patterns instead.

About making this the default option I'm not that convinced. In my use case I have a node_module folder checked-in in my project where I define my modules, in order to avoid require('../../../../../'). Maybe what I can do is an opt-in default configuration.

What do you think?

EDIT: I corrected the glob patter for excluding the tests and node_modules

mantoni commented 9 years ago

I understand the statistics like it's giving me the coverage from parsing the file, but not from running the tests. The coverage stats from coverify suggest that the actual coverage is close to 100%. This looks like a bug to me.

Regarding defaults: It lists exactly the files I want it to check. So there is nothing to exclude for me.

ferlores commented 9 years ago

I found the bug, great catch. The __coverage__ variable was printed out before waiting for the tests to finish https://github.com/ferlores/mochify-istanbul/commit/6cfd3e883fbc5ebf5ed4eb9f5098ebbbfcb0af72

I just published v2.1.0, it also allows you to define multiple --exclude options. Please let me know if it works for you

mantoni commented 9 years ago

Now it works as expected. Thanks :+1:

ferlores commented 9 years ago

Cool. Do you mind linking this repo from the mochify README file? Thanks! On Nov 25, 2014 8:48 AM, "Maximilian Antoni" notifications@github.com wrote:

Closed #2 https://github.com/ferlores/mochify-istanbul/issues/2.

— Reply to this email directly or view it on GitHub https://github.com/ferlores/mochify-istanbul/issues/2#event-198388351.

mantoni commented 9 years ago

Make it a pull request ;-)