From 1c0538927cb4f0a3cba724946b7150be7ad9ccc5 Mon Sep 17 00:00:00 2001 From: Garrick Date: Fri, 30 Jan 2015 14:29:29 -0800 Subject: [PATCH 1/7] Create a child process fork for buster-server-cli to prevent it from being stalled from blocking processes that could happen by buster-test-cli. --- lib/buster-ci.js | 43 +++++++++++++++++++++++++++++++++---------- lib/server.js | 36 ++++++++++++++++++++++++++++++++++++ 2 files changed, 69 insertions(+), 10 deletions(-) create mode 100644 lib/server.js diff --git a/lib/buster-ci.js b/lib/buster-ci.js index 97d598a..3795ed0 100644 --- a/lib/buster-ci.js +++ b/lib/buster-ci.js @@ -8,7 +8,17 @@ var Agent = require("buster-ci-agent"), faye = require("faye"), async = require("async"), when = require("when"), - fs = require('fs'); + fs = require('fs'), + path = require('path'), + // Create a server child process fork to communicate with the server. + // This will prevent long processes in this parent process to stall + // the server child process from responding to slave browsers. + server = require('child_process').fork(path.join(__dirname, 'server')); + +// Be sure to kill the server process when this parent process closes +process.on("close", function(){ + server.kill(); +}); function traverseObject(obj, func) { var prop; @@ -230,11 +240,7 @@ function BusterCi(config) { throw new Error("No agents specified in the config!"); } - this._server = busterServer.create(process.stdout, process.stderr, { - binary: "buster-ci-server", - unexpectedErrorMessage: "" - }); - + this._server = server var outputStream = config.outputFile ? fs.createWriteStream(config.outputFile) : process.stdout; @@ -274,10 +280,27 @@ BusterCi.prototype = { var tasks = { startLocalAgent: startLocalAgent.bind(this), - runServer: this._server.run.bind( - this._server, - ["-p" + (this._serverCfg.port || 1111)] - ) + runServer: (function(cb, result){ + var server = this._server; + // callback for passing server message to the task callback + var callback = function(response){ + // Specifically listen a response containing method == run. + if (response.method == "run") { + cb(response.error); + // Clean up by removing the listener + server.removeListener("message", callback); + } + } + + // Handle server process' message that will trigger the tasks' callback + server.on("message", callback); + + // Tell the server process to run with port arg + server.send({ + method: "run", + args: ["-p" + (this._serverCfg.port || 1111)] + }) + }).bind(this) }; var createFayeClientTasks = []; diff --git a/lib/server.js b/lib/server.js new file mode 100644 index 0000000..b848640 --- /dev/null +++ b/lib/server.js @@ -0,0 +1,36 @@ +"use strict"; + +var busterServer = require("buster-server-cli"); +var server = busterServer.create(process.stdout, process.stderr, { + binary: "buster-ci-server", + unexpectedErrorMessage: "" +}); + +var handleError = function(method, error) { + process.send({ + method: method, + error: error || new Error(method + ' not found') + }) +} + +process.on('message', function(request) { + var method = request.method; + var args = request.args; + + if (method == 'run') { + // Make sure to catch any errors in case the server seems to crash. + try { + server.run(args.concat([function(err, server) { + // Let the parent process know the server is running + process.send({ + method: 'run', + error: err + }) + }])) + } catch (error) { + handleError(method, error) + } + } else { + handleError(method) + } +}) \ No newline at end of file From d6de372c1b20194182698726082a5e09bec64217 Mon Sep 17 00:00:00 2001 From: Garrick Date: Fri, 30 Jan 2015 14:48:42 -0800 Subject: [PATCH 2/7] Need to apply args. --- lib/server.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/server.js b/lib/server.js index b848640..3dee75c 100644 --- a/lib/server.js +++ b/lib/server.js @@ -20,7 +20,7 @@ process.on('message', function(request) { if (method == 'run') { // Make sure to catch any errors in case the server seems to crash. try { - server.run(args.concat([function(err, server) { + server.run.apply(server, args.concat([function(err, bServer) { // Let the parent process know the server is running process.send({ method: 'run', From 41a640eb7c67dc42b5b52c9da3f9c3a97643ce1a Mon Sep 17 00:00:00 2001 From: Garrick Date: Mon, 2 Feb 2015 10:28:15 -0800 Subject: [PATCH 3/7] Be sure to kill the server at the end. --- lib/buster-ci.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/lib/buster-ci.js b/lib/buster-ci.js index 3795ed0..d7a57d1 100644 --- a/lib/buster-ci.js +++ b/lib/buster-ci.js @@ -351,6 +351,9 @@ BusterCi.prototype = { this._logger.error(err); } closeBrowsers.call(this, function (err) { + // Be sure to kill the server process + this._server.kill(); + if (this._localAgent) { this._localAgent.close(); } From cf668857542948dea113aaa86c7f4b383d09ed6e Mon Sep 17 00:00:00 2001 From: Garrick Date: Mon, 2 Feb 2015 13:00:56 -0800 Subject: [PATCH 4/7] Update test helper with stubs for child process node module. Load server child process during buster ci instantiation. Add killServer method to more easily cleanup process close event listener. --- lib/buster-ci.js | 71 ++++++++++++++++++++++++++---------------- lib/server.js | 2 +- test/buster-ci-test.js | 21 ++++++++++--- test/test-helper.js | 34 ++++++++++++++++++-- 4 files changed, 94 insertions(+), 34 deletions(-) diff --git a/lib/buster-ci.js b/lib/buster-ci.js index d7a57d1..17e35c6 100644 --- a/lib/buster-ci.js +++ b/lib/buster-ci.js @@ -10,15 +10,7 @@ var Agent = require("buster-ci-agent"), when = require("when"), fs = require('fs'), path = require('path'), - // Create a server child process fork to communicate with the server. - // This will prevent long processes in this parent process to stall - // the server child process from responding to slave browsers. - server = require('child_process').fork(path.join(__dirname, 'server')); - -// Be sure to kill the server process when this parent process closes -process.on("close", function(){ - server.kill(); -}); + serverPath = path.join(__dirname, 'server'); function traverseObject(obj, func) { var prop; @@ -240,7 +232,14 @@ function BusterCi(config) { throw new Error("No agents specified in the config!"); } - this._server = server + // Create a server child process fork to communicate with the server. + // This will prevent long processes in this parent process to stall + // the server child process from responding to slave browsers. + var server = this._server = require('child_process').fork(serverPath); + // Be sure to kill the server process when this parent process closes + // Creating a new bound function so that the event can be removed during testing for cleanup + this._killServer = this.killServer.bind(this); + process.on("close", this._killServer); var outputStream = config.outputFile ? fs.createWriteStream(config.outputFile) : process.stdout; @@ -276,30 +275,50 @@ function BusterCi(config) { BusterCi.prototype = { + killServer: function(){ + var server = this._server; + if (server) { + // Clean up any attached bound listener. + if (this._killServer) { + process.removeListener("close", this._killServer) + delete this._killServer; + } + + delete this._server; + + server.kill(); + } + }, + run: function (args, cb) { var tasks = { startLocalAgent: startLocalAgent.bind(this), runServer: (function(cb, result){ var server = this._server; - // callback for passing server message to the task callback - var callback = function(response){ - // Specifically listen a response containing method == run. - if (response.method == "run") { - cb(response.error); - // Clean up by removing the listener - server.removeListener("message", callback); + if (server) { + // callback for passing server message to the task callback + var callback = function(response){ + // Specifically listen a response containing method == run. + if (response.method == "run") { + cb(response.error); + // Clean up by removing the listener + server.removeListener("message", callback); + } } - } - // Handle server process' message that will trigger the tasks' callback - server.on("message", callback); + // Handle server process' message that will trigger the tasks' callback + server.on("message", callback); - // Tell the server process to run with port arg - server.send({ - method: "run", - args: ["-p" + (this._serverCfg.port || 1111)] - }) + // Tell the server process to run with port arg + server.send({ + method: "run", + args: ["-p" + (this._serverCfg.port || 1111)] + }) + } else { + cb(new Error('server is undefined')) + } + }).bind(this) }; @@ -352,7 +371,7 @@ BusterCi.prototype = { } closeBrowsers.call(this, function (err) { // Be sure to kill the server process - this._server.kill(); + this.killServer(); if (this._localAgent) { this._localAgent.close(); diff --git a/lib/server.js b/lib/server.js index 3dee75c..ed3d829 100644 --- a/lib/server.js +++ b/lib/server.js @@ -33,4 +33,4 @@ process.on('message', function(request) { } else { handleError(method) } -}) \ No newline at end of file +}) diff --git a/test/buster-ci-test.js b/test/buster-ci-test.js index be01020..05ff9f1 100644 --- a/test/buster-ci-test.js +++ b/test/buster-ci-test.js @@ -12,6 +12,8 @@ var buster = require("buster"), stubServerFayeClient = th.stubServerFayeClient, setFayeClientNotAccessible = th.setFayeClientNotAccessible, asyncTest = th.asyncTest, + childProcessStub = th.childProcessStub, + childProcessForkMock = th.childProcessForkMock, assert = buster.assert, refute = buster.refute, @@ -83,13 +85,13 @@ buster.testCase("buster-ci", { th.tearDown(); }, - "throws if no agents are specified": function () { + "if no agents are specified": function () { assert.exception(function () { var busterCi = new BusterCi({}); }, { message: "no agents" }); }, - "creates server": function () { + "// creates server": function () { var busterCi = new BusterCi(this.config); @@ -236,7 +238,7 @@ buster.testCase("buster-ci", { }.bind(this))); }, - "runs server": function (done) { + "// runs server": function (done) { new BusterCi(this.config).run([], done(function () { @@ -245,7 +247,7 @@ buster.testCase("buster-ci", { })); }, - "runs server with specified port": function (done) { + "// runs server with specified port": function (done) { this.config.server.port = 2222; @@ -635,5 +637,14 @@ buster.testCase("buster-ci", { refute.called(th.fs.createWriteStream); assert.calledWith(th.testCli.create, process.stdout); }.bind(this))); - } + }, + + "kills server when done": function(done){ + var killServerSpy = this.spy(BusterCi.prototype, 'killServer'); + var busterCi = new BusterCi(this.config); + + busterCi.run([], done(function(){ + assert.calledOnce(killServerSpy) + })) + }, }); diff --git a/test/test-helper.js b/test/test-helper.js index 9109f91..9c28eae 100644 --- a/test/test-helper.js +++ b/test/test-helper.js @@ -3,21 +3,48 @@ var buster = require("buster"), async = require("async"), Agent = require("buster-ci-agent"), + childProcess = require("child_process"), proxyquire = require("proxyquire"), + EventEmitter = require('events').EventEmitter, AgentStub = buster.sinon.stub(), busterServer = {}, busterTestCli = {}, faye = {}, fs = {}, + childProcessStub = buster.sinon.stub(), + childProcessForkMock = new EventEmitter, BusterCi = proxyquire("../lib/buster-ci", { "buster-server-cli": busterServer, "buster-ci-agent": AgentStub, "buster-test-cli": busterTestCli, "faye": faye, - "fs": fs + "fs": fs, + "child_process": childProcessStub }), sandbox; +childProcessForkMock.send = function(){}; +childProcessForkMock.message = function(){}; +childProcessForkMock.kill = function(){}; + +function stubChildProcess() { + + sandbox.stub(childProcessForkMock, "kill"); + + sandbox.stub(childProcessForkMock, "send", function(msg){ + if (msg.method == 'run') { + childProcessForkMock.emit('message', { + method: 'run', + error: null + }); + } + }); + sandbox.stub(childProcess, "fork").returns(childProcessForkMock); + + childProcessStub.returns(childProcess); + + return childProcess; +} function stubServer() { @@ -146,7 +173,9 @@ module.exports = { busterServer: busterServer, faye: faye, fs: fs, - + childProcessStub: childProcessStub, + childProcessForkMock: childProcessForkMock, + setUp: function () { async.setImmediate = function (fn) { fn(); @@ -158,6 +187,7 @@ module.exports = { this.agent.listen.callsArg(0); this.testCli = stubTestCli.call(this); this.testCli.run.callsArg(1); + this.childProcess = stubChildProcess.call(this); sandbox.stub(faye, "Client"); sandbox.stub(fs, "createWriteStream"); }, From 438809cfcd6c29d9f7a85cfc8d79ad4244875ffb Mon Sep 17 00:00:00 2001 From: Garrick Date: Mon, 2 Feb 2015 23:33:34 -0800 Subject: [PATCH 5/7] Create a separate server.js acting as the bridge to communicate with the child process, process.js, file. Tests! --- lib/buster-ci.js | 88 ++++++++++++++++------------------ lib/server.js | 95 +++++++++++++++++++++++++------------ lib/server/process.js | 37 +++++++++++++++ test/buster-ci-test.js | 29 +++++------ test/server-test.js | 71 +++++++++++++++++++++++++++ test/server/process-test.js | 48 +++++++++++++++++++ test/test-helper.js | 26 +++++----- 7 files changed, 291 insertions(+), 103 deletions(-) create mode 100644 lib/server/process.js create mode 100644 test/server-test.js create mode 100644 test/server/process-test.js diff --git a/lib/buster-ci.js b/lib/buster-ci.js index 17e35c6..19e1727 100644 --- a/lib/buster-ci.js +++ b/lib/buster-ci.js @@ -10,7 +10,7 @@ var Agent = require("buster-ci-agent"), when = require("when"), fs = require('fs'), path = require('path'), - serverPath = path.join(__dirname, 'server'); + Server = require('./server'); function traverseObject(obj, func) { var prop; @@ -232,14 +232,7 @@ function BusterCi(config) { throw new Error("No agents specified in the config!"); } - // Create a server child process fork to communicate with the server. - // This will prevent long processes in this parent process to stall - // the server child process from responding to slave browsers. - var server = this._server = require('child_process').fork(serverPath); - // Be sure to kill the server process when this parent process closes - // Creating a new bound function so that the event can be removed during testing for cleanup - this._killServer = this.killServer.bind(this); - process.on("close", this._killServer); + this._server = Server.create(); var outputStream = config.outputFile ? fs.createWriteStream(config.outputFile) : process.stdout; @@ -275,51 +268,52 @@ function BusterCi(config) { BusterCi.prototype = { - killServer: function(){ - var server = this._server; - if (server) { - // Clean up any attached bound listener. - if (this._killServer) { - process.removeListener("close", this._killServer) - delete this._killServer; - } + // killServer: function(){ + // var server = this._server; + // if (server) { + // // Clean up any attached bound listener. + // if (this._killServer) { + // process.removeListener("close", this._killServer) + // delete this._killServer; + // } - delete this._server; + // delete this._server; - server.kill(); - } - }, + // server.kill(); + // } + // }, run: function (args, cb) { var tasks = { startLocalAgent: startLocalAgent.bind(this), - runServer: (function(cb, result){ - var server = this._server; - if (server) { - // callback for passing server message to the task callback - var callback = function(response){ - // Specifically listen a response containing method == run. - if (response.method == "run") { - cb(response.error); - // Clean up by removing the listener - server.removeListener("message", callback); - } - } - - // Handle server process' message that will trigger the tasks' callback - server.on("message", callback); - - // Tell the server process to run with port arg - server.send({ - method: "run", - args: ["-p" + (this._serverCfg.port || 1111)] - }) - } else { - cb(new Error('server is undefined')) - } + // runServer: (function(cb, result){ + // var server = this._server; + // if (server) { + // // callback for passing server message to the task callback + // var callback = function(response){ + // // Specifically listen a response containing method == run. + // if (response.method == "run") { + // cb(response.error); + // // Clean up by removing the listener + // server.removeListener("message", callback); + // } + // } + + // // Handle server process' message that will trigger the tasks' callback + // server.on("message", callback); + + // // Tell the server process to run with port arg + // server.send({ + // method: "run", + // args: ["-p" + (this._serverCfg.port || 1111)] + // }) + // } else { + // cb(new Error('server is undefined')) + // } - }).bind(this) + // }).bind(this) + runServer: this._server.run.bind(this._server, ["-p" + (this._serverCfg.port || 1111)]) }; var createFayeClientTasks = []; @@ -371,7 +365,7 @@ BusterCi.prototype = { } closeBrowsers.call(this, function (err) { // Be sure to kill the server process - this.killServer(); + this._server.kill(); if (this._localAgent) { this._localAgent.close(); diff --git a/lib/server.js b/lib/server.js index ed3d829..9677136 100644 --- a/lib/server.js +++ b/lib/server.js @@ -1,36 +1,69 @@ "use strict"; -var busterServer = require("buster-server-cli"); -var server = busterServer.create(process.stdout, process.stderr, { - binary: "buster-ci-server", - unexpectedErrorMessage: "" -}); - -var handleError = function(method, error) { - process.send({ - method: method, - error: error || new Error(method + ' not found') - }) -} +var ChildProcess = require("child_process"); +var path = require("path"); +var serverProcessPath = path.join(__dirname, "./server/process"); + +var Server = function() { + // Create a server child process fork to communicate with the server. + // This will prevent long processes in this parent process to stall + // the server child process from responding to slave browsers. + this._childProcess = ChildProcess.fork(serverProcessPath); + this._killProcess = this.kill.bind(this); + + // Be sure to kill the server process when this parent process closes + // Creating a new bound function so that the event can be removed during testing for cleanup + process.on("close", this._killProcess); + + return this; +}; + +Server.prototype = { + create: function(){ + return new Server(); + }, + + run: function(args, cb) { + var childProcess = this._childProcess; + if (childProcess) { + // callback for passing server message to the task callback + var callback = function(response) { + // Specifically listen a response containing method == run. + if (response.method == "run") { + cb(response.error); + // Clean up by removing the listener + childProcess.removeListener("message", callback); + } + } + + // Handle server process' message that will trigger the tasks' callback + childProcess.on("message", callback); -process.on('message', function(request) { - var method = request.method; - var args = request.args; - - if (method == 'run') { - // Make sure to catch any errors in case the server seems to crash. - try { - server.run.apply(server, args.concat([function(err, bServer) { - // Let the parent process know the server is running - process.send({ - method: 'run', - error: err - }) - }])) - } catch (error) { - handleError(method, error) + // Tell the server process to run with port arg + childProcess.send({ + method: "run", + args: args + }); + } else { + cb(new Error("_childProcess is undefined")); + } + }, + + kill: function() { + var killProccess = this._killProcess; + // Clean up any attached bound listener. + if (killProccess) { + process.removeListener("close", killProccess); + delete this._killProcess; + } + + var childProccess = this._childProcess; + if (childProccess) { + delete this._childProcess; + + childProccess.kill(); } - } else { - handleError(method) } -}) +} + +module.exports = Server.prototype; diff --git a/lib/server/process.js b/lib/server/process.js new file mode 100644 index 0000000..2dbc2de --- /dev/null +++ b/lib/server/process.js @@ -0,0 +1,37 @@ +"use strict"; + +var busterServer = require("buster-server-cli"); +var server = busterServer.create(process.stdout, process.stderr, { + binary: "buster-ci-server", + unexpectedErrorMessage: "" +}); + +var handleError = function(method, error) { + process.send({ + method: method, + error: error || new Error(method + ' not found') + }) +} + +process.on('message', function(request) { + var method = request.method; + var args = request.args; + + if (method == 'run') { + // Make sure to catch any errors in case the server seems to crash. + try { + + server.run.apply(server, args.concat([function(err, bServer) { + // Let the parent process know the server is running + process.send({ + method: 'run', + error: err + }) + }])) + } catch (error) { + handleError(method, error) + } + } else { + handleError(method) + } +}) diff --git a/test/buster-ci-test.js b/test/buster-ci-test.js index 05ff9f1..a646a56 100644 --- a/test/buster-ci-test.js +++ b/test/buster-ci-test.js @@ -14,6 +14,7 @@ var buster = require("buster"), asyncTest = th.asyncTest, childProcessStub = th.childProcessStub, childProcessForkMock = th.childProcessForkMock, + ChildProcess = th.ChildProcess, assert = buster.assert, refute = buster.refute, @@ -91,19 +92,19 @@ buster.testCase("buster-ci", { }, { message: "no agents" }); }, - "// creates server": function () { + "creates server": function () { var busterCi = new BusterCi(this.config); assert.calledOnce(busterServer.create); - assert.calledWith( - busterServer.create, - match.any, - match.any, - match({ - binary: match.string - }) - ); + // assert.calledWith( + // busterServer.create, + // match.any, + // match.any, + // match({ + // binary: match.string + // }) + // ); }, "starts local agent if agent 'localhost' is specified": function (done) { @@ -238,7 +239,7 @@ buster.testCase("buster-ci", { }.bind(this))); }, - "// runs server": function (done) { + "runs server": function (done) { new BusterCi(this.config).run([], done(function () { @@ -247,7 +248,7 @@ buster.testCase("buster-ci", { })); }, - "// runs server with specified port": function (done) { + "runs server with specified port": function (done) { this.config.server.port = 2222; @@ -640,11 +641,11 @@ buster.testCase("buster-ci", { }, "kills server when done": function(done){ - var killServerSpy = this.spy(BusterCi.prototype, 'killServer'); var busterCi = new BusterCi(this.config); + var serverKillSpy = this.spy(busterCi._server, 'kill'); busterCi.run([], done(function(){ - assert.calledOnce(killServerSpy) + assert.calledOnce(serverKillSpy); })) - }, + } }); diff --git a/test/server-test.js b/test/server-test.js new file mode 100644 index 0000000..a81cf8b --- /dev/null +++ b/test/server-test.js @@ -0,0 +1,71 @@ +"use strict"; + +var buster = require("buster"), + EventEmitter = require("events").EventEmitter, + ChildProcess = require("child_process"), + proxyquire = require("proxyquire"), + Server = proxyquire("../lib/server", { + "child_process": ChildProcess + }), + assert = buster.assert, + refute = buster.refute, + childProcessForkMock = new EventEmitter, + sandbox; + + childProcessForkMock.send = function(){}; + childProcessForkMock.message = function(){}; + childProcessForkMock.kill = function(){}; + +function stubChildProcess() { + sandbox.stub(childProcessForkMock, "kill"); + + sandbox.stub(childProcessForkMock, "send", function(msg){ + if (msg.method == "run") { + childProcessForkMock.emit("message", { + method: "run", + error: null + }); + } + }); + + sandbox.stub(ChildProcess, "fork").returns(childProcessForkMock); + + return ChildProcess; +} + +buster.testCase("buster-ci server", { + setUp: function(){ + sandbox = buster.sinon.sandbox.create(); + this.ChildProcess = stubChildProcess.call(this); + }, + + tearDown: function(){ + sandbox.restore(); + }, + "creates a child process fork": function(){ + Server.create(); + assert.calledOnce(this.ChildProcess.fork); + }, + + "run sends a message containing args": function(done){ + var server = Server.create(); + var args = ["-p" + 1111]; + + server.run(args, done(function(){ + assert.calledWith(childProcessForkMock.send, { + method: "run", + args: args + }); + })); + }, + + "kills child process": function(){ + var server = Server.create(); + assert.defined(server._childProcess); + assert.defined(server._killProcess); + server.kill(); + refute.defined(server._childProcess); + refute.defined(server._killProcess); + assert.calledOnce(childProcessForkMock.kill); + } +}); diff --git a/test/server/process-test.js b/test/server/process-test.js new file mode 100644 index 0000000..67d017b --- /dev/null +++ b/test/server/process-test.js @@ -0,0 +1,48 @@ +"use strict"; + +var buster = require("buster"), + busterServer = require("buster-server-cli"), + proxyquire = require("proxyquire"), + assert = buster.assert, + refute = buster.refute, + match = buster.sinon.match; + +buster.testCase("buster-ci server process", { + setUp: function(){ + var server = this.server = busterServer.create(undefined, undefined, {}); + + this.stub(busterServer, "create").returns(server) + + proxyquire("../../lib/server/process", { + "buster-server-cli": busterServer + }); + }, + + tearDown: function(){ + + }, + + "creates server": function(){ + assert.calledOnce(busterServer.create); + }, + + "message runs server": function(done){ + var server = this.server; + var message = { + method: "run", + args: [123] + } + var args = message.args.concat([match.func]); + + this.stub(server, "run"); + + process.send = done(function(){ + assert.calledOnceWith(server.run, args[0], args[1]); + delete process.send; + }); + + process.emit("message", message); + + server.run.callArg(1) + } +}) diff --git a/test/test-helper.js b/test/test-helper.js index 9c28eae..bae279e 100644 --- a/test/test-helper.js +++ b/test/test-helper.js @@ -3,7 +3,7 @@ var buster = require("buster"), async = require("async"), Agent = require("buster-ci-agent"), - childProcess = require("child_process"), + ChildProcess = require("child_process"), proxyquire = require("proxyquire"), EventEmitter = require('events').EventEmitter, AgentStub = buster.sinon.stub(), @@ -13,13 +13,17 @@ var buster = require("buster"), fs = {}, childProcessStub = buster.sinon.stub(), childProcessForkMock = new EventEmitter, + path = require("path"), + Server = proxyquire("../lib/server", { + "child_process": ChildProcess + }), BusterCi = proxyquire("../lib/buster-ci", { - "buster-server-cli": busterServer, + "./server": Server, "buster-ci-agent": AgentStub, "buster-test-cli": busterTestCli, "faye": faye, "fs": fs, - "child_process": childProcessStub + "child_process": ChildProcess }), sandbox; @@ -39,18 +43,16 @@ function stubChildProcess() { }); } }); - sandbox.stub(childProcess, "fork").returns(childProcessForkMock); - childProcessStub.returns(childProcess); + sandbox.stub(ChildProcess, "fork").returns(childProcessForkMock); - return childProcess; + return ChildProcess; } function stubServer() { - - var server = busterServer.create(undefined, undefined, {}); + var server = Server.create(); sandbox.stub(server, "run"); - sandbox.stub(busterServer, "create").returns(server); + sandbox.stub(Server, "create").returns(server); return server; } @@ -170,29 +172,31 @@ module.exports = { AgentStub: AgentStub, BusterCi: BusterCi, - busterServer: busterServer, + busterServer: Server, faye: faye, fs: fs, childProcessStub: childProcessStub, childProcessForkMock: childProcessForkMock, + ChildProcess: ChildProcess, setUp: function () { async.setImmediate = function (fn) { fn(); }; sandbox = buster.sinon.sandbox.create(); + this.ChildProcess = stubChildProcess.call(this); this.server = stubServer.call(this); this.server.run.callsArg(1); this.agent = stubAgent.call(this); this.agent.listen.callsArg(0); this.testCli = stubTestCli.call(this); this.testCli.run.callsArg(1); - this.childProcess = stubChildProcess.call(this); sandbox.stub(faye, "Client"); sandbox.stub(fs, "createWriteStream"); }, tearDown: function () { + this.server.kill(); AgentStub.reset(); sandbox.restore(); }, From 560937a5e0712d0fd791585fb2993d05a31931ea Mon Sep 17 00:00:00 2001 From: Garrick Date: Tue, 3 Feb 2015 11:08:43 -0800 Subject: [PATCH 6/7] Fix server process tests. --- lib/server/process.js | 3 +-- test/server/process-test.js | 48 +++++++++++++++++++++++++------------ 2 files changed, 34 insertions(+), 17 deletions(-) diff --git a/lib/server/process.js b/lib/server/process.js index 2dbc2de..c96a0f1 100644 --- a/lib/server/process.js +++ b/lib/server/process.js @@ -15,12 +15,11 @@ var handleError = function(method, error) { process.on('message', function(request) { var method = request.method; - var args = request.args; + var args = request.args || []; if (method == 'run') { // Make sure to catch any errors in case the server seems to crash. try { - server.run.apply(server, args.concat([function(err, bServer) { // Let the parent process know the server is running process.send({ diff --git a/test/server/process-test.js b/test/server/process-test.js index 67d017b..d6e7d6e 100644 --- a/test/server/process-test.js +++ b/test/server/process-test.js @@ -5,21 +5,30 @@ var buster = require("buster"), proxyquire = require("proxyquire"), assert = buster.assert, refute = buster.refute, - match = buster.sinon.match; + match = buster.sinon.match, + sandbox = buster.sinon.sandbox.create(), + server, + processSend; -buster.testCase("buster-ci server process", { - setUp: function(){ - var server = this.server = busterServer.create(undefined, undefined, {}); +server = busterServer.create(undefined, undefined, {}); +sandbox.stub(server, "run"); +sandbox.stub(busterServer, "create").returns(server); - this.stub(busterServer, "create").returns(server) +proxyquire("../../lib/server/process", { + "buster-server-cli": busterServer +}); - proxyquire("../../lib/server/process", { - "buster-server-cli": busterServer - }); +buster.testCase("buster-ci server process", { + setUp: function(){ + processSend = process.send; }, tearDown: function(){ - + if (processSend) { + process.send = processSend; + } else { + delete process.send; + } }, "creates server": function(){ @@ -27,22 +36,31 @@ buster.testCase("buster-ci server process", { }, "message runs server": function(done){ - var server = this.server; var message = { method: "run", - args: [123] + args: ['-p1111'] } var args = message.args.concat([match.func]); - this.stub(server, "run"); - process.send = done(function(){ assert.calledOnceWith(server.run, args[0], args[1]); - delete process.send; }); process.emit("message", message); - server.run.callArg(1) + server.run.callArg(1); + }, + + "unhandled message sends error": function(done){ + var message = { + method: "abc" + } + + process.send = done(function(msg){ + assert.match(msg, message); + assert.hasPrototype(msg.error, Error.prototype); + }); + + process.emit("message", message); } }) From a79b8201b30f7c59432f7e964683b32503d44ecb Mon Sep 17 00:00:00 2001 From: Garrick Date: Tue, 3 Feb 2015 11:24:32 -0800 Subject: [PATCH 7/7] Clean up commented code and style. --- lib/buster-ci.js | 52 ++++--------------------------------- lib/server.js | 5 ++-- lib/server/process.js | 25 +++++++++--------- test/buster-ci-test.js | 9 ++++--- test/server-test.js | 3 ++- test/server/process-test.js | 9 ++++--- test/test-helper.js | 19 +++++++------- 7 files changed, 42 insertions(+), 80 deletions(-) diff --git a/lib/buster-ci.js b/lib/buster-ci.js index 19e1727..eb7e1eb 100644 --- a/lib/buster-ci.js +++ b/lib/buster-ci.js @@ -1,16 +1,15 @@ +/* global module, require, process */ "use strict"; var Agent = require("buster-ci-agent"), - busterServer = require("buster-server-cli"), busterTestCli = require("buster-test-cli"), formatio = require("formatio"), logger = require("stream-logger"), faye = require("faye"), async = require("async"), when = require("when"), - fs = require('fs'), - path = require('path'), - Server = require('./server'); + fs = require("fs"), + Server = require("./server"); function traverseObject(obj, func) { var prop; @@ -89,8 +88,8 @@ function createFayeClientAgent(agentName, agent, cb) { this._logger.info("create faye client for agent: " + agentName); var agentUrl = "http://" + agentName + ":" + agent.config.port; agent.client = new faye.Client(agentUrl); - agent.client.on('transport:up', cb); - agent.client.on('transport:down', + agent.client.on("transport:up", cb); + agent.client.on("transport:down", cb.bind(null, "Agent " + agentUrl + " not accessible!")); sendMessage.call(this, agent, { command: "ping" }); } @@ -268,51 +267,10 @@ function BusterCi(config) { BusterCi.prototype = { - // killServer: function(){ - // var server = this._server; - // if (server) { - // // Clean up any attached bound listener. - // if (this._killServer) { - // process.removeListener("close", this._killServer) - // delete this._killServer; - // } - - // delete this._server; - - // server.kill(); - // } - // }, - run: function (args, cb) { var tasks = { startLocalAgent: startLocalAgent.bind(this), - // runServer: (function(cb, result){ - // var server = this._server; - // if (server) { - // // callback for passing server message to the task callback - // var callback = function(response){ - // // Specifically listen a response containing method == run. - // if (response.method == "run") { - // cb(response.error); - // // Clean up by removing the listener - // server.removeListener("message", callback); - // } - // } - - // // Handle server process' message that will trigger the tasks' callback - // server.on("message", callback); - - // // Tell the server process to run with port arg - // server.send({ - // method: "run", - // args: ["-p" + (this._serverCfg.port || 1111)] - // }) - // } else { - // cb(new Error('server is undefined')) - // } - - // }).bind(this) runServer: this._server.run.bind(this._server, ["-p" + (this._serverCfg.port || 1111)]) }; diff --git a/lib/server.js b/lib/server.js index 9677136..e27186b 100644 --- a/lib/server.js +++ b/lib/server.js @@ -1,3 +1,4 @@ +/* global module, require, process, __dirname */ "use strict"; var ChildProcess = require("child_process"); @@ -34,7 +35,7 @@ Server.prototype = { // Clean up by removing the listener childProcess.removeListener("message", callback); } - } + }; // Handle server process' message that will trigger the tasks' callback childProcess.on("message", callback); @@ -64,6 +65,6 @@ Server.prototype = { childProccess.kill(); } } -} +}; module.exports = Server.prototype; diff --git a/lib/server/process.js b/lib/server/process.js index c96a0f1..794a588 100644 --- a/lib/server/process.js +++ b/lib/server/process.js @@ -1,3 +1,4 @@ +/* global require, process */ "use strict"; var busterServer = require("buster-server-cli"); @@ -9,28 +10,28 @@ var server = busterServer.create(process.stdout, process.stderr, { var handleError = function(method, error) { process.send({ method: method, - error: error || new Error(method + ' not found') - }) -} + error: error || new Error(method + " not found") + }); +}; -process.on('message', function(request) { +process.on("message", function(request) { var method = request.method; var args = request.args || []; - if (method == 'run') { + if (method == "run") { // Make sure to catch any errors in case the server seems to crash. try { - server.run.apply(server, args.concat([function(err, bServer) { + server.run.apply(server, args.concat([function(err) { // Let the parent process know the server is running process.send({ - method: 'run', + method: "run", error: err - }) - }])) + }); + }])); } catch (error) { - handleError(method, error) + handleError(method, error); } } else { - handleError(method) + handleError(method); } -}) +}); diff --git a/test/buster-ci-test.js b/test/buster-ci-test.js index a646a56..c3f4d62 100644 --- a/test/buster-ci-test.js +++ b/test/buster-ci-test.js @@ -1,3 +1,4 @@ +/* global require, process */ "use strict"; var buster = require("buster"), @@ -57,7 +58,7 @@ buster.testCase("buster-ci", { this.browsersAgentLocalhost = { browsers: this.config.browsers - } + }; this.browsersAgentRemotehost1 = { browsers: { "FF": {}, @@ -256,7 +257,7 @@ buster.testCase("buster-ci", { assert.calledOnce(th.server.run); assert.calledWith(th.server.run, match(function (array) { - return array.indexOf("-p2222") >= 0 + return array.indexOf("-p2222") >= 0; })); })); }, @@ -642,10 +643,10 @@ buster.testCase("buster-ci", { "kills server when done": function(done){ var busterCi = new BusterCi(this.config); - var serverKillSpy = this.spy(busterCi._server, 'kill'); + var serverKillSpy = this.spy(busterCi._server, "kill"); busterCi.run([], done(function(){ assert.calledOnce(serverKillSpy); - })) + })); } }); diff --git a/test/server-test.js b/test/server-test.js index a81cf8b..de27c61 100644 --- a/test/server-test.js +++ b/test/server-test.js @@ -1,3 +1,4 @@ +/* global require */ "use strict"; var buster = require("buster"), @@ -9,7 +10,7 @@ var buster = require("buster"), }), assert = buster.assert, refute = buster.refute, - childProcessForkMock = new EventEmitter, + childProcessForkMock = new EventEmitter(), sandbox; childProcessForkMock.send = function(){}; diff --git a/test/server/process-test.js b/test/server/process-test.js index d6e7d6e..d7a4f9d 100644 --- a/test/server/process-test.js +++ b/test/server/process-test.js @@ -1,3 +1,4 @@ +/* global require, process */ "use strict"; var buster = require("buster"), @@ -38,8 +39,8 @@ buster.testCase("buster-ci server process", { "message runs server": function(done){ var message = { method: "run", - args: ['-p1111'] - } + args: ["-p1111"] + }; var args = message.args.concat([match.func]); process.send = done(function(){ @@ -54,7 +55,7 @@ buster.testCase("buster-ci server process", { "unhandled message sends error": function(done){ var message = { method: "abc" - } + }; process.send = done(function(msg){ assert.match(msg, message); @@ -63,4 +64,4 @@ buster.testCase("buster-ci server process", { process.emit("message", message); } -}) +}); diff --git a/test/test-helper.js b/test/test-helper.js index bae279e..bd26d9d 100644 --- a/test/test-helper.js +++ b/test/test-helper.js @@ -1,3 +1,4 @@ +/* global global, module, require */ "use strict"; var buster = require("buster"), @@ -5,15 +6,13 @@ var buster = require("buster"), Agent = require("buster-ci-agent"), ChildProcess = require("child_process"), proxyquire = require("proxyquire"), - EventEmitter = require('events').EventEmitter, + EventEmitter = require("events").EventEmitter, AgentStub = buster.sinon.stub(), - busterServer = {}, busterTestCli = {}, faye = {}, fs = {}, childProcessStub = buster.sinon.stub(), - childProcessForkMock = new EventEmitter, - path = require("path"), + childProcessForkMock = new EventEmitter(), Server = proxyquire("../lib/server", { "child_process": ChildProcess }), @@ -36,9 +35,9 @@ function stubChildProcess() { sandbox.stub(childProcessForkMock, "kill"); sandbox.stub(childProcessForkMock, "send", function(msg){ - if (msg.method == 'run') { - childProcessForkMock.emit('message', { - method: 'run', + if (msg.method == "run") { + childProcessForkMock.emit("message", { + method: "run", error: null }); } @@ -83,7 +82,7 @@ function stubFayeClient(url) { if (event === "transport:down" && !fayeClient.accessible) { cb(); } - } + }; return fayeClient; } @@ -210,8 +209,8 @@ module.exports = { // 335b48c795d6d21a7d1a442af7837ec2e1ff7c36 var origClearTimeout = global.clearTimeout; sandbox.stub(global, "clearTimeout", function (timerId) { - if (typeof timerId === 'object') { - timerId = timerId.id + if (typeof timerId === "object") { + timerId = timerId.id; } origClearTimeout(timerId); });