test: re-import improved mock system from cloud-ui

[kantord]

re-import improved mocking system from the cloud ui repo. Note, this does not yet remove custom MSW mocks. in order to keep the PR reasonably small I just did the bare minimum changes to actual tests/fixtures here to support the new mocking system.

[kantord]

@claude can you please do a deep review of this branch

[claude]

Claude finished @kantord's task in 2m 37s —— View job

---

Deep Review of Mock System Changes

I've completed a thorough review of the mock system improvements. This is a well-architected refactoring that significantly enhances the testing infrastructure. Here's my detailed analysis:

---

✅ Strengths

1. Excellent Architecture (autoAPIMock.ts)

The new AutoAPIMock system is well-designed:

• Type-safe overrides: The AutoAPIMockInstance<T> interface provides full type safety for mock data
• Fluent API: Chainable methods (override, overrideHandler, scenario) make test setup intuitive
• Clear separation of concerns: Data overrides vs. handler overrides (for errors/network failures)
• Automatic reset: Global registry with resetAllAutoAPIMocks() ensures clean test isolation

2. Scenario System Design

The scenario implementation is clever:

• Header-based activation (line 61-75 in autoAPIMock.ts): Allows browser/dev testing by reading x-mock-scenario header
• Named scenarios: Centralized in scenarioNames.ts for consistency
• Fallback behavior: activateScenario with fallbackToDefault option prevents brittle tests
• Global activation: activateMockScenario() applies scenarios across all mocks simultaneously

3. Migration Strategy

The fixture migration is consistent and backward-compatible:

• Changed from default exports to named exports: export const mockedGetApiV1BetaGroups = AutoAPIMock<T>(...)
• mocker.ts:173-174 handles both old and new formats during transition
• Naming convention (deriveMockName) is predictable and follows pattern

4. Configuration Updates

Path aliases properly configured:

• tsconfig.json & tsconfig.app.json: Added @mocks alias
• vitest.config.ts: Added @mocks to resolve aliases
• vitest.setup.ts: Integrated resetAllAutoAPIMocks() into test lifecycle

---

🐛 Issues Found

High Priority

1. Type Narrowing Issue (mocker.ts:168-178)

   let fixture: AutoAPIMockInstance<unknown> | unknown

   The type is too broad. Consider:

   let fixture: AutoAPIMockInstance<unknown> | Record<string, unknown> | unknown

2. Error Handling Inconsistency (mocker.ts:180-185)
   When a fixture import fails, it returns HTTP 500. However, this could mask legitimate test setup issues. Consider throwing instead of returning 500 during tests, or add a flag to differentiate dev vs test mode.

3. Circular Dependency Risk (autoAPIMock.ts:66-72)
   In the header-based scenario activation, you temporarily mutate overrideHandlerFn, call the handler, then restore. If the handler itself triggers another request to the same endpoint, this could cause issues. Consider using a separate "active scenario" state variable instead.

Medium Priority

