Skip to content

test(frontend): add comprehensive unit test suite (69 cases) - #46

Open
parthdude07 wants to merge 1 commit into
genesis-kb:userauthfrom
parthdude07:test/unit-frontend
Open

test(frontend): add comprehensive unit test suite (69 cases)#46
parthdude07 wants to merge 1 commit into
genesis-kb:userauthfrom
parthdude07:test/unit-frontend

Conversation

@parthdude07

@parthdude07 parthdude07 commented Aug 16, 2026

Copy link
Copy Markdown
Contributor
  • migrateLibrary: 9 tests (schema migration v1→v2, invalid input, future versions)
  • bookmarkStore: 6 tests (localStorage persistence, corrupted JSON recovery, fallback)
  • api service: 10 tests (auth header injection, 401 token clearing, network errors, timeout)
  • AuthContext: 6 tests (mount validation, login/logout, token persistence, modal state)
  • ProtectedRoute: 4 tests (loading spinner, auth rendering, modal triggering, redirect)
  • BookmarkButton: 9 tests (toggle state, click behavior, label display, event propagation)
  • useBookmarks: 11 tests (guest mode CRUD, idempotency, highlight truncation, filtering)
  • useNotes: 7 tests (fetch, sort/filter, add/update/delete, togglePin)
  • useBookmarkReconciliation: 6 tests (stale refresh, deleted flagging, cache cross-reference)

Summary by cubic

Adds a comprehensive frontend unit test suite to lock in auth, bookmarks, notes, and API client behavior. Adds @testing-library/user-event for interaction tests; no runtime behavior changes.

Coverage highlights

  • Components/Context: BookmarkButton, ProtectedRoute, AuthContext.
  • Hooks: useBookmarks (guest/localStorage), useNotes (React Query), useBookmarkReconciliation.
  • Lib/Services: bookmarkStore, migrateLibrary, api and APIError.

Contract assumptions

  • Uses localStorage keys btc-auth-token and btc-library; 401 responses clear the token.
  • API responses are JSON with { success, data } or { success: false, error: { code, message } }; 500s surface error.code and message.
  • ProtectedRoute shows a spinner while loading, opens the login modal when unauthenticated, and redirects to redirectTo if the modal closes without login.
  • Reconciliation reads React Query key ['transcripts'] to refresh snapshots and flag deleted items.

Written for commit ab0994a. Summary will update on new commits.

Review in cubic

- migrateLibrary: 9 tests (schema migration v1→v2, invalid input, future versions)
- bookmarkStore: 6 tests (localStorage persistence, corrupted JSON recovery, fallback)
- api service: 10 tests (auth header injection, 401 token clearing, network errors, timeout)
- AuthContext: 6 tests (mount validation, login/logout, token persistence, modal state)
- ProtectedRoute: 4 tests (loading spinner, auth rendering, modal triggering, redirect)
- BookmarkButton: 9 tests (toggle state, click behavior, label display, event propagation)
- useBookmarks: 11 tests (guest mode CRUD, idempotency, highlight truncation, filtering)
- useNotes: 7 tests (fetch, sort/filter, add/update/delete, togglePin)
- useBookmarkReconciliation: 6 tests (stale refresh, deleted flagging, cache cross-reference)
@coderabbitai

coderabbitai Bot commented Aug 16, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 799220ae-ff61-4cc2-b611-64537c5f5022

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@cubic-dev-ai cubic-dev-ai 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.

10 issues found across 11 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="src/test/lib/bookmarkStore.test.ts">

<violation number="1" location="src/test/lib/bookmarkStore.test.ts:23">
P3: The comment describing the dynamic imports is inaccurate. The import returns the already-cached module singleton every test (`bookmarkStore` is created once at first module evaluation), so re-importing does not recreate the store or re-run module-level setup against fresh mocks. This is harmless for the current tests because `LocalBookmarkStore.load/save` read and write localStorage on each call, but the comment's stated intent (that module-level code runs with mocks in place) is not what happens.</violation>

