feat: add comprehensive test suite and CI workflow (Issue #49)#70
Conversation
|
@godamongstmen897 Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
There was a problem hiding this comment.
Pull request overview
Adds a Jest-based test suite and CI workflow to address Issue #49, while adjusting notification handling and related imports for consistency.
Changes:
- Added new unit tests under
tests/for utils, hooks, services, store, and a component. - Added a GitHub Actions workflow to run
npm teston pushes/PRs. - Updated notification handling/services (import ordering, notification foreground display options, and type assertions).
Reviewed changes
Copilot reviewed 9 out of 10 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
tests/utils/colors.test.ts |
Adds coverage for getColors/getColor theme palettes. |
tests/store/notificationStore.test.ts |
Adds coverage for Zustand notification store actions and preference logic. |
tests/services/mobileAnalytics.test.ts |
Adds coverage for analytics logging via the logger mock. |
tests/hooks/useSafeArea.test.ts |
Adds coverage for useSafeArea behavior (currently not executed in a hook render context). |
tests/components/PrimaryButton.test.tsx |
Adds coverage for PrimaryButton (currently relies on brittle element stringification and ambiguous mock ordering). |
src/utils/notificationHandlers.ts |
Adjusts import ordering and changes notification data assertions. |
src/services/pushNotifications.ts |
Reorders imports and enables additional foreground notification UI options. |
src/__tests__/services/pushNotifications.test.ts |
Reorders imports and updates a trigger type cast for scheduling tests. |
jest.config.js |
Expands Jest roots to include tests/ and changes ts-jest tsconfig configuration. |
.github/workflows/test.yml |
Adds CI workflow to install dependencies and run the Jest suite. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| response: Notifications.NotificationResponse | ||
| ): void { | ||
| const data = response.notification.request.content.data as NotificationData | undefined; | ||
| const data = response.notification.request.content.data as unknown as NotificationData | undefined; |
There was a problem hiding this comment.
Using a double type assertion (as unknown as NotificationData | undefined) bypasses TypeScript’s structural checks and reduces actual type safety (it will accept any payload shape). Consider keeping the minimal assertion and/or adding a small runtime type-guard/parser for NotificationData before switching on data.type so unexpected notification payloads can be rejected safely.
| ): void { | ||
| const { title, body, data } = notification.request.content; | ||
| const notificationData = data as NotificationData | undefined; | ||
| const notificationData = data as unknown as NotificationData | undefined; |
There was a problem hiding this comment.
Same double assertion issue here: data as unknown as NotificationData removes compile-time guarantees and can mask unexpected payload shapes. Prefer validating/narrowing the data object (e.g., checking it’s a plain object and that type is one of NotificationType) before using it to decide preference checks and storage.
| import React from 'react'; | ||
| import PrimaryButton from '../../src/components/common/PrimaryButton'; | ||
|
|
||
| jest.mock('react-native', () => ({ | ||
| TouchableOpacity: 'TouchableOpacity', | ||
| Text: 'Text', | ||
| ActivityIndicator: 'ActivityIndicator', | ||
| View: 'View', | ||
| StyleSheet: { | ||
| create: (styles: unknown) => styles, | ||
| }, | ||
| })); |
There was a problem hiding this comment.
The react-native mock is declared after importing PrimaryButton. If Jest/ts-jest hoisting behavior changes (or if this file is ever treated as ESM), PrimaryButton may load before the mock is applied, and it currently relies on mocked StyleSheet/ActivityIndicator. Move the jest.mock('react-native', ...) call above the PrimaryButton import (or rely on a shared mock in jest.setup.js) to make the test ordering unambiguous.
| it('renders title in solid variant', () => { | ||
| const element = PrimaryButton({ | ||
| title: 'Continue', | ||
| onPress: jest.fn(), | ||
| variant: 'solid', | ||
| }); | ||
|
|
||
| expect(element).toBeTruthy(); | ||
| expect(JSON.stringify(element)).toContain('Continue'); | ||
| }); | ||
|
|
||
| it('renders loading state for gradient variant', () => { | ||
| const element = PrimaryButton({ | ||
| title: 'Submit', | ||
| onPress: jest.fn(), | ||
| loading: true, | ||
| }); | ||
|
|
||
| expect(element).toBeTruthy(); | ||
| expect(JSON.stringify(element)).not.toContain('Submit'); | ||
| }); |
There was a problem hiding this comment.
These assertions rely on JSON.stringifying a raw React element returned by calling the component as a function. This is brittle and doesn’t validate what actually renders (and will break if the component starts using hooks). Prefer rendering via @testing-library/react-native (or react-test-renderer) and asserting on the rendered output (e.g., text present/absent, ActivityIndicator shown when loading).
| import { useSafeArea } from '../../src/hooks/useSafeArea'; | ||
|
|
||
| describe('useSafeArea', () => { | ||
| it('maps inset values and helper paddings', () => { | ||
| const result = useSafeArea(); | ||
|
|
||
| expect(result.top).toBe(24); | ||
| expect(result.bottom).toBe(16); | ||
| expect(result.left).toBe(8); | ||
| expect(result.right).toBe(8); | ||
| expect(result.safePaddingTop(10)).toEqual({ paddingTop: 34 }); | ||
| expect(result.safePaddingBottom(4)).toEqual({ paddingBottom: 20 }); |
There was a problem hiding this comment.
useSafeArea is a React hook (it calls useSafeAreaInsets), but the test calls it directly rather than within a rendered hook/component. This bypasses the Rules of Hooks and can give false confidence (and can start failing if the hook later uses other React hooks). Prefer using renderHook(() => useSafeArea()) from @testing-library/react-native (keeping useSafeAreaInsets mocked as needed).
| import { useSafeArea } from '../../src/hooks/useSafeArea'; | |
| describe('useSafeArea', () => { | |
| it('maps inset values and helper paddings', () => { | |
| const result = useSafeArea(); | |
| expect(result.top).toBe(24); | |
| expect(result.bottom).toBe(16); | |
| expect(result.left).toBe(8); | |
| expect(result.right).toBe(8); | |
| expect(result.safePaddingTop(10)).toEqual({ paddingTop: 34 }); | |
| expect(result.safePaddingBottom(4)).toEqual({ paddingBottom: 20 }); | |
| import { renderHook } from '@testing-library/react-native'; | |
| import { useSafeArea } from '../../src/hooks/useSafeArea'; | |
| describe('useSafeArea', () => { | |
| it('maps inset values and helper paddings', () => { | |
| const { result } = renderHook(() => useSafeArea()); | |
| expect(result.current.top).toBe(24); | |
| expect(result.current.bottom).toBe(16); | |
| expect(result.current.left).toBe(8); | |
| expect(result.current.right).toBe(8); | |
| expect(result.current.safePaddingTop(10)).toEqual({ paddingTop: 34 }); | |
| expect(result.current.safePaddingBottom(4)).toEqual({ paddingBottom: 20 }); |
This pull request introduces a comprehensive set of improvements focused on notification handling, code consistency, and test coverage. The main highlights are the addition of a GitHub Actions workflow for automated testing, enhancements to notification type safety, and the introduction of several new unit tests for key components and utilities.
CI/CD and Testing Automation:
.github/workflows/test.yml) to automatically run the test suite on pushes and pull requests tomainand a feature branch, ensuring continuous integration for the project.Notification Handling and Type Safety:
NotificationDatausingas unknown as NotificationDatainnotificationHandlers.ts, reducing the risk of runtime type errors. [1] [2]pushNotifications.tsto enable new notification display options (shouldShowBanner,shouldShowList), improving the user experience for in-app notifications.Code Consistency and Imports:
pushNotifications.ts,pushNotifications.test.ts, andnotificationHandlers.ts, improving code readability and maintainability. [1] [2] [3]Test Coverage Improvements:
PrimaryButtoncomponent, ensuring correct rendering and loading state handling (tests/components/PrimaryButton.test.tsx).useSafeAreahook, verifying correct mapping of safe area insets and helper paddings (tests/hooks/useSafeArea.test.ts).mobileAnalyticsService, confirming correct event and screen tracking with logger integration (tests/services/mobileAnalytics.test.ts).notificationStore, validating state management and preference logic for notifications (tests/store/notificationStore.test.ts).tests/utils/colors.test.ts).Test File Maintenance:
pushNotifications.test.tsto ensure compatibility with recent notification type changes. [1] [2]These changes collectively improve the reliability, maintainability, and testability of the codebase, especially around notifications and UI components.
Closes #49