From ec457bfafa5801c7450fefa38f18c4d707a5e78d Mon Sep 17 00:00:00 2001 From: Daniel Pouzemski Date: Tue, 12 May 2015 13:03:43 +0200 Subject: [PATCH 1/2] Better follow the node convention with the err as the first argument. Write test for an error case inside app auth. --- lib/auth.js | 6 ++++-- test/auth.spec.js | 15 ++++++++++++++- 2 files changed, 18 insertions(+), 3 deletions(-) diff --git a/lib/auth.js b/lib/auth.js index 8f00b77..338f86f 100644 --- a/lib/auth.js +++ b/lib/auth.js @@ -118,8 +118,10 @@ module.exports = { app_token: appToken }; - this._authenticate(requestData, function(responseData) { - this._onAccessTokenAcquired(responseData, callback); + this._authenticate(requestData, function(err, responseData) { + if (err === null) { + this._onAccessTokenAcquired(responseData, callback); + } }.bind(this)); }, diff --git a/test/auth.spec.js b/test/auth.spec.js index 3686929..de080f3 100644 --- a/test/auth.spec.js +++ b/test/auth.spec.js @@ -199,7 +199,7 @@ describe('auth', function() { var authData = { access_token: 'a321' }; var callback = function() {}; var host = { - _authenticate: sinon.stub().callsArgWith(1, authData), + _authenticate: sinon.stub().callsArgWith(1, null, authData), _onAccessTokenAcquired: sinon.stub() }; @@ -209,6 +209,19 @@ describe('auth', function() { expect(host._onAccessTokenAcquired.calledWithExactly(authData, callback)).toBe(true); }); + it('should not call _onAccessTokenAcquired when auth failed', function() { + var callback = sinon.stub(); + var host = { + _authenticate: sinon.stub().callsArgWith(1, new Error()), + _onAccessTokenAcquired: sinon.stub() + }; + + auth.authenticateWithApp.call(host, 123, 'e123', callback); + + expect(host._onAccessTokenAcquired.called).toBe(false); + expect(callback.called).toBe(false); + }); + }); describe('_getAuthFromStore', function() { From 15cf175457ecb187425c14452e06fcd8ef5356bc Mon Sep 17 00:00:00 2001 From: Daniel Pouzemski Date: Tue, 12 May 2015 13:12:52 +0200 Subject: [PATCH 2/2] Make it right. --- lib/auth.js | 8 ++++++-- test/auth.spec.js | 8 +++++--- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/lib/auth.js b/lib/auth.js index 338f86f..714acd0 100644 --- a/lib/auth.js +++ b/lib/auth.js @@ -119,9 +119,13 @@ module.exports = { }; this._authenticate(requestData, function(err, responseData) { - if (err === null) { - this._onAccessTokenAcquired(responseData, callback); + if (err) { + callback(err); + + return; } + + this._onAccessTokenAcquired(responseData, callback); }.bind(this)); }, diff --git a/test/auth.spec.js b/test/auth.spec.js index de080f3..de11eb8 100644 --- a/test/auth.spec.js +++ b/test/auth.spec.js @@ -209,17 +209,19 @@ describe('auth', function() { expect(host._onAccessTokenAcquired.calledWithExactly(authData, callback)).toBe(true); }); - it('should not call _onAccessTokenAcquired when auth failed', function() { + it('should not call _onAccessTokenAcquired when auth failed and call the callback', function() { var callback = sinon.stub(); + var err = new Error(); var host = { - _authenticate: sinon.stub().callsArgWith(1, new Error()), + _authenticate: sinon.stub().callsArgWith(1, err), _onAccessTokenAcquired: sinon.stub() }; auth.authenticateWithApp.call(host, 123, 'e123', callback); expect(host._onAccessTokenAcquired.called).toBe(false); - expect(callback.called).toBe(false); + expect(callback.called).toBe(true); + expect(callback.calledWithExactly(err)).toBe(true); }); });