chore: run every test suite with bun test - #60
Conversation
Jest was configured with `roots: ['<rootDir>/plugin/src']`, so CI only ever executed the two Expo config-plugin suites. Everything under `package/src/**/__tests__/` was silently skipped: those suites import from `bun:test` and use `mock.module()`, which Jest has no equivalent for, so the migration direction is forced toward Bun. Point `test` and `test:ci` at `bun test`, drop jest/ts-jest/@types/jest and the now-unused jest.config.js, and add @types/bun so `bun:test` resolves under tsc. Also stop hiding the test sources from the typechecker: `tsconfig.json` no longer excludes `src/**/__tests__/**`, which surfaced a real pre-existing type error in normalizeMarkerDescriptors.test.ts (it typed its fixture with the serialized descriptor rather than the public one). The exclude moves to `tsconfig.build.json` — which builder-bob now actually uses via the `project` option — so test declarations stay out of lib/. Finally, wire `typecheck:provider-types` into CI and release; those `@ts-expect-error` type tests never ran either.
|
React Doctor found 8 issues in 5 files · 2 errors & 6 warnings · score 64 / 100 (Needs work) · full project Errors
6 warnings
Reviewed by React Doctor for commit |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (7)
💤 Files with no reviewable changes (1)
Included review availability: 4 reviews are currently available. Your included PR review attempts over the past 7 days set your current allowance at 5 reviews per hour. 📝 WalkthroughSummary by CodeRabbit
WalkthroughThe package replaces Jest with Bun for testing, updates TypeScript build inclusion, adjusts the Bob build target, and adds provider typechecking to CI and release workflows. ChangesBun and provider validation
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to The PR switches package testing and CI coverage to Bun, adds test typechecking, and keeps test declarations out of published output; no actionable merge-blocking risk remains after normal checks and review. Suggested reviewers: 🚥 Pre-merge checks | ✅ 6✅ Passed checks (6 passed)
Full details: Docstring CoverageExplanation No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 1 files. (5 skipped: 5 unsupported.) Full details: Security CheckExplanation No high-confidence security vulnerability was introduced. The diff only changes test tooling, TypeScript build configuration, and adds static provider typechecks to CI and release workflows. The new workflow commands use no attacker-controlled interpolation and do not access secrets. Warning Billing warning: we have not been able to collect payment for this subscription for more than 72 hours. Please update the payment method or pay any pending invoices in Billing to avoid service interruption. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
1 participant
Problem
package/jest.config.jspinned the runner to the config plugin:So CI executed 2 of the 5 test files in the package. Everything under
package/src/**/__tests__/— the LRU cache, marker image resolution, descriptor normalisation — was silently skipped on every push, every PR and every release.Pointing Jest at
srcis not an option: those suitesimport { … } from 'bun:test'and rely onmock.module(), which Jest has no equivalent for. The migration direction is forced toward Bun, and Bun runs the plugin suites unmodified — they only use thedescribe/expect/testglobals, with nojest.*calls anywhere.Two smaller gaps in the same area:
package/tsconfig.jsonexcludedsrc/**/__tests__/**, so the test sources were never typechecked either. That exclude was hiding a real error (below).typecheck:provider-types— the@ts-expect-errorprovider type tests inpackage/type-tests/— had a script but no CI step, so it never ran either.Fix
testandtest:ciboth becomebun test;jest,ts-jestand@types/jestare dropped andjest.config.jsdeleted.@types/bunis added sobun:testresolves undertsconce the tests are in the compilation.Removing the exclude surfaced exactly one pre-existing error,
TS2345innormalizeMarkerDescriptors.test.ts. The fixture was typed with the serialized descriptor fromnative/specs/overlaysand then passed to a function that takes the public one fromtypes/overlays; the two disagree onenteringAnimation(a{ kind }descriptor object there, afalse | 'system' | Configunion here). Fixed on the test side by importing the public type and usingsatisfies, which keeps the literal checked without re-widening it. No production type was touched.One consequence worth calling out, because it is not in the obvious blast radius: builder-bob resolves its own tsconfig, defaulting to
tsconfig.json. With the exclude gone it happily emittedlib/typescript/overlays/__tests__/*.d.ts— test declarations, importingbun:test, into the published build.tsconfig.build.jsonalready existed for exactly this purpose but nothing in the repo referenced it; it was dead config. It is now wired up via the target'sprojectoption and owns the exclude:Tests are therefore typechecked but never emitted. The resulting
lib/typescriptis file-for-file identical tomain's (29.d.ts, verified by diffing against atscrun usingmain's config).Finally,
Typecheck provider typesis added to bothci.ymlandrelease.yml, and the lockfile is regenerated — CI runsbun install --frozen-lockfile, so a stale one breaks the build.Verification
.d.tsemitted intolib/All green on this branch:
cd package && bun testbun run --filter react-native-better-maps test(the CI step, unchanged)bun run lintbun run typecheckbun run typecheck:provider-typesbun run buildmainbun install --frozen-lockfile.release-it.jsonbefore:init→bun run test:cigrep -rn jest package/ --include='*.json' --include='*.js' --include='*.ts'Remaining
jeststrings inbun.lockare transitive only (jest-worker,jest-validate,pretty-formatvia metro / react-native /@expo/metro-file-map).Merge order matters — #58
I test-merged #58 into this branch. It conflicts in
normalizeMarkerDescriptors.test.ts(that PR reworks the same file), and the resolution is straightforward — #58 deletes the round-trip assertion my cast was for, so the cast disappears and only the import/satisfiesfix carries over.The part that needs a decision: #58 keeps
const baseDescriptor: MarkerDescriptorbound to the serialized type, and its newdescriptorEquality.test.tswas written while the exclude was still hiding those files fromtsc. On the merged tree,bun run typecheckreports 10 errors — 5 ×TS2322: Type 'string' is not assignable to type 'OverlayEnteringAnimationKind'indescriptorEquality.test.ts, and 5 more in the reworkednormalizeMarkerDescriptors.test.ts. None are caused by this PR; this PR is what makes CI see them. Whichever of the two lands second has to clear them.The tests themselves are fine either way — the merged tree runs 129 pass / 0 fail across 7 files.
Not included
prettier --checkin CI. 18 files fail onmaintoday, includingnormalizeMarkerDescriptors.test.tsbefore I touched it — the lines Prettier still objects to there are ones this PR does not modify (the mock signature and the dynamic imports). It needs its own--writecommit.pod install/xcodebuild/gradlew).Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.