Build: Add Istanbul for code coverage reports #143
Conversation
|
By analyzing the blame information on this pull request, we identified @markelog, @jzaefferer and @mgol to be potential reviewers |
|
Wow, I wouldn't expect it to be so easy to add. :) |
|
That's instanbul over phantom, without need for ajax requests, shouldn't be hard at all |
| "qunitjs": "1.17.1", | ||
| "testswarm": "~1.1.0" | ||
| }, | ||
| "scripts": { | ||
| "prepublish": "grunt" |
|
Might be good idea to add integration with coveralls too |
| @@ -122,7 +131,7 @@ module.exports = function(grunt) { | |||
| grunt.loadNpmTasks("grunt-contrib-watch"); | |||
| grunt.loadNpmTasks("grunt-contrib-jshint"); | |||
| grunt.loadNpmTasks("grunt-contrib-uglify"); | |||
| grunt.loadNpmTasks("grunt-contrib-qunit"); | |||
| grunt.loadNpmTasks("grunt-qunit-istanbul"); | |||
There was a problem hiding this comment.
Maybe you should consider using load-grunt-tasks like we do in core?
|
Also, you might as well add integration with travis |
|
@dmethvin i see you added travis, but what about my other comments? |
|
The prepublish is gone, something added it automatically. The phantomjs-polyfill isn't needed with grunt-qunit-istanbul afict, the tests pass fine without it so it's not a direct dependency. And if i don't generate coverage info regularly i won't look at it 😸 |
|
If you modify the package dependencies, you have to run In other words polyfill is still there - https://github.com/jquery/jquery-migrate/tree/a79bc6e9bf9e9a4aa3b2bb273db4f99735ba61dd/external/phantomjs-polyfill And - jquery-migrate/test/index.html Line 12 in a79bc6e But if you remove it, the tests will fail,
If you integrate with coveralls, you can add a badge to the readme and also see how coverage changed with every pull request, since coveralls bot will tell you about it. For example see express repo - https://github.com/expressjs/express Also, what about |
I meant hook there |
|
Oh man I forgot about npmcopy, just like I used to forget about bowercopy. The dependencies don't change often enough for it to be a habit. I can add the load-grunt-tasks stuff but in some ways I like doing it explicitly so I know what I've loaded. |
Ok-ok, totally reasonable What about coveralls? I can provide a pull for that this weekend, if that too big task to dig. |
|
Sure, sounds good. I'm sure you can do it in your sleep! |
Coverage test is running against the Phantom build but it's still useful for some basic sanity checking. It will become more so once we move to supporting only a single version of jQuery and don't conditionally fill APIs.