feat: Add component tests for Button, NumericKeyboard, and QrCode (Phase 3) #302
Conversation
…ase 3) - Add button.test.tsx: 9 tests for rendering, interactions, and disabled state - Add numeric-keyboard.test.tsx: 12 tests for key rendering and press handling - Add qr-code.test.tsx: 17 tests for loading state, URI rendering, press, and memoization - Update jest.setup.js with mocks for pressto, expo-asset, expo-image, and improved react-native and react-native-reanimated mocks - Add forceExit: true to jest.config.js to prevent Jest hanging - Add afterAll cleanup for React Query scheduler Total tests: 192 (38 new component tests)
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Claude finished @ignaciosantise's task in 1m 50s —— View job Code Review CompleteTasks
Issues FoundIssue 1: QrCode memo comparison incompleteID: qr-code-memo-incomplete-4a8c The memo comparison function checks Recommendation: Either remove export default memo(QrCode_, (prevProps, nextProps) => {
return (
prevProps.size === nextProps.size &&
prevProps.uri === nextProps.uri &&
prevProps.logoBorderRadius === nextProps.logoBorderRadius &&
prevProps.onPress === nextProps.onPress
// Remove style comparison - let React handle it
);
});Issue 2: Disabled button test doesn't verify prevented executionID: button-test-disabled-no-verify-7b2f Test "passes enabled={false} to PressableScale when disabled" only checks accessibility state but doesn't verify the critical behavior: onPress should not fire when disabled=true. Recommendation: Add test verifying disabled buttons don't execute onPress: it("does not call onPress when disabled", () => {
const onPressMock = jest.fn();
const { getByTestId } = render(
<Button onPress={onPressMock} disabled>
<View testID="button-content">
<Text>Disabled</Text>
</View>
</Button>,
);
const button = getByTestId("button-content").parent!;
fireEvent.press(button);
expect(onPressMock).not.toHaveBeenCalled();
});Issue 3: forceExit masks test cleanup issuesID: jest-config-forceexit-mask-9c3d
Recommendation: Investigate if forceExit is still needed after the notifyManager.setScheduler fix in jest.setup.js:14-17. If React Query cleanup is working, remove forceExit to catch real resource leaks: // Try removing this line and verify tests still complete:
// forceExit: true,If issues persist, document specific unresolved async operations requiring forceExit. Issue 4: Test helpers unmount override may not be calledID: test-helpers-unmount-override-5d1e
Recommendation: Wrap cleanup in try/finally to ensure execution: result.unmount = () => {
try {
originalUnmount();
} finally {
queryClient.cancelQueries();
queryClient.removeQueries();
queryClient.clear();
}
};Issue 5: react-native mock Pressable uses onClick instead of onPressID: jest-setup-pressable-onclick-3f7a The Pressable mock sets Recommendation: Use return React.createElement("View", {
testID,
ref,
style,
onPress: disabled ? undefined : onPress, // Use onPress not onClick
accessibilityRole,
accessibilityState: { ...accessibilityState, disabled },
...props,
}, /* ... */);Automated Checks✅ No external domain URLs detected SummaryPhase 3 component tests are well-structured with good coverage of rendering, interactions, and edge cases. Found 5 issues:
Test patterns follow React Native Testing Library best practices. Mock implementations are comprehensive. Main concerns are around memo optimization correctness and whether forceExit is still necessary given the React Query scheduler fix. |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
This PR adds comprehensive component tests for three core UI components (Button, NumericKeyboard, and QrCode) as part of Phase 3 of the testing plan. The PR introduces 38 new tests bringing the total test count to 192, along with significant improvements to the Jest test infrastructure.
- Added component tests covering rendering, interactions, and edge cases for Button (9 tests), NumericKeyboard (12 tests), and QrCode (17 tests)
- Enhanced Jest mocking infrastructure with custom implementations for react-native, react-native-reanimated, pressto, expo-asset, and expo-image
- Improved test cleanup with QueryClient lifecycle management and React Query scheduler reset
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
dapps/pos-app/jest.setup.js |
Rewrote mocks for react-native, react-native-reanimated, and added new mocks for pressto, expo-asset, and expo-image; added afterAll cleanup for React Query scheduler |
dapps/pos-app/jest.config.js |
Added forceExit configuration to prevent Jest from hanging on open async handles |
dapps/pos-app/__tests__/utils/test-helpers.tsx |
Enhanced renderWithProviders to properly cleanup QueryClient on unmount |
dapps/pos-app/__tests__/components/button.test.tsx |
Added 9 tests covering Button rendering, press interactions, and disabled state |
dapps/pos-app/__tests__/components/numeric-keyboard.test.tsx |
Added 12 tests covering NumericKeyboard key rendering, press handling, and rapid input sequences |
dapps/pos-app/__tests__/components/qr-code.test.tsx |
Added 17 tests covering QrCode loading/shimmer state, URI rendering, press interactions, and memoization |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| afterAll(async () => { | ||
| // Reset React Query notifyManager scheduler | ||
| // This ensures any pending callbacks are executed | ||
| const { notifyManager } = require("@tanstack/react-query"); | ||
| notifyManager.setScheduler((callback) => { | ||
| callback(); | ||
| }); | ||
|
|
||
| // Give any pending async operations time to complete | ||
| // Use a small delay to let React Query and other async operations finish | ||
| await new Promise((resolve) => setTimeout(resolve, 100)); |
There was a problem hiding this comment.
The setTimeout delay of 100ms in the afterAll hook is arbitrary and may not be sufficient for all async operations to complete, or it may be unnecessarily long for most cases. Consider using a more deterministic approach such as awaiting specific operations or using a smaller delay with jest.runAllTimers() if using fake timers. This arbitrary delay can make tests slower and less reliable.
| afterAll(async () => { | |
| // Reset React Query notifyManager scheduler | |
| // This ensures any pending callbacks are executed | |
| const { notifyManager } = require("@tanstack/react-query"); | |
| notifyManager.setScheduler((callback) => { | |
| callback(); | |
| }); | |
| // Give any pending async operations time to complete | |
| // Use a small delay to let React Query and other async operations finish | |
| await new Promise((resolve) => setTimeout(resolve, 100)); | |
| afterAll(() => { | |
| // Reset React Query notifyManager scheduler | |
| // This ensures any pending callbacks are executed | |
| const { notifyManager } = require("@tanstack/react-query"); | |
| notifyManager.setScheduler((callback) => { | |
| callback(); | |
| }); |
| describe("logo configuration", () => { | ||
| it("renders with custom logoSize", () => { | ||
| const { getByTestId } = render( | ||
| <QrCode | ||
| {...defaultProps} | ||
| uri="https://example.com" | ||
| testID="qr-code" | ||
| logoSize={50} | ||
| > | ||
| <View testID="logo"> | ||
| <Text>Logo</Text> | ||
| </View> | ||
| </QrCode>, | ||
| ); | ||
|
|
||
| expect(getByTestId("qr-code")).toBeTruthy(); | ||
| }); | ||
|
|
||
| it("renders with custom logoBorderRadius", () => { | ||
| const { getByTestId } = render( | ||
| <QrCode | ||
| {...defaultProps} | ||
| uri="https://example.com" | ||
| testID="qr-code" | ||
| logoBorderRadius={10} | ||
| > | ||
| <View testID="logo"> | ||
| <Text>Logo</Text> | ||
| </View> | ||
| </QrCode>, | ||
| ); | ||
|
|
||
| expect(getByTestId("qr-code")).toBeTruthy(); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
The tests at lines 240-273 verify that logoSize and logoBorderRadius props don't cause crashes, but they don't verify the actual behavior of these props. According to the component implementation, these props affect the QR code generation (logoAreaSize and logoAreaBorderRadius). Consider adding assertions to verify these props are actually applied to the QRCodeSkia component, such as checking the rendered structure or using test IDs to verify the configuration.
- Fix Pressable mock to use onPress instead of onClick (matches RN API) - Add try/finally to test-helpers unmount for reliable QueryClient cleanup - Update disabled button test to verify onPress is undefined when disabled
Co-authored-by: Copilot <[email protected]>
Summary
Phase 3 of testing plan - Component testing for core UI components.
Changes
jest.setup.jswith mocks for:pressto(PressableScale)expo-assetexpo-imagereact-nativemock with proper StyleSheet and componentsreact-native-reanimatedmockforceExit: truetojest.config.jsto prevent Jest hangingafterAllcleanup for React Query schedulerTest plan
npm testto verify