-
Notifications
You must be signed in to change notification settings - Fork 26
feat: Automated test suite — unit, component #863
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
tuliomir
wants to merge
20
commits into
master
Choose a base branch
from
feat/automated-test-suite-layers-1-3
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from 14 commits
Commits
Show all changes
20 commits
Select commit
Hold shift + click to select a range
ecd05ec
feat: add automated test suite PoC — 57 tests across 3 layers
tuliomir 8f9eeb2
fix: make entire test suite pass — fix pre-existing test failures
tuliomir 67c4b44
chore: allow-scripts entry for jest-circus's unrs-resolver
tuliomir 4f4e9d3
fix: align reown.test.js with rejectAll saga's actual yield order
tuliomir bdbf958
test: add slice-refactor safety net + testing guide
tuliomir 49a4e57
chore: address review nits + pin jest-circus
tuliomir ec3b546
docs: annotate review findings that were intentionally not fixed
tuliomir 940f684
fix(ci): regenerate package-lock.json with Node 22
tuliomir 0864921
fix(lint): import jest globals in reducer test files
tuliomir d19358e
chore: address second review pass — lint hygiene + duplicate test
tuliomir 26638c6
test: address third-pass review — strengthen contracts, dedupe helper
tuliomir ee6848a
docs: add AGENTS.md, CLAUDE.md, and writing-tests skill
tuliomir 7a20c5f
docs: refine test docs per QA audit — gotchas first, less filler
tuliomir d5ff9f0
fix(test): address CodeRabbit lint findings on PR #863
tuliomir 86e8352
test: restore nanoContract coverage lost in 8f9eeb2
tuliomir 037b6e8
fix(test): import jest globals in utils + InitWallet tests
tuliomir 1c6dc07
fix(test): address CodeRabbit lint findings on screen tests
tuliomir adeff1a
test: tighten screen test contracts per CodeRabbit nitpicks
tuliomir 1e114e7
fix: address Copilot review on PR #863
tuliomir a5ae036
docs: break circular AGENTS.md ↔ CLAUDE.md pointer
tuliomir File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,212 @@ | ||
| --- | ||
| name: writing-tests-hathor-wallet-mobile | ||
| description: Use when adding or modifying any test file (paths under __tests__/, or files matching *.test.{ts,tsx,js}). Encodes repo-specific test conventions surfaced through review — jest globals, RTK-migration safety net, helper reuse, mock annotations, and the gotchas an agent won't infer from grepping nearby tests. | ||
| --- | ||
|
|
||
| # Writing tests in hathor-wallet-mobile | ||
|
|
||
| > **If a rule here conflicts with `docs/testing-guide.md`, this file | ||
| > wins** — it's the authoritative source for test conventions. | ||
| > `docs/testing-guide.md` is the long-form "why" reference and the | ||
| > per-PR policy table. | ||
|
|
||
| A frontier model can read the existing tests and infer most idioms. | ||
| This skill covers what it *can't* infer: historical decisions, hidden | ||
| foot-guns, and patterns that look fine but silently misbehave. | ||
|
|
||
| ## Repo-specific gotchas (read these first) | ||
|
|
||
| These are concrete invariants you'd otherwise discover by breaking CI. | ||
|
|
||
| 1. **Don't bump `jest-circus` past 29.7.0.** It's pinned to match | ||
| `jest@29.7.0` (`package.json:120-121`). `jest-circus@30+` requires | ||
| `jest@30+` (breaking config changes); a routine dep bump silently | ||
| breaks `jest.config` parsing. Land both upgrades together in a | ||
| separate PR. See commit `49a4e57`. | ||
|
|
||
| 2. **`@hathor/wallet-lib` sub-paths are already mocked.** | ||
| `jestMockSetup.js:155-162` mocks | ||
| `@hathor/wallet-lib/lib/nano_contracts/utils` and | ||
| `@hathor/wallet-lib/lib/api/axiosWrapper` to dodge the | ||
| *"more than one instance of bitcore-lib found"* error. If your test | ||
| imports another wallet-lib sub-path and behaves oddly, check this | ||
| file before adding a new `jest.mock`. | ||
|
|
||
| 3. **`legacy_createStore` is intentional, not legacy debt.** | ||
| `__tests__/helpers/mockStore.ts:7` uses | ||
| `legacy_createStore as createStore` because RTK | ||
| (`@reduxjs/toolkit`) is not in deps yet — that migration is the | ||
| *reason* this test suite exists. Don't add RTK to satisfy a lint | ||
| suggestion. | ||
|
|
||
| 4. **Do NOT pattern-match from `__tests__/sagas/networkSettings.test.ts`.** | ||
| It has three `describe.skip(...)` blocks (lines 17, 246, 303) and | ||
| uses bare `describe`/`it`/`jest` without importing from | ||
| `@jest/globals`. It compiles only because `no-undef` warnings (not | ||
| errors) are tolerated by `.eslintrc:52`. New tests must import jest | ||
| globals (see below) and skipped describe blocks are tech debt, not | ||
| convention. Use `reducer.{wallet,reown}.test.ts` or | ||
| `__tests__/sagas/wallet.test.ts` as canonical references. | ||
|
|
||
| 5. **`testPathIgnorePatterns` carve-outs** (`package.json:161-165`): | ||
| `__tests__/helpers/` and `__tests__/sagas/nanoContracts/fixtures.js` | ||
| are skipped. New helpers go under `__tests__/helpers/` (auto-skipped). | ||
| Fixture files **must not** end in `.test.{ts,tsx,js}` — jest will | ||
| collect them as suites and fail with *"no tests in file"*. | ||
|
|
||
| 6. **For sagas using `delay(ms)`, use redux-saga-test-plan's | ||
| `.run({ silenceTimeout: true })`.** Example: | ||
| `__tests__/sagas/wallet.test.ts:181`. The repo has no | ||
| `jest.useFakeTimers()` discipline; mocking `delay` directly is the | ||
| wrong instinct. | ||
|
|
||
| 7. **`provide({ call(effect, next) { ... } })` matchers that compare | ||
| `effect.fn?.name === 'bound start'` are fragile.** That's matching | ||
| against `Function.prototype.bind`'s naming convention. If the | ||
| production code refactors `wallet.start.bind(wallet)` into anything | ||
| else, the matcher silently falls through to `next()` and the saga | ||
| runs the real implementation. Prefer reference equality | ||
| (`effect.fn === mockWalletInstance.start`) when possible. See | ||
| `wallet.test.ts:147-164` — that pattern is a known smell, not the | ||
| ideal. | ||
|
|
||
| ## 1. Always import jest globals | ||
|
|
||
| `.eslintrc` has no `env.jest` and no test-file override. Without | ||
| explicit imports, `describe`/`it`/`expect`/`jest`/`beforeEach` trip | ||
| `no-undef` and CI's `npm run lint` fails. | ||
|
|
||
| ```ts | ||
| import { describe, it, expect } from '@jest/globals'; | ||
| // add jest, beforeEach, etc. as needed | ||
| ``` | ||
|
|
||
| ## 2. Test the public contract, not implementation | ||
|
|
||
| Dispatch real action creators against the **root** `reducer`. Do not | ||
| import internal `onXxx` handlers — they'll be renamed or inlined when | ||
| the reducer migrates to RTK slices. | ||
|
|
||
| ```ts | ||
| // ✅ Good — survives any internal reorg | ||
| import { reducer } from '../../src/reducers/reducer'; | ||
| import { resetWalletSuccess } from '../../src/actions'; | ||
| const next = reducer(state, resetWalletSuccess()); | ||
|
|
||
| // ❌ Bad — breaks when onResetWalletSuccess is inlined into a slice | ||
| import { onResetWalletSuccess } from '../../src/reducers/reducer'; | ||
| ``` | ||
|
|
||
| ## 3. Reducer tests pin three contracts | ||
|
|
||
| So a future RTK-slices refactor surfaces drift in explicit, reviewable | ||
| diffs: | ||
|
|
||
| - **Behavior**: action in → state out. | ||
| - **State-shape contract**: sorted-keys equality on each level of | ||
| `getInitialState()` for the sub-tree you touch. | ||
| `expect(Object.keys(state.x).sort()).toEqual([...])`. Use | ||
| sorted-keys equality, **not** `toHaveProperty` — the latter passes | ||
| when keys are *moved* under a sub-tree (`state.foo` → | ||
| `state.foo.foo`), exactly the failure you want to catch. Pin keying | ||
| conventions too (e.g. `state.tokens` is `{ [uid]: token }` — assert | ||
| one sample uid is at the top level). | ||
| - **Action-type contract**: literal `.type` strings for every in-scope | ||
| action creator: `expect(myAction(x).type).toBe('MY_ACTION')`. | ||
|
|
||
| > **Use minimal *valid* payloads in action-type assertions.** Not `{}` | ||
| > / `null` shortcuts. A future RTK `prepare` callback that validates | ||
| > inputs would throw before reaching `.type`, masking the test behind | ||
| > a creator-level error. `setTokens({}).type` is brittle; | ||
| > `setTokens({ [DEFAULT_TOKEN.uid]: DEFAULT_TOKEN }).type` survives. | ||
|
|
||
| Canonical examples: `__tests__/reducers/reducer.{wallet,reown}.test.ts`. | ||
|
|
||
| ## 4. Helper reuse — the two non-obvious rules | ||
|
|
||
| Helpers live in `__tests__/helpers/`. Reuse them; don't redefine. | ||
|
|
||
| 1. **`mockNavigation.ts` returns plain objects, never calls | ||
| `jest.mock`.** Babel only hoists `jest.mock(...)` written at module | ||
| scope of the test file. Wrapping it inside an exported helper makes | ||
| it a runtime no-op — the mock fires *after* modules under test are | ||
| already imported. | ||
| 2. **New helpers must be `.js`, not `.ts`.** The repo has no | ||
| TS-aware import resolver, so `.ts` helpers fail | ||
| `import/no-unresolved` from any consumer. (Workaround until | ||
| `eslint-import-resolver-typescript` is added — tracked as a | ||
| follow-up; meanwhile this rule is load-bearing.) | ||
|
|
||
| ## 5. Saga tests | ||
|
|
||
| Drive sagas end-to-end with `redux-saga-test-plan`. Mock only at the | ||
| I/O boundary (`@hathor/wallet-lib`, fetch, async storage). The reducer | ||
| in a saga test must be the real one — mocking it asserts your mocks, | ||
| not your code. | ||
|
|
||
| **`no-unused-vars` is unreliable for saga imports.** Sagas pass | ||
| function references through `.provide([...])` matchers | ||
| (`if (effect.fn === isWalletServiceEnabled) ...`). Lint sees these as | ||
| "used". But the *opposite* also happens: lint may miss an import that | ||
| *is* used as a function-identity reference. Before deleting an | ||
| "unused" saga import, grep for it across the file. | ||
|
|
||
| ## 6. Component / screen tests | ||
|
|
||
| Mount with `renderWithProviders`. **No `toMatchSnapshot()` blobs** — | ||
| explicit `expect(...).toBe(...)` per invariant is reviewable; a | ||
| 200-line snapshot blob is rubber-stamped. | ||
|
|
||
| ## 7. Mocks that don't match production imports → annotate inline | ||
|
|
||
| `jestMockSetup.js` pattern: if a `jest.mock(...)` factory exposes the | ||
| wrong shape (named-only when production imports default + named, etc.) | ||
| and **no current test exercises that code path**, add a comment block | ||
| listing the named exports a future test would need. Don't preemptively | ||
| restructure — that's scope creep, and reviewers re-raise the mismatch | ||
| when they see it without the context. | ||
|
|
||
| Examples already in the file: `@react-native-firebase/messaging`, | ||
| `@hathor/unleash-client`. Both are documented latent debt with | ||
| remediation recipes attached. | ||
|
|
||
| ## 8. Anti-patterns flagged in past reviews | ||
|
|
||
| - **Silent `return` to skip a test** when fixtures might collide. | ||
| Replace with an explicit guard (`expect(x).not.toEqual(y)`) so a | ||
| failing fixture is never silently masked. | ||
| - **`toStrictEqual` with `Error` instances** is fine — Jest ≥ 24 | ||
| structurally compares `name` + `message`. Don't capture-and-reuse | ||
| the original `Error` reference unless you need referential identity. | ||
|
|
||
| ## 9. Lockfile regen for test deps must use Node 22 | ||
|
|
||
| If your test work adds or upgrades a dev dep, regenerate | ||
| `package-lock.json` on the same Node version CI uses | ||
| (`.github/workflows/main.yml` matrix → 22.x): | ||
|
|
||
| ```sh | ||
| nvm install 22 && nvm use 22 | ||
| npm install --no-audit --no-fund | ||
| ``` | ||
|
|
||
| Different Node majors resolve transitive deps differently. A lock | ||
| generated on Node 24 / npm 11 fails CI's `npm ci` with errors like | ||
| *"Missing: typescript@6.0.3 from lock file"*. | ||
|
|
||
| ## 10. Verify before claiming done — lead with the mutation drill | ||
|
|
||
| > **Mutation drill (cheap, catches "tested but doesn't assert" | ||
| > bugs):** before claiming a contract block is done, break the | ||
| > production code (flip a return value, rename a field) and confirm | ||
| > your test fails red. Then `git restore`. If the test stayed green, | ||
| > your assertion is degenerate. | ||
|
|
||
| Then: | ||
|
|
||
| ```sh | ||
| nvm use 22 | ||
| npx jest --no-coverage # full suite green | ||
| npx eslint <files-touched> # 0 errors | ||
| npm ci --dry-run # lock and package.json in sync | ||
| ``` | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,60 @@ | ||
| # Agent guide for hathor-wallet-mobile | ||
|
|
||
| This file is the entry point for AI coding agents (Claude Code, Cursor, | ||
| Codex, etc.) working in this repo. Read it first; follow the pointers | ||
| for depth. | ||
|
|
||
| ## Repo conventions | ||
|
|
||
| ### 1. Pin every dependency version exactly | ||
|
|
||
| `package.json` MUST use exact versions — no `^`, no `~`. This is a | ||
| supply-chain mitigation and is **mandatory**. When a CodeRabbit-style | ||
| review flags a version mismatch, fix both sides to the same exact | ||
| version. After editing `package.json`, regenerate `package-lock.json` | ||
| following rule (3) below. | ||
|
|
||
| ### 2. Use Node 22 / npm 10 for any lockfile regeneration | ||
|
|
||
| CI runs Node 22.x (`.github/workflows/main.yml` matrix). Running | ||
| `npm install` with a different major (e.g. Node 24 / npm 11) produces | ||
| a divergent lockfile that fails CI's `npm ci`. | ||
|
|
||
| ```sh | ||
| nvm install 22 && nvm use 22 | ||
| npm install --no-audit --no-fund | ||
| ``` | ||
|
|
||
| ### 3. Tests follow `docs/testing-guide.md` | ||
|
|
||
| Read it before writing tests. The short version: | ||
|
|
||
| - **Always import jest globals** in `.test.ts` / `.test.tsx`: | ||
| `import { describe, it, expect } from '@jest/globals';` | ||
| (the repo's ESLint has no `env.jest`, so `no-undef` will fire). | ||
| - **Test the public contract**: dispatch real action creators against | ||
| the *root* `reducer`; never import internal `onXxx` handlers. | ||
| - **Reducer tests pin three contracts**: behavior (action → state), | ||
| initial-state shape (sorted-keys equality), action-type strings. | ||
| Canonical examples: `__tests__/reducers/reducer.{wallet,reown}.test.ts`. | ||
| - **Reuse `__tests__/helpers/`** — don't redefine setup boilerplate. | ||
| Note: helpers use `.js`, not `.ts`; the repo has no TS-aware import | ||
| resolver, so `.ts` helpers fail `import/no-unresolved`. | ||
| - For Claude Code: a more detailed skill auto-loads on test work — see | ||
| `.claude/skills/writing-tests/SKILL.md`. Other agents should read | ||
| `docs/testing-guide.md` directly. | ||
|
|
||
| ### 4. Commits | ||
|
|
||
| - Conventional Commits (`feat:`, `fix:`, `chore:`, `test:`, `docs:`). | ||
| - 50-char title cap, 72-col body wrap. | ||
| - Sign commits with GPG before merge (`git commit -S` or sign-and-amend | ||
| before push). CI may require all commits signed. | ||
|
|
||
| ### 5. PRs | ||
|
|
||
| - Assign to `tuliomir`. Tag `tests` for test PRs, `bug` for fixes, | ||
| `dependencies` for dep changes. | ||
| - Add to project 15 with status "In Progress WIP". | ||
| - Description should be concise; explicit breaking-change notes when | ||
| applicable. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,8 @@ | ||
| # Claude Code instructions | ||
|
|
||
| This file is intentionally a thin pointer. The canonical agent guide for | ||
| this repo is [`AGENTS.md`](./AGENTS.md) — read it first. | ||
|
|
||
| For test work, the more detailed Claude Code skill at | ||
| `.claude/skills/writing-tests/SKILL.md` will auto-load when you start | ||
| modifying files under `__tests__/` or matching `*.test.{ts,tsx,js}`. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,10 @@ | ||
| /** | ||
| * Returns the root reducer's initial state for use in reducer tests. | ||
| * | ||
| * Always import the helper instead of redefining it per file — both the | ||
| * testing-guide.md and prior code review have called out duplicated | ||
| * setup as a smell. | ||
| */ | ||
| import { reducer } from '../../src/reducers/reducer'; | ||
|
|
||
| export const getInitialState = () => reducer(undefined, { type: '@@INIT' }); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,42 @@ | ||
| /** | ||
| * Test helper: mock navigation objects for React Navigation 7. | ||
| * | ||
| * Provides plain mock objects that can be passed as props or returned | ||
| * from `useNavigation()`/`useRoute()` in a `jest.mock(...)` factory at | ||
| * module scope in the consuming test file. | ||
| * | ||
| * Note: this file does NOT call `jest.mock(...)` itself. `jest.mock` is | ||
| * hoisted by Babel only when written at top-level of the test module — | ||
| * wrapping it inside an exported helper would defeat the hoist and run | ||
| * after the modules under test are already imported. | ||
| */ | ||
| import { jest } from '@jest/globals'; | ||
|
|
||
| export function createMockNavigation() { | ||
| return { | ||
| navigate: jest.fn(), | ||
| push: jest.fn(), | ||
| replace: jest.fn(), | ||
| goBack: jest.fn(), | ||
| reset: jest.fn(), | ||
| setOptions: jest.fn(), | ||
| setParams: jest.fn(), | ||
| dispatch: jest.fn(), | ||
| addListener: jest.fn(() => jest.fn()), // returns unsubscribe | ||
| canGoBack: jest.fn(() => true), | ||
| isFocused: jest.fn(() => true), | ||
| getParent: jest.fn(), | ||
| getState: jest.fn(() => ({ | ||
| index: 0, | ||
| routes: [{ name: 'MockRoute', key: 'mock-key' }], | ||
| })), | ||
| }; | ||
| } | ||
|
|
||
| export function createMockRoute(params: Record<string, unknown> = {}) { | ||
| return { | ||
| key: 'mock-route-key', | ||
| name: 'MockRoute', | ||
| params, | ||
| }; | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,21 @@ | ||
| /** | ||
| * Test helper: creates a Redux store with optional preloaded state. | ||
| * | ||
| * Uses the real reducer from the app so that dispatched actions | ||
| * produce the same state transitions as production code. | ||
| */ | ||
| import { legacy_createStore as createStore } from 'redux'; | ||
| import { reducer } from '../../src/reducers/reducer'; | ||
|
|
||
| /** | ||
| * Create a Redux store for testing with optional state overrides. | ||
| * | ||
| * @param preloadedState - Partial state merged on top of the reducer's | ||
| * built-in initialState. Pass only the keys you care about. | ||
| */ | ||
| export function createTestStore(preloadedState: Record<string, unknown> = {}) { | ||
| // Dispatching an unknown action returns initialState from the reducer, | ||
| // which we then merge with the caller's overrides. | ||
| const baseState = reducer(undefined, { type: '@@INIT' }); | ||
| return createStore(reducer, { ...baseState, ...preloadedState } as any); | ||
| } |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.