Skip to content

code coverage grunt tasks #550

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

Merged
merged 14 commits into from
Nov 18, 2013
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -23,4 +23,5 @@ docs/js/examples
docs/downloads
examples/shared/*.js
test/the-files-to-test.generated.js
sauce_connect.log*
*.log*
chrome-user-data
13 changes: 13 additions & 0 deletions Gruntfile.js
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,11 @@ module.exports = function(grunt) {
grunt.registerTask('build:transformer', ['jsx:debug', 'browserify:transformer']);
grunt.registerTask('build:min', ['jsx:release', 'version-check', 'browserify:min']);
grunt.registerTask('build:addons-min', ['jsx:debug', 'browserify:addonsMin']);
grunt.registerTask('build:withCodeCoverageLogging', [
'jsx:debug',
'version-check',
'browserify:withCodeCoverageLogging'
]);
grunt.registerTask('build:test', [
'jsx:test',
'version-check',
Expand All @@ -78,11 +83,19 @@ module.exports = function(grunt) {

grunt.registerTask('webdriver-phantomjs', webdriverPhantomJSTask);

grunt.registerTask('coverage:parse', require('./grunt/tasks/coverage-parse'));

grunt.registerTask('test:webdriver:phantomjs', [
'connect',
'webdriver-phantomjs',
'webdriver-jasmine:local'
]);
grunt.registerTask('test:coverage', [
'build:test',
'build:withCodeCoverageLogging',
'test:webdriver:phantomjs',
'coverage:parse'
]);
grunt.registerTask('test', ['build:test', 'build:basic', 'test:webdriver:phantomjs']);
grunt.registerTask('npm:test', ['build', 'npm:pack']);

Expand Down
15 changes: 14 additions & 1 deletion grunt/config/browserify.js
Original file line number Diff line number Diff line change
Expand Up @@ -95,10 +95,23 @@ var addonsMin = grunt.util._.merge({}, addons, {
after: [minify, bannerify]
});

var withCodeCoverageLogging = {
entries: [
'./build/modules/React.js'
],
outfile: './build/react.js',
debug: true,
standalone: 'React',
transforms: [
require('coverify')
]
};

module.exports = {
basic: basic,
min: min,
transformer: transformer,
addons: addons,
addonsMin: addonsMin
addonsMin: addonsMin,
withCodeCoverageLogging: withCodeCoverageLogging
};
58 changes: 54 additions & 4 deletions grunt/config/server.js
Original file line number Diff line number Diff line change
@@ -1,11 +1,62 @@
'use strict';

module.exports = function(grunt) {
module.exports = function(grunt){
var coverageWriteStream;

grunt.task.registerTask('finalize-coverage-stream', function(){
if (!coverageWriteStream) {
return;
}
var done = this.async();
coverageWriteStream.once('close', done);
coverageWriteStream.end();
coverageWriteStream = null;
});

function consoleLoggerMiddleware(req, res, next) {
if (!(req.method == 'POST' && req._parsedUrl.pathname.replace(/\//g,'') == 'console' && Array.isArray(req.body))) {
return next();
}
res.write('<!doctype html><meta charset=utf-8>');
res.end('Got it, thanks!');

req.body.forEach(function(log){
if (log.message.indexOf('not ok ') === 0) {
log.type = 'error';
} else if (log.message.indexOf('ok ') === 0) {
log.type = 'ok';
} else if (log.message.indexOf('COVER') === 0) {
log.type = 'coverage';
} else if (log.message.indexOf('DONE\t') === 0) {
log.type = 'coverage done';
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for updating the style here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/me almost fails to completely suppress the desire to discuss code formatting style
You're welcome.


if (log.type == 'error') {
grunt.log.error(log.message);
} else if (log.type == 'ok') {
grunt.log.ok(log.message);
} else if (log.type == 'log') {
grunt.log.writeln(log.message);
} else if (log.type == 'coverage') {
if (!coverageWriteStream) {
coverageWriteStream = require('fs').createWriteStream(__dirname + '/../../coverage.log');
}
coverageWriteStream.write(log.message + '\n');
} else if (log.type == 'coverage done') {
grunt.task.run('finalize-coverage-stream');
} else {
grunt.verbose.writeln(log);
}
});
}

function testResultLoggerMiddleware(req, res, next) {
if (!(req.method == 'POST' && req._parsedUrl.pathname.indexOf('/reportTestResults') === 0)) {
return next();
}
res.write('<!doctype html><meta charset=utf-8>');
res.end('Got it, thanks!');

var logType = 'writeln';
var message = req.body;

Expand All @@ -23,8 +74,6 @@ module.exports = function(grunt) {
message = JSON.stringify(message, null, 2);
}
grunt.log[logType]('[%s][%s]', req.headers['user-agent'], Date.now(), message);
res.write('<!doctype html><meta charset=utf-8>');
res.end('Got it, thanks!');
}

return {
Expand All @@ -40,13 +89,14 @@ module.exports = function(grunt) {

return [
connect.json(),
consoleLoggerMiddleware,
testResultLoggerMiddleware,

connect.logger({format:'[:user-agent][:timestamp] :method :url', stream:grunt.verbose}),
connect.static(options.base),
connect.directory(options.base)
];
},
}
}
}
};
Expand Down
2 changes: 1 addition & 1 deletion grunt/tasks/browserify.js
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ module.exports = function() {
};

// TODO: make sure this works, test with this too
config.transforms.forEach(bundle.transform, this);
config.transforms.forEach(bundle.transform, bundle);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What arguments does bundle.transform expect?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

forEach(foo, bar) does the equivalent of foo.call(bar, …).
This was failing because its this wasn't what it expected.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To answer my own question: I guess bundle.transform takes a transform function to apply to the bundle.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

exactly


// Actually bundle it up
var _this = this;
Expand Down
53 changes: 53 additions & 0 deletions grunt/tasks/coverage-parse.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
"use strict";
var grunt = require('grunt');

module.exports = function(){
var ROOT = require('path').normalize(__dirname + '/../..');
var done = this.async();
var uncoveredExpressionCount = 0;
var uncoveredLineCount = 0;

require('fs').createReadStream(ROOT + '/coverage.log')
.pipe(require('coverify/parse')(function(error, results){
if (error) {
grunt.fatal(error);
}

Object.keys(results)
.sort(function(a, b){
return results[a].length - results[b].length;
})
.reverse()
.forEach(function(path){
if (results[path].length === 0) {
return;
}
var relativePath = path.replace(ROOT, '');
uncoveredExpressionCount += results[path].length;
grunt.log.error(results[path].length + ' expressions not covered ' + relativePath);

results[path].forEach(function(c){
uncoveredLineCount += c.code.split('\n').length;
console.log('txmt://open?url=' + encodeURIComponent('file://' + path) + '&line=' + (c.lineNum+1) + '&column=' + (c.column[0]+2));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note to self: figure out how to log a similar URL for Sublime text and/or Emacs.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sublime Text can be made to support txmt:// urls. I don't know about any others.

});
console.log('');
})
;

Object.keys(results).sort().forEach(function(path){
if (results[path].length > 0) {
return;
}
var relativePath = path.replace(ROOT, '');
grunt.log.ok('100% coverage ' + relativePath);
});

if (uncoveredExpressionCount > 0) {
grunt.log.error(uncoveredExpressionCount + ' expressions not covered');
}
if (uncoveredLineCount > 0) {
grunt.log.error(uncoveredLineCount + ' lines not covered');
}
done();
}));
};
2 changes: 1 addition & 1 deletion grunt/tasks/populist.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,6 @@ module.exports = function() {
}).then(function(output) {
grunt.file.write(config.outfile, output);
theFilesToTestScript.end();
done();
theFilesToTestScript.once('close', done);
});
};
3 changes: 2 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,8 @@
"grunt-contrib-connect": "~0.5.0",
"es5-shim": "~2.1.0",
"wd": "~0.2.2",
"sauce-tunnel": "~1.1.0"
"sauce-tunnel": "~1.1.0",
"coverify": "~0.1.1"
},
"engines": {
"node": ">=0.10.0"
Expand Down
4 changes: 3 additions & 1 deletion src/environment/__tests__/ReactWebWorker-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@ describe('ReactWebWorker', function() {
var data = JSON.parse(e.data);
if (data.type == 'error') {
error = data.message + "\n" + data.stack;
} else if (data.type == 'log') {
console.log(data.message);
} else {
expect(data.type).toBe('done');
done = true;
Expand All @@ -42,7 +44,7 @@ describe('ReactWebWorker', function() {
});
runs(function() {
if (error) {
console.log(error);
console.error(error);
throw new Error(error);
}
});
Expand Down
28 changes: 23 additions & 5 deletions src/test/worker.js
Original file line number Diff line number Diff line change
@@ -1,20 +1,38 @@
/* jshint worker: true */
"use strict";

