From 875e8a19eaddcbb95599828db6ff3459b17dcecc Mon Sep 17 00:00:00 2001 From: Charly Chevalier Date: Thu, 16 Jan 2025 11:14:26 +0100 Subject: [PATCH] test(accounts-controller): improve uuid mocks (#5005) ## Explanation Unify the UUID mocking. With this new refactor we longer have to specify each returned values of `mockUUID`. It also handles the non-registered accounts (in this case, a random UUID will be generated). ## References N/A ## Changelog N/A (those changes only impact tests) ## Checklist - [x] I've updated the test suite for new or updated code as appropriate - [x] I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate - [x] I've highlighted breaking changes using the "BREAKING" category above as appropriate - [x] I've prepared draft pull requests for clients and consumer packages to resolve any breaking changes --- .../src/AccountsController.test.ts | 135 +++++++++--------- 1 file changed, 65 insertions(+), 70 deletions(-) diff --git a/packages/accounts-controller/src/AccountsController.test.ts b/packages/accounts-controller/src/AccountsController.test.ts index b28666d152..8c612d1e18 100644 --- a/packages/accounts-controller/src/AccountsController.test.ts +++ b/packages/accounts-controller/src/AccountsController.test.ts @@ -133,9 +133,6 @@ const mockAccount4: InternalAccount = { }, }; -/** - * Mock generated normal account ID to an actual "hard-coded" one. - */ class MockNormalAccountUUID { #accountIds: Record = {}; @@ -158,6 +155,18 @@ class MockNormalAccountUUID { } } +/** + * Mock generated normal account ID to their actual mock ID. This function will + * automatically attaches those accounts to `mockUUID`. A random UUID will be + * generated if an account has not been registered. See {@link MockNormalAccountUUID}. + * + * @param accounts - List of normal accounts to map with their mock ID. + */ +function mockUUIDWithNormalAccounts(accounts: InternalAccount[]) { + const mockAccountUUIDs = new MockNormalAccountUUID(accounts); + mockUUID.mockImplementation(mockAccountUUIDs.mock.bind(mockAccountUUIDs)); +} + /** * Creates an `InternalAccount` object from the given normal account properties. * @@ -466,9 +475,8 @@ describe('AccountsController', () => { describe('onKeyringStateChange', () => { it('uses listMultichainAccounts', async () => { const messenger = buildMessenger(); - mockUUID - .mockReturnValueOnce('mock-id') // call to check if its a new account - .mockReturnValueOnce('mock-id2'); // call to add account + + mockUUIDWithNormalAccounts([mockAccount, mockAccount2]); const { accountsController } = setupAccountsController({ initialState: { @@ -562,10 +570,8 @@ describe('AccountsController', () => { describe('adding accounts', () => { it('add new accounts', async () => { const messenger = buildMessenger(); - mockUUID - .mockReturnValueOnce('mock-id') // call to check if its a new account - .mockReturnValueOnce('mock-id2') // call to check if its a new account - .mockReturnValueOnce('mock-id2'); // call to add account + + mockUUIDWithNormalAccounts([mockAccount, mockAccount2, mockAccount3]); const mockNewKeyringState = { isUnlocked: true, @@ -604,7 +610,7 @@ describe('AccountsController', () => { }); it('add Snap accounts', async () => { - mockUUID.mockReturnValueOnce('mock-id'); // call to check if its a new account + mockUUIDWithNormalAccounts([mockAccount]); const messenger = buildMessenger(); messenger.registerActionHandler( @@ -671,7 +677,8 @@ describe('AccountsController', () => { }); it('handle the event when a Snap deleted the account before the it was added', async () => { - mockUUID.mockReturnValueOnce('mock-id'); // call to check if its a new account + mockUUIDWithNormalAccounts([mockAccount]); + const messenger = buildMessenger(); messenger.registerActionHandler( 'KeyringController:getKeyringsByType', @@ -729,11 +736,8 @@ describe('AccountsController', () => { it('increment the default account number when adding an account', async () => { const messenger = buildMessenger(); - mockUUID - .mockReturnValueOnce('mock-id') // call to check if its a new account - .mockReturnValueOnce('mock-id2') // call to check if its a new account - .mockReturnValueOnce('mock-id3') // call to check if its a new account - .mockReturnValueOnce('mock-id3'); // call to add account + + mockUUIDWithNormalAccounts([mockAccount, mockAccount2, mockAccount3]); const mockNewKeyringState = { isUnlocked: true, @@ -785,11 +789,8 @@ describe('AccountsController', () => { it('use the next number after the total number of accounts of a keyring when adding an account, if the index is lower', async () => { const messenger = buildMessenger(); - mockUUID - .mockReturnValueOnce('mock-id') // call to check if its a new account - .mockReturnValueOnce('mock-id2') // call to check if its a new account - .mockReturnValueOnce('mock-id3') // call to check if its a new account - .mockReturnValueOnce('mock-id3'); // call to add account + + mockUUIDWithNormalAccounts([mockAccount, mockAccount2, mockAccount3]); const mockAccount2WithCustomName = createExpectedInternalAccount({ id: 'mock-id2', @@ -847,7 +848,7 @@ describe('AccountsController', () => { }); it('handle when the account to set as selectedAccount is undefined', async () => { - mockUUID.mockReturnValueOnce('mock-id'); // call to check if its a new account + mockUUIDWithNormalAccounts([mockAccount]); const messenger = buildMessenger(); messenger.registerActionHandler( @@ -897,10 +898,8 @@ describe('AccountsController', () => { it('selectedAccount remains the same after adding a new account', async () => { const messenger = buildMessenger(); - mockUUID - .mockReturnValueOnce('mock-id') // call to check if its a new account - .mockReturnValueOnce('mock-id2') // call to check if its a new account - .mockReturnValueOnce('mock-id2'); // call to add account + + mockUUIDWithNormalAccounts([mockAccount, mockAccount2, mockAccount3]); const mockNewKeyringState = { isUnlocked: true, @@ -942,10 +941,8 @@ describe('AccountsController', () => { it('publishes accountAdded event', async () => { const messenger = buildMessenger(); const messengerSpy = jest.spyOn(messenger, 'publish'); - mockUUID - .mockReturnValueOnce(mockAccount.id) // call to check if its a new account - .mockReturnValueOnce(mockAccount2.id) // call to check if its a new account - .mockReturnValueOnce(mockAccount2.id); // call to add account + + mockUUIDWithNormalAccounts([mockAccount, mockAccount2]); setupAccountsController({ initialState: { @@ -987,7 +984,8 @@ describe('AccountsController', () => { describe('deleting account', () => { it('delete accounts if its gone from the keyring state', async () => { const messenger = buildMessenger(); - mockUUID.mockReturnValueOnce('mock-id2'); + + mockUUIDWithNormalAccounts([mockAccount2]); const mockNewKeyringState = { isUnlocked: true, @@ -1027,11 +1025,8 @@ describe('AccountsController', () => { it('delete accounts and set the most recent lastSelected account', async () => { const messenger = buildMessenger(); - mockUUID - .mockReturnValueOnce('mock-id') - .mockReturnValueOnce('mock-id2') - .mockReturnValueOnce('mock-id') - .mockReturnValueOnce('mock-id2'); + + mockUUIDWithNormalAccounts([mockAccount, mockAccount2]); const mockNewKeyringState = { isUnlocked: true, @@ -1083,11 +1078,8 @@ describe('AccountsController', () => { it('delete accounts and set the most recent lastSelected account when there are accounts that have never been selected', async () => { const messenger = buildMessenger(); - mockUUID - .mockReturnValueOnce('mock-id') - .mockReturnValueOnce('mock-id2') - .mockReturnValueOnce('mock-id') - .mockReturnValueOnce('mock-id2'); + + mockUUIDWithNormalAccounts([mockAccount, mockAccount2]); const mockAccount2WithoutLastSelected = { ...mockAccount2, @@ -1147,7 +1139,8 @@ describe('AccountsController', () => { it('delete the account and select the account with the most recent lastSelected', async () => { const currentTime = Date.now(); const messenger = buildMessenger(); - mockUUID.mockReturnValueOnce('mock-id').mockReturnValueOnce('mock-id2'); + + mockUUIDWithNormalAccounts([mockAccount, mockAccount2]); const mockAccountWithoutLastSelected = { ...mockAccount, @@ -1222,10 +1215,8 @@ describe('AccountsController', () => { it('publishes accountRemoved event', async () => { const messenger = buildMessenger(); const messengerSpy = jest.spyOn(messenger, 'publish'); - mockUUID - .mockReturnValueOnce(mockAccount.id) // call to check if its a new account - .mockReturnValueOnce(mockAccount2.id) // call to check if its a new account - .mockReturnValueOnce(mockAccount2.id); // call to add account + + mockUUIDWithNormalAccounts([mockAccount, mockAccount2]); setupAccountsController({ initialState: { @@ -1278,9 +1269,11 @@ describe('AccountsController', () => { address: '0x456', keyringType: KeyringTypes.hd, }); - mockUUID - .mockReturnValueOnce('mock-id2') // call to check if its a new account - .mockReturnValueOnce('mock-id2'); // call to add account + + mockUUIDWithNormalAccounts([ + mockInitialAccount, + mockReinitialisedAccount, + ]); const mockNewKeyringState = { isUnlocked: true, @@ -1361,9 +1354,7 @@ describe('AccountsController', () => { }); mockExistingAccount2.metadata.lastSelected = lastSelectedForAccount2; - mockUUID - .mockReturnValueOnce('mock-id') // call to check if its a new account - .mockReturnValueOnce('mock-id2'); // call to check if its a new account + mockUUIDWithNormalAccounts([mockAccount, mockAccount2]); const { accountsController } = setupAccountsController({ initialState: { @@ -1447,7 +1438,8 @@ describe('AccountsController', () => { }); it('update accounts with normal accounts', async () => { - mockUUID.mockReturnValueOnce('mock-id').mockReturnValueOnce('mock-id2'); + mockUUIDWithNormalAccounts([mockAccount, mockAccount2]); + const messenger = buildMessenger(); messenger.registerActionHandler( 'KeyringController:getAccounts', @@ -1492,6 +1484,7 @@ describe('AccountsController', () => { keyringType: KeyringTypes.hd, }), ]; + mockUUIDWithNormalAccounts(expectedAccounts); await accountsController.updateAccounts(); @@ -1588,7 +1581,8 @@ describe('AccountsController', () => { }); it('set the account with the correct index', async () => { - mockUUID.mockReturnValueOnce('mock-id').mockReturnValueOnce('mock-id2'); + mockUUIDWithNormalAccounts([mockAccount]); + const messenger = buildMessenger(); messenger.registerActionHandler( 'KeyringController:getAccounts', @@ -1630,6 +1624,7 @@ describe('AccountsController', () => { keyringType: KeyringTypes.hd, }), ]; + mockUUIDWithNormalAccounts(expectedAccounts); await accountsController.updateAccounts(); @@ -1639,7 +1634,8 @@ describe('AccountsController', () => { }); it('filter Snap accounts from normalAccounts', async () => { - mockUUID.mockReturnValueOnce('mock-id'); + mockUUIDWithNormalAccounts([mockAccount]); + const messenger = buildMessenger(); messenger.registerActionHandler( 'KeyringController:getKeyringsByType', @@ -1696,7 +1692,8 @@ describe('AccountsController', () => { }); it('filter Snap accounts from normalAccounts even if the snap account is listed before normal accounts', async () => { - mockUUID.mockReturnValue('mock-id'); + mockUUIDWithNormalAccounts([mockAccount]); + const messenger = buildMessenger(); messenger.registerActionHandler( 'KeyringController:getKeyringsByType', @@ -1762,7 +1759,7 @@ describe('AccountsController', () => { KeyringTypes.qr, 'Custody - JSON - RPC', ])('should add accounts for %s type', async (keyringType) => { - mockUUID.mockReturnValue('mock-id'); + mockUUIDWithNormalAccounts([mockAccount]); const messenger = buildMessenger(); messenger.registerActionHandler( @@ -1811,7 +1808,7 @@ describe('AccountsController', () => { }); it('throw an error if the keyring type is unknown', async () => { - mockUUID.mockReturnValue('mock-id'); + mockUUIDWithNormalAccounts([mockAccount]); const messenger = buildMessenger(); messenger.registerActionHandler( @@ -1892,9 +1889,7 @@ describe('AccountsController', () => { }); mockExistingAccount2.metadata.lastSelected = lastSelectedForAccount2; - mockUUID - .mockReturnValueOnce('mock-id') // call to check if its a new account - .mockReturnValueOnce('mock-id2'); // call to check if its a new account + mockUUIDWithNormalAccounts([mockAccount, mockAccount2]); messenger.registerActionHandler( 'KeyringController:getKeyringsByType', @@ -2544,12 +2539,12 @@ describe('AccountsController', () => { it('return the next account number', async () => { const messenger = buildMessenger(); - mockUUID - .mockReturnValueOnce('mock-id') // call to check if its a new account - .mockReturnValueOnce('mock-id2') // call to check if its a new account - .mockReturnValueOnce('mock-id3') // call to check if its a new account - .mockReturnValueOnce('mock-id2') // call to add account - .mockReturnValueOnce('mock-id3'); // call to add account + + mockUUIDWithNormalAccounts([ + mockAccount, + mockSimpleKeyring1, + mockSimpleKeyring2, + ]); const { accountsController } = setupAccountsController({ initialState: { @@ -2582,13 +2577,13 @@ describe('AccountsController', () => { it('return the next account number even with an index gap', async () => { const messenger = buildMessenger(); - const mockAccountUUIDs = new MockNormalAccountUUID([ + + mockUUIDWithNormalAccounts([ mockAccount, mockSimpleKeyring1, mockSimpleKeyring2, mockSimpleKeyring3, ]); - mockUUID.mockImplementation(mockAccountUUIDs.mock.bind(mockAccountUUIDs)); const { accountsController } = setupAccountsController({ initialState: {