Skip to content

MetricsBar.test.tsx mocks useAsync with only { state, refresh }, omitting reload and mutate — the component is tested against a contract it never receives #429

Description

@Jagadeeshftw

Priority: Medium  ·  Area: Test correctness  ·  Est. effort: 5–8 h

📌 Problem

src/hooks/useAsync.ts returns four members — state, reload, refresh and mutate (useAsync.ts:25-27).

src/components/MetricsBar.test.tsx supplies only two. TypeScript reports it at four call sites — lines 21, 34, 47 and 64 — each a TS2345:

Argument of type '{ state: { status: "loading"; }; refresh: Mock<Procedure>; }'
is not assignable to parameter of type '{ state: AsyncState<unknown>; reload: () => void;
refresh: () => Promise<void>; mutate: (...) => void; }'.

The tests pass anyway, because CI never runs tsc --noEmit over test files (tracked separately) and the missing members are simply undefined at runtime — unless the component calls them.

This makes the tests actively misleading. MetricsBar is verified against a hook shape that the real useAsync never returns. If the component ever calls reload() or mutate(), the tests will not catch the resulting undefined is not a function; and if it should be calling them, nothing verifies that it does.

🎯 Design decision required

State and defend:

  1. Does MetricsBar need reload/mutate at all? Read the component. If it only ever uses state and refresh, the fix is a complete-but-unused mock. If it should be offering a manual reload, the missing member points at a behavioural gap, and that is the more valuable finding.
  2. Mock construction. Hand-writing the full four-member object at four call sites invites the same drift again. A typed factory helper that returns a complete useAsync result — with overrides — keeps mocks honest as the hook evolves. Argue for it or against.
  3. Prevention. With the typecheck gate in place (separate issue) this specific drift becomes impossible. Say whether anything further is warranted.

🧩 Requirements and context

  • Do not silence the errors with as any, @ts-expect-error, or a Partial<> cast on the mock. That reintroduces exactly the problem.
  • The mock must be typed against the real useAsync return type, so a future change to the hook breaks the mock at compile time.
  • All four call sites must be fixed.
  • If the component turns out to need reload, report that as a gap and say whether you addressed it here or filed it.
  • Existing test assertions should not change unless you are fixing a genuine defect — explain if they do.
  • useAsync currently has no test file of its own (separate issue). Your work here defines the contract those tests should pin; cross-reference it.

🛠️ Suggested execution

  1. Run npx tsc --noEmit and confirm the four errors.
  2. Read MetricsBar and establish which hook members it actually uses.
  3. Build a typed mock factory.
  4. Replace all four call sites.
  5. Confirm the errors are gone and the suite still passes.

✅ Acceptance criteria

  • npx tsc --noEmit reports zero errors for MetricsBar.test.tsx; the repo-wide count drops by 4.
  • The mock is typed against the real useAsync return type — a hook change would break it at compile time.
  • No as any, @ts-expect-error or Partial<> cast was used.
  • The PR states which hook members MetricsBar actually uses.
  • Any behavioural gap found is reported.
  • Existing assertions are unchanged, or every change is justified.
  • The full suite passes.

🚫 Out of scope

  • SettlementTable.test.tsx's errors — separate issue.
  • Adding the typecheck CI gate — separate issue.
  • Writing useAsync's own tests — separate issue.

🧪 Verification

npm ci
npx tsc --noEmit 2>&1 | grep -c "error TS"      # report before and after
npx tsc --noEmit 2>&1 | grep MetricsBar          # expect empty
npm test src/components/MetricsBar.test.tsx
npm test

📤 What your PR must include

  1. The four errors before, and the repo-wide count before/after.
  2. Which hook members MetricsBar actually uses.
  3. Your mock-factory decision.
  4. Any behavioural gap discovered.
  5. Closes #<n>.

🔒 Security notes

A test suite that verifies a component against a fabricated interface provides confidence without evidence. Here the divergence is visible to the compiler and has simply never been checked — which is the more general lesson: mocks are assertions about production contracts, and an unchecked mock is an unchecked assertion.

📋 Guidelines

  • Minimum 95% test coverage on changed lines
  • Clear documentation
  • Timeframe: 96 hours from assignment
  • One logical change per commit; no merge commits

💬 Join our community

Working on this, or want to sanity-check your approach before you start? Come and ask — the maintainers are there and happy to help.

Telegram: https://t.me/Grainlify

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

GrantFox OSSGrantFox open-source programMaybe RewardedGrantFox: potentially rewarded contributionThird CampaignGrantFox third campaign issuebugSomething isn't workingpriority:mediumMedium difficulty / self-contained but non-trivial

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions