Skip to content

test: poll for the deep-link terminal event, not a fixed flush - #5

Merged
nicknisi merged 2 commits into
mainfrom
fix/flaky-deeplink-tests
Jul 24, 2026
Merged

nicknisi merged 2 commits into
mainfrom
fix/flaky-deeplink-tests

Conversation

@nicknisi

Copy link
Copy Markdown
Member

The four deep-link callback tests in create-auth-kit.spec.ts gated their assertions on flushAsync() — a fixed 5-microtask + 5 ms window. The callback pipeline unseals real crypto state (via authkit-session) before the code exchange, so under parallel test-file load the window lost the race roughly 1 in 5 full-suite runs, failing with authenticateWithCode/sendSpy at 0 calls while always passing in isolation.

Each test now vi.waitFors its terminal observable event — the renderer broadcast — then runs its remaining assertions. Negative assertions (e.g. "the exchange never ran") stay sound because they execute after the terminal event proves the pipeline completed. flushAsync is removed as dead code.

Verified: 12 consecutive full-suite runs green (153/153); the flake reproduced reliably within ~5 runs before the fix.

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no bugs or issues to report.

Open in Devin Review

@greptile-apps

greptile-apps Bot commented Jul 23, 2026 •

Copy link
Copy Markdown

Greptile Summary

This PR replaces a fixed-window flushAsync() helper (5 microtask flushes + 5 ms setTimeout) with vi.waitFor(() => expect(sendSpy).toHaveBeenCalledTimes(1), { timeout: 5000 }) in four deep-link callback tests, eliminating a ~1-in-5 full-suite flake caused by the crypto-unseal step racing the fixed window. The packageManager field in package.json is also moved to the end of the object (cosmetic only).

  • Flakiness fix: vi.waitFor polls until the terminal event (the renderer broadcast) fires, making the timing contract explicit rather than approximate — negative assertions remain sound because they run only after the pipeline has fully resolved.
  • package.json: packageManager field relocated to the bottom; no semantic change.

Confidence Score: 5/5

Safe to merge — test-only change that removes a timing-dependent helper and substitutes a deterministic poll.

The change targets an isolated async-timing problem in test infrastructure. The vi.waitFor strategy is idiomatic Vitest and the 5000 ms ceiling is generous enough for CI under load. Negative assertions are correctly ordered after the terminal event. No production code is touched.

No files require special attention.

Important Files Changed

Filename Overview
src/main/tests/create-auth-kit.spec.ts Replaces flushAsync() with vi.waitFor polling on the renderer broadcast (sendSpy) in all four deep-link callback tests; removes the dead flushAsync helper and cleans up two redundant expect(sendSpy).toHaveBeenCalledTimes(1) standalone assertions (tests 2 and 4), though an identical redundancy remains at line 268 in test 1 (pre-existing outside-diff comment).
package.json Moves the packageManager field to the end of the object — purely cosmetic, no functional change.

Sequence Diagram

sequenceDiagram
    participant Test
    participant IpcHandler
    participant Ceremony
    participant CryptoUnseal
    participant AuthWithCode
    participant BroadcastSpy as sendSpy

    Test->>IpcHandler: invoke signIn handler
    IpcHandler->>Ceremony: ceremony.open(url + state)
    Ceremony-->>IpcHandler: state sealed in storage
    Test->>Test: deliver open-url deep-link callback
    Note over Test,BroadcastSpy: async pipeline begins
    IpcHandler->>CryptoUnseal: unseal state from storage
    CryptoUnseal-->>IpcHandler: PKCE verifier + state
    IpcHandler->>AuthWithCode: authenticateWithCode(code, verifier)
    AuthWithCode-->>IpcHandler: tokens or error
    IpcHandler->>BroadcastSpy: webContents.send(channel, payload)
    Note over Test,BroadcastSpy: vi.waitFor polls until sendSpy called
    Test->>BroadcastSpy: expect(sendSpy).toHaveBeenCalledTimes(1)
    Test->>Test: run remaining assertions
Loading

Reviews (2): Last reviewed commit: "style: move packageManager to oxfmt's ca..." | Re-trigger Greptile

The four deep-link callback tests gated their assertions on
flushAsync(), a fixed 5-microtask + 5 ms window. The callback
pipeline unseals real crypto state (via authkit-session) before the
code exchange, so under parallel test-file load the fixed window lost
the race roughly 1 in 5 full-suite runs ("authenticateWithCode called
0 times").

Each test now awaits vi.waitFor() on its terminal observable event —
the renderer broadcast (sendSpy called once) — with a 5 s timeout
before running its remaining assertions. Negative assertions are safe
because they run after the pipeline has provably completed. The
flushAsync helper (and its orphaned doc comment) is removed as dead
code.

Verified: 12 consecutive full-suite runs green (153/153); lint and
typecheck pass.
@nicknisi
nicknisi force-pushed the fix/flaky-deeplink-tests branch from 5e982be to dc4121f Compare July 23, 2026 18:23
PR #3 placed "packageManager" after "description" (matching
authkit-nextjs), but oxfmt sorts package.json keys canonically and wants
it last — so `format:check` fails on main and on every PR's merge ref.
PR #3's own CI caught this but was merged before the check was read.
@nicknisi
nicknisi merged commit 3fb8d77 into main Jul 24, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant