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

Proof of concept jscoverage support #77

Merged
merged 1 commit into from
Jan 22, 2013
Merged

Conversation

alexspeller
Copy link
Contributor

Hiya, I was looking for a way of getting code coverage working as part of our CI build. This feature needs some work, but I wonder if you're interested in supporting code coverage in this gem?

The argument could be made for a separate gem, except that (a) it's a tiny amount of code and (b) as it is generated by the tests actually running it either needs to hook in deeply in this gem or run the tests twice which would be annoying.

Please let me know what you think, if you're generally supportive of the idea I can put some more work into this feature.

This uses the jscoverage binary from http://siliconforks.com/jscoverage/

@netzpirat
Copy link
Contributor

This looks very awesome and I really would like to have this in guard-jasmine! I didn't have any time to give it a try on the weekend, but will certainly do on Monday. Thanks a lot!

@alexspeller
Copy link
Contributor Author

Awesome! Here are the issues I see with it at the moment:

  1. The instrumented js files are cached with the same key as the non instrumented files - currently you need to rake tmp:cache:clear to change from coverage on to off, and also restart the web server or the jasmine test server.
  2. Needs to be switched on / off with a config setting, not environment variable probably
  3. Maybe check for existence of jscoverage binary better?
  4. Configurable setting of percentage of code coverage that causes failure, or option to have it not fail based on percentage at all (i.e. just informational)
  5. Better jasmine coverage - there's no coverage of the coffeescript bit yet (but there don't seem to be any specs for that file at all at the moment)
  6. Better ruby specs I'm not sure if I covered the cases for options[:notification] or options[:hide_success]
  7. JSCoverage has a browser interface - maybe this could be mounted on a rack endpoint or something?
  8. Maybe spit out sections of uncovered code?

As you can see, this is a bit of a brain dump. I'll have a look at the caching bug for now, when you have some time let me know what else you think should be included in the first attempt.

@DFrenkel
Copy link

+1 - this would be super-useful! Would also be nice to be able to generate jUnit-style XML output so detailed test results can be integrated with Jenkins

@netzpirat
Copy link
Contributor

I'm so, so sorry, there's really nothing wrong with this pull request, it's just that I have to much other things to do that are more important to me. I'd blindly merge it if someone can rebase it and add a section to the README about the usage.

@randallb
Copy link

randallb commented Jan 8, 2013

@alexspeller I know you don't owe me anything, but any way you can rebase and add a readme so it can get integrated? I don't know enough about git to do that yet, but would love to have test coverage on js.

@alexspeller
Copy link
Contributor Author

@randallb I simply don't have the time at the moment, although I would like to come back to it when I get some time if no-one else has got to it yet. Sorry!

@netzpirat
Copy link
Contributor

I have finally some good news on this pull request! Yesterday I merged it locally and tested it against the Haml_Coffee_Assets CoffeeScript spec suite. Sadly it didn't work initially (because my specs were in the wrong directory), so I started digging into the code and fixed some of the mentioned issues:

The instrumented js files are cached with the same key as the non instrumented files - currently you need to rake tmp:cache:clear to change from coverage on to off, and also restart the web server or the jasmine test server.

I just added it to the documentation and I think I'm going to add a Jasmine cleanup rake task, which also would clean the coverage data (more on this later).

Needs to be switched on / off with a config setting, not environment variable probably

I added a coverage option to the Guard plugin, which basically sets your environment variable. This is needed to support spec generation when using the Jasmine runner in another process.

Maybe check for existence of jscoverage binary better?

Done. There was already a which helper method in the Util module.

Configurable setting of percentage of code coverage that causes failure, or option to have it not fail based on percentage at all (i.e. just informational)

I have switched the coverage from JScoverage to Istanbul, because JSCoverage isn't maintained anymore and it's successor JSCover is now written in Java. Istanbul is actively developed, has already a nice feature set and is pure JavaScript.

There are several report formats and the text reporter can be directly used in the Guard Jasmine reporter. It looks like

--------------------------------+-----------+-----------+-----------+-----------+
File                            |   % Stmts |% Branches |   % Funcs |   % Lines |
--------------------------------+-----------+-----------+-----------+-----------+
   spec/javascripts/            |       100 |       100 |       100 |       100 |
      hamlcoffee_spec.js.coffee |       100 |       100 |       100 |       100 |
      spec.js.coffee            |       100 |       100 |       100 |       100 |
   vendor/assets/javascripts/   |     98.04 |     75.86 |     86.67 |     98.04 |
      hamlcoffee.js.coffee.erb  |     98.04 |     75.86 |     86.67 |     98.04 |
--------------------------------+-----------+-----------+-----------+-----------+
All files                       |     99.44 |     75.86 |     97.47 |     99.44 |
--------------------------------+-----------+-----------+-----------+-----------+

Or

=============================== Coverage summary ===============================
Statements   : 99.44% ( 179/180 )
Branches     : 75.86% ( 22/29 )
Functions    : 97.47% ( 77/79 )
Lines        : 99.44% ( 179/180 )
================================================================================

Better jasmine coverage - there's no coverage of the coffeescript bit yet (but there don't seem to be any specs for that file at all at the moment)

That would be nice, but I don't have the time at the moment and that's also the reason why there are no tests :)

I still need some days to address the rest, but coverage support for Guard::Jasmine is very, very close.

@alexspeller
Copy link
Contributor Author

👍 thanks for your work on this and guard-jasmine in general!

@netzpirat
Copy link
Contributor

Ah, totally forgot: Istanbul uses escodegen as generator and CoffeeScriptRedux escodegen and cscodegen, so I think it's possible to bring CoffeeScript support to Istanbul.

@netzpirat
Copy link
Contributor

🍻

@randallb
Copy link

YAY!! Thanks guys. You're awesome. 😺

@netzpirat
Copy link
Contributor

If anyone is interested in playing with it, please use the coverage branch. All planned features are finished, I just want to use it for some days and see if something pops up. The README has also been updated and theres a lot of information in the coverage section.

@netzpirat netzpirat merged commit e4c15bc into guard:master Jan 22, 2013
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

Successfully merging this pull request may close these issues.

None yet

4 participants