test: React Testing Library + renderer component tests, coverage to 33% (Phase 3)

docs/KNOWN_ISSUES.md

@@ -18,3 +18,24 @@
   on a document-level `pointerdown` outside it.
 - Affected files: src/ui/components/SettingsView/settingRowHover.ts and any row using
   Fuselage `Select`.
+
+## RTL auto-cleanup vs manual `document.body.innerHTML = ''` in `afterEach` orphans portal anchors
+- Status: Confirmed (RTL 14.3.1, @kayahr/jest-electron-runner, single shared `src/.jest/setup.ts`).
+- Symptom: A renderer spec that renders a portal/anchor component (e.g. TooltipProvider ->
+  TooltipPortal -> createAnchor's `#tooltip-root`) crashes the ENTIRE `yarn test:coverage` run with
+  `process.exit(1)` from `src/.jest/setup.ts` (uncaughtException handler), stack originating in
+  React `safelyCallDestroy` / `commitPassiveUnmountInsideDeletedTreeOnFiber`. The thrown error is
+  `NotFoundError: The node to be removed is not a child of this node` from
+  `document.body.removeChild(a)` in `src/ui/components/utils/createAnchor.ts`.
+- Root cause: A spec adds `afterEach(() => { document.body.innerHTML = ''; })`. Jest runs afterEach
+  hooks LIFO; RTL's auto-cleanup `afterEach(cleanup)` is registered at import time so it runs LAST.
+  The manual `innerHTML=''` runs FIRST and removes body-appended portal anchors WITHOUT going through
+  their `deleteAnchor`/effect-cleanup path. RTL `cleanup()` then unmounts the React tree, the portal's
+  unmount effect calls `removeChild` on the already-detached node, and it throws. Because the shared
+  setup converts any uncaughtException into `process.exit(1)`, one orphaned anchor kills the whole run.
+- Workaround / rule: Do NOT manually wipe `document.body.innerHTML` in renderer-spec `afterEach`. RTL
+  auto-cleanup already unmounts the React tree and lets components remove their own anchors. If a test
+  needs a clean body, call the render result's `unmount()` explicitly instead.
+- Affected files: src/ui/components/utils/TooltipProvider.spec.tsx,
+  src/ui/components/utils/ReparentingContainer.spec.tsx, src/ui/components/utils/createAnchor.ts,
+  src/ui/components/utils/TooltipPortal.tsx, src/.jest/setup.ts.

jest.config.js

@@ -12,10 +12,10 @@ module.exports = {
   coveragePathIgnorePatterns: ['/node_modules/', '/app/', '/dist/'],
   coverageThreshold: {
     global: {
-      lines: 25,
-      statements: 25,
-      branches: 22,
-      functions: 18,
+      lines: 32,
+      statements: 32,
+      branches: 28,
+      functions: 26,
     },
   },
   projects: [

package.json

@@ -111,6 +111,10 @@
     "@rollup/plugin-json": "~6.1.0",
     "@rollup/plugin-node-resolve": "~15.2.3",
     "@rollup/plugin-replace": "~5.0.5",
+    "@testing-library/dom": "^9.3.4",
+    "@testing-library/jest-dom": "^6.4.8",
+    "@testing-library/react": "^14.3.1",
+    "@testing-library/user-event": "^14.5.2",
     "@types/archiver": "~7.0.0",
     "@types/dompurify": "~3.2.0",
     "@types/electron-devtools-installer": "~2.2.5",

src/.jest/setup.ts

@@ -1,5 +1,6 @@
 import path from 'path';
 
+import '@testing-library/jest-dom';
 import { app } from 'electron';
 
 expect.extend({

src/jest-dom.d.ts

@@ -0,0 +1,22 @@
+// Registers @testing-library/jest-dom matcher types on jest's global
+// `expect` for type-checking (`tsc --noEmit`). The package ships the same
+// augmentation, but it is only reachable through a nested triple-slash
+// reference that TypeScript does not propagate under this project's classic
+// `node` module resolution. It also cannot live under `src/.jest/`, because
+// TypeScript excludes dot-prefixed directories from the compilation program.
+// The matching runtime side-effect import lives in `src/.jest/setup.ts`.
+import type { TestingLibraryMatchers } from '@testing-library/jest-dom/matchers';
+
+declare global {
+  // eslint-disable-next-line @typescript-eslint/no-namespace
+  namespace jest {
+    // The signature must mirror @types/jest's `Matchers<R, T = {}>` exactly so
+    // the declarations merge; renaming or retyping the parameters breaks it.
+    // eslint-disable-next-line @typescript-eslint/no-unused-vars, @typescript-eslint/ban-types
+    interface Matchers<R = void, T = {}>
+      extends TestingLibraryMatchers<
+        ReturnType<typeof expect.stringContaining>,
+        R
+      > {}
+  }
+}

src/logging/main/index.main.spec.ts

@@ -127,7 +127,10 @@ const loadModule = (): typeof LoggingModule => {
 describe('logging/index', () => {
   beforeEach(() => {
     jest.clearAllMocks();
-    jest.useRealTimers();
+    // Fake timers so the error-buffer flush setInterval scheduled by
+    // configureLogging() is a fake (non-ref'd) handle rather than a real timer
+    // that would keep the process alive and block jest --forceExit.
+    jest.useFakeTimers();
     fakeLog = makeFakeLog(true);
     selectImpl = jest.fn(() => false);
     watchImpl = jest.fn();
@@ -141,6 +144,15 @@ describe('logging/index', () => {
 
   afterEach(() => {
     setProcessType(originalProcessType);
+    // configureLogging() schedules a real, ref'd setInterval (the error-buffer
+    // flush timer) on every module load. It is only cleared on 'before-quit',
+    // which most tests never fire — left alone, each load leaks a live timer
+    // that keeps the libuv loop alive and blocks jest --forceExit. Switching to
+    // fake timers in beforeEach makes that interval a fake handle, and
+    // clearAllTimers here disposes it.
+    jest.clearAllTimers();
+    jest.useRealTimers();
+    jest.restoreAllMocks();
   });
 
   describe('configureLogging at import (main / browser process)', () => {

src/screenSharing/main/ScreenSharingRequestTracker.main.spec.ts

@@ -41,11 +41,20 @@ const getRegisteredListener = (): ResponseListener => {
 
 beforeEach(() => {
   jest.clearAllMocks();
+  // Every createRequest() schedules a real setTimeout (default 60s). Tests that
+  // neither fire the response listener nor call cleanup would leak a live,
+  // ref'd timer that keeps the process alive and blocks jest --forceExit. Fake
+  // timers make those handles fake; clearAllTimers in afterEach disposes them.
+  // setImmediate is left real so flushPromises() (used by the async response
+  // listener tests) still resolves without manual timer advancement.
+  jest.useFakeTimers({ doNotFake: ['setImmediate'] });
   jest.spyOn(console, 'warn').mockImplementation(() => undefined);
   jest.spyOn(console, 'error').mockImplementation(() => undefined);
 });
 
 afterEach(() => {
+  jest.clearAllTimers();
+  jest.useRealTimers();
   jest.restoreAllMocks();
 });
 

src/screenSharing/main/desktopCapturerCache.main.spec.ts

@@ -58,6 +58,10 @@ describe('screenSharing/desktopCapturerCache', () => {
   });
 
   afterEach(() => {
+    // The module schedules no timers of its own, but two tests opt into fake
+    // timers; clear any pending fake handle before restoring real timers so
+    // nothing survives into the next test or blocks jest --forceExit.
+    jest.clearAllTimers();
     jest.useRealTimers();
     clearDesktopCapturerCache();
   });

src/ui/components/AddServerView/index.spec.tsx

@@ -0,0 +1,192 @@
+import { AddServerView } from '.';
+import { ServerUrlResolutionStatus } from '../../../servers/common';
+import { ADD_SERVER_VIEW_SERVER_ADDED } from '../../actions';
+import {
+  renderWithStore,
+  screen,
+  userEvent,
+  fireEvent,
+  waitFor,
+} from '../../test-utils';
+
+// Under the jest-electron renderer, `userEvent.type` mutates the DOM input but
+// React's controlled-input value tracker does not pick the change up, so the
+// component's `useState` value (and therefore the submit handler) never sees the
+// typed text. `fireEvent.change` dispatches a native change event that React's
+// value tracker does recognise, faithfully driving `onChange` -> `setInput`.
+const typeUrl = (value: string): void => {
+  fireEvent.change(screen.getByRole('textbox'), { target: { value } });
+};
+
+jest.mock('react-i18next', () => ({
+  useTranslation: () => ({
+    t: (key: string) => key,
+    i18n: { language: 'en', changeLanguage: jest.fn() },
+  }),
+  Trans: ({ children }: { children: React.ReactNode }) => children,
+  initReactI18next: { type: '3rdParty', init: () => {} },
+}));
+
+const mockDispatch = jest.fn();
+
+jest.mock('react-redux', () => {
+  const actual = jest.requireActual('react-redux');
+  return {
+    __esModule: true,
+    ...actual,
+    useDispatch: () => mockDispatch,
+  };
+});
+
+const mockRequest = jest.fn();
+
+jest.mock('../../../store', () => ({
+  request: (...args: unknown[]) => mockRequest(...args),
+}));
+
+const defaultServerHref = 'https://open.rocket.chat/';
+
+const visibleState = { currentView: 'add-new-server' } as any;
+
+beforeEach(() => {
+  mockDispatch.mockClear();
+  mockRequest.mockReset();
+  // jsdom defaults navigator.onLine to true; assert it explicitly.
+  Object.defineProperty(navigator, 'onLine', {
+    configurable: true,
+    value: true,
+  });
+});
+
+describe('AddServerView', () => {
+  it('renders nothing when the current view is not add-new-server', () => {
+    const { container } = renderWithStore(<AddServerView />, {
+      preloadedState: { currentView: 'add-new-server-disabled' } as any,
+    });
+
+    expect(container).toBeEmptyDOMElement();
+  });
+
+  describe('online', () => {
+    it('renders the url input and connect button', () => {
+      renderWithStore(<AddServerView />, { preloadedState: visibleState });
+
+      expect(screen.getByText('landing.inputUrl')).toBeInTheDocument();
+      expect(screen.getByRole('textbox')).toBeInTheDocument();
+      expect(
+        screen.getByRole('button', { name: 'landing.connect' })
+      ).toBeInTheDocument();
+    });
+
+    it('adds the default server when submitting an empty input', async () => {
+      const user = userEvent.setup();
+      renderWithStore(<AddServerView />, { preloadedState: visibleState });
+
+      await user.click(screen.getByRole('button', { name: 'landing.connect' }));
+
+      expect(mockRequest).not.toHaveBeenCalled();
+      expect(mockDispatch).toHaveBeenCalledWith({
+        type: ADD_SERVER_VIEW_SERVER_ADDED,
+        payload: defaultServerHref,
+      });
+    });
+
+    it('resolves a typed url and dispatches the resolved server on success', async () => {
+      const user = userEvent.setup();
+      mockRequest.mockResolvedValue([
+        'https://chat.example.com/',
+        ServerUrlResolutionStatus.OK,
+      ]);
+
+      renderWithStore(<AddServerView />, { preloadedState: visibleState });
+
+      typeUrl('chat.example.com');
+      await user.click(screen.getByRole('button', { name: 'landing.connect' }));
+
+      await waitFor(() =>
+        expect(mockDispatch).toHaveBeenCalledWith({
+          type: ADD_SERVER_VIEW_SERVER_ADDED,
+          payload: 'https://chat.example.com/',
+        })
+      );
+
+      expect(mockRequest).toHaveBeenCalledTimes(1);
+    });
+
+    it('shows an invalid-url error when resolution fails', async () => {
+      const user = userEvent.setup();
+      mockRequest.mockResolvedValue([
+        'bad-url',
+        ServerUrlResolutionStatus.INVALID_URL,
+      ]);
+
+      renderWithStore(<AddServerView />, { preloadedState: visibleState });
+
+      typeUrl('bad-url');
+      await user.click(screen.getByRole('button', { name: 'landing.connect' }));
+
+      expect(
+        await screen.findByText('error.noValidServerFound')
+      ).toBeInTheDocument();
+      expect(mockDispatch).not.toHaveBeenCalled();
+    });
+
+    it('shows a timeout error when resolution times out', async () => {
+      const user = userEvent.setup();
+      mockRequest.mockResolvedValue([
+        'https://slow.example.com/',
+        ServerUrlResolutionStatus.TIMEOUT,
+      ]);
+
+      renderWithStore(<AddServerView />, { preloadedState: visibleState });
+
+      typeUrl('slow.example.com');
+      await user.click(screen.getByRole('button', { name: 'landing.connect' }));
+
+      expect(
+        await screen.findByText('error.connectTimeout')
+      ).toBeInTheDocument();
+      expect(mockDispatch).not.toHaveBeenCalled();
+    });
+
+    it('clears the validation error when the input is edited again', async () => {
+      const user = userEvent.setup();
+      mockRequest.mockResolvedValue([
+        'bad-url',
+        ServerUrlResolutionStatus.INVALID,
+      ]);
+
+      renderWithStore(<AddServerView />, { preloadedState: visibleState });
+
+      typeUrl('bad-url');
+      await user.click(screen.getByRole('button', { name: 'landing.connect' }));
+
+      expect(
+        await screen.findByText('error.noValidServerFound')
+      ).toBeInTheDocument();
+
+      // Editing the input again resets the validation state and clears the error.
+      typeUrl('bad-urlx');
+
+      await waitFor(() =>
+        expect(
+          screen.queryByText('error.noValidServerFound')
+        ).not.toBeInTheDocument()
+      );
+    });
+  });
+
+  describe('offline', () => {
+    it('shows the offline callout instead of the form', () => {
+      Object.defineProperty(navigator, 'onLine', {
+        configurable: true,
+        value: false,
+      });
+
+      renderWithStore(<AddServerView />, { preloadedState: visibleState });
+
+      expect(screen.getByText('error.offline')).toBeInTheDocument();
+      expect(screen.queryByRole('textbox')).not.toBeInTheDocument();
+    });
+  });
+});