Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test cov #5

Closed
tj opened this issue Sep 6, 2011 · 39 comments
Closed

test cov #5

tj opened this issue Sep 6, 2011 · 39 comments

Comments

@tj
Copy link
Contributor

tj commented Sep 6, 2011

write a proper test cov lib

@Raynos
Copy link
Contributor

Raynos commented Nov 23, 2011

+1 on this. The main thing missing is easy test coverage that just works.

@podviaznikov
Copy link

+1

3 similar comments
@dhendo
Copy link
Contributor

dhendo commented Dec 5, 2011

+1

@timoxley
Copy link

timoxley commented Dec 6, 2011

+1

@BryanDonovan
Copy link
Contributor

+1

@ashwinphatak
Copy link

Will the test coverage functionality be ported over from expresso, based on jscoverage?

@tj
Copy link
Contributor Author

tj commented Dec 12, 2011

nope

@ashwinphatak
Copy link

Ok. Is it because of some technical reason (e.g. doesn't play well with mocha) or due to it being very low on the list of priorities?

@tj
Copy link
Contributor Author

tj commented Dec 13, 2011

just because jscov is the "lame" way to go about it, we could easily write similar with pure javascript, just need some time

@logicalparadox
Copy link

+1 ... were you thinking of writing a separate module to do this or would this be included in mocha? My vote would be a separate module. Either way, please let me know where and how I can contribute :)

@tj
Copy link
Contributor Author

tj commented Jan 3, 2012

definitely a separate module

@daaku
Copy link

daaku commented Jan 25, 2012

+1 ... https://github.com/chrisdickinson/node-runforcover or something using bunker directly?

@alfredwesterveld
Copy link

+1

@chrisdickinson
Copy link

let me know if there's anything you'd particularly like to see in runforcover that would make integrating it with mocha easier.

it's a little specific to require hooks at the moment, but could be made more generic if need be.

thanks!
chris

@tj
Copy link
Contributor Author

tj commented Jan 30, 2012

@chrisdickinson will do! it'll need to support the client-side as well so I almost think the jscov extra directory approach might be fine

@BryanDonovan
Copy link
Contributor

Are there any examples out there that show how to use runforcover with mocha? I tried it, and got it to work (and it worked well) for a single test at a time, but couldn't figure out how to wrap entire test suites, especially with nested tests.

@fengmk2
Copy link
Contributor

fengmk2 commented Feb 15, 2012

Is there any plan for this feature?

@tj
Copy link
Contributor Author

tj commented Feb 15, 2012

there is but I've got a lot of stuff going on so it wont be any time soon

@tj
Copy link
Contributor Author

tj commented Feb 23, 2012

sneak preview of what I have so far:

@fengmk2
Copy link
Contributor

fengmk2 commented Feb 23, 2012

wow, nice job, I think we will see coverage.html soon.

@danielkrau
Copy link

Why don't you use an existing solution?
I think of coveraje. It works for me (also with tests written for mocha).

coveraje outputcoveraje output

@tj
Copy link
Contributor Author

tj commented Feb 23, 2012

because this one can look the way I want :p

@jgallen23
Copy link

looks awesome! (as always)

@danielkrau
Copy link

fork & edit?

@tj
Copy link
Contributor Author

tj commented Feb 23, 2012

@danielkrau it's not difficult to just re-do it, that's only an hour or so work from last night

@tj tj closed this as completed in d4666c2 Feb 24, 2012
@danielkrau
Copy link

@visionmedia oh, ok. if it's that easy...coveraje is clearly not that beautiful, but it works (and now I'm going to investigate your solution)

@BryanDonovan
Copy link
Contributor

Thanks for adding this, TJ. It's working well for me. Had to jump through some hoops since our "lib" is named "src" though. In case anyone else has some trouble, the workaround for me is to have all our tests use a custom require function (which we had anyway, but not it checks if we're running jscov):

var lib_dir = 'src';

