Another shot at code coverage - via JSCover

Nov 19, 2012 at 9:05 PM
Edited Nov 19, 2012 at 9:06 PM

I did this before reading http://chutzpah.codeplex.com/discussions/380448, so perhaps this is redundant, but here goes anyway.

I forked Chutzpah (http://chutzpah.codeplex.com/SourceControl/network/forks/provegard/Chutzpah) and added support for code coverage collection using JSCover. The downside is of course that it requires Java to work. The upside is that it's entirely file-based - no server. The main reason I chose JSCover over JSCoverage, however, is that the latter is superseded by the former.

Note that my primary scenario is not to get code coverage in Visual Studio, but rather on a build server. Therefore, I chose to simply let the console runner dump the contents of the _$jscoverage object (slightly modified, as it's not fit for JSON serialization) to a file. The build server script can then do post-processing that fits the "destination" (HTML publishing, format conversion, etc.).

Interesting?

Coordinator
Nov 20, 2012 at 2:53 PM

Really nice work. I did a quick once over and it looks pretty good. I would give a closer look with comments during a pull request but this is very impressive work.

A couple things that jumped out at me:

1. What will happen when the user does not have Java? Will it just crash or let them know nicely?

2. Is it possible to make the results of the coverage strongly typed? It is currently just an object but would be great if it was a model we could traverse over?

3. I am not a lawyer so I am not sure about this but JSCover is release under GPLv2. Do you know what the implications of packaging that project with Chutzpah would be? 

 

 

Thanks for working on this. Many people have requested this feature so this will be great.

Nov 20, 2012 at 4:20 PM

Thanks, glad you liked it! In response to your questions:

1) If the /coverage option is given and Java.exe cannot be found, an exception will be thrown. This happened to a colleague of mine today, and to be honest the message wasn't very user-friendly since it was embedded in a big scary stack trace. :-)

I couldn't immediately find a mechanism for printing a warning or user-friendly error message, but that would be useful. Perhaps a log service? Or do you have some other idea?

2) Of course! At first I thought that the object would be some sort of dynamic, but I later discovered that it becomes a string. This worked for me because all I do is dump it to a file anyway (and it felt unnecessary to build a full-fledged object just to convert it to a string again right away). But a benefit of a strongly typed object is of course that it can be easily used in the VS plugin for further processing.

3) Oops, didn't think of that! I'm not a lawyer either, but I'll read through the license to see if I find anything. Currently, I chose to bundle JSCover-all.jar, but that's just a convenience. An options is to require the user to download it herself due to possible licensing restrictions.

Some other things worth noticing:

* JSCover chokes on UTF-8 BOM (which I discovered when I used CopyFile rather than Read/Write), *unless* you run it with "-Dfile.encoding=utf-8". I opened a JSCover issue and asked if it would make sense for JSCover to auto-detect the encoding. If not, then Chutzpah might have to do that, or simply give up and require the user to fix the problem.

* When I first ran the coverage collection on a build server, the test count sky-rocketed. The reason was (of course) that during the separate coverage run, Chutzpah would emit TeamCity services messages just as usual. I temporarily solved it by discarding the output altogether, but one idea is to let the /teamcity option take an optional boolean so that TeamCity output can be forcibly turned off.

Coordinator
Nov 21, 2012 at 5:00 AM

1. I have been meaning to add better logging into Chutzpah and have not gotten around to it. Since it might be a common issue maybe performing the check early as part of input validation in the ConsoleRunner project might be helpful.

 

2. Yea I would definitely love to see the strongly typed model since it opens the door to other integration. Would it be a lot of work?

3. Let me know what you find.

Nov 21, 2012 at 9:32 PM

1) I agree that performing the check on startup is a good idea. I'll look into that!

2) No, not a lot of work. Stay tuned for an update in a few days. :-)

3) I found an interesting blog post with some good discussion that may relate to this situation:

http://quoderat.megginson.com/2009/06/16/mixing-gpl-and-non-gpl-a-different-perspective/

I also vaguely recall that once when I installed Java under Ubuntu or Debian, Sun's JDK wasn't in the repos due to licensing problems. But you could download it yourself and install it. And the repos did contain an installer script that did that for you, but you had to accept the license.

