Skip to content

Add Vitest test harness and first batch of logic tests#142

Open
fwextensions wants to merge 1 commit into
devfrom
claude/test-harness-6aba13
Open

Add Vitest test harness and first batch of logic tests#142
fwextensions wants to merge 1 commit into
devfrom
claude/test-harness-6aba13

Conversation

@fwextensions

Copy link
Copy Markdown
Owner

Summary

QuicKey has had no automated tests, in part because it drives interactive Chrome behavior that's hard to exercise headlessly. But almost all of the critical logic — the recent-tabs state machine, the storage migration/locking layer, and the worker↔popup control handoff — is plain code reachable with fakes. This adds a Vitest harness and a first batch of tests covering exactly those areas.

Notably, this batch reproduces three of the four bugs fixed this week as failing tests, so you can watch them go green as the fixes merge.

Harness (test/support/, test/setup.js, vitest.config.js)

  • An in-memory chrome.* fake with real storage.local semantics (get(null)/get(key)/get(array)) and dispatchable event hubs.
  • A navigator.locks fake with real FIFO queuing — a lock is held until its callback's promise settles, then the next waiter is granted on a microtask — so the storage mutex and the __control__ handoff can be driven deterministically.
  • A faithful quickey-storage get/set stand-in that mirrors the real merge-and-save rule (a task returning undefined saves nothing — which is how the getAll no-op is observable).
  • Setup that installs the globals the source reads (chrome, navigator, location, DEBUG) and stubs the analytics trackers (no GA network, no import-time await).
  • Aliases mirror webpack.config.js (@/, lodashlodont).

Two suites

Script Runs Status on this branch
npm test test/unit/ green (29 tests)
npm run test:regression test/regression/ 4 failing, by design
npm run test:all everything 29 pass / 4 fail

Green suite (test/unit/) — behavioral coverage that passes on dev: debounce semantics (including execute(waitForNext)), the recent-tabs add/remove/replace/toggle/updateAll state machine (recency order, penultimate insert, MaxTabsLength trim, URL→id remap on restart), and createStorage get/set, concurrent-write serialization under the lock, the migration chain, and reset-on-invalid.

Regression suite (test/regression/) — each test asserts the fixed behavior of a bug still present on dev, so it fails here and turns green once the fix lands:

Test Documents Fixed by
recent-tabs-getall getAll() never persists refreshed recents (() => { tabsByID } is a no-op) #139
abortable-promise-leak abort listener leaked per AbortablePromise #139
control-thenable claimWhenAvailable awaits the task, so a promise-returning task holds the lock forever #141
control-handoff events arriving during the control gap are dropped, never replayed #141

I verified the full loop locally: with the actual #139 and #141 branch changes applied, all four regression tests pass and the green suite stays green. test/README.md documents the convention — when a fix PR merges, move its now-passing regression test into test/unit/.

Follow-ups (not in this PR)

  • The storage rewrite-skip optimization from Fix IPC memory leak and speed up service worker cold start #139 (don't re-save when the version is unchanged) isn't yet covered — it's a perf change rather than a correctness bug, and asserting it cleanly needs spying on the unawaited init writes. Worth adding to the storage suite later.
  • There's no CI yet. A small GitHub Actions workflow running npm test on PRs would turn this green suite into an actual merge gate.

Testing

  • npm test → 29 passed. npm run test:regression → 4 failed as designed (clean assertion/race failures, no import or harness errors).

🤖 Generated with Claude Code

https://claude.ai/code/session_01Hbx1WJrE7jYxhyvNZ2oLqK


Generated by Claude Code

QuicKey had no automated tests, in part because it drives interactive
Chrome behavior that is hard to exercise headlessly.  But almost all of
the critical logic — the recent-tabs state machine, the storage
migration/locking layer, and the worker/popup control handoff — is
plain code reachable with fakes.  This adds that harness and a first
batch of tests.

Harness (test/support, test/setup.js, vitest.config.js):
- an in-memory chrome.* fake with real storage.local semantics and
  dispatchable event hubs
- a navigator.locks fake with real FIFO queuing (a lock is held until
  its callback's promise settles), so the storage mutex and the
  __control__ handoff can be driven deterministically
- a faithful quickey-storage get/set stand-in that mirrors the real
  merge-and-save rule (a task returning undefined saves nothing)
- setup that installs the globals the source reads (chrome, navigator,
  location, DEBUG) and stubs the analytics trackers

Green suite (test/unit, run by
> QuicKey@0.0.0 test
> vitest run unit

 RUN  v2.1.9 /tmp/claude-0/-home-user-QuicKey/e8759e44-b9b1-5209-a845-fdd5b6294d64/scratchpad/qk-tests

 ✓ test/unit/debounce.test.js (6 tests) 12ms
 ✓ test/unit/storage.test.js (6 tests) 24ms
 ✓ test/unit/recent-tabs.test.js (10 tests) 88ms
stdout | test/unit/control.test.js > control > claimWhenAvailable() acquires the lock and runs the task while it's held
--------- locks /background.html { held: [], pending: [] }
--------- LOCK AVAILABLE /background.html
--------- GOT LOCK /background.html exclusive control-test-acquire

stdout | test/unit/control.test.js > control > a second claimWhenAvailable() on the same lock never runs, since the first holds control forever
--------- locks /background.html {
  held: [ { name: 'control-test-acquire', mode: 'exclusive' } ],
  pending: []
}
--------- LOCK AVAILABLE /background.html
--------- GOT LOCK /background.html exclusive control-test-serialize
--------- locks /background.html {
  held: [
    { name: 'control-test-acquire', mode: 'exclusive' },
    { name: 'control-test-serialize', mode: 'exclusive' }
  ],
  pending: []
}
--------- LOCK AVAILABLE /background.html

 ✓ test/unit/control.test.js (3 tests) 19ms
 ✓ test/unit/smoke.test.js (4 tests) 68ms

 Test Files  5 passed (5)
      Tests  29 passed (29)
   Start at  04:29:46
   Duration  908ms (transform 188ms, setup 171ms, collect 156ms, tests 210ms, environment 1ms, prepare 441ms)): debounce semantics, the
recent-tabs add/remove/replace/toggle/updateAll state machine,
createStorage get/set/locking/migration, and control basics.

Regression suite (test/regression, run by
> QuicKey@0.0.0 test:regression
> vitest run regression

 RUN  v2.1.9 /tmp/claude-0/-home-user-QuicKey/e8759e44-b9b1-5209-a845-fdd5b6294d64/scratchpad/qk-tests

stdout | test/regression/control-thenable.regression.test.js > releases control once a promise-returning task settles
--------- locks /background.html { held: [], pending: [] }
--------- LOCK AVAILABLE /background.html
--------- GOT LOCK /background.html exclusive __control__

stdout | test/regression/control-handoff.regression.test.js > replays an event that arrived while control wasn't held once control is claimed
--------- locks /background.html { held: [], pending: [] }
--------- LOCK AVAILABLE /background.html
--------- GOT LOCK /background.html exclusive __control__

 ❯ test/regression/control-handoff.regression.test.js (1 test | 1 failed) 51ms
   × replays an event that arrived while control wasn't held once control is claimed 50ms
     → expected "spy" to be called with arguments: [ { tabId: 1 } ]

Received:

Number of calls: 0

 ❯ test/regression/recent-tabs-getall.regression.test.js (1 test | 1 failed) 90ms
   × persists a tab's refreshed URL into storage after getAll() re-matches it by id 89ms
     → expected 'https://stale.example.com/' to be 'https://fresh.example.com/' // Object.is equality
 ❯ test/regression/control-thenable.regression.test.js (1 test | 1 failed) 214ms
   × releases control once a promise-returning task settles 213ms
     → claimWhenAvailable() never resolved -- control is stuck held forever
 ❯ test/regression/abortable-promise-leak.regression.test.js (1 test | 1 failed) 10ms
   × removes its abort listener once the promise resolves, instead of leaking one per instance 9ms
     → expected "removeEventListener" to be called 3 times, but got 0 times

 Test Files  4 failed (4)
      Tests  4 failed (4)
   Start at  04:29:47
   Duration  819ms (transform 145ms, setup 139ms, collect 77ms, tests 365ms, environment 1ms, prepare 331ms)):
four tests that assert the FIXED behavior of bugs still present on this
branch, so they fail here and turn green as the fixes land —
recent-tabs getAll persistence and the AbortablePromise listener leak
(PR #139), and claimWhenAvailable's promise handling and the dropped
handoff events (PR #141).  See test/README.md for the convention.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Hbx1WJrE7jYxhyvNZ2oLqK
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants