Skip to content

Fix storage failure tests under jsdom's Storage semantics - #3

Merged
clioo merged 2 commits into
clioo:mainfrom
rashidazarang:fix-storage-failure-tests
Aug 6, 2026
Merged

clioo merged 2 commits into
clioo:mainfrom
rashidazarang:fix-storage-failure-tests

Conversation

@rashidazarang

Copy link
Copy Markdown
Contributor

What

Both persistence-failure tests in src/store.test.ts fail on a clean checkout:

× keeps editing available when a confirmed transaction cannot persist
  → expected 'saved' to be 'error'
× persists normalized documents and handles write/read failures
  → expected true to be false

Why

The tests replaced localStorage.setItem by direct assignment. jsdom's Storage is an exotic object: assigning to a property stores it as an item instead of overriding the method, so the throwing stub never ran — the store saved successfully and the assertions comparing against the failure path broke.

How

Stub with vi.spyOn(Storage.prototype, "setItem" / "getItem") instead, which intercepts the method regardless of how the environment implements the instance, and mockRestore() afterwards so later assertions in the same test see real storage. No production code touched.

Verify

npm test   # 31 passed + 7 companion passed (was: 2 failed)

The persistence-failure tests replaced localStorage.setItem by direct
assignment. jsdom's Storage is an exotic object: assigning to a property
stores it as an item instead of overriding the method, so the throwing
stub never ran and both tests asserted against a save path that had
silently succeeded ("expected 'saved' to be 'error'").

Stub via vi.spyOn(Storage.prototype, ...) instead, which intercepts the
method regardless of how the environment implements the instance, and
restore with mockRestore() so later assertions see real storage.
@clioo
clioo merged commit a5b0f1a into clioo:main Aug 6, 2026
1 of 2 checks passed
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