fix: address PR review feedback for RTL migration

Author: felixrieseberg

tests/renderer/components/commands-spec.tsx

@@ -10,19 +10,19 @@ import { AppState } from '../../../src/renderer/state';
 import { overrideRendererPlatform, resetRendererPlatform } from '../../utils';
 
 vi.mock('../../../src/renderer/components/commands-runner', () => ({
-  Runner: () => <div data-testid="runner" />,
+  Runner: 'runner',
 }));
 
 vi.mock('../../../src/renderer/components/commands-version-chooser', () => ({
-  VersionChooser: () => <div data-testid="version-chooser" />,
+  VersionChooser: 'version-chooser',
 }));
 
 vi.mock('../../../src/renderer/components/commands-address-bar', () => ({
-  AddressBar: () => <div data-testid="address-bar" />,
+  AddressBar: 'address-bar',
 }));
 
 vi.mock('../../../src/renderer/components/commands-action-button', () => ({
-  GistActionButton: () => <div data-testid="action-button" />,
+  GistActionButton: 'action-button',
 }));
 
 describe('Commands component', () => {

tests/renderer/components/dialog-add-version-spec.tsx

@@ -1,6 +1,6 @@
 import * as React from 'react';
 
-import { act, render, screen } from '@testing-library/react';
+import { act, fireEvent, render, screen } from '@testing-library/react';
 import { beforeEach, describe, expect, it, vi } from 'vitest';
 
 import { renderClassComponentWithInstanceRef } from '../../../rtl-spec/test-utils/renderClassComponentWithInstanceRef';
@@ -73,18 +73,21 @@ describe('AddVersionDialog component', () => {
 
     // The FileInput has id="custom-electron-version" with an onClick that
     // calls selectLocalVersion and preventDefault
-    const fileInput = document.querySelector(
-      '#custom-electron-version input[type="file"]',
-    ) as HTMLInputElement;
+    const fileInput = screen.getByLabelText(
+      'Select the folder containing Electron.app...',
+      { selector: 'input' },
+    );
     expect(fileInput).toBeInTheDocument();
 
     const preventDefault = vi.fn();
-    fileInput.dispatchEvent(
+    fireEvent(
+      fileInput,
       Object.assign(new MouseEvent('click', { bubbles: true }), {
         preventDefault,
       }),
     );
 
+    expect(preventDefault).toHaveBeenCalled();
     expect(window.ElectronFiddle.selectLocalVersion).toHaveBeenCalled();
   });
 

tests/renderer/components/dialog-bisect-spec.tsx

@@ -180,8 +180,7 @@ describe.each([8, 15])('BisectDialog component', (numVersions) => {
       await instance.onSubmit();
       expect(Bisector).not.toHaveBeenCalled();
 
-      // Restore valid state for clean teardown
-      (instance as any).state = origState;
+      instance.state = origState;
     });
   });
 
@@ -233,8 +232,7 @@ describe.each([8, 15])('BisectDialog component', (numVersions) => {
       await instance.onAuto();
       expect(Bisector).not.toHaveBeenCalled();
 
-      // Restore valid state for clean teardown
-      (instance as any).state = origState;
+      instance.state = origState;
     });
   });
 

tests/renderer/components/dialog-generic-spec.tsx

