Skip to content

Commit 7f07877

Browse files
committed
chore: clean up WorkspaceMonitor and Inbox tests
- Remove unnecessary dispose() calls from tests that don't test dispose - Combine two "before initial setup" tests into one - Import ContextManager type properly instead of inline import() - Improve test names to describe behavior more precisely
1 parent d70f829 commit 7f07877

File tree

3 files changed

+172
-220
lines changed

3 files changed

+172
-220
lines changed

test/mocks/testHelpers.ts

Lines changed: 41 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -9,13 +9,15 @@ import { vi } from "vitest";
99
import * as vscode from "vscode";
1010

1111
import type { Experiment, User } from "coder/site/src/api/typesGenerated";
12+
import type { WebSocketEventType } from "coder/site/src/utils/OneWayWebSocket";
1213
import type { IncomingMessage } from "node:http";
1314

1415
import type { CoderApi } from "@/api/coderApi";
1516
import type { CliCredentialManager } from "@/core/cliCredentialManager";
1617
import type { Logger } from "@/logging/logger";
1718
import type {
1819
EventHandler,
20+
EventPayloadMap,
1921
ParsedMessageEvent,
2022
UnidirectionalStream,
2123
} from "@/websocket/eventStreamConnection";
@@ -841,50 +843,63 @@ export class MockCancellationToken implements vscode.CancellationToken {
841843
}
842844

