fix: repair broken Jest test suite and run it in CI - #431
Conversation
|
There was a problem hiding this comment.
🟡 Changes recommended
The useAccessibilityInfo tests’ addEventListener mock helper returns undefined, which can break hook effect cleanup that expects a subscription with .remove().
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR repairs the repository’s broken Jest test setup (React Native 0.78 / React 18), modernizes the hook test suites to supported testing-library APIs, and ensures the checks run in CI to prevent future silent breakages.
Changes:
- Fix Jest/Babel/test tooling incompatibilities (RN 0.78 preset, remove dead setup code, align
react-test-rendererwith React 18). - Migrate hook tests from deprecated
@testing-library/react-hooksto@testing-library/react-native(renderHook,waitFor, asyncact). - Add a CI workflow that runs typecheck + lint + test on pushes and PRs.
File summaries
| File | Description |
|---|---|
| src/useRefresh.test.ts | Migrates to @testing-library/react-native and uses async act for timer-driven refresh completion. |
| src/useLayout.test.ts | Updates hook test renderer import to @testing-library/react-native. |
| src/useKeyboard.test.ts | Updates hook test renderer import to @testing-library/react-native. |
| src/useInteractionManager.test.ts | Updates hook test renderer import to @testing-library/react-native. |
| src/useImageDimensions.test.ts | Updates hook test renderer import to @testing-library/react-native. |
| src/useBackHandler.test.ts | Updates hook test renderer import to @testing-library/react-native. |
| src/useAppState.test.ts | Updates hook test renderer import to @testing-library/react-native. |
| src/useAccessibilityInfo.test.ts | Migrates async update waiting to waitFor with RTL native renderHook. |
| package.json | Removes deprecated test dependency, aligns react-test-renderer, adds missing eslint-plugin-react. |
| jest.setup.js | Removes dead RN renderer require, removes global.window shim, improves Keyboard mock to be event-driven. |
| bun.lock | Updates lockfile for dependency removals/additions and version alignment. |
| babel.config.js | Switches preset to @react-native/babel-preset compatible with RN 0.78+. |
| .github/workflows/ci.yml | Adds CI job to run typecheck, lint, and Jest on push/PR. |
Review details
- Files reviewed: 12/13 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| import { useAccessibilityInfo } from "./useAccessibilityInfo" | ||
| import { act, renderHook } from "@testing-library/react-hooks" | ||
| import { act, renderHook, waitFor } from "@testing-library/react-native" | ||
| import { AccessibilityChangeEventName, AccessibilityInfo } from "react-native" | ||
|
|
65798ee to
b93c00d
Compare
There was a problem hiding this comment.
🔵 Needs a closer look
A couple of updated tests mock React Native APIs without returning the cleanup handles (remove/cancel) that the hooks call, which can cause runtime errors during unmount/cleanup and should be corrected before approval.
Review details
Suppressed comments (2)
src/useAppState.test.ts:5
AppState.addEventListeneris mocked without returning the subscription object, butuseAppStatecallssubscription.remove()in its effect cleanup. This mock can cause runtime errors during unmount/cleanup and doesn’t reflect the real API; return{ remove: ... }from the mock and frommockImplementationOnce.
import { act, renderHook } from "@testing-library/react-native"
import { AppState, AppStateStatus } from "react-native"
import { useAppState } from "./useAppState"
jest.mock("react-native", () => ({
src/useInteractionManager.test.ts:6
InteractionManager.runAfterInteractionsis mocked without returning the cancellable handle, butuseInteractionManagercallsinteractionPromise.cancel()in its effect cleanup. This mock can cause runtime errors during unmount/cleanup and doesn’t reflect the real API; return an object withcancelboth in the default mock and the per-testmockImplementationOnce.
import { act, renderHook } from "@testing-library/react-native"
import { InteractionManager } from "react-native"
jest.mock("react-native", () => ({
InteractionManager: {
- Files reviewed: 12/13 changed files
- Comments generated: 0 new
- Review effort level: Lite
The test suite could not run at all on a fresh clone, and neither CI
workflow executed the tests, so the breakage went unnoticed.
Fixes:
- babel.config.js: use @react-native/babel-preset (RN 0.78) instead of
the removed metro-react-native-babel-preset, which was not installed
- jest.setup.js: drop the dead requireActual of the removed
ReactNativeRenderer-prod path, and the unused global.window shim that
clobbered setTimeout under @testing-library/react-native
- jest.setup.js: make the Keyboard mock a real emitter so addListener /
emit actually drive the hook under test
- migrate all test files off the deprecated @testing-library/react-hooks
to @testing-library/react-native's built-in renderHook / waitFor
- align react-test-renderer with react (18.3.1) to fix the renderer crash
- add the missing eslint-plugin-react dev dependency so lint runs
- return cleanup handles ({ remove } / { cancel }) from the AccessibilityInfo,
AppState and InteractionManager mocks so effect cleanup no longer throws
during unmount
- add a CI workflow that runs typecheck, lint and test on push and PR
All 42 tests across 8 suites now pass, along with typecheck and lint.
d63e3de to
9fcd64a
Compare
There was a problem hiding this comment.
🟢 Approval recommended
The updates are consistent with RN 0.78/React 18 tooling expectations and the CI addition directly enforces the intended test/lint/typecheck guarantees going forward.
Review details
- Files reviewed: 12/13 changed files
- Comments generated: 0 new
- Review effort level: Lite
There was a problem hiding this comment.
🟢 Approval recommended
The changes are consistent with the PR’s stated goals and appear complete, low-risk, and self-validating via CI-enabled typecheck/lint/test execution.
Review details
- Files reviewed: 12/13 changed files
- Comments generated: 0 new
- Review effort level: Lite
Problem
On a fresh clone,
bun run testfails before running a single test. Because neither CI workflow (release.yml,publish-canary.yml) runs the tests, this went unnoticed. There were several independent breakages:babel.config.jsreferencesmodule:metro-react-native-babel-preset, which isn't a dependency and was removed in RN 0.73+. RN 0.78 (used here) ships@react-native/babel-presetinstead.jest.setup.jscallsjest.requireActual("react-native/Libraries/ReactNative/oss/ReactNativeRenderer-prod")— that path no longer exists in RN 0.78, and its result was never even used.@testing-library/react-hooks, which can't auto-detect a renderer under React 18.react-test-rendererwas pinned to^19whilereactis18.3.1→TypeError: Cannot read properties of undefined (reading 'S').global.window = {}injest.setup.jsshadowed the global object and brokesetTimeoutunder@testing-library/react-native.Keyboardmock was a no-op (addListenerdidn't store handlers,emitdid nothing), so theuseKeyboardtests couldn't drive the hook.eslint-plugin-reactis imported byeslint.config.mjsbut wasn't a dependency, sobun run lintalso failed.Changes
babel.config.js: switch to@react-native/babel-preset.jest.setup.js: remove the deadrequireActualand theglobal.windowshim; make theKeyboardmock a real emitter.@testing-library/react-hooksto@testing-library/react-native(renderHook/waitFor).useRefresh.test.ts:await act(async …)so the promise microtask flushes under React 18.package.json: drop the unused@testing-library/react-hooks, alignreact-test-rendererto18.3.1, addeslint-plugin-react..github/workflows/ci.ymlrunning typecheck + lint + test on push and PR so this can't silently break again.Verification
No runtime/source (
src/*.tshook) behavior changes — this is test/build/CI tooling only, so no changeset is included.