✨ Wave 2: deep branch coverage for complex hooks — coverage sprint#4124
✨ Wave 2: deep branch coverage for complex hooks — coverage sprint#4124clubanderson merged 1 commit intomainfrom
Conversation
Add comprehensive branch-coverage tests for the 6 largest/most complex hooks in the console codebase: - useCachedData (3,156 lines): New test file covering all 20+ useCached* hooks, fetcher callbacks (no-token, non-JSON, HTTP error, sort/limit), progressive fetcher presence, key formatting, and category defaults. - useSearchIndex (553 lines): +18 tests covering localStorage card scanning (malformed JSON, non-array, missing card_type), stat scanning (fallback to defaults, invisible stats), catalog card de-duplication, node role defaults, custom dashboard href, null/undefined hook data. - useRewards (288 lines): +25 tests covering RewardsProvider lifecycle, awardCoins (repeatable vs one-time, unknown action, no user), coin- based and action-based achievements, event cap at 100, GitHub rewards dedup logic, merged total never-negative, metadata passthrough. - useExecSession: +25 tests covering WebSocket connect/open/auth flow, exec_init message fields, stdout/stderr/exit/error message handling, no-token error, constructor throw, onerror before connection, onclose with code, reconnection scheduling, sendInput/resize JSON messages, disconnect preventing reconnect, unmount cleanup, multi-connect. - useSidebarConfig: +22 tests covering addItem/addItems (primary, secondary, sections), removeItem, updateItem, reorderItems (all targets), toggle/set collapsed, mobile sidebar open/close/toggle, restoreDashboard (add + no-op), resetToDefault, generateFromBehavior, localStorage persistence, exported constants. - useProviderHealth: +7 tests covering AI/cloud provider separation, isDemoFallback suppression while loading, failed state passthrough. Total: 187 tests across 6 files, all passing. Signed-off-by: Andrew Anderson <andy@clubanderson.com>
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
✅ Deploy Preview for kubestellarconsole ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
👋 Hey @clubanderson — thanks for opening this PR!
This is an automated message. |
There was a problem hiding this comment.
Pull request overview
Adds deep branch-coverage tests for several complex console hooks, focusing on conditional branches, edge cases, and error paths—without changing production code.
Changes:
- Expanded test suites for
useSidebarConfig,useSearchIndex,useRewards,useExecSession, anduseProviderHealth - Added a new comprehensive test file for
useCachedDatacovering many exported hooks and internal fetcher branches - Added additional mocks/stubs to exercise error paths and persistence behavior
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| web/src/hooks/tests/useSidebarConfig.test.ts | Adds extensive action + persistence tests for sidebar config hook |
| web/src/hooks/tests/useSearchIndex.test.ts | Adds localStorage parsing edge-case and de-duplication tests |
| web/src/hooks/tests/useRewards.test.tsx | Adds provider-backed tests for awarding, achievements, persistence, and GitHub merge logic |
| web/src/hooks/tests/useProviderHealth.test.ts | Adds split/shape/state passthrough tests with cache + provider mocks |
| web/src/hooks/tests/useExecSession.test.ts | Adds WebSocket lifecycle, error handling, reconnect, and command/resize tests |
| web/src/hooks/tests/useCachedData.test.ts | New deep branch-coverage suite for useCachedData hooks + fetcher behavior |
| import { useSidebarConfig, DISCOVERABLE_DASHBOARDS, PROTECTED_SIDEBAR_IDS } from '../useSidebarConfig' | ||
| import type { SidebarItem } from '../useSidebarConfig' | ||
|
|
||
| const STORAGE_KEY = 'kubestellar-sidebar-config-v11' | ||
| const OLD_STORAGE_KEY = 'kubestellar-sidebar-config-v10' | ||
|
|
||
| describe('useSidebarConfig', () => { | ||
| beforeEach(() => { | ||
| localStorage.clear() | ||
| vi.clearAllMocks() | ||
| // Reset module-level shared state by clearing storage | ||
| // (the module uses sharedConfig singleton that persists across tests) |
There was a problem hiding this comment.
The comment indicates the hook uses a module-level singleton that persists across tests, but the setup only clears localStorage and mocks. If useSidebarConfig reads/writes a shared in-memory singleton, tests can become order-dependent (e.g., addItem mutations leaking into later tests). Consider resetting module state between tests (e.g., vi.resetModules() + dynamic import per test) or calling a dedicated reset exported by the module (if available) so each test starts from a known default config.
| import { useSidebarConfig, DISCOVERABLE_DASHBOARDS, PROTECTED_SIDEBAR_IDS } from '../useSidebarConfig' | |
| import type { SidebarItem } from '../useSidebarConfig' | |
| const STORAGE_KEY = 'kubestellar-sidebar-config-v11' | |
| const OLD_STORAGE_KEY = 'kubestellar-sidebar-config-v10' | |
| describe('useSidebarConfig', () => { | |
| beforeEach(() => { | |
| localStorage.clear() | |
| vi.clearAllMocks() | |
| // Reset module-level shared state by clearing storage | |
| // (the module uses sharedConfig singleton that persists across tests) | |
| import type { SidebarItem } from '../useSidebarConfig' | |
| const STORAGE_KEY = 'kubestellar-sidebar-config-v11' | |
| const OLD_STORAGE_KEY = 'kubestellar-sidebar-config-v10' | |
| let useSidebarConfig: typeof import('../useSidebarConfig')['useSidebarConfig'] | |
| let DISCOVERABLE_DASHBOARDS: typeof import('../useSidebarConfig')['DISCOVERABLE_DASHBOARDS'] | |
| let PROTECTED_SIDEBAR_IDS: typeof import('../useSidebarConfig')['PROTECTED_SIDEBAR_IDS'] | |
| describe('useSidebarConfig', () => { | |
| beforeEach(async () => { | |
| localStorage.clear() | |
| vi.clearAllMocks() | |
| await vi.resetModules() | |
| const mod = await import('../useSidebarConfig') | |
| useSidebarConfig = mod.useSidebarConfig | |
| DISCOVERABLE_DASHBOARDS = mod.DISCOVERABLE_DASHBOARDS | |
| PROTECTED_SIDEBAR_IDS = mod.PROTECTED_SIDEBAR_IDS |
| const compute = DISCOVERABLE_DASHBOARDS.find(d => d.id === 'compute')! | ||
| const before = result.current.config.primaryNav.some(i => i.id === 'compute') | ||
| if (!before) { | ||
| act(() => { result.current.restoreDashboard(compute) }) | ||
| expect(result.current.config.primaryNav.some(i => i.id === 'compute')).toBe(true) | ||
| } |
There was a problem hiding this comment.
This test can become a no-op when compute is already present (it performs no assertions in that case), which weakens its value and can mask regressions. Make the precondition deterministic (e.g., pick a dashboard guaranteed not to exist initially, or remove it first) and always assert the expected outcome.
| const compute = DISCOVERABLE_DASHBOARDS.find(d => d.id === 'compute')! | |
| const before = result.current.config.primaryNav.some(i => i.id === 'compute') | |
| if (!before) { | |
| act(() => { result.current.restoreDashboard(compute) }) | |
| expect(result.current.config.primaryNav.some(i => i.id === 'compute')).toBe(true) | |
| } | |
| const dashboard = DISCOVERABLE_DASHBOARDS.find( | |
| d => !result.current.config.primaryNav.some(i => i.id === d.id), | |
| ) | |
| expect(dashboard).toBeDefined() | |
| const before = result.current.config.primaryNav.some(i => i.id === dashboard!.id) | |
| expect(before).toBe(false) | |
| act(() => { result.current.restoreDashboard(dashboard!) }) | |
| expect(result.current.config.primaryNav.some(i => i.id === dashboard!.id)).toBe(true) |
| if (catalogCards.length > 0) { | ||
| expect(catalogCards[0].href).toContain('addCard=true') | ||
| } |
There was a problem hiding this comment.
This test conditionally asserts and will pass without verifying anything if catalogCards is empty. To keep the test meaningful, assert that at least one catalog card exists for the query (or set up mocks/fixtures so the catalog list is deterministic), then check the href shape.
| if (catalogCards.length > 0) { | |
| expect(catalogCards[0].href).toContain('addCard=true') | |
| } | |
| expect(catalogCards.length).toBeGreaterThan(0) | |
| catalogCards.forEach(card => { | |
| expect(card.href).toContain('addCard=true') | |
| }) |
| const mockUseCacheResult = { | ||
| data: [], | ||
| isLoading: false, | ||
| isRefreshing: false, | ||
| isDemoFallback: false, | ||
| isFailed: false, | ||
| consecutiveFailures: 0, | ||
| refetch: vi.fn(), | ||
| } |
There was a problem hiding this comment.
In TypeScript, data: [] is inferred as never[] in many configurations. Later assignments like mockUseCacheResult.data = [...] as ProviderHealthInfo[] can cause a type error at compile time. Give mockUseCacheResult (or at least data) an explicit type such as ProviderHealthInfo[] or unknown[] to avoid never[] inference.
| ok: true, | ||
| text: vi.fn().mockResolvedValue(JSON.stringify({ pods: [{ name: 'p1' }] })), | ||
| } | ||
| vi.stubGlobal('fetch', vi.fn().mockResolvedValue(mockFetchResponse)) |
There was a problem hiding this comment.
Global stubs are being torn down inline within individual tests. If an assertion fails before vi.unstubAllGlobals() runs, the stubbed fetch can leak into subsequent tests and create cascading failures. Prefer centralizing global cleanup in afterEach (e.g., always calling vi.unstubAllGlobals() there) so teardown happens even when a test fails early.
| const pods = await fetcher() | ||
| expect(Array.isArray(pods)).toBe(true) | ||
|
|
||
| vi.unstubAllGlobals() |
There was a problem hiding this comment.
Global stubs are being torn down inline within individual tests. If an assertion fails before vi.unstubAllGlobals() runs, the stubbed fetch can leak into subsequent tests and create cascading failures. Prefer centralizing global cleanup in afterEach (e.g., always calling vi.unstubAllGlobals() there) so teardown happens even when a test fails early.
|
Thank you for your contribution! Your PR has been merged. Check out what's new:
Stay connected: Slack #kubestellar-dev | Multi-Cluster Survey |
🔄 Auto-Applying Copilot Code ReviewCopilot code review found 3 code suggestion(s) and 3 general comment(s). @copilot Please apply all of the following code review suggestions:
const STORAGE_KEY = 'kub...`
Also address these general comments:
Push all fixes in a single commit. Run Auto-generated by copilot-review-apply workflow. |
Summary
Deep branch-coverage tests for the 6 largest, most complex hooks in the console codebase. These hooks already had basic tests (1-3 tests each); this PR adds 152 new tests to exercise conditional branches, error paths, and edge cases that were previously untested.
Hooks covered (by line count):
useCachedData.tsuseSearchIndex.tsuseRewards.tsxuseExecSession.tsuseSidebarConfig.tsuseProviderHealth.tsApproach
Test plan
npm test -- --run src/hooks/__tests__/{useCachedData,useSearchIndex,useRewards,useExecSession,useSidebarConfig,useProviderHealth}*npm run build