Code Coverage Support added #74

Closed
wants to merge 21 commits into
from

Conversation

Projects
None yet
10 participants
@arunoda

arunoda commented May 28, 2011

Expresso's Code Coverage support added to nodeunit

inorder to use it.
you've define requires as follows
var a = require('a');
instead
var a = require('../lib/a')

Then it'll automatically detect this and added code coverage support.
and display the summery.
If you need more information add -cr option
eg:- nodeunit -cr tests

if you need to ommit Code Coverage you can use -ci option
eg:- nodeunit -ci tests

And this will not affect existing tests written in Nodeunit and above little require modification adds the coverage support

arunoda added some commits May 28, 2011

Node JS Coverage support added via
-c and -cv options
this requires expresso like module loading to be work
need to mention require('main') instead require('../lib/main')
Seamless Code Coverage support added
once you've added lib folders dependencies as require('depname')
nodeunit will automatically detect it and added the code coverage support
--
-cr option to display coverage results in depth including source code view
support added to ignore code coverage using -ci option
eg:- nodeunit -ci tests/
package.json updated with dependencies
@alfredwesterveld

This comment has been minimized.

Show comment Hide comment
@alfredwesterveld

alfredwesterveld Jun 7, 2011

code coverage would be nice

code coverage would be nice

@arunoda

This comment has been minimized.

Show comment Hide comment
@arunoda

arunoda Jun 7, 2011

Yes. that's what I forked this. and you checkout myfork and have a npm install

arunoda commented Jun 7, 2011

Yes. that's what I forked this. and you checkout myfork and have a npm install

@arunoda arunoda closed this Jun 7, 2011

@arunoda

This comment has been minimized.

Show comment Hide comment
@arunoda

arunoda Jun 7, 2011

Yes. that's what I forked this. and you checkout myfork and have a npm install

arunoda commented Jun 7, 2011

Yes. that's what I forked this. and you checkout myfork and have a npm install

@arunoda arunoda reopened this Jun 7, 2011

@s3u

This comment has been minimized.

Show comment Hide comment
@s3u

s3u Aug 29, 2011

Any update on this pull request?

s3u commented Aug 29, 2011

Any update on this pull request?

@arunoda

This comment has been minimized.

Show comment Hide comment
@arunoda

arunoda Aug 29, 2011

Since there is no such update. I'm gonna push forked version of nodeunit to NPM.
How about that?

arunoda commented Aug 29, 2011

Since there is no such update. I'm gonna push forked version of nodeunit to NPM.
How about that?

@caolan

This comment has been minimized.

Show comment Hide comment
@caolan

caolan Oct 29, 2011

Owner

Hi arunoda, thanks for your work on this. However, I'm unable to see any coverage output when I run test suites using your updated code. It runs the tests fine, but there's no coverage report.

I have the code under test in 'lib' and the tests in 'test'. The tests reference the code in lib by doing require('../lib/foo'). I run the tests by doing "nodeunit -cr test" in the project directory. Am I missing something?

Owner

caolan commented Oct 29, 2011

Hi arunoda, thanks for your work on this. However, I'm unable to see any coverage output when I run test suites using your updated code. It runs the tests fine, but there's no coverage report.

I have the code under test in 'lib' and the tests in 'test'. The tests reference the code in lib by doing require('../lib/foo'). I run the tests by doing "nodeunit -cr test" in the project directory. Am I missing something?

@BryanDonovan

This comment has been minimized.

Show comment Hide comment
@BryanDonovan

BryanDonovan Oct 29, 2011

Contributor

I think we need a solution that doesn't require a particular directory structure. Perhaps 'test' and 'lib' can be defaults, but a way to pass in the paths via the command line seems like a necessary feature.

Contributor

BryanDonovan commented Oct 29, 2011

I think we need a solution that doesn't require a particular directory structure. Perhaps 'test' and 'lib' can be defaults, but a way to pass in the paths via the command line seems like a necessary feature.

@arunoda

This comment has been minimized.

Show comment Hide comment
@arunoda

arunoda Oct 30, 2011

Yes :) A simple thing
You've to require('foo') instead of require('../lib/foo')
That's how they detect for code coverage. If not they just ignore it.

On Sun, Oct 30, 2011 at 3:45 AM, Caolan McMahon <
reply@reply.github.com>wrote:

Hi arunoda, thanks for your work on this. However, I'm unable to see any
coverage output when I run test suites using your updated code. It runs the
tests fine, but there's no coverage report.

I have the code under test in 'lib' and the tests in 'test'. The tests
reference the code in lib by doing require('../lib/foo'). I run the tests
by doing "nodeunit -cr test" in the project directory. Am I missing
something?

Reply to this email directly or view it on GitHub:
#74 (comment)

Arunoda Susiripala

@arunoda http://twitter.com/arunoda
http://gplus.to/arunodahttps://github.com/arunoda
http://www.linkedin.com/in/arunoda

arunoda commented Oct 30, 2011

Yes :) A simple thing
You've to require('foo') instead of require('../lib/foo')
That's how they detect for code coverage. If not they just ignore it.

On Sun, Oct 30, 2011 at 3:45 AM, Caolan McMahon <
reply@reply.github.com>wrote:

Hi arunoda, thanks for your work on this. However, I'm unable to see any
coverage output when I run test suites using your updated code. It runs the
tests fine, but there's no coverage report.

I have the code under test in 'lib' and the tests in 'test'. The tests
reference the code in lib by doing require('../lib/foo'). I run the tests
by doing "nodeunit -cr test" in the project directory. Am I missing
something?

Reply to this email directly or view it on GitHub:
#74 (comment)

Arunoda Susiripala

@arunoda http://twitter.com/arunoda
http://gplus.to/arunodahttps://github.com/arunoda
http://www.linkedin.com/in/arunoda

@arunoda

This comment has been minimized.

Show comment Hide comment
@arunoda

arunoda Oct 30, 2011

I agree with you. lib should be default but there should be some option to
add folders to the list.

On Sun, Oct 30, 2011 at 3:51 AM, Bryan Donovan <
reply@reply.github.com>wrote:

I think we need a solution that doesn't require a particular directory
structure. Perhaps 'test' and 'lib' can be defaults, but a way to pass in
the paths via the command line seems like a necessary feature.

Reply to this email directly or view it on GitHub:
#74 (comment)

Arunoda Susiripala

@arunoda http://twitter.com/arunoda
http://gplus.to/arunodahttps://github.com/arunoda
http://www.linkedin.com/in/arunoda

arunoda commented Oct 30, 2011

I agree with you. lib should be default but there should be some option to
add folders to the list.

On Sun, Oct 30, 2011 at 3:51 AM, Bryan Donovan <
reply@reply.github.com>wrote:

I think we need a solution that doesn't require a particular directory
structure. Perhaps 'test' and 'lib' can be defaults, but a way to pass in
the paths via the command line seems like a necessary feature.

Reply to this email directly or view it on GitHub:
#74 (comment)

Arunoda Susiripala

@arunoda http://twitter.com/arunoda
http://gplus.to/arunodahttps://github.com/arunoda
http://www.linkedin.com/in/arunoda

@Raynos

This comment has been minimized.

Show comment Hide comment
@Raynos

Raynos Dec 23, 2011

qbox dependency is overkill

qbox dependency is overkill

This comment has been minimized.

Show comment Hide comment
@arunoda

arunoda Dec 23, 2011

Owner

Why did you say that?

Owner

arunoda replied Dec 23, 2011

Why did you say that?

This comment has been minimized.

Show comment Hide comment
@Raynos

Raynos Dec 23, 2011

Because when you remove the dependency you actually reduce the number of lines of code.

Because when you remove the dependency you actually reduce the number of lines of code.

This comment has been minimized.

Show comment Hide comment
@arunoda

arunoda Dec 23, 2011

Owner
Owner

arunoda replied Dec 23, 2011

This comment has been minimized.

Show comment Hide comment
@arunoda

arunoda Dec 23, 2011

Owner
Owner

arunoda replied Dec 23, 2011

@Raynos

This comment has been minimized.

Show comment Hide comment
@Raynos

Raynos Dec 23, 2011

Just call startTesting here

Just call startTesting here

@Raynos

This comment has been minimized.

Show comment Hide comment
@Raynos

Raynos Dec 23, 2011

change this to an if (!codeCoverage) { startTesting(); }

change this to an if (!codeCoverage) { startTesting(); }

@Raynos

This comment has been minimized.

Show comment Hide comment
@Raynos

Raynos Dec 23, 2011

Rather then having a code coverage ignore option, can we just not enable code coverage by default?

Rather then having a code coverage ignore option, can we just not enable code coverage by default?

This comment has been minimized.

Show comment Hide comment
@arunoda

arunoda Dec 23, 2011

Owner
Owner

arunoda replied Dec 23, 2011

This comment has been minimized.

Show comment Hide comment
@Raynos

Raynos Dec 23, 2011

Yes but your asking to merge something in, where thousands of people use it. And now all these thousands of people will be spawning code coverage by default.

Thats a major breaking change. You can't just introduce that and expect it to get merged.

It's not about personal preferences, It's about how can we introduce non breaking changes gently.

Yes but your asking to merge something in, where thousands of people use it. And now all these thousands of people will be spawning code coverage by default.

Thats a major breaking change. You can't just introduce that and expect it to get merged.

It's not about personal preferences, It's about how can we introduce non breaking changes gently.

This comment has been minimized.

Show comment Hide comment
@arunoda

arunoda Dec 23, 2011

Owner
Owner

arunoda replied Dec 23, 2011

@joeferner

This comment has been minimized.

Show comment Hide comment
@joeferner

joeferner Jan 3, 2012

Great work! I do agree with Raynos though, this is a breaking change. It would be better to have this off by default. It might also be nice to be more explicit on which requires are part of the coverage, maybe nodeunit.require("mydep") this would also reduce the chance of breaking current projects.

Great work! I do agree with Raynos though, this is a breaking change. It would be better to have this off by default. It might also be nice to be more explicit on which requires are part of the coverage, maybe nodeunit.require("mydep") this would also reduce the chance of breaking current projects.

@joeferner

This comment has been minimized.

Show comment Hide comment
@joeferner

joeferner Jan 3, 2012

Found a bug. If you specify more than one file on the command line or use globs on the command line the code coverage will be kicked off multiple times (ie nodeunit tests/*).

The problem appears to be that "setUpCodeCoverage" is called once for each file. It should be moved below.

 if(!codeCoverage.enabled) {
     startTesting();
+} else {
+    setUpCodeCoverage();
 }

Found a bug. If you specify more than one file on the command line or use globs on the command line the code coverage will be kicked off multiple times (ie nodeunit tests/*).

The problem appears to be that "setUpCodeCoverage" is called once for each file. It should be moved below.

 if(!codeCoverage.enabled) {
     startTesting();
+} else {
+    setUpCodeCoverage();
 }
@felipesilva

This comment has been minimized.

Show comment Hide comment
@felipesilva

felipesilva Jul 18, 2012

I would love to see this feature becomes true :) Any news about this pull request?

I would love to see this feature becomes true :) Any news about this pull request?

@joeferner

This comment has been minimized.

Show comment Hide comment
@joeferner

joeferner Jul 18, 2012

@felipesilva while you wait you might want to checkout this out (https://github.com/nearinfinity/node-covershot) we wrote this to be a framework agnostic coverage tool.

@felipesilva while you wait you might want to checkout this out (https://github.com/nearinfinity/node-covershot) we wrote this to be a framework agnostic coverage tool.

@felipesilva

This comment has been minimized.

Show comment Hide comment
@felipesilva

felipesilva Jul 18, 2012

@joeferner wow ;) tks.

@joeferner wow ;) tks.

@haraldrudell

This comment has been minimized.

Show comment Hide comment
@haraldrudell

haraldrudell Aug 22, 2012

mochawrapper

I just wrote a node module that automates coverage reporting. It is based on mocha, jscoverage and node's assert. You do not have to modify require or use make or enviornment variables.

It is hosted on github:
https://github.com/haraldrudell/mochawrapper

mochawrapper

I just wrote a node module that automates coverage reporting. It is based on mocha, jscoverage and node's assert. You do not have to modify require or use make or enviornment variables.

It is hosted on github:
https://github.com/haraldrudell/mochawrapper

@peter-mouland

This comment has been minimized.

Show comment Hide comment
@peter-mouland

peter-mouland Mar 22, 2014

was this ever merged in?

was this ever merged in?

@caolan

This comment has been minimized.

Show comment Hide comment
@caolan

caolan Mar 28, 2014

Owner

This is an old issue and there are tools out there that can now do code coverage with nodeunit. In the future it would be nice to offer some easy integration but for now it works.

Owner

caolan commented Mar 28, 2014

This is an old issue and there are tools out there that can now do code coverage with nodeunit. In the future it would be nice to offer some easy integration but for now it works.

@caolan caolan closed this Mar 28, 2014

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