I think the safer choice is to not ship JSCover-all.jar but have the user download it. If it's not found as part of application startup, we can print an informative message and provide a link to the JSCover download page. What do you think?

Nov 26, 2012 at 6:27 PM

Alright, I just pushed an update. Differences:

  • JSCover-all.jar is no longer bundled. This way, it's neither copied, distributed nor modified.
  • A prerequisites check is made at startup. Chutzpah stops if java.exe or JSCover-all.jar cannot be found. It provides a link to where the user can download the latter.
  • JSCover is run with the --branch flag to enable branch coverage collection. This requires the latest version. I thought about adding a flag for turning it on and off, but never did. Thoughts?
  • JSCover's jscoverage.js (generated during the instrumentation phase) is put in the test harness to do the JSON serialization, rather than Chutzpah having its own version.
  • The coverage object is strongly typed (but a JSON copy is saved as well for easy dumping to file).

Let me know what you think!

Coordinator
Nov 28, 2012 at 2:57 PM

Changes look pretty good. 

1. Could we add a testoption / command line argument that lets you specify where the jscover jar file is? This would make it easier to integrate this into VS since the user wont be able to access the bin directory for chutzpah.

 

2. I just noticed this project (http://migrii.github.com/blanket/) it is also a code coverage project but it is written all in JavaScript? Thinking about in the future supporting multiple different frameworks to perform code coverage is there anything we would need to do to make the code more general?

 

Dec 2, 2012 at 8:46 PM

1. Sure, not a problem.

2. I just had a quick look at the Blanket.js source code. There are fragments of Jasmine support although they say it's not fully there yet. Also, I found the _$jscoverage variable name in the code, which indicates that the resulting coverage object MAY be compatible with that of JSCover - which would be very nice of course. To be able to have some sort of generic coverage tool support, all tools would have to be able to instrument arbitrary code as a separate task. I haven't tested Blanket.js to see if it can do that or if it relies on being part of the HTML page in order to instrument code in script tags. I'll take a look when I have time.

One idea is to have several ICoverageEngine implementations, and select the correct one based on the supplied coverage tool file (i.e., JSCover-all.jar or blanket.js). Essentially ask each ICoverageEngine if it handles the given tool. Once one claims it does, we select that one as the one to use. Is it possible to "lock" an instance in StructureMap once you know which one to use? I.e. some sort of singleton but selected in runtime based on configuration?

Coordinator
Dec 5, 2012 at 3:10 PM

That sounds good. I just came across another pure JS coverage library also: https://github.com/yahoo/istanbul .

It is cool to see new libraries coming out that are all JS. They all seem to be built in Esprima which is a pure JavaScript parser written by the same guy who created PhantomJS.

Dec 5, 2012 at 3:35 PM

I tried to get Blanket to do instrumentation only via Node (as an experiment), but didn't succeed. It's designed to be within a browser.

Istanbul looks promising, it has an explicit instrumentation command.

If Instanbul meets our needs, why bother with other frameworks? In particular, we can skip JSCover and just forget about the Java dependency as well as licensing issues.

I'm also not a big fan of designing for multiple frameworks "just in case". Let's say that I can fit Instanbul into Chutzpah, what degree of "genericness" are you looking for? The instrumentation phase is already located in the coverage engine, and I suppose we can delegate the coverage object parsing to the engine as well (instead of in the reader where it is now). Getting the coverage object from PhantomJS to Chutzpah is harder to do though, since that's JavaScript code. Perhaps the coverage engine can create a piece of JavaScript code that can be put into (jasmine|qunit)-runner.js. What do you think?

Coordinator
Dec 5, 2012 at 4:55 PM

That is odd about Blanket not working in Node but I wonder if it would work fine using PhantomJS since it is a headless browser?

I agree with you, if we can find a good coverage library with a compatible license then it is not worth it to create a general framework for supporting other coverage frameworks. If Blanket or Istanbul meets those requirements than I see no need for JSCover or for it being generic. I was original asking about the generic since the JsCover option was so heavy weight.

 

As for the integration into Chutzpah for Istanbul or Blanket, I would love to hear more details about your ideas. I have some thoughts but since you have been thinking more about this I would love to see how you envision it working.

 

Dec 5, 2012 at 7:16 PM

As for Blanket, I tried requiring it in Node, but in that case it goes into some sort of require.js mode which fails since it's a bundled script file. There are some window checks which trigger a different way of working. So yes, it would probably work in PhantomJS.

If Istanbul can instrument a bunch of files, I think it can be used in the same was that I used JSCover. Basically instrument all relevant files once Coffee/TypeScript compilation is done. Instrumentation could happen inside the file generators of course (I tried that at first), but that is more complex and also slower than doing batch instrumentation.

After tests have run, there's the coverage collection phase, which essentially consists of getting the JS coverage object into a complete, typed .NET object.

Basically, my idea is to leave Chutzpah as the test runner and only trigger the coverage framework before and after.

Coordinator
Dec 5, 2012 at 8:00 PM

Hooking in before and after the test running sounds good to me. This would be super cool to get this working without needed Java. I will also start investigating what the integration scenario inside VS would be.

Dec 10, 2012 at 4:56 PM

Instrumenting with Instanbul is really easy, but it's a NodeJs app with 15 dependencies (I haven't gone through their licenses). Any thoughts on bundling node.exe, Istanbul and all its dependencies with Chutzpah? Having the user download it isn't really a good option as it is a lot more work compared to downloading JSCover...

