test(perps): remove redundant MMDS BottomSheet mocks#33288
Conversation
Perps unit tests were overriding the global testSetup BottomSheet mock with local copies. Rely on the shared mock instead and assert via component-specific testIDs for bottom sheet containers and close actions. Co-authored-by: Cursor <cursoragent@cursor.com>
|
CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes. |
metamask-ci
Bot
added
the
team-design-system
PR template — items to address before "Ready for review"Warnings — informational, address before merging:
See docs/readme/ready-for-review.md for the full Definition of Ready for Review. |
🔍 Smart E2E Test Selection
click to see 🤖 AI reasoning detailsE2E Test Selection:
The changes are low-risk testability improvements to Perps bottom sheet components. SmokePerps is selected because these components are part of the Perps trading flow. SmokeWalletPlatform is selected per the SmokePerps tag description (Perps is a section inside the Trending tab). SmokeConfirmations is NOT selected because no transaction confirmation flows are modified - only bottom sheet testIDs are updated. Performance Test Selection: |
| // ======================================== | ||
|
|
||
| export const PerpsOrderTypeBottomSheetSelectorsIDs = { | ||
| CONTAINER: 'perps-order-type-bottom-sheet', |
There was a problem hiding this comment.
These selectors replace mock-only IDs like bottom-sheet-header and close-button. Tests now target stable per-component testIDs that work with the shared MMDS BottomSheet mock in app/util/test/testSetup.js rather than per-file jest overrides.
| BottomSheetHeader, | ||
| }; | ||
| }); | ||
| jest.mock('../../hooks/usePerpsEventTracking', () => ({ |
There was a problem hiding this comment.
The local @metamask/design-system-react-native BottomSheet mock was removed here. testSetup.js already provides a global BottomSheet stub that renders children synchronously and wires onOpenBottomSheet/onCloseBottomSheet, so duplicating it in each Perps test added maintenance cost without extra coverage.
| return ( | ||
| <BottomSheet | ||
| ref={sheetRef} | ||
| testID={PerpsOrderTypeBottomSheetSelectorsIDs.CONTAINER} |
There was a problem hiding this comment.
CONTAINER and CLOSE_BUTTON are wired into the real component so close/header interactions can be asserted without a custom BottomSheetHeader mock. closeButtonProps forwards the testID to MMDS ButtonIcon, which is the supported way to target the header close control in unit tests.
|
|
||
| expect(mockOnOpenBottomSheet).toHaveBeenCalledTimes(2); | ||
| expect(screen.getByTestId('bottom-sheet')).toBeOnTheScreen(); | ||
| expect( |
There was a problem hiding this comment.
Re-open behavior is now validated by user-visible state (BOTTOM_SHEET and EDIT_CHIP present) instead of counting onOpenBottomSheet calls on a local ref mock. That better matches what users see when the custom slippage sub-sheet dismisses and the preset sheet remounts.
| 'desc', | ||
| ); | ||
| expect(mockOnClose).toHaveBeenCalledTimes(1); | ||
| expect(mockOnClose).toHaveBeenCalled(); |
There was a problem hiding this comment.
With the global BottomSheet mock, onClose fires from both the BottomSheet onClose prop and the callback passed to onCloseBottomSheet inside handleOptionPress. The test now checks that onClose was invoked rather than an exact call count, which reflects the real close path under testSetup.js.
| }; | ||
| }); | ||
|
|
||
| jest.mock('../../../../../../locales/i18n', () => ({ |
There was a problem hiding this comment.
This file previously mocked ListItemSelect, Box, Text, and Icon in addition to BottomSheet. Removing the whole design-system override lets the real MMDS list items render under the global BottomSheet stub, which is closer to production markup and still passes all nine tests.
| onClose={handleClose} | ||
| closeButtonProps={{ testID: 'close-button' }} | ||
| closeButtonProps={{ | ||
| testID: PerpsCandlePeriodBottomSheetSelectorsIDs.CLOSE_BUTTON, |
There was a problem hiding this comment.
The generic close-button testID is replaced with a Perps-scoped selector so the close control stays discoverable when multiple bottom sheets render in the same test tree. This pairs with removing the custom BottomSheetHeader mock in the test file.
| <BottomSheet ref={bottomSheetRef} onClose={onClose}> | ||
| <BottomSheet | ||
| ref={bottomSheetRef} | ||
| onClose={onClose} |
There was a problem hiding this comment.
All 279 Perps unit test suites were run after this change. PerpsOrderBookView.test.tsx still keeps a local BottomSheet override for the unavailable-ref edge case and was intentionally left out of this pass.
| }; | ||
| }); | ||
|
|
||
| jest.mock('../../../../../../locales/i18n', () => ({ |
There was a problem hiding this comment.
PerpsModifyActionSheet and PerpsAdjustMarginActionSheet tests follow the same pattern: only themeUtils and i18n stay mocked. PerpsActionSheet already passes testID through to BottomSheet, so SHEET-level assertions continue to work via the global mock without a local BottomSheetHeader stub.
|
metamask-ci
Bot
added
the
release-8.4.0
3 participants
Description
Perps unit tests were duplicating the global MMDS
BottomSheetmock fromapp/util/test/testSetup.jswith localjest.mock('@metamask/design-system-react-native')overrides. Those local mocks were unnecessary and made assertions depend on mock-specific testIDs likebottom-sheet-headerandclose-button.This PR removes the redundant per-test BottomSheet mocks from six Perps bottom sheet component test files and relies on the shared global mock instead. Components now expose stable, per-feature testIDs for bottom sheet containers and close buttons, and tests assert against those selectors.
Changelog
CHANGELOG entry: null
Related issues
Fixes:
Manual testing steps
N/A — test-only changes. Verified with:
All 279 Perps test suites pass (6332 tests).
Screenshots/Recordings
N/A
Before
After
Pre-merge author checklist
Performance checks (if applicable)
Pre-merge reviewer checklist
Made with Cursor