4miners requested changes on this pull request.

>
const rewiredMultisignatures = rewire(‘../../../modules/multisignatures.js’);

+const validAccount = {

Please use fixtures for that, you already importing those (`accountsFixtures`).

> @@ -99,6 +138,7 @@ describe(‘multisignatures’, () => {
// Create instance of multisignatures module
multisignaturesInstance = new rewiredMultisignatures(
(err, __multisignatures) => {
+ error = err;

Not really needed to use global variable here, you can call `done(err)` instead.

> @@ -138,6 +178,23 @@ describe(‘multisignatures’, () => {
it(‘should call callback with result = self’, () => {
return expect(self).to.be.deep.equal(multisignaturesInstance);
});
+
+ it(‘should return error = null’, () => {
+ return expect(error).to.equal(null);
+ });
+
+ describe(‘__private’, () => {
+ it(‘should call library.logic.transaction.attachAssetType’, () => {
+ return expect(library.logic.transaction.attachAssetType).to.have.been
+ .calledOnce;
+ });
+
+ it(‘assign __private.assetTypes[transactionTypes.MULTI]’, () => {

`should assign`

> + stubs.modules.accounts.generateAddressByPublicKey = sinonSandbox
+ .stub()
+ .withArgs(‘key1’)
+ .returns(‘address1’);
+
+ library.logic.account.getMultiSignature =
+ stubs.logic.account.getMultiSignature;
+ library.db.multisignatures.getMemberPublicKeys =
+ stubs.getMemberPublicKeys;
+ get(‘modules’).accounts.getAccounts = stubs.modules.accounts.getAccounts;
+ get(‘modules’).accounts.generateAddressByPublicKey =
+ stubs.modules.accounts.generateAddressByPublicKey;
+ done();
+ });
+
+ it(‘should accept address as parameter’, done => {

description doesn’t match what is tested, please clarify

> + get(‘modules’).accounts.generateAddressByPublicKey =
+ stubs.modules.accounts.generateAddressByPublicKey;
+ done();
+ });
+
+ it(‘should accept address as parameter’, done => {
+ self.getGroup(validAccount.address, (err, scopeGroup) => {
+ expect(library.logic.account.getMultiSignature).to.be.calledWith({
+ address: validAccount.address,
+ });
+ expect(scopeGroup.address).to.equal(validAccount.address);
+ done();
+ });
+ });
+
+ it(‘should fail if getMultiSignature function returns an error’, done => {

it’s not failing, it’s calling callback with error

> + });
+
+ it(‘should fail if getMultiSignature function returns an error’, done => {
+ library.logic.account.getMultiSignature = sinonSandbox
+ .stub()
+ .callsFake((filters, cb) => {
+ cb(‘Err’, null);
+ });
+ self.getGroup(validAccount.address, (err, scopeGroup) => {
+ expect(err).to.equal(‘Err’);
+ expect(scopeGroup).to.not.exist;
+ done();
+ });
+ });
+
+ it(‘should fail if given address does not return an account’, done => {

wrong description, you’re not testing the behavior of `getMultiSignature`, so you cannot be sure of the reason why this function returning empty account

> + const member = {
+ address: ‘address’,
+ publicKey: ‘publicKey’,
+ secondPublicKey: ‘secondPublicKey’,
+ };
+
+ stubs.modules.accounts.getAccounts = sinonSandbox
+ .stub()
+ .callsFake((param1, param2, cb) => cb(null, [member]));
+
+ self.getGroup(validAccount.address, (err, scopeGroup) => {
+ expect(err).to.not.exist;
+ expect(get(‘modules’).accounts.getAccounts).to.be.calledWith({
+ address: [‘address1’],
+ });
+ expect(scopeGroup.members)

please also test with more than 1 member

> + stubs.getGroupIds = sinonSandbox
+ .stub()
+ .callsFake(() => Promise.resolve([stubGroupId]));
+
+ stubs.getGroup = sinonSandbox
+ .stub()
+ .withArgs(stubGroupId)
+ .callsFake((address, cb) => cb(null, getGroupResponse));
+
+ library.logic.account.get = stubs.logic.account.get;
+ library.db.multisignatures.getGroupIds = stubs.getGroupIds;
+ self.getGroup = stubs.getGroup;
+ done();
+ });
+
+ it(‘should fail library.logic.account.get function returns an error’, done => {

`should call a callback with ApiError instance when…`