Coordinator
Dec 10, 2012 at 6:44 PM

Oy, I didn't realize it required Node.js. I was hoping it would be able to just run in a browser. While me might not have the same licensing issues with Node.js it would balloon the package size for Chutzpah and introduce a heavy-weight dependency.

It might be worth taking another look at Blanket.JS since that one is designed to work in the browser.

Dec 18, 2012 at 1:22 PM

I just wanted you to know that I've started looking at integrating BlanketJS, and I don't foresee any problems with that. I've just had some time shortage. :-) Do you have any deadline for the next release in case you want to have coverage support in place by then?

Blanket only supports line coverage right now, unless I missed something. Do you have any idea about the integration scenario in VS yet?

Coordinator
Dec 18, 2012 at 2:31 PM

That is awesome. I don't have any hard deadline but if you could give me a time frame I can decide to release twice or just once. I am excited to see how it works with Blanket.js. 

 

I have a couple ideas around VS integration. One option would take a bit of effort and has a lot of unknowns. It involves creating a custom data collector (http://msdn.microsoft.com/en-us/library/tfs/dd286737(v=vs.110).aspx). I haven't dug into this much.

 

My preferred option is to figure out if I can detect the user has code covereage option selected then I can just generate our own HTML code coverage report and launch it. That would be ideal and much simpler.

Dec 19, 2012 at 1:47 AM

Hi guys, I'm Alex Seville, the core contributor of Blanket.js.  I love the idea of Blanket being used for Chutzpah, and I'm happy to answer any questions you might have.

A few points have been mentioned, that I thought I could address:

1. Blanket runs in both the browser and nodejs.  It shares some source code, but the node and browser versions are different builds.  The browser version is available on the home page, or via bower (bower install blanket) and the node version is available on npm (npm install blanket).

2. The browser version of blanket does assume the existence of a DOM, but there are some ways to prevent it from searching for script tags to instrument.

3. The browser version runs with phantomjs.  In fact, we have more than half a dozen different test runners (for qunit, requirejs, jasmine, etc...) that all run using phantomjs.

4. For node we support mocha, and mocha uses jscoverage by default, so Blanket copies it's coverage data into the _$jscoverage variable to piggyback off the existing coverage reporters available for mocha.  The browser version can easily do the same (via a custom reporter).

5. We have Blanket running with Jasmine without issue.  I've been in touch with the Jasmine team as well to ensure that we're compatible with the next release of Jasmine.  We have an API for custom adapters and reporters, and have already had a helpful soul create a mocha (for the browser) adapter for us that works.

If you have any other questions feel free to post here, or tweet at me (@alex_seville).

Thanks!

Dec 22, 2012 at 8:23 PM

I've just completed some initial testing with Blanket.js. Unsure if I'll manage to finish it this year. Here's what I've found so far:

* I receive the coverage object (_$blanket) with Jasmine but not with QUnit (this is in the integration test suite). Don't know why yet.

* The coverage object is not fully serializable, so right now I only get line execution counts. Perhaps I can do something with Blanket's JSON reporter.

* The script file names in the coverage object have their file endings stripped off. Not a big deal, but my tests expect to find the appropriate names. I can verify this in Chrome (with --allow-file-access-from-files) - there's a difference between referencing a file with src="file:///path/to/code.js" and src="code.js". In the former case, Chrome writes an error on the console indicating that it cannot GET the file without the extension. In the latter case, I get a coverage report. Alex, perhaps you can shed some light on this?

* Right now, I hack Blanket into the test harness HTML text. This means that the include/exclude patterns don't work since the HTML may contain temp file names (in case of compiled CoffeeScript, for example). I think I need to create some sort of "test harness model" and pass that to the coverage engine, so that it has more information to work with.

Dec 23, 2012 at 1:33 AM

I'm going to assume you're running blanket v1.0.0.

* You should get the window._$blanket object regardless of the test runner.  One reason why it might not appear is if it isn't instrumenting any files for whatever reason.

* Right now we only offer line counts, and not statement counts.  You should be seeing a source code array in the object as well though (".source").

* By file endings, do you mean ".js"? Since we use requirejs as our loader, if you run blanket in Chrome with locally referenced files you will get an error.  However, you shouldn't be getting an error regarding the extension.  Is the test suite you're running part of the Chutzpah source code?  Can I run it locally to see if this is a bug with Blanket?

* For the include/exclude patterns, I assume you're referring to adding "data-cover" to each script that needs to be included.  There is also an option of not adding anything to any of the script references, but adding a "data-cover-only" attribute to the blanket script reference.  You can provide it a string, regex, or array of filename to match.  There's a third option of even manually setting the filter yourself by adding a script block and making a call to "blanket.setFilter(<string, regex, or array of filenames>);".

Dec 24, 2012 at 9:43 AM

Hi Alex! I'm not sure about the Blanket version and I cannot check right now.

* This is probably an error on my part. I'll have to investigate further.

* I noticed that the array has a ".source" object. However, the _$blanket object is marshalled into Chutzpah as JSON, and when JSON.stringify serializes an array, it ignores any properties of the array. Thus, only the array items are serialized. I would like to avoid doing my own _$blanket manipulation, but I noticed that Blanket has a JSON reporter that I might be able to use. Note that I had the exact same problem with JSCover, as _$jscoverage was constructed in the same way. In that case, JSCover provided a function that created a more "serialization-friendly" object.

* Yes, the ".js" extension is stripped off. I realized after posting that it has to do with RequireJS. But I think this will be a non-issue, as I will have to do some reverse mapping of dictionary keys anyway. (But I did find it strange that I got a coverage report in Chrome for src="code.js" but not for src="file:///.../code.js".)

* The include/exclude problem is not really related to Blanket. It's not a problem adding "data-cover" to the script tags. However, if the input is a CoffeeScript file, then Chutzpah compiles it to JavaScript before creating the test harness. So while the original file name is "code.coffee", the "src" attribute of the script tag will contain something like "_Chutzpah.code.js", and the _$blanket dictionary will have "_Chutzpah.code" as key. What I need to do here is keep track of a mapping between original file names and compiled file names, so that a) I know which script tags to add "data-cover" to based on include/exclusion patterns that refer to original file names and b) I can rewrite the dictionary keys afterwards to be the original file names.

I'm off celebrating Christmas right now, so I will post an update in a couple of days.

Dec 29, 2012 at 12:40 PM

Time for a status update!

I've been working with Alex to sort out a few issues with the integration, but since I merged with the main repository I've hit a problem. You've added some code to qunitRunner.js to handle QUnit being included twice:

        // If additional referenced to QUnit are loaded trick them to not pollute global namespace
        window.exports = window.exports || {};

Unfortunately, this means that Blanket will try to load Esprima as a module:

var parse = (typeof exports === 'undefined' ? esprima : require("./esprima")).parse;

But 'require' isn't defined and I get an error at that point.

I'll try to see if I can find some other way to handle the problem with QUnit being included twice. I'll let you know what I find!

Dec 29, 2012 at 9:59 PM

Ok, so I've pushed my changes to my fork. Some notes:

* I broke out the test harness code to a separate TestHarness class. It's created as a model with references to scripts/stylesheets etc., then converted to HTML at a later stage. The code is slightly different from before, so you might want to double-check it. For example, I omitted the referenced files sorting on IsFileUnderTest, because I couldn't see the point of it and all tests pass anyway. :-)

