Unit tests for Multisignatures module - Closes #1723 #2326
Conversation
yatki
force-pushed
the
1723-unit-tests-for-multisigatures
branch
from
cab716c
to
ed14605
Compare
yatki
changed the title
|
@4miners the initial skeleton for unit tests was huge, so idea was to separate them into several PRs. But it's not needed anymore. I updated the PR. |
test/unit/modules/multisignatures.js
Outdated
|
|
||
| const rewiredMultisignatures = rewire('../../../modules/multisignatures.js'); | ||
|
|
||
| const validAccount = { |
There was a problem hiding this comment.
Please use fixtures for that, you already importing those (accountsFixtures).
test/unit/modules/multisignatures.js
Outdated
| @@ -99,6 +138,7 @@ describe('multisignatures', () => { | |||
| // Create instance of multisignatures module | |||
| multisignaturesInstance = new rewiredMultisignatures( | |||
| (err, __multisignatures) => { | |||
| error = err; | |||
There was a problem hiding this comment.
Not really needed to use global variable here, you can call done(err) instead.
test/unit/modules/multisignatures.js
Outdated
| .calledOnce; | ||
| }); | ||
|
|
||
| it('assign __private.assetTypes[transactionTypes.MULTI]', () => { |
test/unit/modules/multisignatures.js
Outdated
| done(); | ||
| }); | ||
|
|
||
| it('should accept address as parameter', done => { |
There was a problem hiding this comment.
description doesn't match what is tested, please clarify
test/unit/modules/multisignatures.js
Outdated
| }); | ||
| }); | ||
|
|
||
| it('should fail if getMultiSignature function returns an error', done => { |
There was a problem hiding this comment.
it's not failing, it's calling callback with error
test/unit/modules/multisignatures.js
Outdated
| }); | ||
| }); | ||
|
|
||
| it('should fail if given address does not return an account', done => { |
There was a problem hiding this comment.
wrong description, you're not testing the behavior of getMultiSignature, so you cannot be sure of the reason why this function returning empty account
test/unit/modules/multisignatures.js
Outdated
| expect(get('modules').accounts.getAccounts).to.be.calledWith({ | ||
| address: ['address1'], | ||
| }); | ||
| expect(scopeGroup.members) |
There was a problem hiding this comment.
please also test with more than 1 member
There was a problem hiding this comment.
I refactored this test to have multiple members instead of adding another one.
test/unit/modules/multisignatures.js
Outdated
| done(); | ||
| }); | ||
|
|
||
| it('should fail library.logic.account.get function returns an error', done => { |
There was a problem hiding this comment.
should call a callback with ApiError instance when...
Review checklist