cleanup testing utilities, documentation, and AI commentary

webclient/src/__test-utils__/globalGuards.ts

@@ -1,15 +1,9 @@
-// Shared lifecycle helpers for test files that need to mutate global state.
-//
-// The root `setupTests.ts` guards catch leaks even when callers forget to
-// clean up, but opt-in helpers make intent explicit at the call site and
-// avoid piling cleanup logic onto the shared safety net.
-
 /**
  * Temporarily override fields on `window.location` and return a restore fn.
  *
- * `Object.defineProperty(window, 'location', ...)` is not a `vi.spyOn` target,
- * so `vi.restoreAllMocks()` will NOT undo it. Always pair with the returned
- * `restore` callback (ideally in `afterEach`).
+ * @critical `Object.defineProperty(window, 'location', ...)` isn't a vi.spyOn
+ * target, so `vi.restoreAllMocks()` will NOT undo it. Always invoke the
+ * returned restore callback.
  */
 export function withMockLocation(overrides: Partial<Location>): () => void {
   const originalDescriptor = Object.getOwnPropertyDescriptor(window, 'location');
@@ -26,23 +20,3 @@ export function withMockLocation(overrides: Partial<Location>): () => void {
     }
   };
 }
-
-/**
- * Push an entry onto a shared event-handler registry array and return a
- * teardown function that removes exactly that entry.
- *
- * Used by ProtobufService specs which install temporary handlers into the
- * (mocked) `GameEvents` / `RoomEvents` / `SessionEvents` arrays. Manual
- * `.push()`/`.pop()` inside a test body corrupts the array if an assertion
- * throws between them — this helper makes the teardown safe to run in
- * `afterEach`.
- */
-export function withEventRegistry<T>(registry: T[], entry: T): () => void {
-  registry.push(entry);
-  return () => {
-    const index = registry.lastIndexOf(entry);
-    if (index !== -1) {
-      registry.splice(index, 1);
-    }
-  };
-}

webclient/src/__test-utils__/index.ts

@@ -1,4 +1,4 @@
-export { withMockLocation, withEventRegistry } from './globalGuards';
+export { withMockLocation } from './globalGuards';
 export { renderWithProviders } from './renderWithProviders';
 export { createMockWebClient } from './mockWebClient';
 export { disconnectedState, connectedState, connectedWithRoomsState, makeUser } from './storeFixtures';

webclient/src/__test-utils__/mockWebClient.ts

@@ -2,8 +2,9 @@ import type { WebClient } from '@app/websocket';
 
 /**
  * Creates a mock WebClient whose `request` property has vi.fn() stubs
- * for every service method that containers/forms call. Inject this into
- * tests via `renderWithProviders({ webClient: createMockWebClient() })`.
+ * for every service method that containers/forms call. Inject via a
+ * vi.hoisted reference returned from a `vi.mock('@app/hooks', ...)` stub
+ * of `useWebClient`; see LoginForm.spec.tsx for the canonical pattern.
  */
 export function createMockWebClient() {
   return {

webclient/src/__test-utils__/renderWithProviders.tsx

@@ -14,12 +14,8 @@ import { actionReducer } from '../store/actions';
 import { ToastProvider } from '../components/Toast/ToastContext';
 import type { RootState } from '../store/store';
 
-// Minimal i18n instance for tests — returns keys as-is. A non-empty
-// `resources` entry is required so i18next registers `en-US` as a known
-// language; otherwise `i18n.resolvedLanguage` stays `undefined`, which
-// LanguageDropdown seeds into a MUI Select and MUI warns "out-of-range
-// value `undefined`". Value is an empty translation map, since tests
-// already assert on i18n keys directly.
+// Non-empty `resources` registers en-US so `resolvedLanguage` is defined;
+// without it MUI warns about out-of-range Select values.
 const testI18n = i18n.createInstance();
 testI18n.use(initReactI18next).init({
   lng: 'en-US',

webclient/src/__test-utils__/storeFixtures.ts

@@ -1,4 +1,5 @@
-import { App, Data, Enriched } from '@app/types';
+import { App, Data } from '@app/types';
+import { WebsocketTypes } from '@app/websocket/types';
 import type { RootState } from '../store/store';
 
 /**
@@ -30,7 +31,7 @@ export const disconnectedState: Partial<RootState> = {
     ignoreList: {},
     status: {
       connectionAttemptMade: false,
-      state: Enriched.StatusEnum.DISCONNECTED,
+      state: WebsocketTypes.StatusEnum.DISCONNECTED,
       description: null,
     },
     info: { message: null, name: null, version: null },
@@ -77,7 +78,7 @@ export const connectedState: Partial<RootState> = {
     initialized: true,
     status: {
       connectionAttemptMade: true,
-      state: Enriched.StatusEnum.LOGGED_IN,
+      state: WebsocketTypes.StatusEnum.LOGGED_IN,
       description: null,
     },
     info: {