* BlanketJsCoverageEngine modifies the TestHarness instance at "model level" to add attributes as necessary to enable coverage collection. It also injects some code in the test harness to "communicate" with the JS test runner.

* Coverage object deserialization is delegated to BlanketJsCoverageEngine by TestCaseStreamReader. The purpose of this is to allow the engine to adapt the coverage object to Chutzpah's representation.

* Currently, Blanket's coverage object is a dictionary where the values are arrays of line execution counts, as I mentioned earlier. We don't get the actual source lines due to JSON serialization issues, but I suppose that's fine. The source code is available elsewhere. :-)

* No coverage when the test file is an HTML file. I can make this happen, but then I'd have to abandon the TestHarness model and patch the HTML directly instead. More code, less flexibility, uglier.

* I solved the QUnit-being-included-twice problem by having a pass where the TestHarness model is cleaned up and any additional QUnit references are removed. This is done based on file name, please double-check if it makes sense. It means that it won't work when the test file is an HTML file, see previous bullet. :-(

The only outstanding issue is that I can't get coverage collection to work when the test file uses RequireJS to pull in additional test files (e.g. JS\Code\RequireJS\all.tests.qunit.js). With Chrome and QUnit, I get two passing tests but only coverage on the top-level file. With Chrome and Jasmine, I get no test execution output at all, but coverage on the top-level file. With Chutzpah/PhantomJS and any test framework, I get coverage on the top-level file and no test results. I think Blanket would have to support the use of RequireJS in this way for things to work, but I'm not sure. Alex, do you have any ideas?

