From 9fe9700eeff7fe3a89aac56ee53355bc2e405b64 Mon Sep 17 00:00:00 2001 From: Andrew Stewart Date: Fri, 5 Sep 2014 08:43:40 -0700 Subject: [PATCH 1/3] Restructure Halt/Disconnect Structure Moves disconnecting event handlers up a level so adaptors/drivers don't have to worry about it. Now, Adaptor/Driver subclasses should simply execute the provided callback to dindicate they're done disconnecting, rather than calling the superclass method as before. --- lib/adaptor.js | 4 +--- lib/connection.js | 1 + lib/device.js | 1 + lib/driver.js | 6 ++---- lib/robot.js | 10 ++++++++-- test/specs/adaptor.spec.js | 10 ---------- test/specs/connection.spec.js | 6 ++++++ test/specs/device.spec.js | 18 +++++++++++------- test/specs/driver.spec.js | 9 --------- 9 files changed, 30 insertions(+), 35 deletions(-) diff --git a/lib/adaptor.js b/lib/adaptor.js index 0425ce2..c9baa16 100644 --- a/lib/adaptor.js +++ b/lib/adaptor.js @@ -47,9 +47,7 @@ Adaptor.prototype.connect = function(callback) { // // Returns nothing Adaptor.prototype.disconnect = function(callback) { - Logger.info("Disconnecting from adaptor '" + this.name + "'."); - this.removeAllListeners(); - callback(null); + callback(); }; // Public: Voids all command functions so they do not interact diff --git a/lib/connection.js b/lib/connection.js index 2c5744a..4282c0f 100644 --- a/lib/connection.js +++ b/lib/connection.js @@ -75,6 +75,7 @@ Connection.prototype.connect = function(callback) { Connection.prototype.disconnect = function(callback) { var msg = this._logstring("Disconnecting from"); Logger.info(msg); + this.removeAllListeners(); return this.adaptor.disconnect(callback); }; diff --git a/lib/device.js b/lib/device.js index ff617fa..183ecb5 100644 --- a/lib/device.js +++ b/lib/device.js @@ -79,6 +79,7 @@ Device.prototype.start = function(callback) { // Returns result of supplied callback Device.prototype.halt = function(callback) { Logger.info("Halting device '" + this.name + "'."); + this.removeAllListeners(); this.driver.halt(callback); }; diff --git a/lib/driver.js b/lib/driver.js index feb4245..8db3ff7 100644 --- a/lib/driver.js +++ b/lib/driver.js @@ -62,7 +62,7 @@ Driver.prototype.setupCommands = function(commands, proxy) { // Returns nothing Driver.prototype.start = function(callback) { Logger.info("Driver " + this.name + " started."); - callback(null); + callback(); }; // Public: Halts the driver @@ -71,7 +71,5 @@ Driver.prototype.start = function(callback) { // // Returns nothing Driver.prototype.halt = function(callback) { - Logger.info("Driver " + this.name + " halted."); - this.removeAllListeners(); - callback(null); + callback(); }; diff --git a/lib/robot.js b/lib/robot.js index 2bcf6da..2a4fcc7 100644 --- a/lib/robot.js +++ b/lib/robot.js @@ -303,7 +303,10 @@ Robot.prototype.halt = function(callback) { for (var d in this.devices) { var device = this.devices[d]; - fns.push(device.halt.bind(device)); + + fns.push(function(callback) { + device.halt.call(device, callback); + }); } Async.parallel(fns, function() { @@ -311,7 +314,10 @@ Robot.prototype.halt = function(callback) { for (var c in this.connections) { var connection = this.connections[c]; - fns.push(connection.disconnect.bind(connection)); + + fns.push(function(callback) { + connection.disconnect.call(connection, callback); + }); } Async.parallel(fns, callback); diff --git a/test/specs/adaptor.spec.js b/test/specs/adaptor.spec.js index 2b16bfe..ab82c00 100644 --- a/test/specs/adaptor.spec.js +++ b/test/specs/adaptor.spec.js @@ -52,19 +52,9 @@ describe("Adaptor", function() { beforeEach(function() { callback = spy(); - stub(Logger, 'info'); adaptor.disconnect(callback); }); - afterEach(function() { - Logger.info.restore(); - }); - - it("logs that it's disconnecting to the adaptor", function() { - var string = "Disconnecting from adaptor 'adaptor'."; - expect(Logger.info).to.be.calledWith(string); - }); - it("triggers the callback", function() { expect(callback).to.be.called; }) diff --git a/test/specs/connection.spec.js b/test/specs/connection.spec.js index c2e8991..40ba06b 100644 --- a/test/specs/connection.spec.js +++ b/test/specs/connection.spec.js @@ -75,12 +75,14 @@ describe("Connection", function() { beforeEach(function() { stub(Logger, 'info').returns(true); stub(connection.adaptor, 'disconnect').returns(true); + stub(connection, 'removeAllListeners'); connection.disconnect(); }); afterEach(function() { connection.adaptor.disconnect.restore(); + connection.removeAllListeners.restore(); Logger.info.restore(); }); @@ -92,5 +94,9 @@ describe("Connection", function() { it("tells the adaptor to disconnect", function() { expect(connection.adaptor.disconnect).to.be.called; }); + + it("disconnects all event listeners", function() { + expect(connection.removeAllListeners).to.be.called; + }); }); }); diff --git a/test/specs/device.spec.js b/test/specs/device.spec.js index 2e72ffb..5b6cdc2 100644 --- a/test/specs/device.spec.js +++ b/test/specs/device.spec.js @@ -101,26 +101,30 @@ describe("Device", function() { describe("#halt", function() { beforeEach(function() { - stub(driver, 'halt').returns(true); + stub(Logger, 'info'); + + driver.halt = stub().returns(true); + device.removeAllListeners = spy(); + + device.halt(); }); afterEach(function() { - driver.halt.restore(); + Logger.info.restore(); }); it("halts the driver", function() { - device.halt(); expect(driver.halt).to.be.called; }); it("logs that it's halt the device", function() { var message = "Halting device 'ping'."; - stub(Logger, 'info'); - - device.halt(); expect(Logger.info).to.be.calledWith(message); - Logger.info.restore(); + }); + + it("disconnects all event listeners", function() { + expect(device.removeAllListeners).to.be.called; }); }); diff --git a/test/specs/driver.spec.js b/test/specs/driver.spec.js index 5c4e61a..75ded4f 100644 --- a/test/specs/driver.spec.js +++ b/test/specs/driver.spec.js @@ -104,18 +104,9 @@ describe("Driver", function() { beforeEach(function() { callback = spy(); - stub(Logger, 'info'); driver.halt(callback); }); - afterEach(function() { - Logger.info.restore(); - }); - - it("logs that it's halting the driver", function() { - expect(Logger.info).to.be.calledWith("Driver driver halted.") - }); - it("triggers the callback", function() { expect(callback).to.be.called; }) From 14d8bbab000abe355590d202a1c615273d67d469 Mon Sep 17 00:00:00 2001 From: Andrew Stewart Date: Fri, 5 Sep 2014 09:55:57 -0700 Subject: [PATCH 2/3] Remove base Adaptor#disconnect and Driver#halt fns Would be more useful to module developers to loudly error rather than silently not work. --- lib/adaptor.js | 9 --------- lib/driver.js | 9 --------- test/specs/adaptor.spec.js | 13 ------------- test/specs/driver.spec.js | 13 ------------- 4 files changed, 44 deletions(-) diff --git a/lib/adaptor.js b/lib/adaptor.js index c9baa16..ef2da1c 100644 --- a/lib/adaptor.js +++ b/lib/adaptor.js @@ -41,15 +41,6 @@ Adaptor.prototype.connect = function(callback) { callback(null); }; -// Public: Disconnects from the adaptor -// -// callback - function to run when the adaptor is disconnected -// -// Returns nothing -Adaptor.prototype.disconnect = function(callback) { - callback(); -}; - // Public: Voids all command functions so they do not interact // with anything after disconnect has been called. // diff --git a/lib/driver.js b/lib/driver.js index 8db3ff7..051276e 100644 --- a/lib/driver.js +++ b/lib/driver.js @@ -64,12 +64,3 @@ Driver.prototype.start = function(callback) { Logger.info("Driver " + this.name + " started."); callback(); }; - -// Public: Halts the driver -// -// callback - function to be triggered when the driver is halted -// -// Returns nothing -Driver.prototype.halt = function(callback) { - callback(); -}; diff --git a/test/specs/adaptor.spec.js b/test/specs/adaptor.spec.js index ab82c00..ef8651f 100644 --- a/test/specs/adaptor.spec.js +++ b/test/specs/adaptor.spec.js @@ -47,19 +47,6 @@ describe("Adaptor", function() { }); }); - describe("#disconnect", function() { - var callback; - - beforeEach(function() { - callback = spy(); - adaptor.disconnect(callback); - }); - - it("triggers the callback", function() { - expect(callback).to.be.called; - }) - }); - describe("#_noop", function() { var hello; diff --git a/test/specs/driver.spec.js b/test/specs/driver.spec.js index 75ded4f..fae488c 100644 --- a/test/specs/driver.spec.js +++ b/test/specs/driver.spec.js @@ -98,17 +98,4 @@ describe("Driver", function() { expect(callback).to.be.called; }); }); - - describe("#halt", function() { - var callback; - - beforeEach(function() { - callback = spy(); - driver.halt(callback); - }); - - it("triggers the callback", function() { - expect(callback).to.be.called; - }) - }); }); From cb43f341387b0d845d29af98b1328a6029ae18ba Mon Sep 17 00:00:00 2001 From: Andrew Stewart Date: Fri, 5 Sep 2014 10:34:03 -0700 Subject: [PATCH 3/3] Fix broken tests --- test/specs/connection.spec.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/specs/connection.spec.js b/test/specs/connection.spec.js index 40ba06b..85444aa 100644 --- a/test/specs/connection.spec.js +++ b/test/specs/connection.spec.js @@ -74,14 +74,14 @@ describe("Connection", function() { describe("#disconnect", function() { beforeEach(function() { stub(Logger, 'info').returns(true); - stub(connection.adaptor, 'disconnect').returns(true); stub(connection, 'removeAllListeners'); + connection.adaptor.disconnect = stub().returns(true); + connection.disconnect(); }); afterEach(function() { - connection.adaptor.disconnect.restore(); connection.removeAllListeners.restore(); Logger.info.restore(); });