From 7b72c6fe5acdc3c8857d56db6b8262c277d256dd Mon Sep 17 00:00:00 2001 From: Mark Stacey Date: Wed, 25 Jan 2023 12:14:36 -0330 Subject: [PATCH] Simplify composable controller tests (#1077) The composable controller tests have been simplified. They no longer use real controllers, instead using mock controllers defined in the test module. This should reduce the churn we've been seeing in these tests as we make breaking changes to controllers. **Checklist** - [x] Tests are included if applicable - [x] Any added code is fully documented --- packages/composable-controller/package.json | 6 - .../src/ComposableController.test.ts | 283 +++--------------- yarn.lock | 12 +- 3 files changed, 49 insertions(+), 252 deletions(-) diff --git a/packages/composable-controller/package.json b/packages/composable-controller/package.json index 929816dbaa..e77f1eb1a0 100644 --- a/packages/composable-controller/package.json +++ b/packages/composable-controller/package.json @@ -32,13 +32,7 @@ "@metamask/base-controller": "workspace:^" }, "devDependencies": { - "@metamask/address-book-controller": "workspace:^", - "@metamask/assets-controllers": "workspace:^", "@metamask/auto-changelog": "^3.1.0", - "@metamask/controller-utils": "workspace:^", - "@metamask/ens-controller": "workspace:^", - "@metamask/network-controller": "workspace:^", - "@metamask/preferences-controller": "workspace:^", "@types/jest": "^26.0.22", "deepmerge": "^4.2.2", "immer": "^9.0.6", diff --git a/packages/composable-controller/src/ComposableController.test.ts b/packages/composable-controller/src/ComposableController.test.ts index 73c25ba530..69a2362a0e 100644 --- a/packages/composable-controller/src/ComposableController.test.ts +++ b/packages/composable-controller/src/ComposableController.test.ts @@ -1,15 +1,5 @@ import * as sinon from 'sinon'; import type { Patch } from 'immer'; -import { - TokensController, - NftController, - AssetsContractController, -} from '@metamask/assets-controllers'; -import { - AddressBookController, - AddressType, -} from '@metamask/address-book-controller'; -import { EnsController } from '@metamask/ens-controller'; import { BaseController, BaseState, @@ -17,17 +7,7 @@ import { ControllerMessenger, RestrictedControllerMessenger, } from '@metamask/base-controller'; -import { PreferencesController } from '@metamask/preferences-controller'; -import { - NetworkController, - NetworkControllerMessenger, - NetworkControllerStateChangeEvent, -} from '@metamask/network-controller'; -import { NetworksChainId } from '@metamask/controller-utils'; -import { - ComposableController, - ComposableControllerRestrictedMessenger, -} from './ComposableController'; +import { ComposableController } from './ComposableController'; // Mock BaseControllerV2 classes @@ -97,81 +77,21 @@ class BarController extends BaseController { } } -const setupControllers = () => { - const mainMessenger = new ControllerMessenger< - never, - NetworkControllerStateChangeEvent - >(); - - const messenger: NetworkControllerMessenger = mainMessenger.getRestricted({ - name: 'NetworkController', - allowedEvents: ['NetworkController:stateChange'], - allowedActions: [], - }); - - const composableMessenger: ComposableControllerRestrictedMessenger = - mainMessenger.getRestricted({ - name: 'ComposableController', - allowedEvents: ['NetworkController:stateChange'], - allowedActions: [], - }); - - const networkController = new NetworkController({ - messenger, - infuraProjectId: 'potato', - }); - const preferencesController = new PreferencesController(); - - const assetContractController = new AssetsContractController({ - onPreferencesStateChange: (listener) => - preferencesController.subscribe(listener), - onNetworkStateChange: (listener) => - messenger.subscribe('NetworkController:stateChange', listener), - }); - - const nftController = new NftController({ - onPreferencesStateChange: (listener) => - preferencesController.subscribe(listener), - onNetworkStateChange: (listener) => - messenger.subscribe('NetworkController:stateChange', listener), - getERC721AssetName: assetContractController.getERC721AssetName.bind( - assetContractController, - ), - getERC721AssetSymbol: assetContractController.getERC721AssetSymbol.bind( - assetContractController, - ), - getERC721TokenURI: assetContractController.getERC721TokenURI.bind( - assetContractController, - ), - getERC721OwnerOf: assetContractController.getERC721OwnerOf.bind( - assetContractController, - ), - getERC1155BalanceOf: assetContractController.getERC1155BalanceOf.bind( - assetContractController, - ), - getERC1155TokenURI: assetContractController.getERC1155TokenURI.bind( - assetContractController, - ), - onNftAdded: jest.fn(), - }); +interface BazControllerState extends BaseState { + baz: string; +} +class BazController extends BaseController { + defaultState = { + baz: 'baz', + }; - const tokensController = new TokensController({ - onPreferencesStateChange: (listener) => - preferencesController.subscribe(listener), - onNetworkStateChange: (listener) => - messenger.subscribe('NetworkController:stateChange', listener), - }); + override name = 'BazController'; - return { - messenger, - composableMessenger, - networkController, - preferencesController, - assetContractController, - nftController, - tokensController, - }; -}; + constructor() { + super(); + this.initialize(); + } +} describe('ComposableController', () => { afterEach(() => { @@ -180,170 +100,59 @@ describe('ComposableController', () => { describe('BaseController', () => { it('should compose controller state', () => { - const { - messenger, - composableMessenger, - networkController, - assetContractController, - nftController, - tokensController, - preferencesController, - } = setupControllers(); - + const composableMessenger = new ControllerMessenger().getRestricted< + 'ComposableController', + never, + never + >({ name: 'ComposableController' }); const controller = new ComposableController( - [ - new AddressBookController(), - nftController, - assetContractController, - new EnsController(), - networkController, - preferencesController, - tokensController, - ], + [new BarController(), new BazController()], composableMessenger, ); expect(controller.state).toStrictEqual({ - AddressBookController: { addressBook: {} }, - AssetsContractController: {}, - NftController: { - allNftContracts: {}, - allNfts: {}, - ignoredNfts: [], - }, - TokensController: { - allTokens: {}, - ignoredTokens: [], - allIgnoredTokens: {}, - suggestedAssets: [], - tokens: [], - detectedTokens: [], - allDetectedTokens: {}, - }, - EnsController: { - ensEntries: {}, - }, - NetworkController: { - network: 'loading', - isCustomNetwork: false, - properties: { isEIP1559Compatible: false }, - providerConfig: { type: 'mainnet', chainId: NetworksChainId.mainnet }, - }, - PreferencesController: { - featureFlags: {}, - frequentRpcList: [], - identities: {}, - ipfsGateway: 'https://ipfs.io/ipfs/', - lostIdentities: {}, - selectedAddress: '', - useTokenDetection: true, - useNftDetection: false, - openSeaEnabled: false, - }, + BarController: { bar: 'bar' }, + BazController: { baz: 'baz' }, }); - - messenger.clearEventSubscriptions('NetworkController:stateChange'); }); it('should compose flat controller state', () => { - const { - messenger, - composableMessenger, - networkController, - assetContractController, - nftController, - tokensController, - preferencesController, - } = setupControllers(); - + const composableMessenger = new ControllerMessenger().getRestricted< + 'ComposableController', + never, + never + >({ name: 'ComposableController' }); const controller = new ComposableController( - [ - new AddressBookController(), - nftController, - assetContractController, - new EnsController(), - networkController, - preferencesController, - tokensController, - ], + [new BarController(), new BazController()], composableMessenger, ); + expect(controller.flatState).toStrictEqual({ - addressBook: {}, - allNftContracts: {}, - allNfts: {}, - allTokens: {}, - ensEntries: {}, - featureFlags: {}, - frequentRpcList: [], - identities: {}, - ignoredNfts: [], - ignoredTokens: [], - allIgnoredTokens: {}, - detectedTokens: [], - allDetectedTokens: {}, - ipfsGateway: 'https://ipfs.io/ipfs/', - lostIdentities: {}, - network: 'loading', - isCustomNetwork: false, - properties: { isEIP1559Compatible: false }, - providerConfig: { type: 'mainnet', chainId: NetworksChainId.mainnet }, - selectedAddress: '', - useTokenDetection: true, - useNftDetection: false, - openSeaEnabled: false, - suggestedAssets: [], - tokens: [], + bar: 'bar', + baz: 'baz', }); - - messenger.clearEventSubscriptions('NetworkController:stateChange'); - }); - - it('should set initial state', () => { - const state = { - addressBook: { - '0x1': { - '0x1234': { - address: 'bar', - chainId: '1', - isEns: false, - memo: '', - name: 'foo', - addressType: AddressType.externallyOwnedAccounts, - }, - }, - }, - }; - const controller = new ComposableController([ - new AddressBookController(undefined, state), - ]); - expect(controller.state).toStrictEqual({ AddressBookController: state }); }); it('should notify listeners of nested state change', () => { - const addressBookController = new AddressBookController(); - const controller = new ComposableController([addressBookController]); + const composableMessenger = new ControllerMessenger().getRestricted< + 'ComposableController', + never, + never + >({ name: 'ComposableController' }); + const barController = new BarController(); + const controller = new ComposableController( + [barController], + composableMessenger, + ); const listener = sinon.stub(); controller.subscribe(listener); - addressBookController.set( - '0x32Be343B94f860124dC4fEe278FDCBD38C102D88', - 'foo', - ); + + barController.updateBar('something different'); + expect(listener.calledOnce).toBe(true); expect(listener.getCall(0).args[0]).toStrictEqual({ - AddressBookController: { - addressBook: { - 1: { - '0x32Be343B94f860124dC4fEe278FDCBD38C102D88': { - address: '0x32Be343B94f860124dC4fEe278FDCBD38C102D88', - chainId: '1', - isEns: false, - memo: '', - name: 'foo', - addressType: undefined, - }, - }, - }, + BarController: { + bar: 'something different', }, }); }); diff --git a/yarn.lock b/yarn.lock index ab4eb0a748..8471a037ce 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1380,7 +1380,7 @@ __metadata: languageName: node linkType: hard -"@metamask/address-book-controller@workspace:^, @metamask/address-book-controller@workspace:packages/address-book-controller": +"@metamask/address-book-controller@workspace:packages/address-book-controller": version: 0.0.0-use.local resolution: "@metamask/address-book-controller@workspace:packages/address-book-controller" dependencies: @@ -1435,7 +1435,7 @@ __metadata: languageName: unknown linkType: soft -"@metamask/assets-controllers@workspace:^, @metamask/assets-controllers@workspace:packages/assets-controllers": +"@metamask/assets-controllers@workspace:packages/assets-controllers": version: 0.0.0-use.local resolution: "@metamask/assets-controllers@workspace:packages/assets-controllers" dependencies: @@ -1523,14 +1523,8 @@ __metadata: version: 0.0.0-use.local resolution: "@metamask/composable-controller@workspace:packages/composable-controller" dependencies: - "@metamask/address-book-controller": "workspace:^" - "@metamask/assets-controllers": "workspace:^" "@metamask/auto-changelog": ^3.1.0 "@metamask/base-controller": "workspace:^" - "@metamask/controller-utils": "workspace:^" - "@metamask/ens-controller": "workspace:^" - "@metamask/network-controller": "workspace:^" - "@metamask/preferences-controller": "workspace:^" "@types/jest": ^26.0.22 deepmerge: ^4.2.2 immer: ^9.0.6 @@ -1627,7 +1621,7 @@ __metadata: languageName: node linkType: hard -"@metamask/ens-controller@workspace:^, @metamask/ens-controller@workspace:packages/ens-controller": +"@metamask/ens-controller@workspace:packages/ens-controller": version: 0.0.0-use.local resolution: "@metamask/ens-controller@workspace:packages/ens-controller" dependencies: