Skip to content

Commit 49e89c9

Browse files
fix: address remaining PR review feedback
- Add waitFor/act for pre-existing RTL tests that need them for MobX observer re-renders - Remove 103 unnecessary (instance as any) casts; use typed instance from renderClassComponentWithInstanceRef. Remaining 17 casts are commented (private methods or Readonly<S> bypass). - Fix no-op tests: bisect command tools, settings unknown page, dialogs settings — all now assert meaningful component output. - Restore dropped coverage: button disabled states, radio ordering, file-selected render path, renderItem text verification. - Remove 8 unnecessary act() wrappers around async methods that don't need synchronous state flush, matching rtl-spec/ convention. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent e351223 commit 49e89c9

14 files changed

Lines changed: 203 additions & 164 deletions

rtl-spec/components/commands-address-bar.spec.tsx

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import React from 'react';
22

3-
import { render } from '@testing-library/react';
3+
import { render, waitFor } from '@testing-library/react';
44
import { userEvent } from '@testing-library/user-event';
55
import { runInAction } from 'mobx';
66
import { beforeEach, describe, expect, it } from 'vitest';
@@ -76,7 +76,9 @@ describe('AddressBar component', () => {
7676
runInAction(() => {
7777
store.activeGistAction = action;
7878
});
79-
const btn = getByRole('button');
80-
expect(btn).toBeDisabled();
79+
await waitFor(() => {
80+
const btn = getByRole('button');
81+
expect(btn).toBeDisabled();
82+
});
8183
});
8284
});

rtl-spec/components/commands-publish-button.spec.tsx

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { Octokit } from '@octokit/rest';
2+
import { act, waitFor } from '@testing-library/react';
23
import { beforeEach, describe, expect, it, vi } from 'vitest';
34