843845
/**
844-
* Mock event stream for testing UnidirectionalStream consumers.
846+
* Mock event stream that implements UnidirectionalStream directly.
847+
* Use pushMessage/pushError for common cases, or emit() for any event type.
845848
*/
846-
export class MockEventStream<T> {
849+
export class MockEventStream<T> implements UnidirectionalStream<T> {
850+
readonly url = "ws://test/mock-stream";
851+
readonly close = vi.fn();
852+
847853
private readonly handlers = new Map<
848854
string,
849-
Array<(...args: unknown[]) => void>
855+
Set<(...args: unknown[]) => void>
850856
>();
851857

852-
readonly stream: UnidirectionalStream<T> = {
853-
url: "ws://test/mock-stream",
854-
addEventListener: vi.fn(
855-
(event: string, callback: (...args: unknown[]) => void) => {
856-
if (!this.handlers.has(event)) {
857-
this.handlers.set(event, []);
858-
}
859-
this.handlers.get(event)!.push(callback);
860-
},
861-
),
862-
removeEventListener: vi.fn(),
863-
close: vi.fn(),
864-
};
858+
addEventListener<E extends WebSocketEventType>(
859+
event: E,
860+
callback: EventHandler<T, E>,
861+
): void {
862+
if (!this.handlers.has(event)) {
863+
this.handlers.set(event, new Set());
864+
}
865+
this.handlers.get(event)!.add(callback as (...args: unknown[]) => void);
866+
}
867+
868+
removeEventListener<E extends WebSocketEventType>(
869+
event: E,
870+
callback: EventHandler<T, E>,
871+
): void {
872+
this.handlers.get(event)?.delete(callback as (...args: unknown[]) => void);
873+
}
874+
875+
emit<E extends WebSocketEventType>(
876+
event: E,
877+
payload: EventPayloadMap<T>[E],
878+
): void {
879+
const handlers = this.handlers.get(event);
880+
if (handlers) {
881+
for (const handler of handlers) {
882+
handler(payload);
883+
}
884+
}
885+
}
865886

866887
pushMessage(parsedMessage: T): void {
867-
const event: ParsedMessageEvent<T> = {
888+
const payload: ParsedMessageEvent<T> = {
868889
sourceEvent: { data: undefined },
869890
parsedMessage,
870891
parseError: undefined,
871892
};
872-
this.fire("message", event);
893+
this.emit("message", payload);
873894
}
874895

875896
pushError(error: Error): void {
876-
const event: ParsedMessageEvent<T> = {
897+
const payload: ParsedMessageEvent<T> = {
877898
sourceEvent: { data: undefined },
878899
parsedMessage: undefined,
879900
parseError: error,
880901
};
881-
this.fire("message", event);
882-
}
883-
884-
private fire(event: string, payload: unknown): void {
885-
for (const handler of this.handlers.get(event) ?? []) {
886-
(handler as EventHandler<T, "message">)(payload as ParsedMessageEvent<T>);
887-
}
902+
this.emit("message", payload);
888903
}
889904
}
890905

test/unit/inbox.test.ts

Lines changed: 20 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -32,85 +32,76 @@ function createNotification(title: string): GetInboxNotificationResponse {
3232
};
3333
}
3434

35-
function createMockClient(
36-
stream: MockEventStream<GetInboxNotificationResponse>,
37-
) {
38-
return {
39-
watchInboxNotifications: vi.fn().mockResolvedValue(stream.stream),
40-
} as unknown as CoderApi;
41-
}
42-
4335
describe("Inbox", () => {
44-
let config: MockConfigurationProvider;
45-
4636
beforeEach(() => {
4737
vi.resetAllMocks();
48-
config = new MockConfigurationProvider();
4938
});
5039

51-
async function createInbox(
40+
async function setup(
5241
stream = new MockEventStream<GetInboxNotificationResponse>(),
5342
) {
54-
const ws = createWorkspace();
55-
const client = createMockClient(stream);
56-
const inbox = await Inbox.create(ws, client, createMockLogger());
57-
return { inbox, stream };
43+
const config = new MockConfigurationProvider();
44+
const inbox = await Inbox.create(
45+
createWorkspace(),
46+
{
47+
watchInboxNotifications: () => Promise.resolve(stream),
48+
} as unknown as CoderApi,
49+
createMockLogger(),
50+
);
51+
return { inbox, stream, config };
5852
}
5953

6054
describe("message handling", () => {
61-
it("shows notification when a message arrives", async () => {
62-
const { inbox, stream } = await createInbox();
55+
it("shows notification with the message title", async () => {
56+
const { stream } = await setup();
6357

6458
stream.pushMessage(createNotification("Out of memory"));
6559

6660
expect(vscode.window.showInformationMessage).toHaveBeenCalledWith(
6761
"Out of memory",
6862
);
69-
inbox.dispose();
7063
});
7164

72-
it("shows multiple notifications for successive messages", async () => {
73-
const { inbox, stream } = await createInbox();
65+
it("shows each notification independently (no dedup)", async () => {
66+
const { stream } = await setup();
7467

7568
stream.pushMessage(createNotification("First alert"));
7669
stream.pushMessage(createNotification("Second alert"));
7770

7871
expect(vscode.window.showInformationMessage).toHaveBeenCalledTimes(2);
79-
inbox.dispose();
8072
});
8173

8274
it("logs parse errors without showing notifications", async () => {
83-
const { inbox, stream } = await createInbox();
75+
const { stream } = await setup();
8476

8577
stream.pushError(new Error("bad json"));
8678

8779
expect(vscode.window.showInformationMessage).not.toHaveBeenCalled();
88-
inbox.dispose();
8980
});
9081

9182
it("closes the socket on dispose", async () => {
9283
const stream = new MockEventStream<GetInboxNotificationResponse>();
93-
const { inbox } = await createInbox(stream);
84+
const { inbox } = await setup(stream);
85+
9486
inbox.dispose();
9587

96-
expect(stream.stream.close).toHaveBeenCalled();
88+
expect(stream.close).toHaveBeenCalled();
9789
});
9890
});
9991

10092
describe("disableNotifications", () => {
10193
it("suppresses notifications when enabled", async () => {
94+
const { stream, config } = await setup();
10295
config.set("coder.disableNotifications", true);
103-
const { inbox, stream } = await createInbox();
10496

10597
stream.pushMessage(createNotification("Out of memory"));
10698

10799
expect(vscode.window.showInformationMessage).not.toHaveBeenCalled();
108-
inbox.dispose();
109100
});
110101

111102
it("shows notifications after re-enabling", async () => {
103+
const { stream, config } = await setup();
112104
config.set("coder.disableNotifications", true);
113-
const { inbox, stream } = await createInbox();
114105

115106
stream.pushMessage(createNotification("suppressed"));
116107
expect(vscode.window.showInformationMessage).not.toHaveBeenCalled();
@@ -121,7 +112,6 @@ describe("Inbox", () => {
121112
expect(vscode.window.showInformationMessage).toHaveBeenCalledWith(
122113
"visible",
123114
);
124-
inbox.dispose();
125115
});
126116
});
127117
});

0 commit comments

Comments
 (0)