From 1ce646e5a62c8d554e0a0da08c7897786821d3b1 Mon Sep 17 00:00:00 2001 From: Victor Wiebe Date: Fri, 15 Nov 2019 09:35:10 -0800 Subject: [PATCH 01/10] test: transferErc20 procedure preliminary tests --- src/procedures/__tests__/TransferErc20.ts | 153 ++++++++++++++++++++++ 1 file changed, 153 insertions(+) create mode 100644 src/procedures/__tests__/TransferErc20.ts diff --git a/src/procedures/__tests__/TransferErc20.ts b/src/procedures/__tests__/TransferErc20.ts new file mode 100644 index 0000000..c090ed3 --- /dev/null +++ b/src/procedures/__tests__/TransferErc20.ts @@ -0,0 +1,153 @@ +import { ImportMock, MockManager } from 'ts-mock-imports'; +import { restore, spy } from 'sinon'; +import * as contractWrappersModule from '@polymathnetwork/contract-wrappers'; +import { BigNumber, TransactionReceiptWithDecodedLogs } from '@polymathnetwork/contract-wrappers'; +import { Procedure } from '~/procedures/Procedure'; +import { ErrorCode, ProcedureType, TransferErc20ProcedureArgs } from '~/types'; +import * as erc20TokenBalanceFactoryModule from '~/entities/factories/Erc20TokenBalanceFactory'; +import * as contextModule from '../../Context'; +import * as wrappersModule from '../../PolymathBase'; +import * as tokenFactoryModule from '../../testUtils/MockedTokenFactoryModule'; +import * as moduleWrapperFactoryModule from '../../testUtils/MockedModuleWrapperFactoryModule'; +import { Wallet } from '~/Wallet'; +import { TransferErc20 } from '~/procedures'; +import { mockFactories } from '~/testUtils/mockFactories'; +import { PolymathError } from '~/PolymathError'; + +const params: TransferErc20ProcedureArgs = { + amount: new BigNumber(10), + receiver: '0x6666666666666666666666666666666666666666', + tokenAddress: '0x7777777777777777777777777777777777777777', +}; + +describe('TransferErc20', () => { + let target: TransferErc20; + let contextMock: MockManager; + let wrappersMock: MockManager; + let tokenFactoryMock: MockManager; + let moduleWrapperFactoryMock: MockManager< + moduleWrapperFactoryModule.MockedModuleWrapperFactoryModule + >; + let polyTokenMock: MockManager; + + // Mock factories + let erc20TokenBalanceFactoryMock: MockManager< + erc20TokenBalanceFactoryModule.Erc20TokenBalanceFactory + >; + + let securityTokenRegistryMock: MockManager; + + let moduleFactoryMock: MockManager; + + let erc20Mock: MockManager; + + beforeEach(() => { + // Mock the context, wrappers, and tokenFactory to test LaunchCappedSto + contextMock = ImportMock.mockClass(contextModule, 'Context'); + wrappersMock = ImportMock.mockClass(wrappersModule, 'PolymathBase'); + tokenFactoryMock = ImportMock.mockClass(tokenFactoryModule, 'MockedTokenFactoryModule'); + moduleWrapperFactoryMock = ImportMock.mockClass( + moduleWrapperFactoryModule, + 'MockedModuleWrapperFactoryModule' + ); + + contextMock.set('contractWrappers', wrappersMock.getMockInstance()); + wrappersMock.set('tokenFactory', tokenFactoryMock.getMockInstance()); + wrappersMock.set('moduleFactory', moduleWrapperFactoryMock.getMockInstance()); + + erc20Mock = ImportMock.mockClass(contractWrappersModule, 'ERC20'); + erc20Mock.mock('balanceOf', Promise.resolve(new BigNumber(20))); + + erc20Mock.mock('address', Promise.resolve(params.tokenAddress)); + + moduleFactoryMock = ImportMock.mockClass(contractWrappersModule, 'ModuleFactory_3_0_0'); + moduleFactoryMock.mock('setupCostInPoly', Promise.resolve(new BigNumber(1))); + moduleFactoryMock.mock('isCostInPoly', Promise.resolve(false)); + moduleFactoryMock.mock('setupCost', Promise.resolve(new BigNumber(1))); + + securityTokenRegistryMock = ImportMock.mockClass( + contractWrappersModule, + 'SecurityTokenRegistry' + ); + + wrappersMock.set('securityTokenRegistry', securityTokenRegistryMock.getMockInstance()); + + securityTokenRegistryMock.mock('isSecurityToken', Promise.resolve(false)); + + wrappersMock.mock('getERC20TokenWrapper', erc20Mock.getMockInstance()); + moduleWrapperFactoryMock.mock('getModuleFactory', moduleFactoryMock.getMockInstance()); + + erc20TokenBalanceFactoryMock = ImportMock.mockClass( + erc20TokenBalanceFactoryModule, + 'Erc20TokenBalanceFactory' + ); + + const factoryMockSetup = mockFactories(); + factoryMockSetup.erc20TokenBalanceFactory = erc20TokenBalanceFactoryMock.getMockInstance(); + + erc20TokenBalanceFactoryMock.mock('refresh', {}); + contextMock.set('factories', factoryMockSetup); + contextMock.set( + 'currentWallet', + new Wallet({ address: () => Promise.resolve(params.receiver) }) + ); + + polyTokenMock = ImportMock.mockClass(contractWrappersModule, 'PolyToken'); + polyTokenMock.mock('address', Promise.resolve(params.receiver)); + polyTokenMock.mock('allowance', Promise.resolve(new BigNumber(0))); + wrappersMock.set('polyToken', polyTokenMock.getMockInstance()); + wrappersMock.mock('isTestnet', Promise.resolve(false)); + + wrappersMock.mock('getModuleFactoryAddress', Promise.resolve(params.receiver)); + + // Instantiate TransferErc20 + target = new TransferErc20(params, contextMock.getMockInstance()); + }); + afterEach(() => { + restore(); + }); + + describe('Types', () => { + test('should extend procedure and have TransferErc20 type', async () => { + expect(target instanceof Procedure).toBe(true); + expect(target.type).toBe(ProcedureType.TransferErc20); + }); + }); + + describe('TransferErc20', () => { + test('should add a transaction to the queue to transfer an erc20 token', async () => { + const addTransactionSpy = spy(target, 'addTransaction'); + // Real call + await target.prepareTransactions(); + + // Verifications + expect(addTransactionSpy.getCall(0).calledWith(erc20Mock.getMockInstance().transfer)).toEqual( + true + ); + expect(addTransactionSpy.callCount).toEqual(1); + }); + + test('should throw if address belongs to a security token, not an erc20 token', async () => { + securityTokenRegistryMock.mock('isSecurityToken', Promise.resolve(true)); + + await expect(target.prepareTransactions()).rejects.toThrow( + new PolymathError({ + code: ErrorCode.ProcedureValidationError, + message: + "This address belongs to a Security Token. To transfer Security Tokens, use the functions in the Security Token's transfers namespace", + }) + ); + }); + + test('should throw if there are not enough funds to make transfer', async () => { + erc20Mock.mock('balanceOf', Promise.resolve(new BigNumber(2))); + + await expect(target.prepareTransactions()).rejects.toThrow( + new PolymathError({ + code: ErrorCode.ProcedureValidationError, + message: 'Not enough funds', + }) + ); + }); + }); +}); From 055527fb004dc3dbff50edbdef5181239f34fe33 Mon Sep 17 00:00:00 2001 From: Victor Wiebe Date: Fri, 15 Nov 2019 14:53:40 -0800 Subject: [PATCH 02/10] refactor: transferErc20 and improve its test --- src/procedures/TransferErc20.ts | 23 +++++++++++++++++------ src/procedures/__tests__/TransferErc20.ts | 9 --------- 2 files changed, 17 insertions(+), 15 deletions(-) diff --git a/src/procedures/TransferErc20.ts b/src/procedures/TransferErc20.ts index ab2ca21..6d04649 100644 --- a/src/procedures/TransferErc20.ts +++ b/src/procedures/TransferErc20.ts @@ -3,6 +3,7 @@ import { Procedure } from './Procedure'; import { TransferErc20ProcedureArgs, ErrorCode, ProcedureType, PolyTransactionTag } from '../types'; import { PolymathError } from '../PolymathError'; import { Erc20TokenBalance } from '../entities'; +import { Factories } from '~/Context'; /** * Procedure to transfer funds of an ERC20 token. If no token address is specified, it defaults to POLY @@ -34,7 +35,9 @@ export class TransferErc20 extends Procedure { } try { - token = await contractWrappers.getERC20TokenWrapper({ address: tokenAddress }); + token = await contractWrappers.getERC20TokenWrapper({ + address: tokenAddress, + }); } catch (err) { throw new PolymathError({ code: ErrorCode.ProcedureValidationError, @@ -70,11 +73,19 @@ export class TransferErc20 extends Procedure { await this.addTransaction(token.transfer, { tag: PolyTransactionTag.TransferErc20, - resolver: async _receipt => { - return factories.erc20TokenBalanceFactory.refresh( - Erc20TokenBalance.generateId({ tokenAddress: address, walletAddress: receiver }) - ); - }, + resolver: createTransferErc20Resolver(factories, address, receiver), })({ to: receiver, value: amount }); } } +export const createTransferErc20Resolver = ( + factories: Factories, + tokenAddress: string, + receiver: string +) => async () => { + return factories.erc20TokenBalanceFactory.refresh( + Erc20TokenBalance.generateId({ + tokenAddress, + walletAddress: receiver, + }) + ); +}; diff --git a/src/procedures/__tests__/TransferErc20.ts b/src/procedures/__tests__/TransferErc20.ts index c090ed3..7e3a92e 100644 --- a/src/procedures/__tests__/TransferErc20.ts +++ b/src/procedures/__tests__/TransferErc20.ts @@ -37,8 +37,6 @@ describe('TransferErc20', () => { let securityTokenRegistryMock: MockManager; - let moduleFactoryMock: MockManager; - let erc20Mock: MockManager; beforeEach(() => { @@ -60,11 +58,6 @@ describe('TransferErc20', () => { erc20Mock.mock('address', Promise.resolve(params.tokenAddress)); - moduleFactoryMock = ImportMock.mockClass(contractWrappersModule, 'ModuleFactory_3_0_0'); - moduleFactoryMock.mock('setupCostInPoly', Promise.resolve(new BigNumber(1))); - moduleFactoryMock.mock('isCostInPoly', Promise.resolve(false)); - moduleFactoryMock.mock('setupCost', Promise.resolve(new BigNumber(1))); - securityTokenRegistryMock = ImportMock.mockClass( contractWrappersModule, 'SecurityTokenRegistry' @@ -75,7 +68,6 @@ describe('TransferErc20', () => { securityTokenRegistryMock.mock('isSecurityToken', Promise.resolve(false)); wrappersMock.mock('getERC20TokenWrapper', erc20Mock.getMockInstance()); - moduleWrapperFactoryMock.mock('getModuleFactory', moduleFactoryMock.getMockInstance()); erc20TokenBalanceFactoryMock = ImportMock.mockClass( erc20TokenBalanceFactoryModule, @@ -94,7 +86,6 @@ describe('TransferErc20', () => { polyTokenMock = ImportMock.mockClass(contractWrappersModule, 'PolyToken'); polyTokenMock.mock('address', Promise.resolve(params.receiver)); - polyTokenMock.mock('allowance', Promise.resolve(new BigNumber(0))); wrappersMock.set('polyToken', polyTokenMock.getMockInstance()); wrappersMock.mock('isTestnet', Promise.resolve(false)); From a075aaf3c031387e2aa628da153318b19740bcde Mon Sep 17 00:00:00 2001 From: Victor Wiebe Date: Mon, 18 Nov 2019 09:39:06 -0800 Subject: [PATCH 03/10] test: coverage increased to 100 for error cases --- src/procedures/__tests__/TransferErc20.ts | 64 ++++++++++++++++++++--- 1 file changed, 58 insertions(+), 6 deletions(-) diff --git a/src/procedures/__tests__/TransferErc20.ts b/src/procedures/__tests__/TransferErc20.ts index 7e3a92e..191896b 100644 --- a/src/procedures/__tests__/TransferErc20.ts +++ b/src/procedures/__tests__/TransferErc20.ts @@ -2,7 +2,7 @@ import { ImportMock, MockManager } from 'ts-mock-imports'; import { restore, spy } from 'sinon'; import * as contractWrappersModule from '@polymathnetwork/contract-wrappers'; import { BigNumber, TransactionReceiptWithDecodedLogs } from '@polymathnetwork/contract-wrappers'; -import { Procedure } from '~/procedures/Procedure'; +import { Procedure } from '../Procedure'; import { ErrorCode, ProcedureType, TransferErc20ProcedureArgs } from '~/types'; import * as erc20TokenBalanceFactoryModule from '~/entities/factories/Erc20TokenBalanceFactory'; import * as contextModule from '../../Context'; @@ -11,8 +11,11 @@ import * as tokenFactoryModule from '../../testUtils/MockedTokenFactoryModule'; import * as moduleWrapperFactoryModule from '../../testUtils/MockedModuleWrapperFactoryModule'; import { Wallet } from '~/Wallet'; import { TransferErc20 } from '~/procedures'; -import { mockFactories } from '~/testUtils/mockFactories'; -import { PolymathError } from '~/PolymathError'; +import * as transferErc20Module from '~/procedures/TransferErc20'; +import { mockFactories } from '../../testUtils/mockFactories'; +import { PolymathError } from '../../PolymathError'; +import { Erc20TokenBalance } from '../../entities'; +import { Factories } from '../../Context'; const params: TransferErc20ProcedureArgs = { amount: new BigNumber(10), @@ -38,9 +41,10 @@ describe('TransferErc20', () => { let securityTokenRegistryMock: MockManager; let erc20Mock: MockManager; + let factoryMockSetup: Factories; beforeEach(() => { - // Mock the context, wrappers, and tokenFactory to test LaunchCappedSto + // Mock the context, wrappers, and tokenFactory to test TransferErc20 contextMock = ImportMock.mockClass(contextModule, 'Context'); wrappersMock = ImportMock.mockClass(wrappersModule, 'PolymathBase'); tokenFactoryMock = ImportMock.mockClass(tokenFactoryModule, 'MockedTokenFactoryModule'); @@ -74,7 +78,7 @@ describe('TransferErc20', () => { 'Erc20TokenBalanceFactory' ); - const factoryMockSetup = mockFactories(); + factoryMockSetup = mockFactories(); factoryMockSetup.erc20TokenBalanceFactory = erc20TokenBalanceFactoryMock.getMockInstance(); erc20TokenBalanceFactoryMock.mock('refresh', {}); @@ -85,7 +89,7 @@ describe('TransferErc20', () => { ); polyTokenMock = ImportMock.mockClass(contractWrappersModule, 'PolyToken'); - polyTokenMock.mock('address', Promise.resolve(params.receiver)); + polyTokenMock.mock('address', Promise.resolve(params.tokenAddress)); wrappersMock.set('polyToken', polyTokenMock.getMockInstance()); wrappersMock.mock('isTestnet', Promise.resolve(false)); @@ -118,6 +122,20 @@ describe('TransferErc20', () => { expect(addTransactionSpy.callCount).toEqual(1); }); + test('should throw if supplied address does not correspond to erc20 token', async () => { + wrappersMock + .mock('getERC20TokenWrapper') + .withArgs({ address: params.tokenAddress }) + .throws(); + + await expect(target.prepareTransactions()).rejects.toThrow( + new PolymathError({ + code: ErrorCode.ProcedureValidationError, + message: 'The supplied address does not correspond to an ERC20 token', + }) + ); + }); + test('should throw if address belongs to a security token, not an erc20 token', async () => { securityTokenRegistryMock.mock('isSecurityToken', Promise.resolve(true)); @@ -130,6 +148,23 @@ describe('TransferErc20', () => { ); }); + test('should add a transaction to the queue to transfer an erc20 token getting token funds if there are not enough funds, only on testnet', async () => { + wrappersMock.mock('isTestnet', Promise.resolve(true)); + erc20Mock.mock('balanceOf', Promise.resolve(new BigNumber(2))); + const addTransactionSpy = spy(target, 'addTransaction'); + // Real call + await target.prepareTransactions(); + + // Verifications + expect(addTransactionSpy.getCall(0).calledWith(erc20Mock.getMockInstance().transfer)).toEqual( + true + ); + expect( + addTransactionSpy.getCall(1).calledWith(wrappersMock.getMockInstance().getPolyTokens) + ).toEqual(true); + expect(addTransactionSpy.callCount).toEqual(2); + }); + test('should throw if there are not enough funds to make transfer', async () => { erc20Mock.mock('balanceOf', Promise.resolve(new BigNumber(2))); @@ -140,5 +175,22 @@ describe('TransferErc20', () => { }) ); }); + + test('should successfully create erc20 transfer resolver', async () => { + const refreshStub = erc20TokenBalanceFactoryMock.mock('refresh', Promise.resolve(undefined)); + const address = params.tokenAddress ? params.tokenAddress : ''; + const erc20TokenBalanceGeneratedId = Erc20TokenBalance.generateId({ + tokenAddress: address, + walletAddress: params.receiver, + }); + const resolverValue = await transferErc20Module.createTransferErc20Resolver( + factoryMockSetup, + address, + params.receiver + )(); + expect(refreshStub.getCall(0).calledWithExactly(erc20TokenBalanceGeneratedId)).toEqual(true); + expect(await resolverValue).toEqual(undefined); + expect(refreshStub.callCount).toEqual(1); + }); }); }); From 705bc6ebbf2b857964252fb7f991dc77b2b8e203 Mon Sep 17 00:00:00 2001 From: Victor Wiebe Date: Tue, 19 Nov 2019 06:51:56 -0800 Subject: [PATCH 04/10] refactor: improve transferErc20 test --- src/procedures/__tests__/TransferErc20.ts | 22 ++++++++-------------- 1 file changed, 8 insertions(+), 14 deletions(-) diff --git a/src/procedures/__tests__/TransferErc20.ts b/src/procedures/__tests__/TransferErc20.ts index 191896b..fbdaa61 100644 --- a/src/procedures/__tests__/TransferErc20.ts +++ b/src/procedures/__tests__/TransferErc20.ts @@ -3,8 +3,8 @@ import { restore, spy } from 'sinon'; import * as contractWrappersModule from '@polymathnetwork/contract-wrappers'; import { BigNumber, TransactionReceiptWithDecodedLogs } from '@polymathnetwork/contract-wrappers'; import { Procedure } from '../Procedure'; -import { ErrorCode, ProcedureType, TransferErc20ProcedureArgs } from '~/types'; -import * as erc20TokenBalanceFactoryModule from '~/entities/factories/Erc20TokenBalanceFactory'; +import { ErrorCode, ProcedureType, TransferErc20ProcedureArgs } from '../../types'; +import * as erc20TokenBalanceFactoryModule from '../../entities/factories/Erc20TokenBalanceFactory'; import * as contextModule from '../../Context'; import * as wrappersModule from '../../PolymathBase'; import * as tokenFactoryModule from '../../testUtils/MockedTokenFactoryModule'; @@ -22,6 +22,7 @@ const params: TransferErc20ProcedureArgs = { receiver: '0x6666666666666666666666666666666666666666', tokenAddress: '0x7777777777777777777777777777777777777777', }; +const currentWallet = '0x8888888888888888888888888888888888888888'; describe('TransferErc20', () => { let target: TransferErc20; @@ -66,11 +67,9 @@ describe('TransferErc20', () => { contractWrappersModule, 'SecurityTokenRegistry' ); - - wrappersMock.set('securityTokenRegistry', securityTokenRegistryMock.getMockInstance()); - securityTokenRegistryMock.mock('isSecurityToken', Promise.resolve(false)); + wrappersMock.set('securityTokenRegistry', securityTokenRegistryMock.getMockInstance()); wrappersMock.mock('getERC20TokenWrapper', erc20Mock.getMockInstance()); erc20TokenBalanceFactoryMock = ImportMock.mockClass( @@ -83,18 +82,13 @@ describe('TransferErc20', () => { erc20TokenBalanceFactoryMock.mock('refresh', {}); contextMock.set('factories', factoryMockSetup); - contextMock.set( - 'currentWallet', - new Wallet({ address: () => Promise.resolve(params.receiver) }) - ); + contextMock.set('currentWallet', new Wallet({ address: () => Promise.resolve(currentWallet) })); polyTokenMock = ImportMock.mockClass(contractWrappersModule, 'PolyToken'); polyTokenMock.mock('address', Promise.resolve(params.tokenAddress)); wrappersMock.set('polyToken', polyTokenMock.getMockInstance()); wrappersMock.mock('isTestnet', Promise.resolve(false)); - wrappersMock.mock('getModuleFactoryAddress', Promise.resolve(params.receiver)); - // Instantiate TransferErc20 target = new TransferErc20(params, contextMock.getMockInstance()); }); @@ -128,7 +122,7 @@ describe('TransferErc20', () => { .withArgs({ address: params.tokenAddress }) .throws(); - await expect(target.prepareTransactions()).rejects.toThrow( + expect(target.prepareTransactions()).rejects.toThrow( new PolymathError({ code: ErrorCode.ProcedureValidationError, message: 'The supplied address does not correspond to an ERC20 token', @@ -139,7 +133,7 @@ describe('TransferErc20', () => { test('should throw if address belongs to a security token, not an erc20 token', async () => { securityTokenRegistryMock.mock('isSecurityToken', Promise.resolve(true)); - await expect(target.prepareTransactions()).rejects.toThrow( + expect(target.prepareTransactions()).rejects.toThrow( new PolymathError({ code: ErrorCode.ProcedureValidationError, message: @@ -168,7 +162,7 @@ describe('TransferErc20', () => { test('should throw if there are not enough funds to make transfer', async () => { erc20Mock.mock('balanceOf', Promise.resolve(new BigNumber(2))); - await expect(target.prepareTransactions()).rejects.toThrow( + expect(target.prepareTransactions()).rejects.toThrow( new PolymathError({ code: ErrorCode.ProcedureValidationError, message: 'Not enough funds', From 3e689730a4cda28bbdbd27092b2c9122d568c583 Mon Sep 17 00:00:00 2001 From: Victor Wiebe Date: Tue, 19 Nov 2019 14:03:54 -0800 Subject: [PATCH 05/10] refactor: add tags and improve test quality --- src/procedures/__tests__/TransferErc20.ts | 46 ++++++++++++++--------- 1 file changed, 28 insertions(+), 18 deletions(-) diff --git a/src/procedures/__tests__/TransferErc20.ts b/src/procedures/__tests__/TransferErc20.ts index fbdaa61..ae29cdd 100644 --- a/src/procedures/__tests__/TransferErc20.ts +++ b/src/procedures/__tests__/TransferErc20.ts @@ -3,15 +3,20 @@ import { restore, spy } from 'sinon'; import * as contractWrappersModule from '@polymathnetwork/contract-wrappers'; import { BigNumber, TransactionReceiptWithDecodedLogs } from '@polymathnetwork/contract-wrappers'; import { Procedure } from '../Procedure'; -import { ErrorCode, ProcedureType, TransferErc20ProcedureArgs } from '../../types'; +import { + ErrorCode, + PolyTransactionTag, + ProcedureType, + TransferErc20ProcedureArgs, +} from '../../types'; import * as erc20TokenBalanceFactoryModule from '../../entities/factories/Erc20TokenBalanceFactory'; import * as contextModule from '../../Context'; import * as wrappersModule from '../../PolymathBase'; import * as tokenFactoryModule from '../../testUtils/MockedTokenFactoryModule'; import * as moduleWrapperFactoryModule from '../../testUtils/MockedModuleWrapperFactoryModule'; -import { Wallet } from '~/Wallet'; -import { TransferErc20 } from '~/procedures'; -import * as transferErc20Module from '~/procedures/TransferErc20'; +import { Wallet } from '../../Wallet'; +import { TransferErc20 } from '../../procedures'; +import * as transferErc20Module from '../../procedures/TransferErc20'; import { mockFactories } from '../../testUtils/mockFactories'; import { PolymathError } from '../../PolymathError'; import { Erc20TokenBalance } from '../../entities'; @@ -23,6 +28,7 @@ const params: TransferErc20ProcedureArgs = { tokenAddress: '0x7777777777777777777777777777777777777777', }; const currentWallet = '0x8888888888888888888888888888888888888888'; +const polyTokenAddress = '0x9999999999999999999999999999999999999999'; describe('TransferErc20', () => { let target: TransferErc20; @@ -85,7 +91,7 @@ describe('TransferErc20', () => { contextMock.set('currentWallet', new Wallet({ address: () => Promise.resolve(currentWallet) })); polyTokenMock = ImportMock.mockClass(contractWrappersModule, 'PolyToken'); - polyTokenMock.mock('address', Promise.resolve(params.tokenAddress)); + polyTokenMock.mock('address', Promise.resolve(polyTokenAddress)); wrappersMock.set('polyToken', polyTokenMock.getMockInstance()); wrappersMock.mock('isTestnet', Promise.resolve(false)); @@ -104,7 +110,7 @@ describe('TransferErc20', () => { }); describe('TransferErc20', () => { - test('should add a transaction to the queue to transfer an erc20 token', async () => { + test('should add a transaction to the queue to transfer an erc20 token with specified token address to a specified receiving address', async () => { const addTransactionSpy = spy(target, 'addTransaction'); // Real call await target.prepareTransactions(); @@ -113,16 +119,17 @@ describe('TransferErc20', () => { expect(addTransactionSpy.getCall(0).calledWith(erc20Mock.getMockInstance().transfer)).toEqual( true ); + expect(addTransactionSpy.getCall(0).lastArg.tag).toEqual(PolyTransactionTag.TransferErc20); expect(addTransactionSpy.callCount).toEqual(1); }); - test('should throw if supplied address does not correspond to erc20 token', async () => { + test('should throw if supplied address does not correspond to a valid erc20 token', async () => { wrappersMock .mock('getERC20TokenWrapper') .withArgs({ address: params.tokenAddress }) .throws(); - expect(target.prepareTransactions()).rejects.toThrow( + await expect(target.prepareTransactions()).rejects.toThrow( new PolymathError({ code: ErrorCode.ProcedureValidationError, message: 'The supplied address does not correspond to an ERC20 token', @@ -133,7 +140,7 @@ describe('TransferErc20', () => { test('should throw if address belongs to a security token, not an erc20 token', async () => { securityTokenRegistryMock.mock('isSecurityToken', Promise.resolve(true)); - expect(target.prepareTransactions()).rejects.toThrow( + await expect(target.prepareTransactions()).rejects.toThrow( new PolymathError({ code: ErrorCode.ProcedureValidationError, message: @@ -142,27 +149,30 @@ describe('TransferErc20', () => { ); }); - test('should add a transaction to the queue to transfer an erc20 token getting token funds if there are not enough funds, only on testnet', async () => { + test('should add a transaction to the queue to transfer an erc20 token getting poly token funds if there are not enough funds, only on testnet', async () => { wrappersMock.mock('isTestnet', Promise.resolve(true)); + polyTokenMock.mock('address', Promise.resolve(params.tokenAddress)); erc20Mock.mock('balanceOf', Promise.resolve(new BigNumber(2))); const addTransactionSpy = spy(target, 'addTransaction'); // Real call await target.prepareTransactions(); // Verifications - expect(addTransactionSpy.getCall(0).calledWith(erc20Mock.getMockInstance().transfer)).toEqual( - true - ); expect( - addTransactionSpy.getCall(1).calledWith(wrappersMock.getMockInstance().getPolyTokens) + addTransactionSpy.getCall(0).calledWith(wrappersMock.getMockInstance().getPolyTokens) ).toEqual(true); + expect(addTransactionSpy.getCall(0).lastArg.tag).toEqual(PolyTransactionTag.GetTokens); + expect(addTransactionSpy.getCall(1).calledWith(erc20Mock.getMockInstance().transfer)).toEqual( + true + ); + expect(addTransactionSpy.getCall(1).lastArg.tag).toEqual(PolyTransactionTag.TransferErc20); expect(addTransactionSpy.callCount).toEqual(2); }); - test('should throw if there are not enough funds to make transfer', async () => { + test('should throw error if there are not enough funds to make an erc20 transfer', async () => { erc20Mock.mock('balanceOf', Promise.resolve(new BigNumber(2))); - expect(target.prepareTransactions()).rejects.toThrow( + await expect(target.prepareTransactions()).rejects.toThrow( new PolymathError({ code: ErrorCode.ProcedureValidationError, message: 'Not enough funds', @@ -171,7 +181,7 @@ describe('TransferErc20', () => { }); test('should successfully create erc20 transfer resolver', async () => { - const refreshStub = erc20TokenBalanceFactoryMock.mock('refresh', Promise.resolve(undefined)); + const refreshStub = erc20TokenBalanceFactoryMock.mock('refresh', Promise.resolve()); const address = params.tokenAddress ? params.tokenAddress : ''; const erc20TokenBalanceGeneratedId = Erc20TokenBalance.generateId({ tokenAddress: address, @@ -183,7 +193,7 @@ describe('TransferErc20', () => { params.receiver )(); expect(refreshStub.getCall(0).calledWithExactly(erc20TokenBalanceGeneratedId)).toEqual(true); - expect(await resolverValue).toEqual(undefined); + expect(resolverValue).toEqual(undefined); expect(refreshStub.callCount).toEqual(1); }); }); From 17745457e57278fa1449ed6c4af7830e4536b929 Mon Sep 17 00:00:00 2001 From: Victor Wiebe Date: Tue, 19 Nov 2019 14:06:36 -0800 Subject: [PATCH 06/10] refactor: use promise resolve --- src/procedures/__tests__/TransferErc20.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/procedures/__tests__/TransferErc20.ts b/src/procedures/__tests__/TransferErc20.ts index ae29cdd..777b96c 100644 --- a/src/procedures/__tests__/TransferErc20.ts +++ b/src/procedures/__tests__/TransferErc20.ts @@ -86,7 +86,7 @@ describe('TransferErc20', () => { factoryMockSetup = mockFactories(); factoryMockSetup.erc20TokenBalanceFactory = erc20TokenBalanceFactoryMock.getMockInstance(); - erc20TokenBalanceFactoryMock.mock('refresh', {}); + erc20TokenBalanceFactoryMock.mock('refresh', Promise.resolve()); contextMock.set('factories', factoryMockSetup); contextMock.set('currentWallet', new Wallet({ address: () => Promise.resolve(currentWallet) })); From 47675d935f18cb779f339efed8bc27fc2da8092b Mon Sep 17 00:00:00 2001 From: Victor Wiebe Date: Wed, 20 Nov 2019 04:58:06 -0800 Subject: [PATCH 07/10] fix: add extra mocks to ensure that calledWith is valid --- src/procedures/__tests__/TransferErc20.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/procedures/__tests__/TransferErc20.ts b/src/procedures/__tests__/TransferErc20.ts index 777b96c..8e74d6f 100644 --- a/src/procedures/__tests__/TransferErc20.ts +++ b/src/procedures/__tests__/TransferErc20.ts @@ -112,6 +112,7 @@ describe('TransferErc20', () => { describe('TransferErc20', () => { test('should add a transaction to the queue to transfer an erc20 token with specified token address to a specified receiving address', async () => { const addTransactionSpy = spy(target, 'addTransaction'); + erc20Mock.mock('transfer', Promise.resolve('Transfer')); // Real call await target.prepareTransactions(); @@ -153,6 +154,8 @@ describe('TransferErc20', () => { wrappersMock.mock('isTestnet', Promise.resolve(true)); polyTokenMock.mock('address', Promise.resolve(params.tokenAddress)); erc20Mock.mock('balanceOf', Promise.resolve(new BigNumber(2))); + erc20Mock.mock('transfer', Promise.resolve('Transfer')); + wrappersMock.mock('getPolyTokens', Promise.resolve('GetPolyTokens')); const addTransactionSpy = spy(target, 'addTransaction'); // Real call await target.prepareTransactions(); From 8ea59180415b2cb052fbfa8a9b81d641eba3834a Mon Sep 17 00:00:00 2001 From: Victor Wiebe Date: Thu, 21 Nov 2019 13:59:19 -0800 Subject: [PATCH 08/10] fix: review comments, new test, new comments --- src/procedures/__tests__/TransferErc20.ts | 28 ++++++++++++++++++++--- 1 file changed, 25 insertions(+), 3 deletions(-) diff --git a/src/procedures/__tests__/TransferErc20.ts b/src/procedures/__tests__/TransferErc20.ts index 8e74d6f..46340d1 100644 --- a/src/procedures/__tests__/TransferErc20.ts +++ b/src/procedures/__tests__/TransferErc20.ts @@ -124,6 +124,27 @@ describe('TransferErc20', () => { expect(addTransactionSpy.callCount).toEqual(1); }); + test('should add a transaction to the queue to transfer an erc20 token with specified token address without a specified receiving address', async () => { + target = new TransferErc20( + { ...params, tokenAddress: undefined }, + contextMock.getMockInstance() + ); + polyTokenMock.mock('balanceOf', Promise.resolve(new BigNumber(20))); + + const addTransactionSpy = spy(target, 'addTransaction'); + polyTokenMock.mock('transfer', Promise.resolve('Transfer')); + + // Real call + await target.prepareTransactions(); + + // Verifications + expect( + addTransactionSpy.getCall(0).calledWith(polyTokenMock.getMockInstance().transfer) + ).toEqual(true); + expect(addTransactionSpy.getCall(0).lastArg.tag).toEqual(PolyTransactionTag.TransferErc20); + expect(addTransactionSpy.callCount).toEqual(1); + }); + test('should throw if supplied address does not correspond to a valid erc20 token', async () => { wrappersMock .mock('getERC20TokenWrapper') @@ -150,9 +171,10 @@ describe('TransferErc20', () => { ); }); - test('should add a transaction to the queue to transfer an erc20 token getting poly token funds if there are not enough funds, only on testnet', async () => { + test('should add an extra transaction to get POLY from the faucet if the balance is insufficient, specifically on testnet', async () => { wrappersMock.mock('isTestnet', Promise.resolve(true)); - polyTokenMock.mock('address', Promise.resolve(params.tokenAddress)); + polyTokenMock.mock('address', Promise.resolve(polyTokenAddress)); + erc20Mock.mock('address', Promise.resolve(polyTokenAddress)); erc20Mock.mock('balanceOf', Promise.resolve(new BigNumber(2))); erc20Mock.mock('transfer', Promise.resolve('Transfer')); wrappersMock.mock('getPolyTokens', Promise.resolve('GetPolyTokens')); @@ -183,7 +205,7 @@ describe('TransferErc20', () => { ); }); - test('should successfully create erc20 transfer resolver', async () => { + test('should successfully refresh the corresponding ERC20 Balance Entity', async () => { const refreshStub = erc20TokenBalanceFactoryMock.mock('refresh', Promise.resolve()); const address = params.tokenAddress ? params.tokenAddress : ''; const erc20TokenBalanceGeneratedId = Erc20TokenBalance.generateId({ From b217a7cac13d43e33efde23155d51eaf60d80866 Mon Sep 17 00:00:00 2001 From: Victor Wiebe Date: Fri, 22 Nov 2019 08:22:21 -0800 Subject: [PATCH 09/10] refactor: better way to use optional param --- src/procedures/__tests__/TransferErc20.ts | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/procedures/__tests__/TransferErc20.ts b/src/procedures/__tests__/TransferErc20.ts index 46340d1..f6cc635 100644 --- a/src/procedures/__tests__/TransferErc20.ts +++ b/src/procedures/__tests__/TransferErc20.ts @@ -207,14 +207,13 @@ describe('TransferErc20', () => { test('should successfully refresh the corresponding ERC20 Balance Entity', async () => { const refreshStub = erc20TokenBalanceFactoryMock.mock('refresh', Promise.resolve()); - const address = params.tokenAddress ? params.tokenAddress : ''; const erc20TokenBalanceGeneratedId = Erc20TokenBalance.generateId({ - tokenAddress: address, + tokenAddress: params.tokenAddress!, walletAddress: params.receiver, }); const resolverValue = await transferErc20Module.createTransferErc20Resolver( factoryMockSetup, - address, + params.tokenAddress!, params.receiver )(); expect(refreshStub.getCall(0).calledWithExactly(erc20TokenBalanceGeneratedId)).toEqual(true); From 66114299c41b9080ced8d692d8400cdaa0b634ef Mon Sep 17 00:00:00 2001 From: Victor Wiebe Date: Fri, 22 Nov 2019 12:31:08 -0800 Subject: [PATCH 10/10] fix: review issues, comment and redundant code --- src/procedures/__tests__/TransferErc20.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/procedures/__tests__/TransferErc20.ts b/src/procedures/__tests__/TransferErc20.ts index f6cc635..e2fac01 100644 --- a/src/procedures/__tests__/TransferErc20.ts +++ b/src/procedures/__tests__/TransferErc20.ts @@ -124,7 +124,7 @@ describe('TransferErc20', () => { expect(addTransactionSpy.callCount).toEqual(1); }); - test('should add a transaction to the queue to transfer an erc20 token with specified token address without a specified receiving address', async () => { + test('should add a transaction to the queue to transfer poly as the parameters do not include token address', async () => { target = new TransferErc20( { ...params, tokenAddress: undefined }, contextMock.getMockInstance() @@ -173,7 +173,6 @@ describe('TransferErc20', () => { test('should add an extra transaction to get POLY from the faucet if the balance is insufficient, specifically on testnet', async () => { wrappersMock.mock('isTestnet', Promise.resolve(true)); - polyTokenMock.mock('address', Promise.resolve(polyTokenAddress)); erc20Mock.mock('address', Promise.resolve(polyTokenAddress)); erc20Mock.mock('balanceOf', Promise.resolve(new BigNumber(2))); erc20Mock.mock('transfer', Promise.resolve('Transfer'));