From e2c92d697a5cad926d85fcdd56cad292a0d3f43a Mon Sep 17 00:00:00 2001 From: Angela Lu Date: Mon, 22 Mar 2021 20:41:54 -0700 Subject: [PATCH 1/5] refactor catch errors to async await format --- .eslintrc.js | 1 - src/approval/ApprovalController.test.ts | 35 +--- src/assets/AssetsController.test.ts | 93 ++++----- src/message-manager/MessageManager.test.ts | 95 ++++----- .../PersonalMessageManager.test.ts | 95 ++++----- .../TypedMessageManager.test.ts | 162 +++++++-------- src/transaction/TransactionController.test.ts | 189 ++++++++---------- 7 files changed, 286 insertions(+), 384 deletions(-) diff --git a/.eslintrc.js b/.eslintrc.js index 3c4b1b1575..5ff7482f97 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -55,7 +55,6 @@ module.exports = { 'jest/no-restricted-matchers': 'off', 'jest/no-try-expect': 'off', 'jest/prefer-strict-equal': 'off', - 'jest/valid-expect-in-promise': 'off', }, settings: { 'import/resolver': { diff --git a/src/approval/ApprovalController.test.ts b/src/approval/ApprovalController.test.ts index f149da18bc..499f85960a 100644 --- a/src/approval/ApprovalController.test.ts +++ b/src/approval/ApprovalController.test.ts @@ -312,33 +312,22 @@ describe('approval controller', () => { it('rejects approval promise', async () => { numDeletions = 1; - - const approvalPromise = approvalController.add({ id: 'foo', origin: 'bar.baz', type: TYPE }).catch((error) => { - expect(error).toMatchObject(getError('failure')); - }); - + const approvalPromise = approvalController.add({ id: 'foo', origin: 'bar.baz', type: TYPE }); approvalController.reject('foo', new Error('failure')); - await approvalPromise; + await expect(approvalPromise).rejects.toThrow('failure'); expect(deleteSpy.callCount).toEqual(numDeletions); }); it('rejects multiple approval promises out of order', async () => { numDeletions = 2; - const rejectionPromise1 = approvalController.add({ id: 'foo1', origin: 'bar.baz', type: TYPE }).catch((error) => { - expect(error).toMatchObject(getError('failure1')); - }); - const rejectionPromise2 = approvalController - .add({ id: 'foo2', origin: 'bar.baz', type: 'myType2' }) - .catch((error) => { - expect(error).toMatchObject(getError('failure2')); - }); + const rejectionPromise1 = approvalController.add({ id: 'foo1', origin: 'bar.baz', type: TYPE }); + const rejectionPromise2 = approvalController.add({ id: 'foo2', origin: 'bar.baz', type: 'myType2' }); approvalController.reject('foo2', new Error('failure2')); - await rejectionPromise2; - approvalController.reject('foo1', new Error('failure1')); - await rejectionPromise1; + await expect(rejectionPromise2).rejects.toThrow('failure2'); + await expect(rejectionPromise1).rejects.toThrow('failure1'); expect(deleteSpy.callCount).toEqual(numDeletions); }); @@ -354,12 +343,8 @@ describe('approval controller', () => { const promise1 = approvalController.add({ id: 'foo1', origin: 'bar.baz', type: TYPE }); const promise2 = approvalController.add({ id: 'foo2', origin: 'bar.baz', type: 'myType2' }); - const promise3 = approvalController.add({ id: 'foo3', origin: 'fizz.buzz', type: TYPE }).catch((error) => { - expect(error).toMatchObject(getError('failure3')); - }); - const promise4 = approvalController.add({ id: 'foo4', origin: 'bar.baz', type: 'myType4' }).catch((error) => { - expect(error).toMatchObject(getError('failure4')); - }); + const promise3 = approvalController.add({ id: 'foo3', origin: 'fizz.buzz', type: TYPE }); + const promise4 = approvalController.add({ id: 'foo4', origin: 'bar.baz', type: 'myType4' }); approvalController.resolve('foo2', 'success2'); @@ -367,10 +352,10 @@ describe('approval controller', () => { expect(result).toEqual('success2'); approvalController.reject('foo4', new Error('failure4')); - await promise4; + await expect(promise4).rejects.toThrow('failure4'); approvalController.reject('foo3', new Error('failure3')); - await promise3; + await expect(promise3).rejects.toThrow('failure3'); expect(approvalController.has({ origin: 'fizz.buzz' })).toEqual(false); expect(approvalController.has({ origin: 'bar.baz' })).toEqual(true); diff --git a/src/assets/AssetsController.test.ts b/src/assets/AssetsController.test.ts index f0aa34a5bc..65bc6c38e1 100644 --- a/src/assets/AssetsController.test.ts +++ b/src/assets/AssetsController.test.ts @@ -444,67 +444,54 @@ describe('AssetsController', () => { }); it('should reject a valid suggested asset via watchAsset', async () => { - await new Promise(async (resolve) => { - const { result, suggestedAssetMeta } = await assetsController.watchAsset( - { - address: '0xe9f786dfdd9ae4d57e830acb52296837765f0e5b', - decimals: 18, - symbol: 'TKN', - }, - 'ERC20', - ); - assetsController.rejectWatchAsset('foo'); - assetsController.rejectWatchAsset(suggestedAssetMeta.id); - assetsController.hub.once(`${suggestedAssetMeta.id}:finished`, () => { - expect(assetsController.state.suggestedAssets).toHaveLength(0); - }); - result.catch((error) => { - expect(error.message).toContain('User rejected to watch the asset.'); - resolve(''); - }); + const { result, suggestedAssetMeta } = await assetsController.watchAsset( + { + address: '0xe9f786dfdd9ae4d57e830acb52296837765f0e5b', + decimals: 18, + symbol: 'TKN', + }, + 'ERC20', + ); + assetsController.rejectWatchAsset('foo'); + assetsController.rejectWatchAsset(suggestedAssetMeta.id); + assetsController.hub.once(`${suggestedAssetMeta.id}:finished`, () => { + expect(assetsController.state.suggestedAssets).toHaveLength(0); }); + await expect(result).rejects.toThrow('User rejected to watch the asset.'); }); it('should accept a valid suggested asset via watchAsset', async () => { - await new Promise(async (resolve) => { - const { result, suggestedAssetMeta } = await assetsController.watchAsset( - { - address: '0xe9f786dfdd9ae4d57e830acb52296837765f0e5b', - decimals: 18, - symbol: 'TKN', - }, - 'ERC20', - ); - result.then((res) => { - expect(assetsController.state.suggestedAssets).toHaveLength(0); - expect(res).toBe('0xe9f786dfdd9ae4d57e830acb52296837765f0e5b'); - resolve(''); - }); - await assetsController.acceptWatchAsset(suggestedAssetMeta.id); + const { result, suggestedAssetMeta } = await assetsController.watchAsset( + { + address: '0xe9f786dfdd9ae4d57e830acb52296837765f0e5b', + decimals: 18, + symbol: 'TKN', + }, + 'ERC20', + ); + await assetsController.acceptWatchAsset(suggestedAssetMeta.id); + await result.then((res) => { + expect(assetsController.state.suggestedAssets).toHaveLength(0); + expect(res).toBe('0xe9f786dfdd9ae4d57e830acb52296837765f0e5b'); }); }); it('should fail a valid suggested asset via watchAsset with wrong type', async () => { - await new Promise(async (resolve) => { - const { result, suggestedAssetMeta } = await assetsController.watchAsset( - { - address: '0xe9f786dfdd9be4d57e830acb52296837765f0e5b', - decimals: 18, - symbol: 'TKN', - }, - 'ERC20', - ); - const { suggestedAssets } = assetsController.state; - const index = suggestedAssets.findIndex(({ id }) => suggestedAssetMeta.id === id); - const newSuggestedAssetMeta = suggestedAssets[index]; - suggestedAssetMeta.type = 'ERC721'; - assetsController.update({ suggestedAssets: [...suggestedAssets, newSuggestedAssetMeta] }); - await assetsController.acceptWatchAsset(suggestedAssetMeta.id); - result.catch((error) => { - expect(error.message).toContain('Asset of type ERC721 not supported'); - resolve(''); - }); - }); + const { result, suggestedAssetMeta } = await assetsController.watchAsset( + { + address: '0xe9f786dfdd9be4d57e830acb52296837765f0e5b', + decimals: 18, + symbol: 'TKN', + }, + 'ERC20', + ); + const { suggestedAssets } = assetsController.state; + const index = suggestedAssets.findIndex(({ id }) => suggestedAssetMeta.id === id); + const newSuggestedAssetMeta = suggestedAssets[index]; + suggestedAssetMeta.type = 'ERC721'; + assetsController.update({ suggestedAssets: [...suggestedAssets, newSuggestedAssetMeta] }); + await assetsController.acceptWatchAsset(suggestedAssetMeta.id); + await expect(result).rejects.toThrow('Asset of type ERC721 not supported'); }); it('should not add duplicate tokens to the ignoredToken list', async () => { diff --git a/src/message-manager/MessageManager.test.ts b/src/message-manager/MessageManager.test.ts index 04e78077c5..377dbea679 100644 --- a/src/message-manager/MessageManager.test.ts +++ b/src/message-manager/MessageManager.test.ts @@ -42,69 +42,56 @@ describe('PersonalMessageManager', () => { }); it('should reject a message', async () => { - await new Promise(async (resolve) => { - const controller = new MessageManager(); - const from = '0xc38bf1ad06ef69f0c04e29dbeb4152b4175f0a8d'; - const data = '0x879a053d4800c6354e76c7985a865d2922c82fb5b'; - const result = controller.addUnapprovedMessageAsync({ - data, - from, - }); - const unapprovedMessages = controller.getUnapprovedMessages(); - const keys = Object.keys(unapprovedMessages); - controller.hub.once(`${keys[0]}:finished`, () => { - expect(unapprovedMessages[keys[0]].messageParams.from).toBe(from); - expect(unapprovedMessages[keys[0]].status).toBe('rejected'); - }); - controller.rejectMessage(keys[0]); - result.catch((error) => { - expect(error.message).toContain('User denied message signature'); - resolve(); - }); + const controller = new MessageManager(); + const from = '0xc38bf1ad06ef69f0c04e29dbeb4152b4175f0a8d'; + const data = '0x879a053d4800c6354e76c7985a865d2922c82fb5b'; + const result = controller.addUnapprovedMessageAsync({ + data, + from, }); + const unapprovedMessages = controller.getUnapprovedMessages(); + const keys = Object.keys(unapprovedMessages); + controller.hub.once(`${keys[0]}:finished`, () => { + expect(unapprovedMessages[keys[0]].messageParams.from).toBe(from); + expect(unapprovedMessages[keys[0]].status).toBe('rejected'); + }); + controller.rejectMessage(keys[0]); + await expect(result).rejects.toThrow('User denied message signature'); }); it('should sign a message', async () => { - await new Promise(async (resolve) => { - const controller = new MessageManager(); - const from = '0xc38bf1ad06ef69f0c04e29dbeb4152b4175f0a8d'; - const data = '0x879a053d4800c6354e76c7985a865d2922c82fb5b'; - const rawSig = '0x5f7a0'; - const result = controller.addUnapprovedMessageAsync({ - data, - from, - }); - const unapprovedMessages = controller.getUnapprovedMessages(); - const keys = Object.keys(unapprovedMessages); - controller.hub.once(`${keys[0]}:finished`, () => { - expect(unapprovedMessages[keys[0]].messageParams.from).toBe(from); - expect(unapprovedMessages[keys[0]].status).toBe('signed'); - }); - controller.setMessageStatusSigned(keys[0], rawSig); - result.then((sig) => { - expect(sig).toEqual(rawSig); - resolve(); - }); + const controller = new MessageManager(); + const from = '0xc38bf1ad06ef69f0c04e29dbeb4152b4175f0a8d'; + const data = '0x879a053d4800c6354e76c7985a865d2922c82fb5b'; + const rawSig = '0x5f7a0'; + const result = controller.addUnapprovedMessageAsync({ + data, + from, + }); + const unapprovedMessages = controller.getUnapprovedMessages(); + const keys = Object.keys(unapprovedMessages); + controller.hub.once(`${keys[0]}:finished`, () => { + expect(unapprovedMessages[keys[0]].messageParams.from).toBe(from); + expect(unapprovedMessages[keys[0]].status).toBe('signed'); + }); + controller.setMessageStatusSigned(keys[0], rawSig); + await result.then((sig) => { + expect(sig).toEqual(rawSig); }); }); it('should throw when unapproved finishes', async () => { - await new Promise(async (resolve) => { - const controller = new MessageManager(); - const from = '0xc38bf1ad06ef69f0c04e29dbeb4152b4175f0a8d'; - const data = '0x879a053d4800c6354e76c7985a865d2922c82fb5b'; - const result = controller.addUnapprovedMessageAsync({ - data, - from, - }); - const unapprovedMessages = controller.getUnapprovedMessages(); - const keys = Object.keys(unapprovedMessages); - controller.hub.emit(`${keys[0]}:finished`, unapprovedMessages[keys[0]]); - result.catch((error) => { - expect(error.message).toContain('Unknown problem'); - resolve(); - }); + const controller = new MessageManager(); + const from = '0xc38bf1ad06ef69f0c04e29dbeb4152b4175f0a8d'; + const data = '0x879a053d4800c6354e76c7985a865d2922c82fb5b'; + const result = controller.addUnapprovedMessageAsync({ + data, + from, }); + const unapprovedMessages = controller.getUnapprovedMessages(); + const keys = Object.keys(unapprovedMessages); + controller.hub.emit(`${keys[0]}:finished`, unapprovedMessages[keys[0]]); + await expect(result).rejects.toThrow('Unknown problem'); }); it('should add a valid unapproved message', async () => { diff --git a/src/message-manager/PersonalMessageManager.test.ts b/src/message-manager/PersonalMessageManager.test.ts index c1addc9aa2..ebd24660a3 100644 --- a/src/message-manager/PersonalMessageManager.test.ts +++ b/src/message-manager/PersonalMessageManager.test.ts @@ -42,69 +42,56 @@ describe('PersonalMessageManager', () => { }); it('should reject a message', async () => { - await new Promise(async (resolve) => { - const controller = new PersonalMessageManager(); - const from = '0xc38bf1ad06ef69f0c04e29dbeb4152b4175f0a8d'; - const data = '0x879a053d4800c6354e76c7985a865d2922c82fb5b'; - const result = controller.addUnapprovedMessageAsync({ - data, - from, - }); - const unapprovedMessages = controller.getUnapprovedMessages(); - const keys = Object.keys(unapprovedMessages); - controller.hub.once(`${keys[0]}:finished`, () => { - expect(unapprovedMessages[keys[0]].messageParams.from).toBe(from); - expect(unapprovedMessages[keys[0]].status).toBe('rejected'); - }); - controller.rejectMessage(keys[0]); - result.catch((error) => { - expect(error.message).toContain('User denied message signature'); - resolve(); - }); + const controller = new PersonalMessageManager(); + const from = '0xc38bf1ad06ef69f0c04e29dbeb4152b4175f0a8d'; + const data = '0x879a053d4800c6354e76c7985a865d2922c82fb5b'; + const result = controller.addUnapprovedMessageAsync({ + data, + from, }); + const unapprovedMessages = controller.getUnapprovedMessages(); + const keys = Object.keys(unapprovedMessages); + controller.hub.once(`${keys[0]}:finished`, () => { + expect(unapprovedMessages[keys[0]].messageParams.from).toBe(from); + expect(unapprovedMessages[keys[0]].status).toBe('rejected'); + }); + controller.rejectMessage(keys[0]); + await expect(result).rejects.toThrow('User denied message signature'); }); it('should sign a message', async () => { - await new Promise(async (resolve) => { - const controller = new PersonalMessageManager(); - const from = '0xc38bf1ad06ef69f0c04e29dbeb4152b4175f0a8d'; - const data = '0x879a053d4800c6354e76c7985a865d2922c82fb5b'; - const rawSig = '0x5f7a0'; - const result = controller.addUnapprovedMessageAsync({ - data, - from, - }); - const unapprovedMessages = controller.getUnapprovedMessages(); - const keys = Object.keys(unapprovedMessages); - controller.hub.once(`${keys[0]}:finished`, () => { - expect(unapprovedMessages[keys[0]].messageParams.from).toBe(from); - expect(unapprovedMessages[keys[0]].status).toBe('signed'); - }); - controller.setMessageStatusSigned(keys[0], rawSig); - result.then((sig) => { - expect(sig).toEqual(rawSig); - resolve(); - }); + const controller = new PersonalMessageManager(); + const from = '0xc38bf1ad06ef69f0c04e29dbeb4152b4175f0a8d'; + const data = '0x879a053d4800c6354e76c7985a865d2922c82fb5b'; + const rawSig = '0x5f7a0'; + const result = controller.addUnapprovedMessageAsync({ + data, + from, + }); + const unapprovedMessages = controller.getUnapprovedMessages(); + const keys = Object.keys(unapprovedMessages); + controller.hub.once(`${keys[0]}:finished`, () => { + expect(unapprovedMessages[keys[0]].messageParams.from).toBe(from); + expect(unapprovedMessages[keys[0]].status).toBe('signed'); + }); + controller.setMessageStatusSigned(keys[0], rawSig); + await result.then((sig) => { + expect(sig).toEqual(rawSig); }); }); it('should throw when unapproved finishes', async () => { - await new Promise(async (resolve) => { - const controller = new PersonalMessageManager(); - const from = '0xc38bf1ad06ef69f0c04e29dbeb4152b4175f0a8d'; - const data = '0x879a053d4800c6354e76c7985a865d2922c82fb5b'; - const result = controller.addUnapprovedMessageAsync({ - data, - from, - }); - const unapprovedMessages = controller.getUnapprovedMessages(); - const keys = Object.keys(unapprovedMessages); - controller.hub.emit(`${keys[0]}:finished`, unapprovedMessages[keys[0]]); - result.catch((error) => { - expect(error.message).toContain('Unknown problem'); - resolve(); - }); + const controller = new PersonalMessageManager(); + const from = '0xc38bf1ad06ef69f0c04e29dbeb4152b4175f0a8d'; + const data = '0x879a053d4800c6354e76c7985a865d2922c82fb5b'; + const result = controller.addUnapprovedMessageAsync({ + data, + from, }); + const unapprovedMessages = controller.getUnapprovedMessages(); + const keys = Object.keys(unapprovedMessages); + controller.hub.emit(`${keys[0]}:finished`, unapprovedMessages[keys[0]]); + await expect(result).rejects.toThrow('Unknown problem'); }); it('should add a valid unapproved message', async () => { diff --git a/src/message-manager/TypedMessageManager.test.ts b/src/message-manager/TypedMessageManager.test.ts index 7a61c2f7b2..6d60cde295 100644 --- a/src/message-manager/TypedMessageManager.test.ts +++ b/src/message-manager/TypedMessageManager.test.ts @@ -54,108 +54,90 @@ describe('TypedMessageManager', () => { }); it('should reject a message', async () => { - await new Promise(async (resolve) => { - const controller = new TypedMessageManager(); - const from = '0xc38bf1ad06ef69f0c04e29dbeb4152b4175f0a8d'; - const version = 'V1'; - const messageData = typedMessage; - const result = controller.addUnapprovedMessageAsync( - { - data: messageData, - from, - }, - version, - ); - const unapprovedMessages = controller.getUnapprovedMessages(); - const keys = Object.keys(unapprovedMessages); - controller.hub.once(`${keys[0]}:finished`, () => { - expect(unapprovedMessages[keys[0]].messageParams.from).toBe(from); - expect(unapprovedMessages[keys[0]].status).toBe('rejected'); - }); - controller.rejectMessage(keys[0]); - result.catch((error) => { - expect(error.message).toContain('User denied message signature'); - resolve(); - }); + const controller = new TypedMessageManager(); + const from = '0xc38bf1ad06ef69f0c04e29dbeb4152b4175f0a8d'; + const version = 'V1'; + const messageData = typedMessage; + const result = controller.addUnapprovedMessageAsync( + { + data: messageData, + from, + }, + version, + ); + const unapprovedMessages = controller.getUnapprovedMessages(); + const keys = Object.keys(unapprovedMessages); + controller.hub.once(`${keys[0]}:finished`, () => { + expect(unapprovedMessages[keys[0]].messageParams.from).toBe(from); + expect(unapprovedMessages[keys[0]].status).toBe('rejected'); }); + controller.rejectMessage(keys[0]); + await expect(result).rejects.toThrow('User denied message signature'); }); it('should sign a message', async () => { - await new Promise(async (resolve) => { - const controller = new TypedMessageManager(); - const from = '0xc38bf1ad06ef69f0c04e29dbeb4152b4175f0a8d'; - const version = 'V1'; - const rawSig = '0x5f7a0'; - const messageData = typedMessage; - const result = controller.addUnapprovedMessageAsync( - { - data: messageData, - from, - }, - version, - ); - const unapprovedMessages = controller.getUnapprovedMessages(); - const keys = Object.keys(unapprovedMessages); - controller.hub.once(`${keys[0]}:finished`, () => { - expect(unapprovedMessages[keys[0]].messageParams.from).toBe(from); - expect(unapprovedMessages[keys[0]].status).toBe('signed'); - }); - controller.setMessageStatusSigned(keys[0], rawSig); - result.then((sig) => { - expect(sig).toEqual(rawSig); - resolve(); - }); + const controller = new TypedMessageManager(); + const from = '0xc38bf1ad06ef69f0c04e29dbeb4152b4175f0a8d'; + const version = 'V1'; + const rawSig = '0x5f7a0'; + const messageData = typedMessage; + const result = controller.addUnapprovedMessageAsync( + { + data: messageData, + from, + }, + version, + ); + const unapprovedMessages = controller.getUnapprovedMessages(); + const keys = Object.keys(unapprovedMessages); + controller.hub.once(`${keys[0]}:finished`, () => { + expect(unapprovedMessages[keys[0]].messageParams.from).toBe(from); + expect(unapprovedMessages[keys[0]].status).toBe('signed'); + }); + controller.setMessageStatusSigned(keys[0], rawSig); + await result.then((sig) => { + expect(sig).toEqual(rawSig); }); }); it("should set message status as 'errored'", async () => { - await new Promise(async (resolve) => { - const controller = new TypedMessageManager(); - const from = '0xc38bf1ad06ef69f0c04e29dbeb4152b4175f0a8d'; - const version = 'V1'; - const messageData = typedMessage; - const result = controller.addUnapprovedMessageAsync( - { - data: messageData, - from, - }, - version, - ); - const unapprovedMessages = controller.getUnapprovedMessages(); - const keys = Object.keys(unapprovedMessages); - controller.hub.once(`${keys[0]}:finished`, () => { - expect(unapprovedMessages[keys[0]].messageParams.from).toBe(from); - expect(unapprovedMessages[keys[0]].status).toBe('errored'); - }); - controller.setMessageStatusErrored(keys[0], 'error message'); - result.catch((error) => { - expect(error.message).toContain('MetaMask Typed Message Signature: error message'); - resolve(); - }); + const controller = new TypedMessageManager(); + const from = '0xc38bf1ad06ef69f0c04e29dbeb4152b4175f0a8d'; + const version = 'V1'; + const messageData = typedMessage; + const result = controller.addUnapprovedMessageAsync( + { + data: messageData, + from, + }, + version, + ); + const unapprovedMessages = controller.getUnapprovedMessages(); + const keys = Object.keys(unapprovedMessages); + controller.hub.once(`${keys[0]}:finished`, () => { + expect(unapprovedMessages[keys[0]].messageParams.from).toBe(from); + expect(unapprovedMessages[keys[0]].status).toBe('errored'); }); + controller.setMessageStatusErrored(keys[0], 'error message'); + await expect(result).rejects.toThrow('MetaMask Typed Message Signature: error message'); }); it('should throw when unapproved finishes', async () => { - await new Promise(async (resolve) => { - const controller = new TypedMessageManager(); - const from = '0xc38bf1ad06ef69f0c04e29dbeb4152b4175f0a8d'; - const version = 'V1'; - const messageData = typedMessage; - const result = controller.addUnapprovedMessageAsync( - { - data: messageData, - from, - }, - version, - ); - const unapprovedMessages = controller.getUnapprovedMessages(); - const keys = Object.keys(unapprovedMessages); - controller.hub.emit(`${keys[0]}:finished`, unapprovedMessages[keys[0]]); - result.catch((error) => { - expect(error.message).toContain('Unknown problem'); - resolve(); - }); - }); + const controller = new TypedMessageManager(); + const from = '0xc38bf1ad06ef69f0c04e29dbeb4152b4175f0a8d'; + const version = 'V1'; + const messageData = typedMessage; + const result = controller.addUnapprovedMessageAsync( + { + data: messageData, + from, + }, + version, + ); + const unapprovedMessages = controller.getUnapprovedMessages(); + const keys = Object.keys(unapprovedMessages); + controller.hub.emit(`${keys[0]}:finished`, unapprovedMessages[keys[0]]); + await expect(result).rejects.toThrow('Unknown problem'); }); it('should add a valid unapproved message', async () => { diff --git a/src/transaction/TransactionController.test.ts b/src/transaction/TransactionController.test.ts index d28e256864..b8866d5a78 100644 --- a/src/transaction/TransactionController.test.ts +++ b/src/transaction/TransactionController.test.ts @@ -606,28 +606,23 @@ describe('TransactionController', () => { }); it('should cancel a transaction', async () => { - await new Promise(async (resolve) => { - const controller = new TransactionController({ provider: PROVIDER }); - controller.context = { - NetworkController: MOCK_NETWORK, - } as any; - controller.onComposed(); - const from = '0xc38bf1ad06ef69f0c04e29dbeb4152b4175f0a8d'; - const { result } = await controller.addTransaction({ - from, - to: from, - }); - controller.cancelTransaction('foo'); - controller.hub.once(`${controller.state.transactions[0].id}:finished`, () => { - expect(controller.state.transactions[0].transaction.from).toBe(from); - expect(controller.state.transactions[0].status).toBe(TransactionStatus.rejected); - }); - controller.cancelTransaction(controller.state.transactions[0].id); - result.catch((error) => { - expect(error.message).toContain('User rejected the transaction'); - resolve(''); - }); + const controller = new TransactionController({ provider: PROVIDER }); + controller.context = { + NetworkController: MOCK_NETWORK, + } as any; + controller.onComposed(); + const from = '0xc38bf1ad06ef69f0c04e29dbeb4152b4175f0a8d'; + const { result } = await controller.addTransaction({ + from, + to: from, + }); + controller.cancelTransaction('foo'); + controller.hub.once(`${controller.state.transactions[0].id}:finished`, () => { + expect(controller.state.transactions[0].transaction.from).toBe(from); + expect(controller.state.transactions[0].status).toBe(TransactionStatus.rejected); }); + controller.cancelTransaction(controller.state.transactions[0].id); + await expect(result).rejects.toThrow('User rejected the transaction'); }); it('should wipe transactions', async () => { @@ -666,30 +661,25 @@ describe('TransactionController', () => { }); it('should fail to approve an invalid transaction', async () => { - await new Promise(async (resolve) => { - const controller = new TransactionController({ - provider: PROVIDER, - sign: () => { - throw new Error('foo'); - }, - }); - controller.context = { - NetworkController: MOCK_NETWORK, - } as any; - controller.onComposed(); - const from = '0xe6509775f3f3614576c0d83f8647752f87cd6659'; - const to = '0xc38bf1ad06ef69f0c04e29dbeb4152b4175f0a8d'; - const { result } = await controller.addTransaction({ from, to }); - result.catch((error) => { - const { transaction, status } = controller.state.transactions[0]; - expect(transaction.from).toBe(from); - expect(transaction.to).toBe(to); - expect(status).toBe(TransactionStatus.failed); - expect(error.message).toContain('foo'); - resolve(''); - }); - await controller.approveTransaction(controller.state.transactions[0].id); + const controller = new TransactionController({ + provider: PROVIDER, + sign: () => { + throw new Error('foo'); + }, }); + controller.context = { + NetworkController: MOCK_NETWORK, + } as any; + controller.onComposed(); + const from = '0xe6509775f3f3614576c0d83f8647752f87cd6659'; + const to = '0xc38bf1ad06ef69f0c04e29dbeb4152b4175f0a8d'; + const { result } = await controller.addTransaction({ from, to }); + const { transaction, status } = controller.state.transactions[0]; + expect(transaction.from).toBe(from); + expect(transaction.to).toBe(to); + expect(status).toBe(TransactionStatus.unapproved); + await controller.approveTransaction(controller.state.transactions[0].id); + await expect(result).rejects.toThrow('foo'); }); it('should fail transaction if gas calculation fails', async () => { @@ -714,52 +704,42 @@ describe('TransactionController', () => { }); it('should fail if no sign method defined', async () => { - await new Promise(async (resolve) => { - const controller = new TransactionController({ - provider: PROVIDER, - }); - controller.context = { - NetworkController: MOCK_NETWORK, - } as any; - controller.onComposed(); - const from = '0xe6509775f3f3614576c0d83f8647752f87cd6659'; - const to = '0xc38bf1ad06ef69f0c04e29dbeb4152b4175f0a8d'; - const { result } = await controller.addTransaction({ from, to }); - result.catch((error) => { - const { transaction, status } = controller.state.transactions[0]; - expect(transaction.from).toBe(from); - expect(transaction.to).toBe(to); - expect(status).toBe(TransactionStatus.failed); - expect(error.message).toContain('No sign method defined'); - resolve(''); - }); - await controller.approveTransaction(controller.state.transactions[0].id); + const controller = new TransactionController({ + provider: PROVIDER, }); + controller.context = { + NetworkController: MOCK_NETWORK, + } as any; + controller.onComposed(); + const from = '0xe6509775f3f3614576c0d83f8647752f87cd6659'; + const to = '0xc38bf1ad06ef69f0c04e29dbeb4152b4175f0a8d'; + const { result } = await controller.addTransaction({ from, to }); + const { transaction, status } = controller.state.transactions[0]; + expect(transaction.from).toBe(from); + expect(transaction.to).toBe(to); + expect(status).toBe(TransactionStatus.unapproved); + await controller.approveTransaction(controller.state.transactions[0].id); + await expect(result).rejects.toThrow('No sign method defined'); }); it('should fail if no chainId defined', async () => { - await new Promise(async (resolve) => { - const controller = new TransactionController({ - provider: PROVIDER, - sign: async (transaction: any) => transaction, - }); - controller.context = { - NetworkController: MOCK_NETWORK_WITHOUT_CHAIN_ID, - } as any; - controller.onComposed(); - const from = '0xe6509775f3f3614576c0d83f8647752f87cd6659'; - const to = '0xc38bf1ad06ef69f0c04e29dbeb4152b4175f0a8d'; - const { result } = await controller.addTransaction({ from, to }); - result.catch((error) => { - const { transaction, status } = controller.state.transactions[0]; - expect(transaction.from).toBe(from); - expect(transaction.to).toBe(to); - expect(status).toBe(TransactionStatus.failed); - expect(error.message).toContain('No chainId defined'); - resolve(''); - }); - await controller.approveTransaction(controller.state.transactions[0].id); + const controller = new TransactionController({ + provider: PROVIDER, + sign: async (transaction: any) => transaction, }); + controller.context = { + NetworkController: MOCK_NETWORK_WITHOUT_CHAIN_ID, + } as any; + controller.onComposed(); + const from = '0xe6509775f3f3614576c0d83f8647752f87cd6659'; + const to = '0xc38bf1ad06ef69f0c04e29dbeb4152b4175f0a8d'; + const { result } = await controller.addTransaction({ from, to }); + const { transaction, status } = controller.state.transactions[0]; + expect(transaction.from).toBe(from); + expect(transaction.to).toBe(to); + expect(status).toBe(TransactionStatus.unapproved); + await controller.approveTransaction(controller.state.transactions[0].id); + await expect(result).rejects.toThrow('No chainId defined'); }); it('should approve a transaction', async () => { @@ -965,29 +945,24 @@ describe('TransactionController', () => { }); it('should stop a transaction', async () => { - await new Promise(async (resolve) => { - const controller = new TransactionController({ - provider: PROVIDER, - sign: async (transaction: any) => transaction, - }); - const from = '0xc38bf1ad06ef69f0c04e29dbeb4152b4175f0a8d'; - controller.context = { - NetworkController: MOCK_NETWORK, - } as any; - controller.onComposed(); - const { result } = await controller.addTransaction({ - from, - gas: '0x0', - gasPrice: '0x1', - to: from, - value: '0x0', - }); - result.catch((error) => { - expect(error.message).toContain('User cancelled the transaction'); - resolve(''); - }); - controller.stopTransaction(controller.state.transactions[0].id); + const controller = new TransactionController({ + provider: PROVIDER, + sign: async (transaction: any) => transaction, + }); + const from = '0xc38bf1ad06ef69f0c04e29dbeb4152b4175f0a8d'; + controller.context = { + NetworkController: MOCK_NETWORK, + } as any; + controller.onComposed(); + const { result } = await controller.addTransaction({ + from, + gas: '0x0', + gasPrice: '0x1', + to: from, + value: '0x0', }); + controller.stopTransaction(controller.state.transactions[0].id); + await expect(result).rejects.toThrow('User cancelled the transaction'); }); it('should fail to stop a transaction if no sign method', async () => { From 4b13a4872e465a07818c490d0444f1752846c290 Mon Sep 17 00:00:00 2001 From: Angela Lu Date: Tue, 23 Mar 2021 13:36:47 -0700 Subject: [PATCH 2/5] finish promise callback removal --- src/assets/AssetsController.test.ts | 7 +++---- src/message-manager/MessageManager.test.ts | 5 ++--- src/message-manager/PersonalMessageManager.test.ts | 5 ++--- src/message-manager/TypedMessageManager.test.ts | 5 ++--- 4 files changed, 9 insertions(+), 13 deletions(-) diff --git a/src/assets/AssetsController.test.ts b/src/assets/AssetsController.test.ts index 65bc6c38e1..7352c049e6 100644 --- a/src/assets/AssetsController.test.ts +++ b/src/assets/AssetsController.test.ts @@ -470,10 +470,9 @@ describe('AssetsController', () => { 'ERC20', ); await assetsController.acceptWatchAsset(suggestedAssetMeta.id); - await result.then((res) => { - expect(assetsController.state.suggestedAssets).toHaveLength(0); - expect(res).toBe('0xe9f786dfdd9ae4d57e830acb52296837765f0e5b'); - }); + const res = await result; + expect(assetsController.state.suggestedAssets).toHaveLength(0); + expect(res).toBe('0xe9f786dfdd9ae4d57e830acb52296837765f0e5b'); }); it('should fail a valid suggested asset via watchAsset with wrong type', async () => { diff --git a/src/message-manager/MessageManager.test.ts b/src/message-manager/MessageManager.test.ts index 377dbea679..53c6773918 100644 --- a/src/message-manager/MessageManager.test.ts +++ b/src/message-manager/MessageManager.test.ts @@ -75,9 +75,8 @@ describe('PersonalMessageManager', () => { expect(unapprovedMessages[keys[0]].status).toBe('signed'); }); controller.setMessageStatusSigned(keys[0], rawSig); - await result.then((sig) => { - expect(sig).toEqual(rawSig); - }); + const sig = await result; + expect(sig).toBe(rawSig); }); it('should throw when unapproved finishes', async () => { diff --git a/src/message-manager/PersonalMessageManager.test.ts b/src/message-manager/PersonalMessageManager.test.ts index ebd24660a3..0f2ae5f5ed 100644 --- a/src/message-manager/PersonalMessageManager.test.ts +++ b/src/message-manager/PersonalMessageManager.test.ts @@ -75,9 +75,8 @@ describe('PersonalMessageManager', () => { expect(unapprovedMessages[keys[0]].status).toBe('signed'); }); controller.setMessageStatusSigned(keys[0], rawSig); - await result.then((sig) => { - expect(sig).toEqual(rawSig); - }); + const sig = await result; + expect(sig).toBe(rawSig); }); it('should throw when unapproved finishes', async () => { diff --git a/src/message-manager/TypedMessageManager.test.ts b/src/message-manager/TypedMessageManager.test.ts index 6d60cde295..b8c60e974c 100644 --- a/src/message-manager/TypedMessageManager.test.ts +++ b/src/message-manager/TypedMessageManager.test.ts @@ -95,9 +95,8 @@ describe('TypedMessageManager', () => { expect(unapprovedMessages[keys[0]].status).toBe('signed'); }); controller.setMessageStatusSigned(keys[0], rawSig); - await result.then((sig) => { - expect(sig).toEqual(rawSig); - }); + const sig = await result; + expect(sig).toBe(rawSig); }); it("should set message status as 'errored'", async () => { From 4263bd0e478fcf3a0b67afe3e321053702a90d33 Mon Sep 17 00:00:00 2001 From: Angela Lu Date: Tue, 23 Mar 2021 13:54:25 -0700 Subject: [PATCH 3/5] restore ordering --- src/transaction/TransactionController.test.ts | 46 +++++++++---------- 1 file changed, 23 insertions(+), 23 deletions(-) diff --git a/src/transaction/TransactionController.test.ts b/src/transaction/TransactionController.test.ts index b8866d5a78..8abfc1439d 100644 --- a/src/transaction/TransactionController.test.ts +++ b/src/transaction/TransactionController.test.ts @@ -661,25 +661,25 @@ describe('TransactionController', () => { }); it('should fail to approve an invalid transaction', async () => { - const controller = new TransactionController({ - provider: PROVIDER, - sign: () => { - throw new Error('foo'); - }, - }); - controller.context = { - NetworkController: MOCK_NETWORK, - } as any; - controller.onComposed(); - const from = '0xe6509775f3f3614576c0d83f8647752f87cd6659'; - const to = '0xc38bf1ad06ef69f0c04e29dbeb4152b4175f0a8d'; - const { result } = await controller.addTransaction({ from, to }); - const { transaction, status } = controller.state.transactions[0]; - expect(transaction.from).toBe(from); - expect(transaction.to).toBe(to); - expect(status).toBe(TransactionStatus.unapproved); - await controller.approveTransaction(controller.state.transactions[0].id); - await expect(result).rejects.toThrow('foo'); + const controller = new TransactionController({ + provider: PROVIDER, + sign: () => { + throw new Error('foo'); + }, + }); + controller.context = { + NetworkController: MOCK_NETWORK, + } as any; + controller.onComposed(); + const from = '0xe6509775f3f3614576c0d83f8647752f87cd6659'; + const to = '0xc38bf1ad06ef69f0c04e29dbeb4152b4175f0a8d'; + const { result } = await controller.addTransaction({ from, to }); + await controller.approveTransaction(controller.state.transactions[0].id); + const { transaction, status } = controller.state.transactions[0]; + expect(transaction.from).toBe(from); + expect(transaction.to).toBe(to); + expect(status).toBe(TransactionStatus.failed); + await expect(result).rejects.toThrow('foo'); }); it('should fail transaction if gas calculation fails', async () => { @@ -714,11 +714,11 @@ describe('TransactionController', () => { const from = '0xe6509775f3f3614576c0d83f8647752f87cd6659'; const to = '0xc38bf1ad06ef69f0c04e29dbeb4152b4175f0a8d'; const { result } = await controller.addTransaction({ from, to }); + await controller.approveTransaction(controller.state.transactions[0].id); const { transaction, status } = controller.state.transactions[0]; expect(transaction.from).toBe(from); expect(transaction.to).toBe(to); - expect(status).toBe(TransactionStatus.unapproved); - await controller.approveTransaction(controller.state.transactions[0].id); + expect(status).toBe(TransactionStatus.failed); await expect(result).rejects.toThrow('No sign method defined'); }); @@ -734,11 +734,11 @@ describe('TransactionController', () => { const from = '0xe6509775f3f3614576c0d83f8647752f87cd6659'; const to = '0xc38bf1ad06ef69f0c04e29dbeb4152b4175f0a8d'; const { result } = await controller.addTransaction({ from, to }); + await controller.approveTransaction(controller.state.transactions[0].id); const { transaction, status } = controller.state.transactions[0]; expect(transaction.from).toBe(from); expect(transaction.to).toBe(to); - expect(status).toBe(TransactionStatus.unapproved); - await controller.approveTransaction(controller.state.transactions[0].id); + expect(status).toBe(TransactionStatus.failed); await expect(result).rejects.toThrow('No chainId defined'); }); From 41eb582a462ff2690ae823d2abd305121d76c61c Mon Sep 17 00:00:00 2001 From: Angela Lu Date: Tue, 23 Mar 2021 14:05:16 -0700 Subject: [PATCH 4/5] promise does not resolve --- src/transaction/TransactionController.test.ts | 47 ++++++++++--------- 1 file changed, 25 insertions(+), 22 deletions(-) diff --git a/src/transaction/TransactionController.test.ts b/src/transaction/TransactionController.test.ts index 8abfc1439d..d7eb4747bc 100644 --- a/src/transaction/TransactionController.test.ts +++ b/src/transaction/TransactionController.test.ts @@ -617,9 +617,12 @@ describe('TransactionController', () => { to: from, }); controller.cancelTransaction('foo'); - controller.hub.once(`${controller.state.transactions[0].id}:finished`, () => { - expect(controller.state.transactions[0].transaction.from).toBe(from); - expect(controller.state.transactions[0].status).toBe(TransactionStatus.rejected); + await new Promise(async (resolve) => { + controller.hub.once(`${controller.state.transactions[0].id}:finished`, () => { + expect(controller.state.transactions[0].transaction.from).toBe(from); + expect(controller.state.transactions[0].status).toBe(TransactionStatus.rejected); + resolve(''); + }); }); controller.cancelTransaction(controller.state.transactions[0].id); await expect(result).rejects.toThrow('User rejected the transaction'); @@ -661,25 +664,25 @@ describe('TransactionController', () => { }); it('should fail to approve an invalid transaction', async () => { - const controller = new TransactionController({ - provider: PROVIDER, - sign: () => { - throw new Error('foo'); - }, - }); - controller.context = { - NetworkController: MOCK_NETWORK, - } as any; - controller.onComposed(); - const from = '0xe6509775f3f3614576c0d83f8647752f87cd6659'; - const to = '0xc38bf1ad06ef69f0c04e29dbeb4152b4175f0a8d'; - const { result } = await controller.addTransaction({ from, to }); - await controller.approveTransaction(controller.state.transactions[0].id); - const { transaction, status } = controller.state.transactions[0]; - expect(transaction.from).toBe(from); - expect(transaction.to).toBe(to); - expect(status).toBe(TransactionStatus.failed); - await expect(result).rejects.toThrow('foo'); + const controller = new TransactionController({ + provider: PROVIDER, + sign: () => { + throw new Error('foo'); + }, + }); + controller.context = { + NetworkController: MOCK_NETWORK, + } as any; + controller.onComposed(); + const from = '0xe6509775f3f3614576c0d83f8647752f87cd6659'; + const to = '0xc38bf1ad06ef69f0c04e29dbeb4152b4175f0a8d'; + const { result } = await controller.addTransaction({ from, to }); + await controller.approveTransaction(controller.state.transactions[0].id); + const { transaction, status } = controller.state.transactions[0]; + expect(transaction.from).toBe(from); + expect(transaction.to).toBe(to); + expect(status).toBe(TransactionStatus.failed); + await expect(result).rejects.toThrow('foo'); }); it('should fail transaction if gas calculation fails', async () => { From 56c1c91be8498e2fb786969e823951cc7a603933 Mon Sep 17 00:00:00 2001 From: Angela Lu Date: Wed, 24 Mar 2021 15:40:30 -0700 Subject: [PATCH 5/5] use listener to ensure expects are called --- src/transaction/TransactionController.test.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/transaction/TransactionController.test.ts b/src/transaction/TransactionController.test.ts index d7eb4747bc..edf39d3503 100644 --- a/src/transaction/TransactionController.test.ts +++ b/src/transaction/TransactionController.test.ts @@ -617,7 +617,7 @@ describe('TransactionController', () => { to: from, }); controller.cancelTransaction('foo'); - await new Promise(async (resolve) => { + const transactionListener = new Promise(async (resolve) => { controller.hub.once(`${controller.state.transactions[0].id}:finished`, () => { expect(controller.state.transactions[0].transaction.from).toBe(from); expect(controller.state.transactions[0].status).toBe(TransactionStatus.rejected); @@ -626,6 +626,7 @@ describe('TransactionController', () => { }); controller.cancelTransaction(controller.state.transactions[0].id); await expect(result).rejects.toThrow('User rejected the transaction'); + await transactionListener; }); it('should wipe transactions', async () => {