<violation number="2" location="src/test/lib/bookmarkStore.test.ts:67">
P2: The quota-exceeded pruning path is untested despite the header and PR description claiming coverage. The lone save-error test deliberately throws a plain `Error`, which can't match the `DOMException`/`QuotaExceededError` check in `save()`, so it only exercises the generic `console.error` branch. Add a test that stubs `setItem` to throw a `DOMException` with `name: 'QuotaExceededError'` (or code 22) and verifies highlights are pruned and the retry is attempted.</violation>

<violation number="3" location="src/test/lib/bookmarkStore.test.ts:81">
P2: The 'InMemoryStore (fallback)' test does not test InMemoryStore at all. It imports the shared `bookmarkStore` singleton, which in the test environment is the `LocalBookmarkStore` (created at module load when `isLocalStorageAvailable()` returns true), and merely roundtrips save/load through localStorage. The fallback (private/incognito) path is a distinct class (`InMemoryStore`) that is never exercised, so the test is mislabeled and provides false coverage of the fallback path described in the PR. The InMemoryStore class is not exported, and the `bookmarkStore` module singleton is cached, so there is no way this test reaches the fallback implementation. Export `InMemoryStore` (or run the module with localStorage disabled) and instantiate it directly so the fallback path is genuinely covered.</violation>
</file>

<file name="src/test/services/api.test.ts">

<violation number="1" location="src/test/services/api.test.ts:35">
P3: The api.test.ts suite never exercises the timeout path. The PR description claims "timeout" coverage for the api service, and this file sets up `vi.useFakeTimers({ shouldAdvanceTime: true })`, presumably for that purpose, but no test triggers the `AbortError` → 408 `TIMEOUT` branch in `createTimeoutController`/`request`. Add a test that makes fetch honor the AbortSignal and assert the resulting APIError has `statusCode: 408` and `code: 'TIMEOUT'`, otherwise the timeout handling (and the fake-timer setup) is unverified.</violation>
</file>

<file name="src/test/components/ProtectedRoute.test.tsx">

<violation number="1" location="src/test/components/ProtectedRoute.test.tsx:125">
P2: The redirect test can never fail and doesn't test the scenario it claims. In `ProtectedRoute`, both `useEffect` hooks run on mount in declaration order: effect 1 sets `modalWasOpened.current = true`, then effect 2 sees it and calls `navigate` on the very first render. So `mockNavigate` is already called with `/home` before any modal opens/closes. Wrapping the assertion in `if (mockNavigate.mock.calls.length > 0)` inside `waitFor` also means the test passes even if navigation never happens, since `waitFor` resolves when the callback doesn't throw. Drop the guard and assert directly, and actually simulate the modal opening then closing (e.g. re-render with `isLoginModalOpen: true` then `false`) so the test validates the intended open→close→redirect flow.</violation>
</file>

<file name="src/test/lib/migrateLibrary.test.ts">

<violation number="1" location="src/test/lib/migrateLibrary.test.ts:80">
P3: The 'returns v2 data as-is when shape is valid' test asserts `expect(result).toBe(v2Data)`, tying the test to the implementation detail that `migrateLibrary` returns the stored reference without copying. If the migration is later changed to defensively copy/normalize valid v2 data (a common safety practice), this test breaks even though behavior is unchanged. Assert on the result's shape/content instead (e.g. `toEqual` with the expected library state), which reflects the contract the tests should protect.</violation>
</file>

<file name="src/test/hooks/useBookmarkReconciliation.test.tsx">

<violation number="1" location="src/test/hooks/useBookmarkReconciliation.test.tsx:41">
P3: The 'cache not loaded' case seeds no data, so the hook's queryFn (`queryClient.getQueryData(['transcripts'])`) resolves to `undefined`. React Query v5 rejects a queryFn resolving to undefined with its 'Query data cannot be undefined' error, leaving the query in an error state and emitting console noise. The test still passes because the hook treats `allTranscripts` as falsy and returns bookmarks as-is, but the test does not exercise the intended no-cache branch cleanly and the assertion would pass whether or not the fallback logic worked. Return a defined value (e.g. `?? null`) from the hook's queryFn so the no-cache path is a clean 'success with no data' rather than an error.</violation>
</file>

<file name="src/test/contexts/AuthContext.test.tsx">