if (typeof console == 'undefined') {
this.console = {
error: function(e){
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you make these methods take multiple arguments, like the standard console.error and console.log methods?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

YAGNI. As soon as we need that, absolutely.

postMessage(JSON.stringify({
type: 'error',
message: e.message,
stack: e.stack
}));
},
log: function(message){
postMessage(JSON.stringify({
type: 'log',
message: message
}));
}
}
}

console.log('worker BEGIN');

// The UMD wrapper tries to store on `global` if `window` isn't available
var global = {};
importScripts("phantomjs-shims.js");

try {
importScripts("../../build/react.js");
} catch (e) {
postMessage(JSON.stringify({
type: 'error',
message: e.message,
stack: e.stack
}));
console.error(e);
}

postMessage(JSON.stringify({
type: 'done'
}));

console.log('worker END');
28 changes: 19 additions & 9 deletions test/lib/reportTestResults.browser.js
Original file line number Diff line number Diff line change
@@ -1,34 +1,44 @@
var __DEBUG__ = location.search.substring(1).indexOf('debug') != -1;

if (typeof console == 'undefined') console = {
log: function(){},
warn: function(){},
error: function(){}
};

var __consoleReport__ = [];

console._log = console.log;
console.log = function(message){
console._log(message);
postDataToURL({type:'log', message:message}, '/reportTestResults');
if (__DEBUG__) postDataToURL({type:'log', message:message}, '/reportTestResults');
else __consoleReport__.push({type:'log', message:message});
}

console._error = console.error;
console.error = function(message){
console._error(message);
postDataToURL({type:'error', message:message}, '/reportTestResults');
if (__DEBUG__) postDataToURL({type:'error', message:message}, '/reportTestResults');
else __consoleReport__.push({type:'error', message:message});
}

console._flush = function(){
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

right here

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh derp, nvm

postDataToURL(__consoleReport__, '/console');
__consoleReport__.length = 0;
}

;(function(env){
env.addReporter(new jasmine.JSReporter());
if (location.search.substring(1).indexOf('debug') != -1){
env.addReporter(new TAPReporter(console.log.bind(console)));
}
env.addReporter(new TAPReporter(console.log.bind(console)));

function report(){
if (typeof jasmine.getJSReport != 'function') {
console.log("typeof jasmine.getJSReport != 'function'");
return setTimeout(report, 100);
}
postDataToURL(jasmine.getJSReport(), '/reportTestResults', function(error, results){
if (error) return console.error(error);
});
if (!__DEBUG__) {
console.log('DONE\t' + navigator.userAgent);
console._flush();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is console._flush() necessary here? Is it a widely-implemented method? Maybe call it only if it exists?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I implemented it a few lines up.

}
}

var oldCallback = env.currentRunner().finishCallback;
Expand Down