var require_src = function(filepath) {
    if (process.env.JSCOV) {
        lib_dir = 'lib-cov';
    }

    var root_dir = path.resolve(__dirname, "../../");
    return require(path.join(root_dir, lib_dir, filepath));
};

// In a test file:a
var foo = require_src('bar/foo');
// instead of
var foo = require('../../src/bar/foo');

And our Makefile sets JSCOV=1 when running 'make test-cov'.

This is pretty hacky though. Am I just not noticing a simpler solution?

@tj
Copy link
Contributor Author

tj commented Mar 4, 2012

@BryanDonovan the directory names is not important. All you have to do instrument the lib, in your case something like:

$ jscoverage src src-cov

create an ./index.js file so that you can alter which ./src lib is actually used:

module.exports = process.env.MYLIB_COV
  ? require('./src-cov')
  : require('./src');

That's pretty much it, it's not as transparent as it used to be with node require.paths but it's not all that hacky. In general it is bad-practice to require nested mods though (require('foo/bar/baz') so that may make things a bit more difficult if you're doing that

@BryanDonovan
Copy link
Contributor

Ah, thanks. I'll try that. This application was set up before I came on board, so I just went with what others were doing.

Why is requiring nested mods a bad practice? For us it's not so much nested, it's just an organization structure. Something like:

our_app/src/apps/app_one/model.js
our_app/src/apps/app_two/model.js
our_app/src/common/some_shared_module.js

I.e., we have several "apps" in parallel in a bigger application so they can share common code without having to deal with git submodules, etc.

One thing that I hated at first about node (or our setup) was having to require every little file. But now I kind of like it. It's very explicit what you're requiring. But maybe there's a big downside I've overlooked?

@tj
Copy link
Contributor Author

tj commented Mar 4, 2012

yeah it's annoying and nice at the same time haha. It's fine for your own application I guess, but for libraries you should expose things via the single require(), something like require('express').Router vs require('express/router') for example. I guess your hack isn't so bad then for your use-case, I would definitely try and expose models etc via the main module, might help some of the .../.../.././/././/. pain

@BryanDonovan
Copy link
Contributor

Oh, I see what you're saying. Yeah, for a library (and some internal libraries) we use a single require.

@BryanDonovan
Copy link
Contributor

Oh, another question: It seems that if a file is not loaded at all by the test suite, then it doesn't show up in the coverage report. I suppose this would be solved by using an index file that loads all the source code or something, but it seems like that would make some mocking/stubbing difficult. It would be nice if it reported on all files that are in lib-cov, not just the ones that were required by the tests. This may be a limitation of jscoverage though?

@tj
Copy link
Contributor Author

tj commented Mar 4, 2012

yeah that's because jscoverage populates the $_jscoverage global with the source etc

@BryanDonovan
Copy link
Contributor

Ok, I figured it was something like that. Thanks for all the help.

@alfredwesterveld
Copy link

I was wondering if the native javascript code coverage is abandoned because at post #5 (comment) you said you would try and built that which I think would be awesome, but then again I think it might take significant time.

Is this jscoverage is just a temporary (quick fix)solution, because it is not native?

@tj
Copy link
Contributor Author

tj commented Mar 19, 2012

@alfredwesterveld jscov works really well so there's not really a great reason to reinvent it, especially since it's a one-time install. Maybe in the future with esprima but it's not a huge deal

@domenic
Copy link
Contributor

domenic commented Mar 19, 2012

I might be missing something, but we tried to use it on Windows and it wouldn't install. So there's that.

Coveraje works pretty well, and I've also seen node-cover.

@tj
Copy link
Contributor Author

tj commented Mar 19, 2012

yeah I guess windows too

@tj
Copy link
Contributor Author

tj commented Mar 19, 2012

effectively anything that produces the same result as jscoverage would work just fine with what is existing right now, which isn't too hard but it's not top-priority for me

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests