Skip to content

test(registry): pin useRegistryUpdateMutation coverage to fix Coveralls flake#2035

Merged
samuv merged 2 commits intomainfrom
coveralls-fix
Apr 17, 2026
Merged

test(registry): pin useRegistryUpdateMutation coverage to fix Coveralls flake#2035
samuv merged 2 commits intomainfrom
coveralls-fix

Conversation

@samuv
Copy link
Copy Markdown
Collaborator

@samuv samuv commented Apr 17, 2026

The coverage/coveralls check has been flapping between "no change" and "Coverage decreased (-0.02%) to 63.188%" on PRs that touch only docs/CI (e.g. #2032). Coveralls pinpoints the regression to exactly one line: renderer/src/common/components/settings/registry/use-registry-update-mutation.ts:59 — the generic throw e rethrow inside putRegistry's .catch. That line has no dedicated test; its coverage is entirely ambient from renderer/src/common/components/settings/tabs/__tests__/registry-tab.test.tsx, which never forces a non-matching PUT error. Depending on MSW teardown and test scheduling it sometimes flipped to uncovered, producing the -2.25% on this file (-0.02% total) that Coveralls was reporting.

  • Add renderer/src/common/components/settings/registry/__tests__/use-registry-update-mutation.test.ts — 12 direrenderHooktests that pin every branch ofuseRegistryUpdateMutation`
  • Happy paths: type = default (empty PUT body), type = url ({ url } + cache updated), type = local_path ({ local_path }), type = api_url with both auth fields (PUT + postApiV1BetaRegistryAuthLogin + invalidateQueries), type = api_url with only client_id (partial auth), type = api_url with neither field (no auth, login skipped), and the post-success removeQueries for registry-name-servers
  • putRegistry error branches: api_url + OIDC pattern → REGISTRY_WRONG_ISSUER_TOAST, api_url + auth-required pattern → REGISTRY_AUTH_FIELDS_REQUIRED_TOAST, api_url + non-matching string → throw e (line 59 via path A), url + JSON error → throw e (line 59 via path B)
  • authenticateWithRegistry error branch: PUT succeeds, login fails → mutateAsync rejects with REGISTRY_WRONG_AUTH_TOAST, invalidateQueries for the registry key was called, fire-and-forget logout POST was made
  • Uses the existing es (mockedPutApiV1BetaRegistryByName, mockedPostApiV1BetaRegistryAuthLogin, mockedPostApiV1BetaRegistryAuthLogout) — no new infrastructure

Why this removes the jitter

v8 records per-line hit counts. Once line 59 is hit deterministically by an explicit assertion, it stays at covered on every run, so base and head SHAs report identical coverage for this file. Verified locally across three back-to-back pnpm run test:coverage runs on the relevant files — the lcov block for use-registry-update-mutation.ts is byte-identical, and DA:59 is pinned at hit-count 3.

Not changed in this PR (deliberate, follow-up)

  • No change to .github/workflows/_unit-tests.yml, .github/workflows/on-pr.yml, or vitest.config.ts — the workflow still runs on every PR and uploads to Coveralls, it just stops producing a non-deterministic delta
  • The duplicate coverage/coveralls status post (two identical entries on the same (sha, context) pair, ~2.5 minutes apart) is orthogonal to the flake and left low-up: one-line github-token: '' on coverallsapp/github-action in _unit-tests.yml, or disabling the Coveralls GitHub App's PR-status integration in repo settings. After this PR the single remaining status would already be green.

…ls flake

Coveralls has been reporting a -0.02% regression on PRs that touch only docs/CI because line 59 of use-registry-update-mutation.ts (the generic `throw e` rethrow inside putRegistry) was only ever hit ambiently via registry-tab.test.tsx. Depending on MSW teardown and test scheduling it sometimes flipped to uncovered, producing a spurious -2.25% delta on this file (~-0.02% total).

Add a direct unit test that exercises every branch of the hook: default/url/local_path happy paths, api_url with full/partial/no auth, OIDC_DISCOVERY and AUTH_FIELDS_REQUIRED error patterns, non-matching PUT errors for both api_url and url types (covers `throw e` from both guard paths), and the authenticateWithRegistry login-failure path (REGISTRY_WRONG_AUTH_TOAST + invalidate + fire-and-forget logout). Verified lcov output is byte-identical across three consecutive coverage runs, with DA:59 pinned at hit-count 3.
Copilot AI review requested due to automatic review settings April 17, 2026 12:06
@samuv samuv self-assigned this Apr 17, 2026
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds deterministic unit tests for useRegistryUpdateMutation to stabilize Coveralls line coverage and prevent flapping caused by previously-ambient execution of the throw e rethrow branch in putRegistry.

Changes:

  • Add a new hook-focused test suite that exercises all useRegistryUpdateMutation branches (success + error paths).
  • Explicitly covers putRegistry error classification (OIDC discovery/auth-required) and the generic rethrow behavior.
  • Covers authenticateWithRegistry failure behavior (rejects with known toast + invalidation + logout call).

…on deterministically

Addresses Copilot review feedback on #2035:
- Pin the exact thrown value in the two rethrow tests so line 59
  (`throw e`) is protected against regressions that would wrap the
  original error in a new Error.
- In the login-failure test, await the mutation promise directly
  (`await expect(promise).rejects.toMatchObject(...)`) instead of the
  side-effecting `.catch` + local variable pattern, so the toast
  assertion no longer races the React Query observer notification.

Made-with: Cursor
@samuv samuv enabled auto-merge (squash) April 17, 2026 12:22
@samuv samuv merged commit 127a73a into main Apr 17, 2026
16 checks passed
@samuv samuv deleted the coveralls-fix branch April 17, 2026 12:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants