From 51f17d0031b89416388b7139dc76a74a9aa66a04 Mon Sep 17 00:00:00 2001 From: Vit Stanislav Date: Fri, 18 May 2018 08:56:57 +0200 Subject: [PATCH 1/7] :white_check_mark: Re-enable test of account api util --- src/utils/api/account.test.js | 27 +++++++++++++++------------ 1 file changed, 15 insertions(+), 12 deletions(-) diff --git a/src/utils/api/account.test.js b/src/utils/api/account.test.js index db87ac44d..410351de3 100644 --- a/src/utils/api/account.test.js +++ b/src/utils/api/account.test.js @@ -6,14 +6,17 @@ import { getAccount, setSecondPassphrase, send, describe('Utils: Account', () => { const address = '1449310910991872227L'; - describe.skip('getAccount', () => { + describe('getAccount', () => { + let activePeer; let activePeerMock; - const activePeer = { - getAccount: () => { }, - }; beforeEach(() => { - activePeerMock = mock(activePeer); + activePeer = { + accounts: { + get: () => { }, + }, + }; + activePeerMock = mock(activePeer.accounts).expects('get').returnsPromise(); }); afterEach(() => { @@ -21,23 +24,23 @@ describe('Utils: Account', () => { activePeerMock.restore(); }); - it('should return a promise that is resolved when activePeer.getAccount() calls its callback with data.success == true', () => { + it('should return a promise that is resolved when activePeer.accounts.get() resolves one account', () => { const account = { address, balance: 0, publicKey: null }; - const response = { success: true, account }; + const response = { data: [account] }; - activePeerMock.expects('getAccount').withArgs(address).callsArgWith(1, response); const requestPromise = getAccount(activePeer, address); + activePeerMock.resolves(response); return expect(requestPromise).to.eventually.eql({ ...account, serverPublicKey: null, }); }); - it('should return a promise that is resolved even when activePeer.getAccount() calls its callback with data.success == false and "Account not found"', () => { - const response = { success: false, error: 'Account not found' }; + it('should return a promise that is resolved even when activePeer.accounts.get() resolves no accounts', () => { + const response = { data: [] }; const account = { address, balance: 0 }; - activePeerMock.expects('getAccount').withArgs(address).callsArgWith(1, response); + activePeerMock.resolves(response); const requestPromise = getAccount(activePeer, address); return expect(requestPromise).to.eventually.eql(account); }); @@ -45,7 +48,7 @@ describe('Utils: Account', () => { it('should otherwise return a promise that is rejected', () => { const response = { success: false }; - activePeerMock.expects('getAccount').withArgs(address).callsArgWith(1, response); + activePeerMock.rejects(response); const requestPromise = getAccount(activePeer, address); return expect(requestPromise).to.eventually.be.rejectedWith(response); }); From 4ea61946d79f73aa08612dcbf4c009974eb6dc36 Mon Sep 17 00:00:00 2001 From: Vit Stanislav Date: Fri, 18 May 2018 08:57:59 +0200 Subject: [PATCH 2/7] :white_check_mark: Re-enable tests of login middleware --- src/store/middlewares/login.test.js | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/src/store/middlewares/login.test.js b/src/store/middlewares/login.test.js index dd97dd60e..51c9fdf53 100644 --- a/src/store/middlewares/login.test.js +++ b/src/store/middlewares/login.test.js @@ -1,6 +1,6 @@ import Lisk from 'lisk-elements'; import { expect } from 'chai'; -import { spy, stub, mock } from 'sinon'; +import { spy, stub, match } from 'sinon'; import middleware from './login'; import actionTypes from '../../constants/actions'; import * as accountApi from '../../utils/api/account'; @@ -42,16 +42,11 @@ describe('Login middleware', () => { expect(next).to.have.been.calledWith(sampleAction); }); - it.skip(`should action data to only have activePeer on ${actionTypes.activePeerSet} action`, () => { + it(`should action data to only have activePeer on ${actionTypes.activePeerSet} action`, () => { middleware(store)(next)(activePeerSetAction); - const peerMock = mock(activePeer.node); - peerMock.expects('getConstants').withArgs() - .returnsPromise().resolves({ nethash }); - peerMock.restore(); - peerMock.verify(); expect(next).to.have.been.calledWith({ type: actionTypes.activePeerSet, - data: activePeer, + data: match.hasNested('activePeer', activePeer), }); }); @@ -66,7 +61,7 @@ describe('Login middleware', () => { delegateApiMock.restore(); }); - it.skip(`should fetch account and delegate info on ${actionTypes.activePeerSet} action (delegate)`, () => { + it(`should fetch account and delegate info on ${actionTypes.activePeerSet} action (delegate)`, () => { const accountApiMock = stub(accountApi, 'getAccount').returnsPromise().resolves({ success: true, balance: 0 }); const delegateApiMock = stub(delegateApi, 'getDelegate').returnsPromise().resolves({ success: true, From 1aa50fe0b3d8b1471017af7632b0441cb003f86e Mon Sep 17 00:00:00 2001 From: Vit Stanislav Date: Fri, 18 May 2018 11:35:42 +0200 Subject: [PATCH 3/7] :bug: Fix setting active peer for mainnet and testnet --- src/actions/peers.js | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/src/actions/peers.js b/src/actions/peers.js index c66387d91..0e1b5a232 100644 --- a/src/actions/peers.js +++ b/src/actions/peers.js @@ -1,9 +1,7 @@ import i18next from 'i18next'; import Lisk from 'lisk-elements'; import actionTypes from '../constants/actions'; -// import { getNethash } from './../utils/api/nethash'; import { errorToastDisplayed } from './toaster'; -import netHashes from '../constants/netHashes'; import { loadingStarted, loadingFinished } from '../utils/loading'; const peerSet = (data, config) => ({ @@ -39,19 +37,20 @@ export const activePeerSet = data => config.ssl = protocol === 'https:'; config.port = port || (config.ssl ? 443 : 80); config.nodes = [`${protocol}//${hostname}:${port}`]; + } else if (config.testnet) { + config.nodes = Lisk.constants.TESTNET_NODES; + config.nethash = Lisk.constants.TESTNET_NETHASH; + } else { + config.nodes = Lisk.constants.MAINNET_NODES; + config.nethash = Lisk.constants.MAINNET_NETHASH; } - if (config.testnet === undefined && config.port !== undefined) { - config.testnet = config.port === '7000'; - } + if (config.custom) { - const getNethash = new Lisk.APIClient(config.nodes, { nethash: config.nethash }); + const liskAPIClient = new Lisk.APIClient(config.nodes, { nethash: config.nethash }); loadingStarted('getConstants'); - getNethash.node.getConstants().then((response) => { + liskAPIClient.node.getConstants().then((response) => { loadingFinished('getConstants'); - config.testnet = response.data.nethash === netHashes.testnet; - if (!config.testnet && response.data.nethash !== netHashes.mainnet) { - config.nethash = response.data.nethash; - } + config.nethash = response.data.nethash; dispatch(peerSet(data, config)); }).catch(() => { loadingFinished('getConstants'); From a2faa28f2ea658b02f5705cc5be2719c03d4a5e3 Mon Sep 17 00:00:00 2001 From: Vit Stanislav Date: Fri, 18 May 2018 11:36:42 +0200 Subject: [PATCH 4/7] :white_check_mark: Re-enable and fix tests for activePeerSet action --- src/actions/peers.test.js | 102 +++++++++++++++---------------------- src/constants/netHashes.js | 6 --- 2 files changed, 41 insertions(+), 67 deletions(-) delete mode 100644 src/constants/netHashes.js diff --git a/src/actions/peers.test.js b/src/actions/peers.test.js index 563d1701a..17886b244 100644 --- a/src/actions/peers.test.js +++ b/src/actions/peers.test.js @@ -2,13 +2,12 @@ import { expect } from 'chai'; import { spy, stub, match } from 'sinon'; import Lisk from 'lisk-elements'; import actionTypes from '../constants/actions'; -import netHashes from '../constants/netHashes'; import { activePeerSet, activePeerUpdate } from './peers'; -describe.skip('actions: peers', () => { +describe('actions: peers', () => { const passphrase = 'wagon stock borrow episode laundry kitten salute link globe zero feed marble'; const nethash = '198f2b61a8eb95fbeed58b8216780b68f697f26b849acf00c8c93bb9b24f783d'; - const nethashApi = new Lisk.APIClient(['http://localhost:4000'], { nethash }); + describe('activePeerUpdate', () => { it('should create an action to update the active peer', () => { const data = { @@ -25,92 +24,73 @@ describe.skip('actions: peers', () => { describe('activePeerSet', () => { let dispatch; - let getNetHash; + let getNetHashMock; beforeEach(() => { dispatch = spy(); - const node = nethashApi.node; - getNetHash = stub(node, 'getConstants'); + // TODO: this doesn't work because Lisk.APIClient is called with 'new' + getNetHashMock = stub(Lisk, 'APIClient').returns({ + node: { + getConstants: new Promise((resolve) => { + resolve({ data: { nethash } }); + }), + }, + }); }); afterEach(() => { - getNetHash.restore(); - }); - - it('creates active peer config', () => { - getNetHash.returnsPromise(); - const data = { - passphrase, - network: { - name: 'Custom Node', - custom: true, - address: 'http://localhost:4000', - testnet: true, - nethash, - }, - }; - - activePeerSet(data)(dispatch); - getNetHash.resolves({ data: { nethash } }); - - expect(dispatch).to.have.been.calledOnce(); + getNetHashMock.restore(); }); it('dispatch activePeerSet action also when address http missing', () => { - const network = { address: 'localhost:8000' }; - - activePeerSet({ passphrase, network })(dispatch); - - expect(dispatch).to.have.been.calledWith(match.hasNested('data.activePeer.options.address', 'localhost:8000')); - }); - - it('dispatch activePeerSet with testnet config set to true when the network is a custom node and nethash is testnet', () => { - getNetHash.returnsPromise(); const network = { - address: 'http://localhost:4000', - custom: true, + address: 'localhost:8000', + nethash: Lisk.constants.MAINNET_NETHASH, }; activePeerSet({ passphrase, network })(dispatch); - getNetHash.resolves({ data: { nethash: netHashes.testnet } }); - expect(dispatch).to.have.been.calledWith(match.hasNested('data.activePeer.testnet', true)); + expect(dispatch).to.have.been.calledWith(match.hasNested('data.options.address', 'localhost:8000')); }); - it('dispatch activePeerSet with testnet config set to false when the network is a custom node and nethash is testnet', () => { - getNetHash.returnsPromise(); - const network = { - address: 'http://localhost:4000', - custom: true, - }; - - activePeerSet({ passphrase, network })(dispatch); - getNetHash.resolves({ data: { nethash: 'some other nethash' } }); + it('dispatch activePeerSet action with mainnet nodes if network.address is undefined', () => { + activePeerSet({ passphrase, network: {} })(dispatch); - expect(dispatch).to.have.been.calledWith(match.hasNested('data.activePeer.testnet', false)); + expect(dispatch).to.have.been.calledWith(match.hasNested('data.options.nodes', Lisk.constants.MAINNET_NODES)); }); - it('dispatch activePeerSet action even if network is undefined', () => { + it('dispatch activePeerSet action with mainnet nodes if network is undefined', () => { activePeerSet({ passphrase })(dispatch); - expect(dispatch).to.have.been.calledWith(); + expect(dispatch).to.have.been.calledWith(match.hasNested('data.options.nodes', Lisk.constants.MAINNET_NODES)); }); - it('dispatch activePeerSet action even if network.address is undefined', () => { - activePeerSet({ passphrase, network: {} })(dispatch); + it('dispatch activePeerSet action with testnet nodes if testnet option is set', () => { + const network = { + testnet: true, + }; - expect(dispatch).to.have.been.calledWith(); + activePeerSet({ passphrase, network })(dispatch); + + expect(dispatch).to.have.been.calledWith(match.hasNested('data.options.nodes', Lisk.constants.TESTNET_NODES)); }); - it('should set to testnet if not defined in config but port is 7000', () => { - const network7000 = { address: 'http://127.0.0.1:7000', nethash }; - const network4000 = { address: 'http://127.0.0.1:4000', nethash }; + // skipped because I don't know how to stub liskAPIClient.node.getConstants() + it.skip('dispatch activePeerSet action with custom node', () => { + const data = { + passphrase, + network: { + name: 'Custom Node', + custom: true, + address: 'http://localhost:4000', + testnet: true, + nethash, + }, + }; - activePeerSet({ passphrase, network: network7000 })(dispatch); - expect(dispatch).to.have.been.calledWith(match.hasNested('data.activePeer.options.testnet', true)); + activePeerSet(data)(dispatch); - activePeerSet({ passphrase, network: network4000 })(dispatch); - expect(dispatch).to.have.been.calledWith(match.hasNested('data.activePeer.options.testnet', false)); + expect(dispatch).to.have.been.calledWith(match.hasNested('data.options.nodes', [data.network.address])); }); }); }); diff --git a/src/constants/netHashes.js b/src/constants/netHashes.js deleted file mode 100644 index 5c9baac29..000000000 --- a/src/constants/netHashes.js +++ /dev/null @@ -1,6 +0,0 @@ -const netHash = { - testnet: 'da3ed6a45429278bac2666961289ca17ad86595d33b31037615d4b8e8f158bba', - mainnet: 'ed14889723f24ecc54871d058d98ce91ff2f973192075c0155ba2b7b70ad2511', -}; - -export default netHash; From d9f29dd6d0863c1379825b9517ce9bf68347d1f8 Mon Sep 17 00:00:00 2001 From: Vit Stanislav Date: Fri, 18 May 2018 14:33:15 +0200 Subject: [PATCH 5/7] :white_check_mark: Add unit tests for custom active peer --- src/actions/peers.test.js | 46 +++++++++++++++++++++++++++++---------- 1 file changed, 34 insertions(+), 12 deletions(-) diff --git a/src/actions/peers.test.js b/src/actions/peers.test.js index 17886b244..e15912667 100644 --- a/src/actions/peers.test.js +++ b/src/actions/peers.test.js @@ -24,22 +24,26 @@ describe('actions: peers', () => { describe('activePeerSet', () => { let dispatch; - let getNetHashMock; + let APIClientBackup; + let getConstantsMock; beforeEach(() => { dispatch = spy(); - // TODO: this doesn't work because Lisk.APIClient is called with 'new' - getNetHashMock = stub(Lisk, 'APIClient').returns({ - node: { - getConstants: new Promise((resolve) => { - resolve({ data: { nethash } }); - }), - }, - }); + getConstantsMock = stub().returnsPromise(); + APIClientBackup = Lisk.APIClient; + + // TODO: find a better way of mocking Lisk.APIClient + Lisk.APIClient = class MockAPIClient { + constructor() { + this.node = { + getConstants: getConstantsMock, + }; + } + }; }); afterEach(() => { - getNetHashMock.restore(); + Lisk.APIClient = APIClientBackup; }); it('dispatch activePeerSet action also when address http missing', () => { @@ -75,8 +79,7 @@ describe('actions: peers', () => { expect(dispatch).to.have.been.calledWith(match.hasNested('data.options.nodes', Lisk.constants.TESTNET_NODES)); }); - // skipped because I don't know how to stub liskAPIClient.node.getConstants() - it.skip('dispatch activePeerSet action with custom node', () => { + it('dispatch activePeerSet action with custom node', () => { const data = { passphrase, network: { @@ -87,10 +90,29 @@ describe('actions: peers', () => { nethash, }, }; + getConstantsMock.resolves({ data: { nethash } }); activePeerSet(data)(dispatch); expect(dispatch).to.have.been.calledWith(match.hasNested('data.options.nodes', [data.network.address])); }); + + it('dispatch error toast action if getConstants() API call fails', () => { + const data = { + passphrase, + network: { + name: 'Custom Node', + custom: true, + address: 'http://localhost:4000', + testnet: true, + nethash, + }, + }; + getConstantsMock.rejects({}); + + activePeerSet(data)(dispatch); + + expect(dispatch).to.have.been.calledWith(match.has('type', actionTypes.toastDisplayed)); + }); }); }); From fb9b9cc56d651df97cc33d82e7569e5a4a4faa82 Mon Sep 17 00:00:00 2001 From: Vit Stanislav Date: Wed, 23 May 2018 13:44:22 +0200 Subject: [PATCH 6/7] Fix an eslint violation after rebase --- src/store/middlewares/login.test.js | 1 - 1 file changed, 1 deletion(-) diff --git a/src/store/middlewares/login.test.js b/src/store/middlewares/login.test.js index 51c9fdf53..0208f1942 100644 --- a/src/store/middlewares/login.test.js +++ b/src/store/middlewares/login.test.js @@ -10,7 +10,6 @@ describe('Login middleware', () => { let store; let next; const passphrase = 'wagon stock borrow episode laundry kitten salute link globe zero feed marble'; - const nethash = '198f2b61a8eb95fbeed58b8216780b68f697f26b849acf00c8c93bb9b24f783d'; const activePeer = new Lisk.APIClient(['http://localhost:4000'], {}); const activePeerSetAction = { From 30174a609ddd339de0f9e8f072c07e26a2cc7f9b Mon Sep 17 00:00:00 2001 From: Vit Stanislav Date: Thu, 24 May 2018 09:44:59 +0200 Subject: [PATCH 7/7] Update peers actions after lisk-elements moved some constants ... to Lisk.APIClient --- src/actions/peers.js | 8 ++++---- src/actions/peers.test.js | 9 +++++---- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/src/actions/peers.js b/src/actions/peers.js index 0e1b5a232..9f694ebcd 100644 --- a/src/actions/peers.js +++ b/src/actions/peers.js @@ -38,11 +38,11 @@ export const activePeerSet = data => config.port = port || (config.ssl ? 443 : 80); config.nodes = [`${protocol}//${hostname}:${port}`]; } else if (config.testnet) { - config.nodes = Lisk.constants.TESTNET_NODES; - config.nethash = Lisk.constants.TESTNET_NETHASH; + config.nodes = Lisk.APIClient.constants.TESTNET_NODES; + config.nethash = Lisk.APIClient.constants.TESTNET_NETHASH; } else { - config.nodes = Lisk.constants.MAINNET_NODES; - config.nethash = Lisk.constants.MAINNET_NETHASH; + config.nodes = Lisk.APIClient.constants.MAINNET_NODES; + config.nethash = Lisk.APIClient.constants.MAINNET_NETHASH; } if (config.custom) { diff --git a/src/actions/peers.test.js b/src/actions/peers.test.js index e15912667..7adb3b089 100644 --- a/src/actions/peers.test.js +++ b/src/actions/peers.test.js @@ -40,6 +40,7 @@ describe('actions: peers', () => { }; } }; + Lisk.APIClient.constants = APIClientBackup.constants; }); afterEach(() => { @@ -49,7 +50,7 @@ describe('actions: peers', () => { it('dispatch activePeerSet action also when address http missing', () => { const network = { address: 'localhost:8000', - nethash: Lisk.constants.MAINNET_NETHASH, + nethash: Lisk.APIClient.constants.MAINNET_NETHASH, }; activePeerSet({ passphrase, network })(dispatch); @@ -60,13 +61,13 @@ describe('actions: peers', () => { it('dispatch activePeerSet action with mainnet nodes if network.address is undefined', () => { activePeerSet({ passphrase, network: {} })(dispatch); - expect(dispatch).to.have.been.calledWith(match.hasNested('data.options.nodes', Lisk.constants.MAINNET_NODES)); + expect(dispatch).to.have.been.calledWith(match.hasNested('data.options.nodes', Lisk.APIClient.constants.MAINNET_NODES)); }); it('dispatch activePeerSet action with mainnet nodes if network is undefined', () => { activePeerSet({ passphrase })(dispatch); - expect(dispatch).to.have.been.calledWith(match.hasNested('data.options.nodes', Lisk.constants.MAINNET_NODES)); + expect(dispatch).to.have.been.calledWith(match.hasNested('data.options.nodes', Lisk.APIClient.constants.MAINNET_NODES)); }); it('dispatch activePeerSet action with testnet nodes if testnet option is set', () => { @@ -76,7 +77,7 @@ describe('actions: peers', () => { activePeerSet({ passphrase, network })(dispatch); - expect(dispatch).to.have.been.calledWith(match.hasNested('data.options.nodes', Lisk.constants.TESTNET_NODES)); + expect(dispatch).to.have.been.calledWith(match.hasNested('data.options.nodes', Lisk.APIClient.constants.TESTNET_NODES)); }); it('dispatch activePeerSet action with custom node', () => {