From f9ae7053d0d9f605b9d7a98f923b445b94c77725 Mon Sep 17 00:00:00 2001 From: SleepWalker Date: Tue, 6 Dec 2016 23:08:51 +0200 Subject: [PATCH] #169: remove unhandled promise rejections in tests. Added loging for that cases --- src/components/user/factory.js | 2 +- src/services/authFlow/AcceptRulesState.js | 5 +++- src/services/authFlow/ActivationState.js | 5 +++- src/services/authFlow/AuthFlow.js | 7 +++--- src/services/authFlow/CompleteState.js | 7 ++++-- src/services/authFlow/LoginState.js | 5 +++- src/services/authFlow/RecoverPasswordState.js | 5 +++- src/services/authFlow/RegisterState.js | 5 +++- .../authFlow/ResendActivationState.js | 5 +++- tests/components/accounts/actions.test.js | 2 +- tests/components/user/actions.test.js | 24 +++++++------------ tests/polyfills.test.js | 10 ++++---- .../authFlow/AcceptRulesState.test.js | 5 +++- .../services/authFlow/ActivationState.test.js | 4 +++- tests/services/authFlow/LoginState.test.js | 4 +++- .../authFlow/RecoverPasswordState.test.js | 4 +++- tests/services/authFlow/RegisterState.test.js | 4 +++- .../authFlow/ResendActivationState.test.js | 4 +++- 18 files changed, 67 insertions(+), 40 deletions(-) diff --git a/src/components/user/factory.js b/src/components/user/factory.js index 58af5cf..e584505 100644 --- a/src/components/user/factory.js +++ b/src/components/user/factory.js @@ -12,7 +12,7 @@ let promise; * * @param {object} store - redux store * - * @return {Promise} - a promise, that resolves in User state + * @return {Promise} - a promise, that resolves in User state */ export function factory(store) { if (promise) { diff --git a/src/services/authFlow/AcceptRulesState.js b/src/services/authFlow/AcceptRulesState.js index 755302d..c1ae468 100644 --- a/src/services/authFlow/AcceptRulesState.js +++ b/src/services/authFlow/AcceptRulesState.js @@ -1,3 +1,5 @@ +import logger from 'services/logger'; + import AbstractState from './AbstractState'; import CompleteState from './CompleteState'; @@ -14,7 +16,8 @@ export default class AcceptRulesState extends AbstractState { resolve(context) { context.run('acceptRules') - .then(() => context.setState(new CompleteState())); + .then(() => context.setState(new CompleteState())) + .catch((err = {}) => err.errors || logger.warn(err)); } reject(context) { diff --git a/src/services/authFlow/ActivationState.js b/src/services/authFlow/ActivationState.js index 491c98c..55e1878 100644 --- a/src/services/authFlow/ActivationState.js +++ b/src/services/authFlow/ActivationState.js @@ -1,3 +1,5 @@ +import logger from 'services/logger'; + import AbstractState from './AbstractState'; import CompleteState from './CompleteState'; import ResendActivationState from './ResendActivationState'; @@ -18,7 +20,8 @@ export default class ActivationState extends AbstractState { resolve(context, payload) { context.run('activate', payload) - .then(() => context.setState(new CompleteState())); + .then(() => context.setState(new CompleteState())) + .catch((err = {}) => err.errors || logger.warn(err)); } reject(context) { diff --git a/src/services/authFlow/AuthFlow.js b/src/services/authFlow/AuthFlow.js index f205df4..ec361ba 100644 --- a/src/services/authFlow/AuthFlow.js +++ b/src/services/authFlow/AuthFlow.js @@ -1,5 +1,7 @@ import { routeActions } from 'react-router-redux'; +import logger from 'services/logger'; + import RegisterState from './RegisterState'; import LoginState from './LoginState'; import OAuthState from './OAuthState'; @@ -88,7 +90,7 @@ export default class AuthFlow { if (this.onReady) { const callback = this.onReady; this.onReady = () => {}; - return resp.then(callback); + return resp.then(callback, (err) => err || logger.warn(err)); } return resp; @@ -119,7 +121,7 @@ export default class AuthFlow { * (state's enter function's promise resolved) */ handleRequest(request, replace, callback = function() {}) { - const {path, params = {}, query = {}} = request; + const {path} = request; this.replace = replace; this.onReady = callback; @@ -175,7 +177,6 @@ export default class AuthFlow { break; default: - console.log('Unsupported request', {path, query, params}); replace('/404'); break; } diff --git a/src/services/authFlow/CompleteState.js b/src/services/authFlow/CompleteState.js index 3e0aae0..00ac4f2 100644 --- a/src/services/authFlow/CompleteState.js +++ b/src/services/authFlow/CompleteState.js @@ -71,8 +71,11 @@ export default class CompleteState extends AbstractState { if (resp.redirectUri.indexOf('static_page') === 0) { context.setState(new FinishState()); } else { - context.run('redirect', resp.redirectUri); - return Promise.reject(); // do not allow loader to be hidden and app to be rendered + return new Promise(() => { + // do not resolve promise to make loader visible and + // overcome app rendering + context.run('redirect', resp.redirectUri); + }); } }, (resp) => { if (resp.unauthorized) { diff --git a/src/services/authFlow/LoginState.js b/src/services/authFlow/LoginState.js index cb63349..04161c6 100644 --- a/src/services/authFlow/LoginState.js +++ b/src/services/authFlow/LoginState.js @@ -1,3 +1,5 @@ +import logger from 'services/logger'; + import AbstractState from './AbstractState'; import PasswordState from './PasswordState'; @@ -20,6 +22,7 @@ export default class LoginState extends AbstractState { resolve(context, payload) { context.run('login', payload) - .then(() => context.setState(new PasswordState())); + .then(() => context.setState(new PasswordState())) + .catch((err = {}) => err.errors || logger.warn(err)); } } diff --git a/src/services/authFlow/RecoverPasswordState.js b/src/services/authFlow/RecoverPasswordState.js index 5d935c0..f4cdac1 100644 --- a/src/services/authFlow/RecoverPasswordState.js +++ b/src/services/authFlow/RecoverPasswordState.js @@ -1,3 +1,5 @@ +import logger from 'services/logger'; + import AbstractState from './AbstractState'; import LoginState from './LoginState'; import CompleteState from './CompleteState'; @@ -18,7 +20,8 @@ export default class RecoverPasswordState extends AbstractState { resolve(context, payload) { context.run('recoverPassword', payload) - .then(() => context.setState(new CompleteState())); + .then(() => context.setState(new CompleteState())) + .catch((err = {}) => err.errors || logger.warn(err)); } goBack(context) { diff --git a/src/services/authFlow/RegisterState.js b/src/services/authFlow/RegisterState.js index 728752d..af0bd4c 100644 --- a/src/services/authFlow/RegisterState.js +++ b/src/services/authFlow/RegisterState.js @@ -1,3 +1,5 @@ +import logger from 'services/logger'; + import AbstractState from './AbstractState'; import CompleteState from './CompleteState'; import ActivationState from './ActivationState'; @@ -16,7 +18,8 @@ export default class RegisterState extends AbstractState { resolve(context, payload) { context.run('register', payload) - .then(() => context.setState(new CompleteState())); + .then(() => context.setState(new CompleteState())) + .catch((err = {}) => err.errors || logger.warn(err)); } reject(context, payload) { diff --git a/src/services/authFlow/ResendActivationState.js b/src/services/authFlow/ResendActivationState.js index db157fc..890d63d 100644 --- a/src/services/authFlow/ResendActivationState.js +++ b/src/services/authFlow/ResendActivationState.js @@ -1,3 +1,5 @@ +import logger from 'services/logger'; + import AbstractState from './AbstractState'; import CompleteState from './CompleteState'; import ActivationState from './ActivationState'; @@ -16,7 +18,8 @@ export default class ResendActivationState extends AbstractState { resolve(context, payload) { context.run('resendActivation', payload) - .then(() => context.setState(new ActivationState())); + .then(() => context.setState(new ActivationState())) + .catch((err = {}) => err.errors || logger.warn(err)); } reject(context) { diff --git a/tests/components/accounts/actions.test.js b/tests/components/accounts/actions.test.js index 29c404e..4544a32 100644 --- a/tests/components/accounts/actions.test.js +++ b/tests/components/accounts/actions.test.js @@ -126,7 +126,7 @@ describe('components/accounts/actions', () => { sessionStorage.removeItem(expectedKey); return authenticate(account)(dispatch).then(() => { - expect(sessionStorage.getItem(expectedKey), 'not to be null') + expect(sessionStorage.getItem(expectedKey), 'not to be null'); sessionStorage.removeItem(expectedKey); }); }); diff --git a/tests/components/user/actions.test.js b/tests/components/user/actions.test.js index 693dce4..479acdd 100644 --- a/tests/components/user/actions.test.js +++ b/tests/components/user/actions.test.js @@ -1,4 +1,5 @@ import expect from 'unexpected'; +import sinon from 'sinon'; import { routeActions } from 'react-router-redux'; @@ -56,24 +57,15 @@ describe('components/user/actions', () => { }); }); - it('should post to /api/authentication/logout with user jwt', () => { - // TODO: need an integration test with a middleware, because this - // test is not reliable to check, whether logout request will have token inside - request.post.returns(new Promise((resolve) => { - setTimeout(() => { - // we must not overwrite user's token till request starts - expect(dispatch, 'was not called'); - - resolve(); - }, 0); - })); - - return callThunk(logout).then(() => { + it('should post to /api/authentication/logout with user jwt', () => + callThunk(logout).then(() => { expect(request.post, 'to have a call satisfying', [ - '/api/authentication/logout', {}, {} + '/api/authentication/logout', {}, { + token: expect.it('not to be empty') + } ]); - }); - }); + }) + ); testChangedToGuest(); testAccountsReset(); diff --git a/tests/polyfills.test.js b/tests/polyfills.test.js index 70d199d..9f816b3 100644 --- a/tests/polyfills.test.js +++ b/tests/polyfills.test.js @@ -2,14 +2,14 @@ import expect from 'unexpected'; describe('promise.prototype.finally', () => { it('should be invoked after promise resolved', () => - expect(new Promise((resolve) => { + expect(new Promise((resolve) => Promise.resolve().finally(resolve) - }), 'to be fulfilled') + ), 'to be fulfilled') ); it('should be invoked after promise rejected', () => - expect(new Promise((resolve) => { - Promise.reject().finally(resolve) - }), 'to be fulfilled') + expect(new Promise((resolve) => + expect(Promise.reject().finally(resolve), 'to be rejected') + ), 'to be fulfilled') ); }); diff --git a/tests/services/authFlow/AcceptRulesState.test.js b/tests/services/authFlow/AcceptRulesState.test.js index f0f05f1..d1c0c0f 100644 --- a/tests/services/authFlow/AcceptRulesState.test.js +++ b/tests/services/authFlow/AcceptRulesState.test.js @@ -1,3 +1,5 @@ +import sinon from 'sinon'; + import AcceptRulesState from 'services/authFlow/AcceptRulesState'; import CompleteState from 'services/authFlow/CompleteState'; @@ -50,7 +52,8 @@ describe('AcceptRulesState', () => { describe('#resolve', () => { it('should call acceptRules', () => { - expectRun(mock, 'acceptRules').returns({then() {}}); + expectRun(mock, 'acceptRules') + .returns(Promise.resolve()); state.resolve(context); }); diff --git a/tests/services/authFlow/ActivationState.test.js b/tests/services/authFlow/ActivationState.test.js index 999bc31..826b261 100644 --- a/tests/services/authFlow/ActivationState.test.js +++ b/tests/services/authFlow/ActivationState.test.js @@ -1,3 +1,5 @@ +import sinon from 'sinon'; + import ActivationState from 'services/authFlow/ActivationState'; import CompleteState from 'services/authFlow/CompleteState'; import ResendActivationState from 'services/authFlow/ResendActivationState'; @@ -73,7 +75,7 @@ describe('ActivationState', () => { mock, 'activate', sinon.match.same(payload) - ).returns({then() {}}); + ).returns(Promise.resolve()); state.resolve(context, payload); }); diff --git a/tests/services/authFlow/LoginState.test.js b/tests/services/authFlow/LoginState.test.js index 54602a5..4688559 100644 --- a/tests/services/authFlow/LoginState.test.js +++ b/tests/services/authFlow/LoginState.test.js @@ -1,3 +1,5 @@ +import sinon from 'sinon'; + import LoginState from 'services/authFlow/LoginState'; import PasswordState from 'services/authFlow/PasswordState'; @@ -52,7 +54,7 @@ describe('LoginState', () => { mock, 'login', sinon.match.same(payload) - ).returns({then() {}}); + ).returns(Promise.resolve()); state.resolve(context, payload); }); diff --git a/tests/services/authFlow/RecoverPasswordState.test.js b/tests/services/authFlow/RecoverPasswordState.test.js index dc2e9f7..10f2f44 100644 --- a/tests/services/authFlow/RecoverPasswordState.test.js +++ b/tests/services/authFlow/RecoverPasswordState.test.js @@ -1,3 +1,5 @@ +import sinon from 'sinon'; + import RecoverPasswordState from 'services/authFlow/RecoverPasswordState'; import CompleteState from 'services/authFlow/CompleteState'; import LoginState from 'services/authFlow/LoginState'; @@ -67,7 +69,7 @@ describe('RecoverPasswordState', () => { mock, 'recoverPassword', sinon.match(expectedPayload) - ).returns({then() {}}); + ).returns(Promise.resolve()); state.resolve(context, expectedPayload); }); diff --git a/tests/services/authFlow/RegisterState.test.js b/tests/services/authFlow/RegisterState.test.js index 3f1ff25..94db655 100644 --- a/tests/services/authFlow/RegisterState.test.js +++ b/tests/services/authFlow/RegisterState.test.js @@ -1,3 +1,5 @@ +import sinon from 'sinon'; + import RegisterState from 'services/authFlow/RegisterState'; import CompleteState from 'services/authFlow/CompleteState'; import ActivationState from 'services/authFlow/ActivationState'; @@ -52,7 +54,7 @@ describe('RegisterState', () => { mock, 'register', sinon.match.same(payload) - ).returns({then() {}}); + ).returns(Promise.resolve()); state.resolve(context, payload); }); diff --git a/tests/services/authFlow/ResendActivationState.test.js b/tests/services/authFlow/ResendActivationState.test.js index 53e1430..d4308a8 100644 --- a/tests/services/authFlow/ResendActivationState.test.js +++ b/tests/services/authFlow/ResendActivationState.test.js @@ -1,3 +1,5 @@ +import sinon from 'sinon'; + import ResendActivationState from 'services/authFlow/ResendActivationState'; import CompleteState from 'services/authFlow/CompleteState'; import ActivationState from 'services/authFlow/ActivationState'; @@ -69,7 +71,7 @@ describe('ResendActivationState', () => { mock, 'resendActivation', sinon.match.same(payload) - ).returns({then() {}}); + ).returns(Promise.resolve()); state.resolve(context, payload); });