🌱 Add unit tests for useLocalAgent, progress hooks, and useMCS#3966
🌱 Add unit tests for useLocalAgent, progress hooks, and useMCS#3966clubanderson merged 1 commit intomainfrom
Conversation
|
[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. |
|
Thank you for your contribution! Your PR has been merged. Check out what's new:
Stay connected: Slack #kubestellar-dev | Multi-Cluster Survey |
There was a problem hiding this comment.
Pull request overview
Adds new Vitest unit test suites for several React hooks that manage local agent connectivity, WebSocket-driven progress updates, and Multi-Cluster Services (MCS) API polling, improving regression coverage for connection lifecycle, parsing, polling, and cleanup behavior.
Changes:
- Add
useLocalAgentunit tests covering connection state transitions, polling, and unmount cleanup. - Add/expand WebSocket hook tests for
useClusterProgressand add a new suite foruseUpdateProgress. - Add a comprehensive
useMCStest suite foruseMCSStatus,useServiceExports, anduseServiceImports, including demo mode and polling scenarios.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| web/src/hooks/tests/useLocalAgent.test.ts | New tests for local agent polling/state transitions and cleanup. |
| web/src/hooks/tests/useClusterProgress.test.ts | New WebSocket parsing/reconnect/cleanup tests for cluster progress updates. |
| web/src/hooks/tests/useUpdateProgress.test.ts | New WebSocket parsing, step-history, reconnect, and cleanup tests for update progress. |
| web/src/hooks/tests/useMCS.test.ts | New tests for MCS hooks: fetch success/error paths, demo mode, refetch, polling, cleanup. |
| /** | ||
| * Tests for useClusterProgress hook. | ||
| * | ||
| * Validates WebSocket connection, message parsing for local_cluster_progress | ||
| * events, dismiss behaviour, and cleanup on unmount. | ||
| */ | ||
|
|
||
| import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest' | ||
| import { renderHook, act } from '@testing-library/react' | ||
|
|
There was a problem hiding this comment.
This suite duplicates coverage with the existing web/src/hooks/useClusterProgress.test.ts (same hook, overlapping cases). Keeping two test files for the same hook increases maintenance cost and can make future refactors harder (updates must be applied in two places). Consider consolidating into a single test file (preferably under __tests__/) and removing/merging the older one.
| // Assign mock to global before importing the hook | ||
| Object.defineProperty(globalThis, 'WebSocket', { | ||
| writable: true, | ||
| value: MockWebSocket, | ||
| }) | ||
|
|
There was a problem hiding this comment.
Object.defineProperty(globalThis, 'WebSocket', ...) is done without configurable: true and the mock is never restored. This can leak the mocked WebSocket into other test files and may prevent other suites from re-stubbing WebSocket cleanly. Prefer vi.stubGlobal('WebSocket', MockWebSocket) and vi.unstubAllGlobals() in afterEach (or at least set configurable: true and restore the original).
| // Assign mock to global before importing the hook | ||
| Object.defineProperty(globalThis, 'WebSocket', { | ||
| writable: true, | ||
| value: MockWebSocket, | ||
| }) |
There was a problem hiding this comment.
Object.defineProperty(globalThis, 'WebSocket', ...) is done without configurable: true and the mock is never restored. This can leak the mocked WebSocket into other test files and may prevent other suites from re-stubbing WebSocket cleanly. Prefer vi.stubGlobal('WebSocket', MockWebSocket) and vi.unstubAllGlobals() in afterEach (or at least set configurable: true and restore the original).
| vi.useFakeTimers() | ||
| vi.resetModules() | ||
|
|
||
| // Set up fresh global fetch mock before importing | ||
| global.fetch = vi.fn() | ||
|
|
||
| const mod = await import('../useLocalAgent') | ||
| useLocalAgent = mod.useLocalAgent |
There was a problem hiding this comment.
This test file assigns global.fetch = vi.fn() but never restores the original fetch. Depending on Vitest worker reuse, this can leak into other suites. Prefer vi.stubGlobal('fetch', vi.fn()) and vi.unstubAllGlobals() (or restore the original in afterEach) to keep globals isolated.
🔄 Auto-Applying Copilot Code ReviewCopilot code review found 0 code suggestion(s) and 4 general comment(s). Also address these general comments:
Push all fixes in a single commit. Run Auto-generated by copilot-review-apply workflow. |
…plicate test - Replace Object.defineProperty/global assignment with vi.stubGlobal() for WebSocket and fetch mocks so vitest tracks and restores them - Add vi.unstubAllGlobals() to afterEach in all 3 test files - Re-establish WebSocket mock in beforeEach (cleared by unstubAllGlobals) - Delete duplicate useClusterProgress.test.ts from hooks/ (canonical copy lives in hooks/__tests__/) Signed-off-by: Andrew Anderson <andy@clubanderson.com>
…plicate test (#3971) - Replace Object.defineProperty/global assignment with vi.stubGlobal() for WebSocket and fetch mocks so vitest tracks and restores them - Add vi.unstubAllGlobals() to afterEach in all 3 test files - Re-establish WebSocket mock in beforeEach (cleared by unstubAllGlobals) - Delete duplicate useClusterProgress.test.ts from hooks/ (canonical copy lives in hooks/__tests__/) Signed-off-by: Andrew Anderson <andy@clubanderson.com>
Summary
useLocalAgenthook (8 tests): initial state, connected/disconnected transitions, polling, cleanup on unmountuseClusterProgresshook (10 tests): WebSocket message parsing, status transitions, dismiss, reconnect, cleanupuseUpdateProgresshook (11 tests): WebSocket message parsing, step history tracking, stale detection, dismiss, cleanupuseMCShooks (26 tests):useMCSStatus,useServiceExports,useServiceImports— API fetching, error handling, BackendUnavailableError, demo mode fallback, polling, cleanupTest plan
npx vitest run src/hooks/__tests__/useLocalAgent.test.ts src/hooks/__tests__/useClusterProgress.test.ts src/hooks/__tests__/useUpdateProgress.test.ts src/hooks/__tests__/useMCS.test.tsnpm run build)Fixes #3955 #3956 #3957