@@ -33,26 +33,32 @@ describe('GenericDialog component', () => {
       renderDialogWithType(GenericDialogType.warning);
       expect(screen.getByText('Test label')).toBeInTheDocument();
       expect(screen.getByText('Okay')).toBeInTheDocument();
+      expect(
+        document.querySelector('[icon="warning-sign"]'),
+      ).toBeInTheDocument();
     });
 
     it('confirmation', () => {
       renderDialogWithType(GenericDialogType.confirm);
       expect(screen.getByText('Test label')).toBeInTheDocument();
       expect(screen.getByText('Okay')).toBeInTheDocument();
+      expect(document.querySelector('[icon="help"]')).toBeInTheDocument();
     });
 
     it('success', () => {
       renderDialogWithType(GenericDialogType.success);
       expect(screen.getByText('Test label')).toBeInTheDocument();
       expect(screen.getByText('Okay')).toBeInTheDocument();
+      expect(document.querySelector('[icon="info-sign"]')).toBeInTheDocument();
     });
 
     it('with an input prompt', () => {
       store.genericDialogOptions.wantsInput = true;
       renderDialogWithType(GenericDialogType.confirm);
-      expect(screen.getByText('Test label')).toBeInTheDocument();
+      const label = screen.getByText('Test label');
+      expect(label).toBeInTheDocument();
       // The InputGroup renders an input element with id="input"
-      expect(document.getElementById('input')).toBeInTheDocument();
+      expect(label.parentNode?.querySelector('input')).toBeInTheDocument();
     });
 
     it('with an input prompt and placeholder', () => {
@@ -85,7 +91,7 @@ describe('GenericDialog component', () => {
     store.genericDialogOptions.wantsInput = true;
     render(<GenericDialog appState={store} />);
 
-    const input = document.getElementById('input') as HTMLInputElement;
+    const input = document.getElementById('input')!;
     expect(input).toBeInTheDocument();
 
     // Focus the input and press Enter

tests/renderer/components/dialog-token-spec.tsx

@@ -131,7 +131,9 @@ describe('TokenDialog component', () => {
     });
 
     act(() => {
-      instance.handleChange({ target: { value: 'hi' } } as any);
+      instance.handleChange({
+        target: { value: 'hi' },
+      } as React.ChangeEvent<HTMLInputElement>);
     });
 
     expect(instance.state.tokenInput).toBe('hi');

tests/renderer/components/output-spec.tsx

@@ -1,37 +1,42 @@
-import * as React from 'react';
-
-import { render } from '@testing-library/react';
 import type * as MonacoType from 'monaco-editor';
-import { MosaicContext } from 'react-mosaic-component';
+import type { MosaicContext } from 'react-mosaic-component';
 import { beforeEach, describe, expect, it, vi } from 'vitest';
 
+import { renderClassComponentWithInstanceRef } from '../../../rtl-spec/test-utils/renderClassComponentWithInstanceRef';
 import { Output } from '../../../src/renderer/components/output';
+import { WrapperEditorId } from '../../../src/renderer/components/output-editors-wrapper';
 import { AppState } from '../../../src/renderer/state';
 import { MonacoMock } from '../../mocks/mocks';
 
-const mockMosaicActions = {
-  expand: vi.fn(),
-  remove: vi.fn(),
-  hide: vi.fn(),
-  replaceWith: vi.fn(),
-  updateTree: vi.fn(),
-  getRoot: vi.fn(),
-};
-
-const mockContextValue = {
-  mosaicActions: mockMosaicActions,
-  mosaicId: 'output',
-} as any;
-
-function renderOutput(store: AppState, monaco: typeof MonacoType) {
-  const ref = React.createRef<any>();
-  const renderResult = render(
-    <MosaicContext.Provider value={mockContextValue}>
-      <Output appState={store} monaco={monaco} monacoOptions={{}} ref={ref} />
-    </MosaicContext.Provider>,
+const mockContext = vi.hoisted(
+  () =>
+    ({
+      mosaicActions: {
+        expand: vi.fn(),
+        remove: vi.fn(),
+        hide: vi.fn(),
+        replaceWith: vi.fn(),
+        updateTree: vi.fn(),
+        getRoot: vi.fn(),
+      },
+      mosaicId: 'output',
+    }) as unknown as MosaicContext<WrapperEditorId>,
+);
+
+// Provide a default value for MosaicContext so that the component's
+// `static contextType = MosaicContext` picks up `mockContext` without
+// needing a Provider in the render tree.
+vi.mock('react-mosaic-component', async () => {
+  const React = await import('react');
+  const actual = await vi.importActual<typeof import('react-mosaic-component')>(
+    'react-mosaic-component',
   );
-  return { instance: ref.current!, renderResult };
-}
+
+  return {
+    ...actual,
+    MosaicContext: React.createContext(mockContext),
+  };
+});
 
 describe('Output component', () => {
   let store: AppState;
@@ -40,26 +45,40 @@ describe('Output component', () => {
   beforeEach(() => {
     monaco = window.monaco;
     ({ state: store } = window.app);
-    vi.clearAllMocks();
+    vi.mocked(mockContext.mosaicActions.replaceWith).mockClear();
+    vi.mocked(mockContext.mosaicActions.getRoot).mockReset();
   });
 
   it('renders the output container', () => {
-    const { renderResult } = renderOutput(store, monaco);
+    const { renderResult } = renderClassComponentWithInstanceRef(Output, {
+      appState: store,
+      monaco,
+      monacoOptions: {},
+    });
     const outputDiv = renderResult.container.querySelector('.output');
     expect(outputDiv).toBeInTheDocument();
     expect(outputDiv).toHaveStyle('display: inline-block');
   });
 
   it('correctly sets the language', () => {
-    const { instance } = renderOutput(store, monaco);
+    const { instance } = renderClassComponentWithInstanceRef(Output, {
+      appState: store,
+      monaco,
+      monacoOptions: {},
+    });
     expect(instance.language).toBe('consoleOutputLanguage');
   });
 
   describe('initMonaco()', () => {
     it('attempts to create an editor', async () => {
-      const { instance } = renderOutput(store, monaco);
-
-      instance.outputRef.current = 'ref' as any;
+      const { instance } = renderClassComponentWithInstanceRef(Output, {
+        appState: store,
+        monaco,
+        monacoOptions: {},
+      });
+
+      // outputRef is private — cast needed to test initMonaco directly
+      (instance as any).outputRef.current = 'ref';
       await instance.initMonaco();
 
       expect(monaco.editor.create).toHaveBeenCalled();
@@ -68,9 +87,14 @@ describe('Output component', () => {
   });
 
   it('componentWillUnmount() attempts to dispose the editor', async () => {
-    const { instance } = renderOutput(store, monaco);
+    const { instance } = renderClassComponentWithInstanceRef(Output, {
+      appState: store,
+      monaco,
+      monacoOptions: {},
+    });
 
-    instance.outputRef.current = 'ref' as any;
+    // outputRef is private
+    (instance as any).outputRef.current = 'ref';
     await instance.initMonaco();
     instance.componentWillUnmount();
 
@@ -80,22 +104,25 @@ describe('Output component', () => {
   });
 
   it('hides the console with react-mosaic-component', async () => {
-    const { instance } = renderOutput(store, monaco);
-
     // direction is required to be recognized as a valid root node
-    mockMosaicActions.getRoot.mockReturnValue({
+    vi.mocked(mockContext.mosaicActions.getRoot).mockReturnValue({
       splitPercentage: 25,
       direction: 'row',
+    } as ReturnType<typeof mockContext.mosaicActions.getRoot>);
+
+    const { instance } = renderClassComponentWithInstanceRef(Output, {
+      appState: store,
+      monaco,
+      monacoOptions: {},
     });
 
-    instance.outputRef.current = 'ref' as any;
+    // outputRef is private
+    (instance as any).outputRef.current = 'ref';
     await instance.initMonaco();
-
-    // Trigger toggleConsole explicitly
     instance.toggleConsole();
 
-    expect(mockMosaicActions.replaceWith).toHaveBeenCalled();
-    expect(mockMosaicActions.replaceWith).toHaveBeenCalledWith(
+    expect(mockContext.mosaicActions.replaceWith).toHaveBeenCalled();
+    expect(mockContext.mosaicActions.replaceWith).toHaveBeenCalledWith(
       [],
       expect.objectContaining({ splitPercentage: 25 }),
     );
@@ -114,12 +141,16 @@ describe('Output component', () => {
       },
     ];
 
-    const { instance } = renderOutput(store, monaco);
+    const { instance } = renderClassComponentWithInstanceRef(Output, {
+      appState: store,
+      monaco,
+      monacoOptions: {},
+    });
 
-    instance.outputRef.current = 'ref' as any;
+    // outputRef and updateModel are private
+    (instance as any).outputRef.current = 'ref';
     await instance.initMonaco();
-    // updateModel is private — cast needed to test it directly
-    (instance as any).updateModel();
+    await (instance as any).updateModel();
 
     expect(monaco.editor.createModel).toHaveBeenCalled();
     expect(instance.editor?.revealLine).toHaveBeenCalled();
@@ -133,15 +164,19 @@ describe('Output component', () => {
       },
     ];
 
-    const { instance } = renderOutput(store, monaco);
+    const { instance } = renderClassComponentWithInstanceRef(Output, {
+      appState: store,
+      monaco,
+      monacoOptions: {},
+    });
     // updateModel is private — cast needed to spy on it
     const spy = vi.spyOn(instance as any, 'updateModel');
 
-    instance.outputRef.current = 'ref' as any;
+    // outputRef is private
+    (instance as any).outputRef.current = 'ref';
     await instance.initMonaco();
 
-    // updateModel is private
-    (instance as any).updateModel();
+    await (instance as any).updateModel();
 
     // new output
     store.output = [
@@ -159,13 +194,17 @@ describe('Output component', () => {
   });
 
   it('handles componentDidUpdate', async () => {
-    const { instance } = renderOutput(store, monaco);
+    const { instance } = renderClassComponentWithInstanceRef(Output, {
+      appState: store,
+      monaco,
+      monacoOptions: {},
+    });
     const spy = vi.spyOn(instance, 'toggleConsole');
 
-    instance.outputRef.current = 'ref' as any;
+    // outputRef and updateModel are private
+    (instance as any).outputRef.current = 'ref';
     await instance.initMonaco();
 
-    // updateModel is private
     await (instance as any).updateModel();
     expect(spy).toHaveBeenCalled();
   });

tests/renderer/components/settings-electron-spec.tsx

@@ -292,10 +292,9 @@ describe('ElectronSettings component', () => {
       );
       store.hideChannels.mockImplementation((ids: ElectronReleaseChannel[]) =>
         runInAction(() => {
-          for (const id of ids) {
-            const idx = store.channelsToShow.indexOf(id);
-            if (idx !== -1) store.channelsToShow.splice(idx, 1);
-          }
+          store.channelsToShow = store.channelsToShow.filter(
+            (id: ElectronReleaseChannel) => !ids.includes(id),
+          );
         }),
       );
 
@@ -340,7 +339,7 @@ describe('ElectronSettings component', () => {
 
       const disabledVersions = container.querySelectorAll('.disabled-version');
       expect(disabledVersions).toHaveLength(1);
-      expect(disabledVersions[0]).toHaveClass('bp3-disabled');
+      expect(disabledVersions[0]).toHaveAttribute('disabled');
     });
   });
 });