Skip to content

Commit ba8794d

Browse files
jaaydenhjaaydenh
committed
fix: updates for PR review
1 parent 176f542 commit ba8794d

File tree

2 files changed

+73
-105
lines changed

2 files changed

+73
-105
lines changed

‎site/src/pages/CreateWorkspacePage/CreateWorkspacePageExperimental.test.tsx

Lines changed: 65 additions & 100 deletions
Original file line numberDiff line numberDiff line change
@@ -79,33 +79,28 @@ function createMockWebSocket(
7979

8080
addEventListener: <E extends keyof WebSocketEventMap>(
8181
eventType: E,
82-
callback: WebSocketEventMap[E],
82+
cb: (event: WebSocketEventMap[E]) => void,
8383
) => {
8484
if (closed) {
8585
return;
8686
}
8787

8888
const subscribers = store[eventType];
89-
const cb = callback as unknown as CallbackStore[E][0];
9089
if (!subscribers.includes(cb)) {
9190
subscribers.push(cb);
9291
}
9392
},
9493

9594
removeEventListener: <E extends keyof WebSocketEventMap>(
9695
eventType: E,
97-
callback: WebSocketEventMap[E],
96+
cb: (event: WebSocketEventMap[E]) => void,
9897
) => {
9998
if (closed) {
10099
return;
101100
}
102101

103-
const subscribers = store[eventType];
104-
const cb = callback as unknown as CallbackStore[E][0];
105-
if (subscribers.includes(cb)) {
106-
const updated = store[eventType].filter((c) => c !== cb);
107-
store[eventType] = updated as unknown as CallbackStore[E];
108-
}
102+
const updated = store[eventType].filter((c) => c !== cb);
103+
store[eventType] = updated as unknown as CallbackStore[E];
109104
},
110105

111106
close: () => {
@@ -189,25 +184,25 @@ const mockDynamicParametersResponseWithError: DynamicParametersResponse = {
189184
],
190185
};
191186

192-
const renderCreateWorkspacePageExperimental = (
193-
route = `/templates/${MockTemplate.name}/workspace`,
194-
) => {
195-
return renderWithAuth(<CreateWorkspacePageExperimental />, {
196-
route,
197-
path: "/templates/:template/workspace",
198-
extraRoutes: [
199-
{
200-
path: "/:username/:workspace",
201-
element: <div>Workspace Page</div>,
202-
},
203-
],
204-
});
205-
};
206-
207187
describe("CreateWorkspacePageExperimental", () => {
208188
let mockWebSocket: WebSocket;
209189
let publisher: MockPublisher;
210190

191+
const renderCreateWorkspacePageExperimental = (
192+
route = `/templates/${MockTemplate.name}/workspace`,
193+
) => {
194+
return renderWithAuth(<CreateWorkspacePageExperimental />, {
195+
route,
196+
path: "/templates/:template/workspace",
197+
extraRoutes: [
198+
{
199+
path: "/:username/:workspace",
200+
element: <div>Workspace Page</div>,
201+
},
202+
],
203+
});
204+
};
205+
211206
beforeEach(() => {
212207
jest.clearAllMocks();
213208

@@ -234,14 +229,12 @@ describe("CreateWorkspacePageExperimental", () => {
234229
callbacks.onClose();
235230
});
236231

237-
setTimeout(() => {
238-
publisher.publishOpen(new Event("open"));
239-
publisher.publishMessage(
240-
new MessageEvent("message", {
241-
data: JSON.stringify(mockDynamicParametersResponse),
242-
}),
243-
);
244-
}, 10);
232+
publisher.publishOpen(new Event("open"));
233+
publisher.publishMessage(
234+
new MessageEvent("message", {
235+
data: JSON.stringify(mockDynamicParametersResponse),
236+
}),
237+
);
245238

246239
return mockWebSocket;
247240
});
@@ -269,7 +262,7 @@ describe("CreateWorkspacePageExperimental", () => {
269262
);
270263

271264
await waitFor(() => {
272-
expect(screen.getByText("Instance Type")).toBeInTheDocument();
265+
expect(screen.getByText(/instance type/i)).toBeInTheDocument();
273266
expect(screen.getByText("CPU Count")).toBeInTheDocument();
274267
expect(screen.getByText("Enable Monitoring")).toBeInTheDocument();
275268
expect(screen.getByText("Tags")).toBeInTheDocument();
@@ -280,9 +273,7 @@ describe("CreateWorkspacePageExperimental", () => {
280273
renderCreateWorkspacePageExperimental();
281274
await waitForLoaderToBeRemoved();
282275

283-
await waitFor(() => {
284-
expect(screen.getByText("Instance Type")).toBeInTheDocument();
285-
});
276+
expect(screen.getByText(/instance type/i)).toBeInTheDocument();
286277

287278
expect(mockWebSocket.send).toBeDefined();
288279

@@ -381,18 +372,16 @@ describe("CreateWorkspacePageExperimental", () => {
381372
callbacks.onMessage(JSON.parse(event.data));
382373
});
383374

384-
setTimeout(() => {
385-
publisher.publishOpen(new Event("open"));
386-
publisher.publishMessage(
387-
new MessageEvent("message", {
388-
data: JSON.stringify({
389-
id: 0,
390-
parameters: [mockDropdownParameter],
391-
diagnostics: [],
392-
}),
375+
publisher.publishOpen(new Event("open"));
376+
publisher.publishMessage(
377+
new MessageEvent("message", {
378+
data: JSON.stringify({
379+
id: 0,
380+
parameters: [mockDropdownParameter],
381+
diagnostics: [],
393382
}),
394-
);
395-
}, 0);
383+
}),
384+
);
396385

397386
return mockWebSocket;
398387
});
@@ -411,20 +400,18 @@ describe("CreateWorkspacePageExperimental", () => {
411400
diagnostics: [],
412401
};
413402

414-
setTimeout(() => {
403+
await waitFor(() => {
415404
publisher.publishMessage(
416405
new MessageEvent("message", { data: JSON.stringify(response1) }),
417406
);
418407

419408
publisher.publishMessage(
420409
new MessageEvent("message", { data: JSON.stringify(response2) }),
421410
);
422-
}, 0);
423-
424-
await waitFor(() => {
425-
expect(screen.queryByText("CPU Count")).toBeInTheDocument();
426-
expect(screen.queryByText("Instance Type")).not.toBeInTheDocument();
427411
});
412+
413+
expect(screen.queryByText("CPU Count")).toBeInTheDocument();
414+
expect(screen.queryByText("Instance Type")).not.toBeInTheDocument();
428415
});
429416
});
430417

@@ -433,28 +420,18 @@ describe("CreateWorkspacePageExperimental", () => {
433420
renderCreateWorkspacePageExperimental();
434421
await waitForLoaderToBeRemoved();
435422

436-
await waitFor(() => {
437-
expect(screen.getByText("Instance Type")).toBeInTheDocument();
438-
expect(
439-
screen.getByRole("combobox", { name: /instance type/i }),
440-
).toBeInTheDocument();
441-
});
423+
expect(screen.getByText(/instance type/i)).toBeInTheDocument();
442424

443425
const select = screen.getByRole("combobox", { name: /instance type/i });
444426

445427
await waitFor(async () => {
446428
await userEvent.click(select);
447429
});
448430

449-
expect(
450-
screen.getByRole("option", { name: /t3\.micro/i }),
451-
).toBeInTheDocument();
452-
expect(
453-
screen.getByRole("option", { name: /t3\.small/i }),
454-
).toBeInTheDocument();
455-
expect(
456-
screen.getByRole("option", { name: /t3\.medium/i }),
457-
).toBeInTheDocument();
431+
// Each option appears in both the trigger and the dropdown
432+
expect(screen.getAllByText(/t3\.micro/i)).toHaveLength(2);
433+
expect(screen.getAllByText(/t3\.small/i)).toHaveLength(2);
434+
expect(screen.getAllByText(/t3\.medium/i)).toHaveLength(2);
458435
});
459436

460437
it("renders number parameter with slider", async () => {
@@ -539,13 +516,11 @@ describe("CreateWorkspacePageExperimental", () => {
539516
callbacks.onMessage(JSON.parse(event.data));
540517
});
541518

542-
setTimeout(() => {
543-
publisher.publishMessage(
544-
new MessageEvent("message", {
545-
data: JSON.stringify(mockDynamicParametersResponseWithError),
546-
}),
547-
);
548-
}, 10);
519+
publisher.publishMessage(
520+
new MessageEvent("message", {
521+
data: JSON.stringify(mockDynamicParametersResponseWithError),
522+
}),
523+
);
549524

550525
return mockWebSocket;
551526
});
@@ -603,28 +578,24 @@ describe("CreateWorkspacePageExperimental", () => {
603578
callbacks.onMessage(JSON.parse(event.data));
604579
});
605580

606-
setTimeout(() => {
607-
publisher.publishOpen(new Event("open"));
581+
publisher.publishOpen(new Event("open"));
608582

609-
publisher.publishMessage(
610-
new MessageEvent("message", {
611-
data: JSON.stringify(mockResponseInitial),
612-
}),
613-
);
614-
}, 10);
583+
publisher.publishMessage(
584+
new MessageEvent("message", {
585+
data: JSON.stringify(mockResponseInitial),
586+
}),
587+
);
615588

616589
const originalSend = socket.send;
617590
socket.send = jest.fn((data) => {
618591
originalSend.call(socket, data);
619592

620593
if (typeof data === "string" && data.includes('"200"')) {
621-
setTimeout(() => {
622-
publisher.publishMessage(
623-
new MessageEvent("message", {
624-
data: JSON.stringify(mockResponseWithError),
625-
}),
626-
);
627-
}, 10);
594+
publisher.publishMessage(
595+
new MessageEvent("message", {
596+
data: JSON.stringify(mockResponseWithError),
597+
}),
598+
);
628599
}
629600
});
630601

@@ -746,9 +717,7 @@ describe("CreateWorkspacePageExperimental", () => {
746717

747718
await waitForLoaderToBeRemoved();
748719

749-
await waitFor(() => {
750-
expect(screen.getByText("Instance Type")).toBeInTheDocument();
751-
});
720+
expect(screen.getByText(/instance type/i)).toBeInTheDocument();
752721

753722
await waitFor(() => {
754723
expect(screen.getByText("Create workspace")).toBeInTheDocument();
@@ -764,9 +733,7 @@ describe("CreateWorkspacePageExperimental", () => {
764733
renderCreateWorkspacePageExperimental();
765734
await waitForLoaderToBeRemoved();
766735

767-
await waitFor(() => {
768-
expect(screen.getByText("Instance Type")).toBeInTheDocument();
769-
});
736+
expect(screen.getByText(/instance type/i)).toBeInTheDocument();
770737

771738
const nameInput = screen.getByRole("textbox", {
772739
name: /workspace name/i,
@@ -813,10 +780,8 @@ describe("CreateWorkspacePageExperimental", () => {
813780
);
814781
await waitForLoaderToBeRemoved();
815782

816-
await waitFor(() => {
817-
expect(screen.getByText("Instance Type")).toBeInTheDocument();
818-
expect(screen.getByText("CPU Count")).toBeInTheDocument();
819-
});
783+
expect(screen.getByText(/instance type/i)).toBeInTheDocument();
784+
expect(screen.getByText("CPU Count")).toBeInTheDocument();
820785
});
821786

822787
it("uses custom template version when specified", async () => {

‎site/src/pages/CreateWorkspacePage/CreateWorkspacePageExperimental.tsx

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -271,16 +271,19 @@ const CreateWorkspacePageExperimental: FC = () => {
271271
return [...latestResponse.parameters].sort((a, b) => a.order - b.order);
272272
}, [latestResponse?.parameters]);
273273

274+
const shouldShowLoader =
275+
!templateQuery.data ||
276+
isLoadingFormData ||
277+
isLoadingExternalAuth ||
278+
autoCreateReady ||
279+
(!latestResponse && !wsError);
280+
274281
return (
275282
<>
276283
<Helmet>
277284
<title>{pageTitle(title)}</title>
278285
</Helmet>
279-
{(!latestResponse && !wsError) ||
280-
!templateQuery.data ||
281-
isLoadingFormData ||
282-
isLoadingExternalAuth ||
283-
autoCreateReady ? (
286+
{shouldShowLoader ? (
284287
<Loader />
285288
) : (
286289
<CreateWorkspacePageViewExperimental

0 commit comments

Comments
 (0)