4. No Type Safety for Scenario Names (autoAPIMock.ts:62-63)

   const headerScenario = info.request.headers.get(SCENARIO_HEADER)
   if (headerScenario) {
     const scenarioFn = scenarios.get(headerScenario as MockScenarioName)

   The cast as MockScenarioName is unsafe. If an invalid header is passed, it silently fails. Consider validating against MockScenarios object first.

5. Registry Memory Leak (autoAPIMock.ts:50, 121)
   The registry uses a Set but never removes instances. If mocks are dynamically created (unlikely but possible), this could leak memory. Consider:

   export function unregisterMock(instance: AutoAPIMockInstance<unknown>): void {
     registry.delete(instance)
   }

6. Inconsistent Naming Convention (mockTemplate.ts:5-15)
   The deriveMockName function has hardcoded HTTP method replacements:

   .replace(/^Get/, 'get')
   .replace(/^Post/, 'post')

   This works but is fragile. If operation names don't start with these exact patterns, naming breaks. Consider using the actual HTTP method from the operation instead.

7. Missing JSDoc on Public API (mocks/index.ts)
   The main export file lacks documentation. Add JSDoc comments explaining:

   • When to use override vs overrideHandler
   • How scenarios work
   • Example usage

Low Priority

8. Magic Number (mocker.ts:23-26)
   JSF configuration uses hardcoded values:

   jsf.option({ minItems: 1 })
   jsf.option({ maxItems: 3 })

   Consider extracting to constants with explanatory names.

9. Potential Race Condition (mocker.ts:169-179)
   The dynamic import with fallback to import.meta.glob could have timing issues if a fixture is generated mid-test. Not critical but worth noting.

10. Type Generation Assumption (mockTemplate.ts:30)

    [`import type { ${typeName} } from '@api/types.gen'`]

    Assumes types are always in @api/types.gen. If the OpenAPI schema changes structure, this breaks silently.

---

💡 Suggestions for Future Improvements

1. Add TypeScript Strict Mode Guards

   // In autoAPIMock.ts generatedHandler
   if (!overrideHandlerFn) {
     return HttpResponse.json(defaultValue as JsonBodyType)
   }
   return overrideHandlerFn(defaultValue, info)

2. Consider Adding Mock Analytics
   Track which scenarios are used most frequently to identify dead code:

   const scenarioUsageStats = new Map<MockScenarioName, number>()

3. Add Scenario Validation
   When defining a scenario, validate it actually does something:

   scenario(name: MockScenarioName, fn: ScenarioFn<T>) {
     const before = overrideHandlerFn
     fn(instance)
     const after = overrideHandlerFn
     if (before === after) {
       console.warn(`Scenario "${name}" doesn't modify mock behavior`)
     }
     scenarios.set(name, fn)
     return instance
   }

4. Document Migration Path
   Add a comment in mocker.ts explaining when the backward compatibility (line 174) can be removed.

---

📋 Test Coverage Observations

Looking at install-group-button.test.tsx:

• ✅ Good: Tests still use MSW server.use() directly for one-off overrides
• ✅ Good: Tests access defaultValue (line 40) showing the new API is being used
• ⚠️ Missing: No tests demonstrate the new scenario system in action. Consider adding a test that uses activateScenario to validate the feature works end-to-end.

---

🎯 Overall Assessment

Score: 8.5/10

This is a high-quality refactoring that:

• Improves type safety significantly
• Adds powerful scenario testing capabilities
• Maintains backward compatibility
• Has clean, readable code

The issues identified are mostly minor edge cases. The architecture is solid and the implementation is production-ready. My main recommendation is to address the high-priority items (error handling consistency and circular dependency risk) before merging.

Recommendation: Approve with minor changes ✨

---

[Copilot]

Pull request overview

This PR re-imports an improved mock system from cloud-ui that enhances API mocking capabilities with type-safe test overrides and reusable scenarios. The new AutoAPIMock wrapper provides a cleaner API for test-scoped overrides while maintaining backward compatibility with existing fixtures.

Key changes:

• Introduces AutoAPIMock<T> wrapper with .override(), .overrideHandler(), and .activateScenario() methods for flexible test customization
• Migrates all mock fixtures from default exports to named exports with consistent naming convention (e.g., mockedGetApiV1BetaGroups)
• Adds comprehensive documentation in docs/mocks.md and .github/copilot-instructions.md

Reviewed changes

Copilot reviewed 23 out of 24 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
vitest.setup.ts	Adds resetAllAutoAPIMocks() call in beforeEach to reset mock overrides between tests
vitest.config.ts	Adds @mocks path alias for convenient imports
tsconfig.json	Adds @mocks and @mocks/* path mappings for TypeScript resolution
tsconfig.app.json	Mirrors the @mocks path mappings from tsconfig.json
renderer/src/features/registry-servers/components/tests/install-group-button.test.tsx	Updates to use new named export mockedGetApiV1BetaGroups and accesses fixture data via .defaultValue
renderer/src/common/mocks/test.setup.ts	Adds standalone test setup file (appears to duplicate existing setup in root vitest.setup.ts)
renderer/src/common/mocks/scenarioNames.ts	Defines global scenario names (Empty, ServerError) with type safety
renderer/src/common/mocks/mocker.ts	Updates fixture generation to use AutoAPIMock wrapper and named exports; removes Ajv validation
renderer/src/common/mocks/mockTemplate.ts	Implements deriveMockName() function and updates buildMockModule() to generate fixtures with named exports
renderer/src/common/mocks/index.ts	Exports public API of the mock system
renderer/src/common/mocks/fixtures/workloads_restart/post.ts	Migrates to AutoAPIMock with named export mockedPostApiV1BetaWorkloadsRestart
renderer/src/common/mocks/fixtures/workloads_name_edit/post.ts	Migrates to AutoAPIMock with named export mockedPostApiV1BetaWorkloadsByNameEdit
renderer/src/common/mocks/fixtures/workloads_name/get.ts	Migrates to AutoAPIMock with named export mockedGetApiV1BetaWorkloadsByName
renderer/src/common/mocks/fixtures/workloads/post.ts	Migrates to AutoAPIMock with named export mockedPostApiV1BetaWorkloads
renderer/src/common/mocks/fixtures/groups_name/get.ts	Migrates to AutoAPIMock with named export mockedGetApiV1BetaGroupsByName
renderer/src/common/mocks/fixtures/groups_name/delete.ts	Migrates to AutoAPIMock with named export mockedDeleteApiV1BetaGroupsByName
renderer/src/common/mocks/fixtures/groups/post.ts	Migrates to AutoAPIMock with named export mockedPostApiV1BetaGroups
renderer/src/common/mocks/fixtures/groups/get.ts	Migrates to AutoAPIMock with named export mockedGetApiV1BetaGroups
renderer/src/common/mocks/fixtures/discovery_clients/get.ts	Migrates to AutoAPIMock with named export mockedGetApiV1BetaDiscoveryClients
renderer/src/common/mocks/fixtures/clients_register/post.ts	Migrates to AutoAPIMock with named export mockedPostApiV1BetaClientsRegister
renderer/src/common/mocks/fixtures/README.md	Removes outdated documentation (replaced by comprehensive docs/mocks.md)
renderer/src/common/mocks/autoAPIMock.ts	Implements the core AutoAPIMock wrapper with override capabilities, scenario support, and registry for bulk resets
docs/mocks.md	Adds comprehensive documentation covering fixture generation, test-scoped overrides, scenarios, and request recording
.github/copilot-instructions.md	Documents API mocking patterns and examples for GitHub Copilot

[peppescg]

🚀