<violation number="1" location="src/test/contexts/AuthContext.test.tsx:186">
P3: If the `expect(...).toThrow(...)` assertion in this test fails, `spy.mockRestore()` is never reached and `console.error` stays suppressed for the rest of the test run, hiding the cause of later failures. This `describe` block has no `afterEach` cleanup (the `vi.restoreAllMocks()` is scoped to the `AuthContext` describe), so there is no safety net. Wrap the assertion in `try/finally` so the spy is always restored.</violation>
</file>

<file name="src/test/hooks/useNotes.test.tsx">

<violation number="1" location="src/test/hooks/useNotes.test.tsx:53">
P3: `vi.stubGlobal('crypto', { randomUUID: ... })` replaces the entire global `crypto` object, dropping `getRandomValues`, `subtle`, and every other method, and it is never restored. The hook only needs `randomUUID`, so preserve the rest of the browser crypto (and restore after) instead of shadowing the whole API, which could silently break unrelated code that runs during these tests.</violation>
</file>

<file name="src/test/hooks/useBookmarks.test.tsx">

<violation number="1" location="src/test/hooks/useBookmarks.test.tsx:53">
P3: This stub replaces the entire global crypto object and is never restored (no afterEach/vi.unstubAllGlobals). Replacing the whole object discards other crypto APIs such as getRandomValues/subtle that other code in the same test worker may rely on, and it silently overrides the real implementation rather than just the one API under test. Scope the override to randomUUID only and restore it in afterEach to keep the test isolated and non-polluting.</violation>
</file>

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

})
})

