chore: improve mock asset controller data source tests#7958
Merged
Prithpal-Sooriya merged 3 commits intomainfrom Feb 18, 2026
Merged
chore: improve mock asset controller data source tests#7958Prithpal-Sooriya merged 3 commits intomainfrom
Prithpal-Sooriya merged 3 commits intomainfrom
Conversation
This commit introduces a new mock implementation of the AssetControllerMessenger, which facilitates testing by simulating various actions and events related to asset management. The mock includes methods for creating a mock provider, registering action handlers, and simulating network states. Additionally, it updates the RpcDataSource and StakedBalanceDataSource tests to utilize the new mock, improving test reliability and coverage. Refactors existing tests to remove unnecessary complexity and improve clarity, ensuring that the mock accurately reflects the expected behavior of the real messenger in a controlled environment.
|
|
||
| // Test escape hatch for mocking areas that do not need explicit types | ||
| // eslint-disable-next-line @typescript-eslint/no-explicit-any | ||
| type TestMockType = any; |
Contributor
Author
There was a problem hiding this comment.
Only 1 or 2 places are using the escape hatch 🤞🏾
Comment on lines
+49
to
+97
| rootMessenger.delegate({ | ||
| messenger: assetsControllerMessenger, | ||
| actions: [ | ||
| // AssetsController | ||
| 'AccountTreeController:getAccountsFromSelectedAccountGroup', | ||
| // RpcDataSource | ||
| 'TokenListController:getState', | ||
| 'NetworkController:getState', | ||
| 'NetworkController:getNetworkClientById', | ||
| // RpcDataSource, StakedBalanceDataSource | ||
| 'NetworkEnablementController:getState', | ||
| // SnapDataSource | ||
| 'SnapController:getRunnableSnaps', | ||
| 'SnapController:handleRequest', | ||
| 'PermissionController:getPermissions', | ||
| // BackendWebsocketDataSource | ||
| 'BackendWebSocketService:connect', | ||
| 'BackendWebSocketService:disconnect', | ||
| 'BackendWebSocketService:forceReconnection', | ||
| 'BackendWebSocketService:sendMessage', | ||
| 'BackendWebSocketService:sendRequest', | ||
| 'BackendWebSocketService:getConnectionInfo', | ||
| 'BackendWebSocketService:getSubscriptionsByChannel', | ||
| 'BackendWebSocketService:channelHasSubscription', | ||
| 'BackendWebSocketService:findSubscriptionsByChannelPrefix', | ||
| 'BackendWebSocketService:addChannelCallback', | ||
| 'BackendWebSocketService:removeChannelCallback', | ||
| 'BackendWebSocketService:getChannelCallbacks', | ||
| 'BackendWebSocketService:subscribe', | ||
| ], | ||
| events: [ | ||
| // AssetsController | ||
| 'AccountTreeController:selectedAccountGroupChange', | ||
| 'KeyringController:lock', | ||
| 'KeyringController:unlock', | ||
| 'PreferencesController:stateChange', | ||
| // RpcDataSource, StakedBalanceDataSource | ||
| 'NetworkController:stateChange', | ||
| 'TransactionController:transactionConfirmed', | ||
| 'TransactionController:incomingTransactionsReceived', | ||
| // StakedBalanceDataSource | ||
| 'NetworkEnablementController:stateChange', | ||
| // SnapDataSource | ||
| 'AccountsController:accountBalancesUpdated', | ||
| 'PermissionController:stateChange', | ||
| // BackendWebsocketDataSource | ||
| 'BackendWebSocketService:connectionStateChanged', | ||
| ], | ||
| }); |
Contributor
Author
There was a problem hiding this comment.
Yeah lets def sync on the AssetController messenger.
This messenger has way too many responsibilities.
Ideally each data source should have its own responsibilities.
Comment on lines
+65
to
+77
| 'BackendWebSocketService:connect', | ||
| 'BackendWebSocketService:disconnect', | ||
| 'BackendWebSocketService:forceReconnection', | ||
| 'BackendWebSocketService:sendMessage', | ||
| 'BackendWebSocketService:sendRequest', | ||
| 'BackendWebSocketService:getConnectionInfo', | ||
| 'BackendWebSocketService:getSubscriptionsByChannel', | ||
| 'BackendWebSocketService:channelHasSubscription', | ||
| 'BackendWebSocketService:findSubscriptionsByChannelPrefix', | ||
| 'BackendWebSocketService:addChannelCallback', | ||
| 'BackendWebSocketService:removeChannelCallback', | ||
| 'BackendWebSocketService:getChannelCallbacks', | ||
| 'BackendWebSocketService:subscribe', |
Contributor
Author
There was a problem hiding this comment.
Does the asset controller really need ALL the backend actions?
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
packages/assets-controller/src/data-sources/StakedBalanceDataSource.ts
Outdated
Show resolved
Hide resolved
…DataSource This commit cleans up the `destroy` method in the `StakedBalanceDataSource` class by removing the unsubscription logic for transaction and network events, which is no longer necessary. Corresponding test cases have also been removed to reflect this change.
| this.#handleStakedBalanceUpdate.bind(this), | ||
| ); | ||
|
|
||
| const unsubConfirmed = this.#messenger.subscribe( |
Contributor
Author
There was a problem hiding this comment.
.subscribe does not return a callback, so this was not needed.
Comment on lines
-922
to
-925
| this.#unsubscribeTransactionConfirmed?.(); | ||
| this.#unsubscribeIncomingTransactions?.(); | ||
| this.#unsubscribeNetworkStateChange?.(); | ||
| this.#unsubscribeNetworkEnablementControllerStateChange?.(); |
Contributor
Author
There was a problem hiding this comment.
There is no callback to unsubscribe from.
| }); | ||
| }); | ||
|
|
||
| describe('destroy', () => { |
Contributor
Author
There was a problem hiding this comment.
Test is irrelevant. The controller destruction will clear messenger subscriptions.
BUT we share this messenger with other data sources, so a data source destruction might impact other data sources 💀
We can discuss delegating and generating diff messengers per data source.
packages/assets-controller/src/data-sources/StakedBalanceDataSource.test.ts
Show resolved
Hide resolved
salimtb
approved these changes
Feb 18, 2026
Contributor
salimtb
left a comment
salimtb
left a comment
There was a problem hiding this comment.
tested and work as expected
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Explanation
This commit introduces a new mock implementation of the AssetControllerMessenger, which facilitates testing by simulating various actions and events related to asset management. The mock includes methods for creating a mock provider, registering action handlers, and simulating network states. Additionally, it updates the RpcDataSource and StakedBalanceDataSource tests to utilize the new mock, improving test reliability and coverage.
Refactors existing tests to remove unnecessary complexity and improve clarity, ensuring that the mock accurately reflects the expected behavior of the real messenger in a controlled environment.
References
Checklist
Note
Medium Risk
Mostly test-only refactors, but it changes
StakedBalanceDataSourcesubscription teardown behavior to rely on messenger-level cleanup, which could affect consumers with custom messenger implementations.Overview
Improves
assets-controllerdata source tests by introducing a sharedMockAssetControllerMessengerfixture that delegates the relevant actions/events and provides helpers for registering RPC/staking handlers and mocking aWeb3Provider.Updates
RpcDataSourceandStakedBalanceDataSourcetests to use the shared fixture, simplify network-state/event publishing via the root messenger, and tighten event-driven staking refresh assertions. Also exportsSTAKING_INTERFACEfor reuse in mocks and makes small type-safety cleanups inRpcDataSourcearound state/token-list access and native asset ID construction.Written by Cursor Bugbot for commit b2c402c. This will update automatically on new commits. Configure here.