[LWM] test(pay-card): assert view model hook return - #21432
Conversation
live-github-bot
Bot
changed the title
Web Tools Build Status
|
There was a problem hiding this comment.
🟢 Approval recommended
The change is test-only, uses the repo’s existing hook test wrapper patterns, and maintains/clarifies the same behavioral assertions without introducing production code risk.
Pull request overview
This PR refactors the PayTab MVVM hook unit test to exercise usePayTabViewModel via renderHook within an actual React Navigation stack screen, avoiding useRoute mocking while keeping the test file as plain .ts.
Changes:
- Replaced the
.tsxprobe-component test with a hook-focused test usingrenderHookand a real stack route wrapper (initialParams). - Kept coverage for default OAuth env config, env updates without remount, deep-link
code, and missing params behavior. - Added a changeset entry for the mobile package describing the test change.
File summaries
| File | Description |
|---|---|
| apps/ledger-live-mobile/src/mvvm/features/PayTab/screens/PayTab/usePayTabViewModel.test.tsx | Removed the prior component-probe based test implementation. |
| apps/ledger-live-mobile/src/mvvm/features/PayTab/screens/PayTab/usePayTabViewModel.test.ts | New hook-based test that mounts usePayTabViewModel under a real navigation route via innerWrapper. |
| .changeset/calm-hooks-return.md | Declares a patch changeset for the mobile package tied to this test refactor. |
Review details
- Files reviewed: 3/3 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Rsdoctor Bundle Diff AnalysisFound 7 projects in monorepo, 2 projects with changes. 📊 Quick Summary
📋 Detailed Reports (Click to expand)📁 desktop-webviewDappPreloaderPath:
📦 Download Diff Report: desktop-webviewDappPreloader Bundle Diff 📁 desktop-webviewPreloaderPath:
📦 Download Diff Report: desktop-webviewPreloader Bundle Diff Generated by Rsdoctor GitHub Action |
There was a problem hiding this comment.
🟡 Changes recommended
The implemented test still mocks useRoute, which conflicts with the PR title/description and the changeset text, so the PR messaging and/or test approach needs to be reconciled.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 1
- Review effort level: Lite
tonykhaov
changed the title
There was a problem hiding this comment.
🔵 Needs a closer look
The new test still mocks useRoute, which contradicts the PR’s stated goal of exercising the hook through a real navigation route and leaves that integration untested.
Review details
Suppressed comments (1)
apps/ledger-live-mobile/src/mvvm/features/PayTab/screens/PayTab/usePayTabViewModel.test.ts:17
- This test still mocks
useRoute(and therefore does not exerciseusePayTabViewModelinside a real React Navigation stack / real route params), which contradicts the PR description and leaves the route-param parsing behavior untested.
Consider removing the jest.mock("@react-navigation/native") override and instead wrapping renderHook in a minimal NavigationContainer + stack screen with initialParams (you can keep the file as .ts via createElement, as described).
const mockRoute: {
params?: PayTabNavigatorParamList[typeof ScreenName.PayTab];
} = {
params: undefined,
};
jest.mock("@react-navigation/native", () => ({
...jest.requireActual("@react-navigation/native"),
useRoute: () => mockRoute,
}));
- Files reviewed: 2/2 changed files
- Comments generated: 0 new
- Review effort level: Lite
|
📝 Description
usePayTabViewModelbuilds the Card OAuth client config from env and the login callback from the PayTab route. That return was untested.This PR adds a unit test that calls the hook with
renderHook, mocksuseRoutefor redirect params, and uses realsetEnv/getEnvso a debug-settings env change is picked up without remounting. The file is.ts.🔗 Context