Skip to content

Commit d7462dc

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 4e1cbd3 commit d7462dc

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 { 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";
@@ -836,50 +838,63 @@ export class MockCancellationToken implements vscode.CancellationToken {
836838
}
837839

838840
/**
839-
* Mock event stream for testing UnidirectionalStream consumers.
841+
* Mock event stream that implements UnidirectionalStream directly.
842+
* Use pushMessage/pushError for common cases, or emit() for any event type.
840843
*/
841-
export class MockEventStream<T> {
844+
export class MockEventStream<T> implements UnidirectionalStream<T> {
845+
readonly url = "ws://test/mock-stream";
846+
readonly close = vi.fn();
847+
842848
private readonly handlers = new Map<
843849
string,
844-
Array<(...args: unknown[]) => void>
850+
Set<(...args: unknown[]) => void>
845851
>();
846852

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

861882
pushMessage(parsedMessage: T): void {
862-
const event: ParsedMessageEvent<T> = {
883+
const payload: ParsedMessageEvent<T> = {
863884
sourceEvent: { data: undefined },
864885
parsedMessage,
865886
parseError: undefined,
866887
};
867-
this.fire("message", event);
888+
this.emit("message", payload);
868889
}
869890

870891
pushError(error: Error): void {
871-
const event: ParsedMessageEvent<T> = {
892+
const payload: ParsedMessageEvent<T> = {
872893
sourceEvent: { data: undefined },
873894
parsedMessage: undefined,
874895
parseError: error,
875896
};
876-
this.fire("message", event);
877-
}
878-
879-
private fire(event: string, payload: unknown): void {
880-
for (const handler of this.handlers.get(event) ?? []) {
881-
(handler as EventHandler<T, "message">)(payload as ParsedMessageEvent<T>);
882-
}
897+
this.emit("message", payload);
883898
}
884899
}
885900

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)