From 9179f8dfdfe184d999b3fc35c52ab79da7782e0c Mon Sep 17 00:00:00 2001 From: Paul Pflugradt Date: Fri, 15 Aug 2014 12:20:28 +0200 Subject: [PATCH 1/6] Bugfix moved from grunt.util.spawn to node spawn to properly catch syntax errors. fixed tests --- .gitignore | 2 ++ Gruntfile.js | 50 +++++++++++++++++++++++++++++--------- tasks/lib/server.js | 34 +++++++++----------------- test/custom_delay_test.js | 2 +- test/custom_output_test.js | 2 +- test/custom_port_test.js | 2 +- test/server_malformed.js | 25 +++++++++++++++++++ 7 files changed, 79 insertions(+), 38 deletions(-) create mode 100644 test/server_malformed.js diff --git a/.gitignore b/.gitignore index b785247..9a06d10 100644 --- a/.gitignore +++ b/.gitignore @@ -1,3 +1,5 @@ node_modules npm-debug.log tmp +*.sublime-project +*.sublime-workspace \ No newline at end of file diff --git a/Gruntfile.js b/Gruntfile.js index 05c4eed..f51c59d 100644 --- a/Gruntfile.js +++ b/Gruntfile.js @@ -17,6 +17,7 @@ module.exports = function(grunt) { 'Gruntfile.js', 'tasks/**/*.js', 'test/*.js', + '!test/server_malformed.js' ], options: { jshintrc: '.jshintrc', @@ -80,10 +81,15 @@ module.exports = function(grunt) { custom_cmd: { options: { script: './test/server.coffee', - cmd: "coffee", + opts: ["node_modules/coffee-script/bin/coffee"], output: "Express server listening on port .+" } }, + malformed: { + options: { + script: './test/server_malformed.js' + } + }, custom_args: { options: { args: [1, 2], @@ -146,19 +152,39 @@ module.exports = function(grunt) { // plugin's task(s), then test the result. grunt.registerTask('test', [ 'clean', - 'express:defaults', 'nodeunit:defaults', 'express:defaults:stop', - 'express:custom_cmd', 'nodeunit:custom_cmd', 'express:custom_cmd:stop', - 'express:custom_args', 'nodeunit:custom_args', 'express:custom_args:stop', - 'express:custom_port', 'nodeunit:custom_port', 'express:custom_port:stop', - 'express:custom_node_env', 'nodeunit:custom_node_env', 'express:custom_node_env:stop', - 'express:custom_delay', 'nodeunit:custom_delay', 'express:custom_delay:stop', - 'express:custom_output', 'nodeunit:custom_output', 'express:custom_output:stop', - 'express:stoppable', 'express:stoppable:stop', 'nodeunit:stoppable', + 'nodeunit:defaults', + 'express:defaults', + 'express:defaults:stop', + 'express:malformed', + 'express:custom_cmd', + 'nodeunit:custom_cmd', + 'express:custom_cmd:stop', + 'express:custom_args', + 'nodeunit:custom_args', + 'express:custom_args:stop', + 'express:custom_port', + 'nodeunit:custom_port', + 'express:custom_port:stop', + 'express:custom_node_env', + 'nodeunit:custom_node_env', + 'express:custom_node_env:stop', + 'express:custom_delay', + 'nodeunit:custom_delay', + 'express:custom_delay:stop', + 'express:custom_output', + 'nodeunit:custom_output', + 'express:custom_output:stop', + 'express:stoppable', + 'express:stoppable:stop', + 'nodeunit:stoppable', // Multiple servers - 'express:custom_port', 'express:defaults', - 'nodeunit:defaults', 'nodeunit:custom_port', - 'express:custom_port:stop', 'express:defaults:stop', + 'express:custom_port', + 'nodeunit:defaults', + 'express:defaults', + 'nodeunit:custom_port', + 'express:custom_port:stop', + 'express:defaults:stop', ]); // By default, lint and run all tests. diff --git a/tasks/lib/server.js b/tasks/lib/server.js index eb8d681..11fd205 100644 --- a/tasks/lib/server.js +++ b/tasks/lib/server.js @@ -7,6 +7,7 @@ */ 'use strict'; +var spawn = require('child_process').spawn; module.exports = function(grunt, target) { if (!process._servers) { @@ -24,7 +25,6 @@ module.exports = function(grunt, target) { done = null; } }; - return { start: function start(options) { if (server) { @@ -75,13 +75,14 @@ module.exports = function(grunt, target) { } if (options.background) { - var donefunc = (options.delay || options.output) ? function() {} : finished; - server = process._servers[target] = grunt.util.spawn({ - cmd: options.cmd, - args: options.opts.concat(options.args), - env: process.env, - fallback: options.fallback - }, donefunc); + server = process._servers[target] = spawn( + options.cmd, + options.opts.concat(options.args), + { + env: process.env, + stdio: ['ignore', 'pipe', process.stderr] + } + ); if (options.delay) { setTimeout(finished, options.delay); @@ -96,32 +97,19 @@ module.exports = function(grunt, target) { } }); } - server.stderr.on('data', function(data) { - if (!options.debug) { - finished(); - } else { - var message = "" + data; - var regex = new RegExp('debugger listening', "gi"); - if (!message.match(regex)) { - finished(); - } - } - }); server.stdout.pipe(process.stdout); - server.stderr.pipe(process.stderr); + server.on('close',this.stop); } else { // Server is ran in current process server = process._servers[target] = require(options.script); } - - process.on('exit', finished); process.on('exit', this.stop); }, stop: function stop() { if (server && server.kill) { grunt.log.writeln('Stopping'.red + ' Express server'); - + server.removeAllListeners('close'); server.kill('SIGTERM'); process.removeListener('exit', finished); process.removeListener('exit', stop); diff --git a/test/custom_delay_test.js b/test/custom_delay_test.js index d435f39..f03e7e5 100644 --- a/test/custom_delay_test.js +++ b/test/custom_delay_test.js @@ -16,7 +16,7 @@ module.exports.custom_delay = { get('http://localhost:3000/hello.txt', function(res, body) { test.equal(res.statusCode, 200, 'should return 200'); - test.equal(body, 'Howdy!\n', 'should return static page'); + test.equal(body.trim(), 'Howdy!', 'should return static page'); test.done(); }, function(err) { test.done(); diff --git a/test/custom_output_test.js b/test/custom_output_test.js index 7bc6d32..068d3dc 100644 --- a/test/custom_output_test.js +++ b/test/custom_output_test.js @@ -16,7 +16,7 @@ module.exports.custom_output = { get('http://localhost:3000/hello.txt', function(res, body) { test.equal(res.statusCode, 200, 'should return 200'); - test.equal(body, 'Howdy!\n', 'should return static page'); + test.equal(body.trim(), 'Howdy!', 'should return static page'); test.done(); }, function(err) { test.done(); diff --git a/test/custom_port_test.js b/test/custom_port_test.js index 647594f..3eb0845 100644 --- a/test/custom_port_test.js +++ b/test/custom_port_test.js @@ -16,7 +16,7 @@ module.exports.custom_port = { get('http://localhost:8080/hello.txt', function(res, body) { test.equal(res.statusCode, 200, 'should return 200'); - test.equal(body, 'Howdy!\n', 'should return static page'); + test.equal(body.trim(), 'Howdy!', 'should return static page'); test.done(); }, function(err) { test.done(); diff --git a/test/server_malformed.js b/test/server_malformed.js new file mode 100644 index 0000000..b55c95f --- /dev/null +++ b/test/server_malformed.js @@ -0,0 +1,25 @@ +"use strict"; + +/** + * Test Server + */ + +var app = require('./app'); +var start = Date.now(); +var log = function(message) { + console.log("[" + (Date.now() - start) + "] " + message); +}; + +log"Begin server.js"); + +setTimeout(function() { + module.exports = app.listen(app.get('port'), function() { + log("Express server listening on port " + app.get('port')); + }); +}, 50); + +setTimeout(function() { + log("250ms timeout"); +}, 250); + +log("End server.js"); From 9a8882bb23aa35de592f0e2fa2f182308693c140 Mon Sep 17 00:00:00 2001 From: Paul Pflugradt Date: Fri, 15 Aug 2014 21:09:41 +0200 Subject: [PATCH 2/6] implemented #48 --- tasks/express.js | 3 ++- tasks/lib/server.js | 18 ++++++++++++++++-- 2 files changed, 18 insertions(+), 3 deletions(-) diff --git a/tasks/express.js b/tasks/express.js index 3b94c8f..aec5f97 100644 --- a/tasks/express.js +++ b/tasks/express.js @@ -31,7 +31,8 @@ module.exports = function(grunt) { port: process.env.PORT || 3000, delay: 0, output: ".+", - debug: false + debug: false, + logs: undefined }); options.script = path.resolve(options.script); diff --git a/tasks/lib/server.js b/tasks/lib/server.js index 11fd205..96dd83d 100644 --- a/tasks/lib/server.js +++ b/tasks/lib/server.js @@ -75,12 +75,16 @@ module.exports = function(grunt, target) { } if (options.background) { + var errtype = process.stderr; + if(options.logs && options.logs.err) { + errtype = 'pipe'; + } server = process._servers[target] = spawn( options.cmd, options.opts.concat(options.args), { env: process.env, - stdio: ['ignore', 'pipe', process.stderr] + stdio: ['ignore', 'pipe', errtype] } ); @@ -97,7 +101,17 @@ module.exports = function(grunt, target) { } }); } - server.stdout.pipe(process.stdout); + var out = process.stdout; + if(options.logs) { + var fs = require('fs'), path = require('path'); + if(options.logs.out) { + out = fs.createWriteStream(path.resolve(options.logs.out), {flags: 'a'}); + } + if(options.logs.err && errtype === 'pipe') { + server.stderr.pipe(fs.createWriteStream(path.resolve(options.logs.err), {flags: 'a'})); + } + } + server.stdout.pipe(out); server.on('close',this.stop); } else { // Server is ran in current process From 4de7dcd0066a124fa7e04fffaa5bdc617a2b55e1 Mon Sep 17 00:00:00 2001 From: Paul Pflugradt Date: Fri, 15 Aug 2014 22:06:33 +0200 Subject: [PATCH 3/6] Added documentation For the new logs feature --- README.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/README.md b/README.md index a20fc6e..3c004ae 100644 --- a/README.md +++ b/README.md @@ -93,6 +93,10 @@ or within each individual server task. // Set --debug debug: false + + // Object with properties `out` and `err` both will take a path to a log file and + // append the output of the server. Make sure the folders exist. + logs: undefined } } ``` From 975affc24f9a34ac6f640e53f97ecaa78fdc610b Mon Sep 17 00:00:00 2001 From: Kevin Cai Date: Mon, 15 Sep 2014 16:32:29 -0700 Subject: [PATCH 4/6] Added breakOnFirstLine option --- tasks/express.js | 23 ++++++++++++----------- tasks/lib/server.js | 6 +++++- 2 files changed, 17 insertions(+), 12 deletions(-) diff --git a/tasks/express.js b/tasks/express.js index aec5f97..4b601b7 100644 --- a/tasks/express.js +++ b/tasks/express.js @@ -22,17 +22,18 @@ module.exports = function(grunt) { var server = servers[this.target]; var action = this.args.shift() || 'start'; var options = this.options({ - cmd: process.argv[0], - opts: [ ], - args: [ ], - node_env: undefined, - background: true, - fallback: function() { /* Prevent EADDRINUSE from breaking Grunt */ }, - port: process.env.PORT || 3000, - delay: 0, - output: ".+", - debug: false, - logs: undefined + cmd: process.argv[0], + opts: [ ], + args: [ ], + node_env: undefined, + background: true, + fallback: function() { /* Prevent EADDRINUSE from breaking Grunt */ }, + port: process.env.PORT || 3000, + delay: 0, + output: ".+", + debug: false, + breakOnFirstLine: false, + logs: undefined }); options.script = path.resolve(options.script); diff --git a/tasks/lib/server.js b/tasks/lib/server.js index 96dd83d..a1d07d7 100644 --- a/tasks/lib/server.js +++ b/tasks/lib/server.js @@ -68,7 +68,11 @@ module.exports = function(grunt, target) { // Set debug mode for node-inspector if (options.debug) { - options.opts.unshift('--debug'); + if (options.breakOnFirstLine) { + options.opts.unshift('--debug-brk'); + } else { + options.opts.unshift('--debug'); + } if (options.cmd === 'coffee') { options.opts.unshift('--nodejs'); } From 13e91aa733fc9a8194a6dacc370550b95db06cad Mon Sep 17 00:00:00 2001 From: Kevin Cai Date: Mon, 15 Sep 2014 16:37:07 -0700 Subject: [PATCH 5/6] Modified README file for breakOnFirstLine option --- README.md | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 3c004ae..cf7c709 100644 --- a/README.md +++ b/README.md @@ -92,11 +92,15 @@ or within each individual server task. output: ".+", // Set --debug - debug: false + debug: false, + + // Set --debug-brk + breakOnFirstLine: false, // Object with properties `out` and `err` both will take a path to a log file and // append the output of the server. Make sure the folders exist. logs: undefined + } } ``` From 756097ece10e65d34f608695c8c28082cee8265b Mon Sep 17 00:00:00 2001 From: Kevin Cai Date: Mon, 15 Sep 2014 16:40:40 -0700 Subject: [PATCH 6/6] Modified README file --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index cf7c709..f63eaa1 100644 --- a/README.md +++ b/README.md @@ -94,7 +94,7 @@ or within each individual server task. // Set --debug debug: false, - // Set --debug-brk + // Set --debug-brk (requires debug to be true) breakOnFirstLine: false, // Object with properties `out` and `err` both will take a path to a log file and