Jan 7, 2013 at 9:51 PM

Status update: Almost everything is working now. There is a timeout issue that needs to be sorted out, and the performance isn't really top-notch. Having a separate instrumentation pass before launching the runner (as with JSCover) would be a lot faster, but the nice thing with having the instrumentation in the runner is that script files that are RequireJSed are covered as well.

Jan 30, 2013 at 6:33 PM

I am very excited to have code coverage as a part of Chutzpah, it is a great project.  Is there an update on this?

Feb 7, 2013 at 1:16 PM
Sorry for updating... everything should be working now with Blanket. Should I prepare a pull request?
Feb 7, 2013 at 1:16 PM
Sorry for not updating, that is... :-)
Coordinator
Feb 7, 2013 at 4:57 PM
Once you merge with the latest version pull request away!
Developer
Feb 7, 2013 at 10:53 PM
I've been lurking around this discussion for a while, and it's really exciting to see it come to fruition. Awesome work!

The JSCoverage implementation I started was never quite right, and was kinda forcing the tool to do something it was not designed for (not to mention bulking up the Chutzpah installer). So much cleaner and nicer using a native JS library like Blanket.js.

Makes me wonder what else we could use modern JS tools like Esprima for... test discovery maybe?
Mar 12, 2013 at 3:57 PM
Do you think this will make it in the next version?
Coordinator
Mar 12, 2013 at 3:58 PM
Yep! I have it checked in already.
Mar 12, 2013 at 4:28 PM
That's excellent. I look forward to using it. Thanks.
Mar 13, 2013 at 2:55 AM
Let me know when it's released and I'll mention Chutzpah on the Blanket Features & Compatibility page:
https://github.com/alex-seville/blanket/blob/master/docs/compatibility_and_features.md
Mar 16, 2013 at 9:58 PM
I am having some issues seeing any test results - any chance you could reply or email me AlexSeville?

Thanks
Greg
Mar 16, 2013 at 10:36 PM
Hi Greg, the best thing to do might be to open an issue on the project:
https://github.com/alex-seville/blanket/issues

That way myself or someone else might be able to fix the issue, and you can post stack traces or images.