45
import {
@@ -273,7 +274,7 @@ describe('Action button component', () => {
273274
// create a button that's primed to update gistId
274275
state.gistId = gistId;
275276
({ instance } = createActionButton());
276-
instance.setState({ actionType: GistActionType.update });
277+
act(() => instance.setState({ actionType: GistActionType.update }));
277278

278279
mocktokit.gists.get.mockImplementation(() => {
279280
return {
@@ -316,7 +317,7 @@ describe('Action button component', () => {
316317

317318
// create a button primed to delete gistId
318319
({ instance } = createActionButton());
319-
instance.setState({ actionType: GistActionType.delete });
320+
act(() => instance.setState({ actionType: GistActionType.delete }));
320321
});
321322

322323
it('attempts to delete an existing Gist', async () => {
@@ -350,10 +351,14 @@ describe('Action button component', () => {
350351
expect(container.querySelector('fieldset')).not.toBeDisabled();
351352

352353
state.activeGistAction = gistActionState;
353-
expect(container.querySelector('fieldset')).toBeDisabled();
354+
await waitFor(() => {
355+
expect(container.querySelector('fieldset')).toBeDisabled();
356+
});
354357

355358
state.activeGistAction = GistActionState.none;
356-
expect(container.querySelector('fieldset')).not.toBeDisabled();
359+
await waitFor(() => {
360+
expect(container.querySelector('fieldset')).not.toBeDisabled();
361+
});
357362
}
358363

359364
it('while publishing', async () => {

tests/renderer/components/commands-spec.tsx

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -54,9 +54,11 @@ describe('Commands component', () => {
5454
it('can show the bisect command tools', () => {
5555
store.isBisectCommandShowing = true;
5656
render(<Commands appState={store} />);
57-
// BisectHandler is not mocked, so it renders directly
58-
// We check for a bisect-related element in the DOM
59-
expect(document.querySelector('.commands')).toBeInTheDocument();
57+
// BisectHandler is not mocked, so it renders directly.
58+
// The mock store has Bisector set, so active bisect buttons appear.
59+
expect(
60+
screen.getByRole('button', { name: 'Cancel bisect' }),
61+
).toBeInTheDocument();
6062
});
6163

6264
it('handleDoubleClick()', () => {
@@ -65,6 +67,7 @@ describe('Commands component', () => {
6567
});
6668

6769
const tag = { tagName: 'DIV' };
70+
// handleDoubleClick is private — cast needed to test it directly
6871
(instance as any).handleDoubleClick({ target: tag, currentTarget: tag });
6972

7073
expect(window.ElectronFiddle.macTitlebarClicked).toHaveBeenCalled();
@@ -75,6 +78,7 @@ describe('Commands component', () => {
7578
appState: store,
7679
});
7780

81+
// handleDoubleClick is private — cast needed to test it directly
7882
(instance as any).handleDoubleClick({
7983
target: { tagName: 'INPUT' },
8084
currentTarget: { tagName: 'DIV' },

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

Lines changed: 47 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -40,14 +40,37 @@ describe('AddThemeDialog component', () => {
4040
({ state: store } = window.app);
4141
});
4242

43-
// TODO(dsanders11): Update this test to be accurate
4443
it('renders', () => {
4544
store.isThemeDialogShowing = true;
4645
render(<AddThemeDialog appState={store} />);
4746

4847
expect(screen.getByText('Add theme')).toBeInTheDocument();
4948
expect(screen.getByText('Add')).toBeInTheDocument();
5049
expect(screen.getByText('Cancel')).toBeInTheDocument();
50+
// Add button should be disabled when no file is selected
51+
expect(screen.getByRole('button', { name: 'Add' })).toBeDisabled();
52+
});
53+
54+
it('renders with a file selected', () => {
55+
store.isThemeDialogShowing = true;
56+
const { instance } = renderClassComponentWithInstanceRef(AddThemeDialog, {
57+
appState: store,
58+
});
59+
60+
act(() => {
61+
instance.setState({
62+
file: new FileMock(
63+
['{}'],
64+
'theme.json',
65+
'/test/theme.json',
66+
'application/json',
67+
),
68+
});
69+
});
70+
71+
// File name should be displayed and Add button should be enabled
72+
expect(screen.getByText('theme.json')).toBeInTheDocument();
73+
expect(screen.getByRole('button', { name: 'Add' })).not.toBeDisabled();
5174
});
5275

5376
describe('createNewThemeFromMonaco()', () => {
@@ -58,10 +81,7 @@ describe('AddThemeDialog component', () => {
5881
});
5982

6083
try {
61-
await (instance as any).createNewThemeFromMonaco(
62-
'',
63-
{} as LoadedFiddleTheme,
64-
);
84+
await instance.createNewThemeFromMonaco('', {} as LoadedFiddleTheme);
6585
} catch (err: any) {
6686
expect(err.message).toEqual(`Filename not found`);
6787
expect(window.ElectronFiddle.createThemeFile).toHaveBeenCalledTimes(0);
@@ -76,7 +96,7 @@ describe('AddThemeDialog component', () => {
7696
});
7797

7898
act(() => {
79-
(instance as any).setState({
99+
instance.setState({
80100
file: new FileMock(
81101
[JSON.stringify(defaultLight.editor)],
82102
'file.json',
@@ -91,10 +111,7 @@ describe('AddThemeDialog component', () => {
91111
file: themePath,
92112
} as LoadedFiddleTheme);
93113

94-
await (instance as any).createNewThemeFromMonaco(
95-
'testingLight',
96-
defaultLight,
97-
);
114+
await instance.createNewThemeFromMonaco('testingLight', defaultLight);
98115

99116
expect(window.ElectronFiddle.createThemeFile).toHaveBeenCalledWith(
100117
expect.objectContaining({
@@ -115,15 +132,13 @@ describe('AddThemeDialog component', () => {
115132
appState: store,
116133
});
117134

118-
(instance as any).createNewThemeFromMonaco = vi.fn();
119-
(instance as any).onClose = vi.fn();
135+
instance.createNewThemeFromMonaco = vi.fn();
136+
instance.onClose = vi.fn();
120137

121-
await (instance as any).onSubmit();
138+
await instance.onSubmit();
122139

123-
expect((instance as any).createNewThemeFromMonaco).toHaveBeenCalledTimes(
124-
0,
125-
);
126-
expect((instance as any).onClose).toHaveBeenCalledTimes(0);
140+
expect(instance.createNewThemeFromMonaco).toHaveBeenCalledTimes(0);
141+
expect(instance.onClose).toHaveBeenCalledTimes(0);
127142
});
128143

129144
it('loads a theme if a file is currently set', async () => {
@@ -140,19 +155,17 @@ describe('AddThemeDialog component', () => {
140155
);
141156
const spy = vi.spyOn(file, 'text');
142157
act(() => {
143-
(instance as any).setState({ file });
158+
instance.setState({ file });
144159
});
145160

146-
(instance as any).createNewThemeFromMonaco = vi.fn();
147-
(instance as any).onClose = vi.fn();
161+
instance.createNewThemeFromMonaco = vi.fn();
162+
instance.onClose = vi.fn();
148163

149-
await (instance as any).onSubmit();
164+
await instance.onSubmit();
150165

151166
expect(spy).toHaveBeenCalledTimes(1);
152-
expect((instance as any).createNewThemeFromMonaco).toHaveBeenCalledTimes(
153-
1,
154-
);
155-
expect((instance as any).onClose).toHaveBeenCalledTimes(1);
167+
expect(instance.createNewThemeFromMonaco).toHaveBeenCalledTimes(1);
168+
expect(instance.onClose).toHaveBeenCalledTimes(1);
156169
});
157170

158171
it('shows an error dialog for a malformed theme', async () => {
@@ -170,12 +183,12 @@ describe('AddThemeDialog component', () => {
170183
);
171184
const spy = vi.spyOn(file, 'text').mockResolvedValue('{}');
172185
act(() => {
173-
(instance as any).setState({ file });
186+
instance.setState({ file });
174187
});
175188

176-
(instance as any).onClose = vi.fn();
189+
instance.onClose = vi.fn();
177190

178-
await (instance as any).onSubmit();
191+
await instance.onSubmit();
179192

180193
expect(spy).toHaveBeenCalledTimes(1);
181194
expect(store.showErrorDialog).toHaveBeenCalledWith(
@@ -193,11 +206,11 @@ describe('AddThemeDialog component', () => {
193206

194207
const files = ['one', 'two'];
195208
act(() => {
196-
(instance as any).onChangeFile({
209+
instance.onChangeFile({
197210
target: { files } as unknown as EventTarget,
198211
} as React.FormEvent<HTMLInputElement>);
199212
});
200-
expect((instance as any).state.file).toBe(files[0]);
213+
expect(instance.state.file).toBe(files[0]);
201214
});
202215

203216
it('handles no input', () => {
@@ -206,12 +219,10 @@ describe('AddThemeDialog component', () => {
206219
appState: store,
207220
});
208221

209-
act(() => {
210-
(instance as any).onChangeFile({
211-
target: { files: null } as unknown as EventTarget,
212-
} as React.FormEvent<HTMLInputElement>);
213-
});
214-
expect((instance as any).state.file).toBeUndefined();
222+
instance.onChangeFile({
223+
target: { files: null } as unknown as EventTarget,
224+
} as React.FormEvent<HTMLInputElement>);
225+
expect(instance.state.file).toBeUndefined();
215226
});
216227
});
217228
});

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

Lines changed: 23 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ describe('AddVersionDialog component', () => {
2828
});
2929

3030
act(() => {
31-
(instance as any).setState({
31+
instance.setState({
3232
isValidVersion: true,
3333
isValidElectron: true,
3434
folderPath: mockFile,
@@ -40,18 +40,19 @@ describe('AddVersionDialog component', () => {
4040
expect(screen.getByText('Cancel')).toBeInTheDocument();
4141

4242
act(() => {
43-
(instance as any).setState({
43+
instance.setState({
4444
isValidVersion: false,
4545
isValidElectron: true,
4646
folderPath: mockFile,
4747
});
4848
});
4949

50-
// Still renders the dialog
50+
// Still renders the dialog, but Add button should be disabled
5151
expect(screen.getByText('Add local Electron build')).toBeInTheDocument();
52+
expect(screen.getByRole('button', { name: 'Add' })).toBeDisabled();
5253

5354
act(() => {
54-
(instance as any).setState({
55+
instance.setState({
5556
isValidVersion: true,
5657
isValidElectron: true,
5758
existingLocalVersion: {
@@ -99,13 +100,11 @@ describe('AddVersionDialog component', () => {
99100
isValidElectron: true,
100101
localName: 'Test',
101102
});
102-
await act(async () => {
103-
await (instance as any).selectLocalVersion();
104-
});
103+
await instance.selectLocalVersion();
105104

106-
expect((instance as any).state.isValidElectron).toBe(true);
107-
expect((instance as any).state.folderPath).toBe('/test/');
108-
expect((instance as any).state.localName).toBe('Test');
105+
expect(instance.state.isValidElectron).toBe(true);
106+
expect(instance.state.folderPath).toBe('/test/');
107+
expect(instance.state.localName).toBe('Test');
109108
});
110109
});
111110

@@ -118,12 +117,12 @@ describe('AddVersionDialog component', () => {
118117
);
119118

120119
act(() => {
121-
(instance as any).onChangeVersion({
120+
instance.onChangeVersion({
122121
target: { value: '3.3.3' },
123-
});
122+
} as any);
124123
});
125-
expect((instance as any).state.isValidVersion).toBe(true);
126-
expect((instance as any).state.version).toBe('3.3.3');
124+
expect(instance.state.isValidVersion).toBe(true);
125+
expect(instance.state.version).toBe('3.3.3');
127126
});
128127

129128
it('handles invalid input', () => {
@@ -134,16 +133,16 @@ describe('AddVersionDialog component', () => {
134133
);
135134

136135
act(() => {
137-
(instance as any).onChangeVersion({ target: { value: 'foo' } });
136+
instance.onChangeVersion({ target: { value: 'foo' } } as any);
138137
});
139-
expect((instance as any).state.isValidVersion).toBe(false);
140-
expect((instance as any).state.version).toBe('foo');
138+
expect(instance.state.isValidVersion).toBe(false);
139+
expect(instance.state.version).toBe('foo');
141140

142141
act(() => {
143-
(instance as any).onChangeVersion({ target: {} });
142+
instance.onChangeVersion({ target: {} } as any);
144143
});
145-
expect((instance as any).state.isValidVersion).toBe(false);
146-
expect((instance as any).state.version).toBe('');
144+
expect(instance.state.isValidVersion).toBe(false);
145+
expect(instance.state.version).toBe('');
147146
});
148147
});
149148

@@ -155,9 +154,7 @@ describe('AddVersionDialog component', () => {
155154
{ appState: store },
156155
);
157156

158-
await act(async () => {
159-
await (instance as any).onSubmit();
160-
});
157+
await instance.onSubmit();
161158

162159
expect(store.addLocalVersion).toHaveBeenCalledTimes(0);
163160
});
@@ -170,15 +167,13 @@ describe('AddVersionDialog component', () => {
170167
);
171168

172169
act(() => {
173-
(instance as any).setState({
170+
instance.setState({
174171
version: '3.3.3',
175172
folderPath: '/test/path',
176173
});
177174
});
178175

179-
await act(async () => {
180-
await (instance as any).onSubmit();
181-
});
176+
await instance.onSubmit();
182177

183178
expect(store.addLocalVersion).toHaveBeenCalledTimes(1);
184179
expect(store.addLocalVersion).toHaveBeenCalledWith(
@@ -197,7 +192,7 @@ describe('AddVersionDialog component', () => {
197192
);
198193

199194
act(() => {
200-
(instance as any).setState({
195+
instance.setState({
201196
isValidElectron: true,
202197
folderPath: '/test/path',
203198
version: '3.3.3',

0 commit comments

Comments
 (0)