describe('InMemoryStore (fallback)', () => {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2: The 'InMemoryStore (fallback)' test does not test InMemoryStore at all. It imports the shared bookmarkStore singleton, which in the test environment is the LocalBookmarkStore (created at module load when isLocalStorageAvailable() returns true), and merely roundtrips save/load through localStorage. The fallback (private/incognito) path is a distinct class (InMemoryStore) that is never exercised, so the test is mislabeled and provides false coverage of the fallback path described in the PR. The InMemoryStore class is not exported, and the bookmarkStore module singleton is cached, so there is no way this test reaches the fallback implementation. Export InMemoryStore (or run the module with localStorage disabled) and instantiate it directly so the fallback path is genuinely covered.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/test/lib/bookmarkStore.test.ts, line 81:

<comment>The 'InMemoryStore (fallback)' test does not test InMemoryStore at all. It imports the shared `bookmarkStore` singleton, which in the test environment is the `LocalBookmarkStore` (created at module load when `isLocalStorageAvailable()` returns true), and merely roundtrips save/load through localStorage. The fallback (private/incognito) path is a distinct class (`InMemoryStore`) that is never exercised, so the test is mislabeled and provides false coverage of the fallback path described in the PR. The InMemoryStore class is not exported, and the `bookmarkStore` module singleton is cached, so there is no way this test reaches the fallback implementation. Export `InMemoryStore` (or run the module with localStorage disabled) and instantiate it directly so the fallback path is genuinely covered.</comment>

<file context>
@@ -0,0 +1,100 @@
+  })
+})
+
+describe('InMemoryStore (fallback)', () => {
+  it('load() returns empty state and save() persists in memory', async () => {
+    // Simulate no localStorage available by importing InMemoryStore path.
</file context>

// After the useEffects run, navigate should be called with redirectTo
await waitFor(() => {
// The redirect may happen after two renders (once for modal open, once for close)
if (mockNavigate.mock.calls.length > 0) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2: The redirect test can never fail and doesn't test the scenario it claims. In ProtectedRoute, both useEffect hooks run on mount in declaration order: effect 1 sets modalWasOpened.current = true, then effect 2 sees it and calls navigate on the very first render. So mockNavigate is already called with /home before any modal opens/closes. Wrapping the assertion in if (mockNavigate.mock.calls.length > 0) inside waitFor also means the test passes even if navigation never happens, since waitFor resolves when the callback doesn't throw. Drop the guard and assert directly, and actually simulate the modal opening then closing (e.g. re-render with isLoginModalOpen: true then false) so the test validates the intended open→close→redirect flow.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/test/components/ProtectedRoute.test.tsx, line 125:

<comment>The redirect test can never fail and doesn't test the scenario it claims. In `ProtectedRoute`, both `useEffect` hooks run on mount in declaration order: effect 1 sets `modalWasOpened.current = true`, then effect 2 sees it and calls `navigate` on the very first render. So `mockNavigate` is already called with `/home` before any modal opens/closes. Wrapping the assertion in `if (mockNavigate.mock.calls.length > 0)` inside `waitFor` also means the test passes even if navigation never happens, since `waitFor` resolves when the callback doesn't throw. Drop the guard and assert directly, and actually simulate the modal opening then closing (e.g. re-render with `isLoginModalOpen: true` then `false`) so the test validates the intended open→close→redirect flow.</comment>

<file context>
@@ -0,0 +1,130 @@
+    // After the useEffects run, navigate should be called with redirectTo
+    await waitFor(() => {
+      // The redirect may happen after two renders (once for modal open, once for close)
+      if (mockNavigate.mock.calls.length > 0) {
+        expect(mockNavigate).toHaveBeenCalledWith('/home', { replace: true })
+      }
</file context>

expect(stored.bookmarks).toHaveLength(1)
})

it('save() returns state even on non-quota error', async () => {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2: The quota-exceeded pruning path is untested despite the header and PR description claiming coverage. The lone save-error test deliberately throws a plain Error, which can't match the DOMException/QuotaExceededError check in save(), so it only exercises the generic console.error branch. Add a test that stubs setItem to throw a DOMException with name: 'QuotaExceededError' (or code 22) and verifies highlights are pruned and the retry is attempted.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/test/lib/bookmarkStore.test.ts, line 67:

<comment>The quota-exceeded pruning path is untested despite the header and PR description claiming coverage. The lone save-error test deliberately throws a plain `Error`, which can't match the `DOMException`/`QuotaExceededError` check in `save()`, so it only exercises the generic `console.error` branch. Add a test that stubs `setItem` to throw a `DOMException` with `name: 'QuotaExceededError'` (or code 22) and verifies highlights are pruned and the retry is attempted.</comment>

<file context>
@@ -0,0 +1,100 @@
+    expect(stored.bookmarks).toHaveLength(1)
+  })
+
+  it('save() returns state even on non-quota error', async () => {
+    const spy = vi.spyOn(Storage.prototype, 'setItem').mockImplementation(() => {
+      throw new Error('Generic storage error')
</file context>

})

it('load() returns DEFAULT_LIBRARY when localStorage is empty', async () => {
// Dynamically import so module-level code runs with our mocks in place

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P3: The comment describing the dynamic imports is inaccurate. The import returns the already-cached module singleton every test (bookmarkStore is created once at first module evaluation), so re-importing does not recreate the store or re-run module-level setup against fresh mocks. This is harmless for the current tests because LocalBookmarkStore.load/save read and write localStorage on each call, but the comment's stated intent (that module-level code runs with mocks in place) is not what happens.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/test/lib/bookmarkStore.test.ts, line 23:

<comment>The comment describing the dynamic imports is inaccurate. The import returns the already-cached module singleton every test (`bookmarkStore` is created once at first module evaluation), so re-importing does not recreate the store or re-run module-level setup against fresh mocks. This is harmless for the current tests because `LocalBookmarkStore.load/save` read and write localStorage on each call, but the comment's stated intent (that module-level code runs with mocks in place) is not what happens.</comment>

<file context>
@@ -0,0 +1,100 @@
+  })
+
+  it('load() returns DEFAULT_LIBRARY when localStorage is empty', async () => {
+    // Dynamically import so module-level code runs with our mocks in place
+    const { bookmarkStore } = await import('@/lib/bookmarkStore')
+    const result = bookmarkStore.load()
</file context>

@@ -0,0 +1,170 @@
/**

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P3: The api.test.ts suite never exercises the timeout path. The PR description claims "timeout" coverage for the api service, and this file sets up vi.useFakeTimers({ shouldAdvanceTime: true }), presumably for that purpose, but no test triggers the AbortError → 408 TIMEOUT branch in createTimeoutController/request. Add a test that makes fetch honor the AbortSignal and assert the resulting APIError has statusCode: 408 and code: 'TIMEOUT', otherwise the timeout handling (and the fake-timer setup) is unverified.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/test/services/api.test.ts, line 35:

<comment>The api.test.ts suite never exercises the timeout path. The PR description claims "timeout" coverage for the api service, and this file sets up `vi.useFakeTimers({ shouldAdvanceTime: true })`, presumably for that purpose, but no test triggers the `AbortError` → 408 `TIMEOUT` branch in `createTimeoutController`/`request`. Add a test that makes fetch honor the AbortSignal and assert the resulting APIError has `statusCode: 408` and `code: 'TIMEOUT'`, otherwise the timeout handling (and the fake-timer setup) is unverified.</comment>

<file context>
@@ -0,0 +1,170 @@
+  })
+})
+
+describe('api.request (via api.get/post)', () => {
+  const originalFetch = global.fetch
+
</file context>

}

const result = migrateLibrary(v2Data)
expect(result).toBe(v2Data) // same reference, no copy

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P3: The 'returns v2 data as-is when shape is valid' test asserts expect(result).toBe(v2Data), tying the test to the implementation detail that migrateLibrary returns the stored reference without copying. If the migration is later changed to defensively copy/normalize valid v2 data (a common safety practice), this test breaks even though behavior is unchanged. Assert on the result's shape/content instead (e.g. toEqual with the expected library state), which reflects the contract the tests should protect.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/test/lib/migrateLibrary.test.ts, line 80:

<comment>The 'returns v2 data as-is when shape is valid' test asserts `expect(result).toBe(v2Data)`, tying the test to the implementation detail that `migrateLibrary` returns the stored reference without copying. If the migration is later changed to defensively copy/normalize valid v2 data (a common safety practice), this test breaks even though behavior is unchanged. Assert on the result's shape/content instead (e.g. `toEqual` with the expected library state), which reflects the contract the tests should protect.</comment>

<file context>
@@ -0,0 +1,107 @@
+    }
+
+    const result = migrateLibrary(v2Data)
+    expect(result).toBe(v2Data) // same reference, no copy
+  })
+
</file context>

]

it('returns bookmarks as-is when transcript cache is not loaded', async () => {
// Don't pre-seed cache

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P3: The 'cache not loaded' case seeds no data, so the hook's queryFn (queryClient.getQueryData(['transcripts'])) resolves to undefined. React Query v5 rejects a queryFn resolving to undefined with its 'Query data cannot be undefined' error, leaving the query in an error state and emitting console noise. The test still passes because the hook treats allTranscripts as falsy and returns bookmarks as-is, but the test does not exercise the intended no-cache branch cleanly and the assertion would pass whether or not the fallback logic worked. Return a defined value (e.g. ?? null) from the hook's queryFn so the no-cache path is a clean 'success with no data' rather than an error.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/test/hooks/useBookmarkReconciliation.test.tsx, line 41:

<comment>The 'cache not loaded' case seeds no data, so the hook's queryFn (`queryClient.getQueryData(['transcripts'])`) resolves to `undefined`. React Query v5 rejects a queryFn resolving to undefined with its 'Query data cannot be undefined' error, leaving the query in an error state and emitting console noise. The test still passes because the hook treats `allTranscripts` as falsy and returns bookmarks as-is, but the test does not exercise the intended no-cache branch cleanly and the assertion would pass whether or not the fallback logic worked. Return a defined value (e.g. `?? null`) from the hook's queryFn so the no-cache path is a clean 'success with no data' rather than an error.</comment>

<file context>
@@ -0,0 +1,149 @@
+  ]
+
+  it('returns bookmarks as-is when transcript cache is not loaded', async () => {
+    // Don't pre-seed cache
+    const { result } = renderHook(
+      () => useBookmarkReconciliation(baseBookmarks, baseHighlights),
</file context>

Comment on lines +186 to +195
const spy = vi.spyOn(console, 'error').mockImplementation(() => {})

function BadComponent() {
useAuth()
return null
}

expect(() => render(<BadComponent />)).toThrow('useAuth must be used within an AuthProvider')

spy.mockRestore()

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P3: If the expect(...).toThrow(...) assertion in this test fails, spy.mockRestore() is never reached and console.error stays suppressed for the rest of the test run, hiding the cause of later failures. This describe block has no afterEach cleanup (the vi.restoreAllMocks() is scoped to the AuthContext describe), so there is no safety net. Wrap the assertion in try/finally so the spy is always restored.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/test/contexts/AuthContext.test.tsx, line 186:

<comment>If the `expect(...).toThrow(...)` assertion in this test fails, `spy.mockRestore()` is never reached and `console.error` stays suppressed for the rest of the test run, hiding the cause of later failures. This `describe` block has no `afterEach` cleanup (the `vi.restoreAllMocks()` is scoped to the `AuthContext` describe), so there is no safety net. Wrap the assertion in `try/finally` so the spy is always restored.</comment>

<file context>
@@ -0,0 +1,197 @@
+describe('useAuth (outside provider)', () => {
+  it('throws when used outside AuthProvider', () => {
+    // Suppress React error boundary noise
+    const spy = vi.spyOn(console, 'error').mockImplementation(() => {})
+
+    function BadComponent() {
</file context>
Suggested change
const spy = vi.spyOn(console, 'error').mockImplementation(() => {})
function BadComponent() {
useAuth()
return null
}
expect(() => render(<BadComponent />)).toThrow('useAuth must be used within an AuthProvider')
spy.mockRestore()
it('throws when used outside AuthProvider', () => {
const spy = vi.spyOn(console, 'error').mockImplementation(() => {})
function BadComponent() {
useAuth()
return null
}
try {
expect(() => render(<BadComponent />)).toThrow('useAuth must be used within an AuthProvider')
} finally {
spy.mockRestore()
}
})

}))

// Mock crypto.randomUUID
vi.stubGlobal('crypto', {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P3: vi.stubGlobal('crypto', { randomUUID: ... }) replaces the entire global crypto object, dropping getRandomValues, subtle, and every other method, and it is never restored. The hook only needs randomUUID, so preserve the rest of the browser crypto (and restore after) instead of shadowing the whole API, which could silently break unrelated code that runs during these tests.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/test/hooks/useNotes.test.tsx, line 53:

<comment>`vi.stubGlobal('crypto', { randomUUID: ... })` replaces the entire global `crypto` object, dropping `getRandomValues`, `subtle`, and every other method, and it is never restored. The hook only needs `randomUUID`, so preserve the rest of the browser crypto (and restore after) instead of shadowing the whole API, which could silently break unrelated code that runs during these tests.</comment>

<file context>
@@ -0,0 +1,224 @@
+}))
+
+// Mock crypto.randomUUID
+vi.stubGlobal('crypto', {
+  randomUUID: () => 'temp-uuid-' + Math.random().toString(36).slice(2, 8),
+})
</file context>

}))

// Mock crypto.randomUUID
vi.stubGlobal('crypto', {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P3: This stub replaces the entire global crypto object and is never restored (no afterEach/vi.unstubAllGlobals). Replacing the whole object discards other crypto APIs such as getRandomValues/subtle that other code in the same test worker may rely on, and it silently overrides the real implementation rather than just the one API under test. Scope the override to randomUUID only and restore it in afterEach to keep the test isolated and non-polluting.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/test/hooks/useBookmarks.test.tsx, line 53:

<comment>This stub replaces the entire global crypto object and is never restored (no afterEach/vi.unstubAllGlobals). Replacing the whole object discards other crypto APIs such as getRandomValues/subtle that other code in the same test worker may rely on, and it silently overrides the real implementation rather than just the one API under test. Scope the override to randomUUID only and restore it in afterEach to keep the test isolated and non-polluting.</comment>

<file context>
@@ -0,0 +1,233 @@
+}))
+
+// Mock crypto.randomUUID
+vi.stubGlobal('crypto', {
+  randomUUID: () => 'mock-uuid-' + Math.random().toString(36).slice(2, 8),
+})
</file context>

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.

1 participant