Skip to content

chore: mock localStorage in web tests#24663

Closed
timonrieger wants to merge 2 commits intoimmich-app:mainfrom
timonrieger:tests/fix-localstorage-mock-web-tests
Closed

chore: mock localStorage in web tests#24663
timonrieger wants to merge 2 commits intoimmich-app:mainfrom
timonrieger:tests/fix-localstorage-mock-web-tests

Conversation

@timonrieger
Copy link
Copy Markdown
Collaborator

@timonrieger timonrieger commented Dec 18, 2025

Test Fix

Fixed test environment issue where svelte-persisted-store was failing to access localStorage during test initialization, causing 20 test files to fail. The fix ensures localStorage is properly available in the test environment, resolving all localStorage-related errors and allowing all tests to run correctly.

Test results:

  • Run with: cd web && pnpm test
  • Before fix: See before.log - 20 failed test files due to localStorage errors
  • After fix: See after.log - 35 passed test files, localStorage errors resolved

Note: The remaining failing tests in timeline-manager.svelte.spec.ts are unrelated to this fix and are out of scope for this PR. These appear to be pre-existing test issues.

@timonrieger timonrieger changed the title test(web): mock localStorage for svelte-persisted-store in setup.ts chore: mock localStorage in web tests Dec 18, 2025
@timonrieger timonrieger force-pushed the tests/fix-localstorage-mock-web-tests branch from c263a5e to de32c72 Compare January 5, 2026 17:15
Copy link
Copy Markdown
Member

@danieldietzler danieldietzler left a comment

Choose a reason for hiding this comment

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

Thanks!

@danieldietzler
Copy link
Copy Markdown
Member

The remaining failing test is definitely related to your PR since it targets localStorage specifically

@timonrieger
Copy link
Copy Markdown
Collaborator Author

timonrieger commented Jan 6, 2026

The remaining failing test is definitely related to your PR since it targets localStorage specifically

What do you mean? As written above

Before fix: See before.log - 20 failed test files due to localStorage errors
After fix: See after.log - 35 passed test files, localStorage errors resolved

EDIT: you mean this one? https://github.com/immich-app/immich/actions/runs/20723239706/job/59492380262?pr=24663

@timonrieger
Copy link
Copy Markdown
Collaborator Author

timonrieger commented Jan 6, 2026

hmm it seems like the test fixes have already been pushed to main. I cannot reproduce the test failures anymore I saw when opening the PR regarding localstorage
left: main | right: this PR branch
image

@danieldietzler
Copy link
Copy Markdown
Member

EDIT: you mean this one? immich-app/immich/actions/runs/20723239706/job/59492380262?pr=24663

I do. And yes, I am not talking about those undefined values in the tests. I'm specifically talking about that one test that expect showAssetOwners to be true while it's actually false. That seems to be a regression from this PR

@timonrieger
Copy link
Copy Markdown
Collaborator Author

timonrieger commented Jan 6, 2026

Could not figure out why, i assume some dependency update fixed the local storage issues. Closing this therefore, as this PR introduced on more test failure now.

@timonrieger timonrieger closed this Jan 6, 2026
@timonrieger timonrieger deleted the tests/fix-localstorage-mock-web-tests branch January 14, 2026 21:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants