Skip to content

The home-page success test can pass while an unrouted fetch is swallowed by the page's own error handling #6

Open

Description

@brandonperfetti

Summary

installFetchRouter throws on any URL no route matches, which is the right design — but in tests/app/home-page.test.tsx that throw cannot reach the test. The component under test is built to survive failed GitHub requests, so it catches the error and renders fallback values, and console.error is stubbed silent in beforeEach. A request the fixtures do not cover is therefore absorbed twice over, and the test still passes.

Raised by CodeRabbit on #5 and confirmed against the source before filing. Not a production defect — the only runtime change in that PR was three og:image string swaps — so #5 was merged rather than held.

Context

[source: tests/helpers/github-fixtures.ts] the router fails loudly by design:

const route = routes.find((candidate) => candidate.match(url));
if (!route) {
  throw new Error(`Unrouted fetch in test: ${url.href}`);
}

[source: tests/app/home-page.test.tsx] three things then combine to eat it:

  1. The "still renders when every GitHub request fails" test proves the page renders fallbacks rather than propagating fetch failures — so the same handling absorbs a thrown router error.
  2. vi.spyOn(console, "error").mockImplementation(() => undefined) in beforeEach removes the remaining signal.
  3. The success test registers four routes (contributions, authedRepos, pinned, commits). Any call outside that set — /search/issues, /pulls, /releases all have fixtures defined but unregistered here — goes unrouted.

The blast radius is bounded, not zero. The success test asserts hard on the primary paths (12 contributions in 30 days, Repos tracked → 3, Languages → 2, pinned-first ordering, 2 commits), so a failure there would still surface. What can pass unnoticed is a secondary request whose output the assertions do not reach — which is also exactly the kind of coverage a future change is most likely to silently drop.

Change

Assert that every request made during the test matched a fixture route. installFetchRouter already returns calls, so the harness needs no new plumbing; the cheapest correct fix is to have it also collect unrouted URLs rather than only throwing, and expose them for assertion.

Non-goals:

  • Removing the console.error stub. It exists to keep expected-failure output off the test log and should stay.
  • Changing the router's throw-on-unrouted behavior. That is correct and worth keeping for tests whose subject does not swallow errors.
  • Broadening home-page assertions to cover every chart. This ticket is about detecting an unrouted call, not about asserting more output.

Acceptance

  • A deliberately unregistered route in the home-page success test causes that test to fail [measured].
  • The existing "every GitHub request fails" test still passes — it routes match: () => true, so it has no unrouted calls and must not regress.
  • Full suite green: 52 tests, no network.

Relates: #4, #5

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions