Skip to content

Commit 77214b6

Browse files
committed
refactor: simplify shared workspaces provider and tests
- collapse per-query branches into a readonly WORKSPACE_QUERY_CONFIG table - drop the unreachable re-entry branch in the workspace refresh loop - remove test-only getCurrentUserId; tests read getSnapshot via a helper - extract the repeated tree-view wiring in extension.ts into a helper - drive metadata tests through the real watcher over MockEventStream - move shared test doubles (session, workspaces client, flush) into testHelpers - replace describe-scoped state + beforeEach/afterEach with a setup() helper
1 parent adfc6d0 commit 77214b6

6 files changed

Lines changed: 336 additions & 394 deletions

File tree

src/deployment/deploymentManager.ts

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -109,12 +109,6 @@ export class DeploymentManager
109109
return this.#session.deployment;
110110
}
111111

112-
public getCurrentUserId(): string | undefined {
113-
return this.#session.kind === "signedIn"
114-
? this.#session.user.id
115-
: undefined;
116-
}
117-
118112
public getSnapshot(): WorkspaceSessionSnapshot {
119113
if (this.#session.kind === "signedIn") {
120114
return {

src/extension.ts

Lines changed: 20 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -188,44 +188,28 @@ async function doActivate(
188188

189189
// createTreeView, unlike registerTreeDataProvider, gives us the tree view API
190190
// (so we can see when it is visible) but otherwise they have the same effect.
191-
const myWsTree = vscode.window.createTreeView(MY_WORKSPACES_TREE_ID, {
192-
treeDataProvider: myWorkspacesProvider,
193-
});
194-
ctx.subscriptions.push(myWsTree);
195-
myWorkspacesProvider.setVisibility(myWsTree.visible);
196-
myWsTree.onDidChangeVisibility(
197-
(event) => {
198-
myWorkspacesProvider.setVisibility(event.visible);
199-
},
200-
undefined,
201-
ctx.subscriptions,
202-
);
203-
204-
const sharedWsTree = vscode.window.createTreeView(SHARED_WORKSPACES_TREE_ID, {
205-
treeDataProvider: sharedWorkspacesProvider,
206-
});
207-
ctx.subscriptions.push(sharedWsTree);
208-
sharedWorkspacesProvider.setVisibility(sharedWsTree.visible);
209-
sharedWsTree.onDidChangeVisibility(
210-
(event) => {
211-
sharedWorkspacesProvider.setVisibility(event.visible);
212-
},
213-
undefined,
214-
ctx.subscriptions,
215-
);
191+
const registerWorkspaceTreeView = (
192+
viewId: string,
193+
provider: WorkspaceProvider,
194+
) => {
195+
const tree = vscode.window.createTreeView(viewId, {
196+
treeDataProvider: provider,
197+
});
198+
ctx.subscriptions.push(tree);
199+
provider.setVisibility(tree.visible);
200+
tree.onDidChangeVisibility(
201+
(event) => provider.setVisibility(event.visible),
202+
undefined,
203+
ctx.subscriptions,
204+
);
205+
};
216206

217-
const allWsTree = vscode.window.createTreeView(ALL_WORKSPACES_TREE_ID, {
218-
treeDataProvider: allWorkspacesProvider,
219-
});
220-
ctx.subscriptions.push(allWsTree);
221-
allWorkspacesProvider.setVisibility(allWsTree.visible);
222-
allWsTree.onDidChangeVisibility(
223-
(event) => {
224-
allWorkspacesProvider.setVisibility(event.visible);
225-
},
226-
undefined,
227-
ctx.subscriptions,
207+
registerWorkspaceTreeView(MY_WORKSPACES_TREE_ID, myWorkspacesProvider);
208+
registerWorkspaceTreeView(
209+
SHARED_WORKSPACES_TREE_ID,
210+
sharedWorkspacesProvider,
228211
);
212+
registerWorkspaceTreeView(ALL_WORKSPACES_TREE_ID, allWorkspacesProvider);
229213

230214
// Register globally available commands. Many of these have visibility
231215
// controlled by contexts, see `when` in the package.json.

src/workspace/workspacesProvider.ts

Lines changed: 38 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,31 @@ export enum WorkspaceQuery {
3131
Shared = "shared:true",
3232
}
3333

34+
/** Per-view rendering behavior, keyed by workspace query. */
35+
interface WorkspaceQueryConfig {
36+
readonly showOwner: boolean;
37+
readonly showMetadata: boolean;
38+
readonly excludeOwn: boolean;
39+
}
40+
41+
const WORKSPACE_QUERY_CONFIG = {
42+
[WorkspaceQuery.Mine]: {
43+
showOwner: false,
44+
showMetadata: true,
45+
excludeOwn: false,
46+
},
47+
[WorkspaceQuery.All]: {
48+
showOwner: true,
49+
showMetadata: false,
50+
excludeOwn: false,
51+
},
52+
[WorkspaceQuery.Shared]: {
53+
showOwner: true,
54+
showMetadata: false,
55+
excludeOwn: true,
56+
},
57+
} as const satisfies Record<WorkspaceQuery, WorkspaceQueryConfig>;
58+
3459
export interface WorkspaceProviderOptions {
3560
readonly refreshIntervalMs?: number;
3661
}
@@ -53,6 +78,7 @@ export class WorkspaceProvider
5378
AgentMetadataWatcher
5479
>();
5580
private readonly sessionChangeDisposable: vscode.Disposable;
81+
private readonly config: WorkspaceQueryConfig;
5682
private timeout: NodeJS.Timeout | undefined;
5783
private fetching = false;
5884
private refreshQueued = false;
@@ -66,6 +92,7 @@ export class WorkspaceProvider
6692
private readonly sessionState: WorkspaceSessionState,
6793
private readonly options: WorkspaceProviderOptions = {},
6894
) {
95+
this.config = WORKSPACE_QUERY_CONFIG[getWorkspacesQuery];
6996
this.sessionChangeDisposable = this.sessionState.onDidChange(() => {
7097
this.clear();
7198
this.requestRefresh();
@@ -121,13 +148,11 @@ export class WorkspaceProvider
121148
this.setWorkspaces([]);
122149
}
123150

124-
shouldScheduleRefresh = !hadError && !this.refreshQueued;
151+
shouldScheduleRefresh = !hadError;
125152
}
126153
} finally {
127154
this.fetching = false;
128-
if (this.refreshQueued && !this.disposed && this.visible) {
129-
void this.runRefreshLoop();
130-
} else if (shouldScheduleRefresh && !this.disposed && this.visible) {
155+
if (shouldScheduleRefresh && !this.disposed && this.visible) {
131156
this.maybeScheduleRefresh();
132157
}
133158
}
@@ -176,8 +201,7 @@ export class WorkspaceProvider
176201
// TODO: I think it might make more sense for the tree items to contain
177202
// their own watchers, rather than recreate the tree items every time and
178203
// have this separate map held outside the tree.
179-
const showMetadata = this.getWorkspacesQuery === WorkspaceQuery.Mine;
180-
if (showMetadata) {
204+
if (this.config.showMetadata) {
181205
const agents = extractAllAgents(workspaces);
182206
for (const agent of agents) {
183207
// If we have an existing watcher, re-use it.
@@ -204,22 +228,26 @@ export class WorkspaceProvider
204228
}
205229
}
206230

207-
const showOwner = this.getWorkspacesQuery !== WorkspaceQuery.Mine;
208-
209231
// Create tree items for each workspace
210232
return workspaces.map(
211233
(workspace: Workspace) =>
212-
new WorkspaceTreeItem(workspace, showOwner, showMetadata),
234+
new WorkspaceTreeItem(
235+
workspace,
236+
this.config.showOwner,
237+
this.config.showMetadata,
238+
),
213239
);
214240
}
215241

216242
private filterWorkspaces(
217243
workspaces: readonly Workspace[],
218244
session: Extract<WorkspaceSessionSnapshot, { kind: "signedIn" }>,
219245
): readonly Workspace[] {
220-
if (this.getWorkspacesQuery !== WorkspaceQuery.Shared) {
246+
if (!this.config.excludeOwn) {
221247
return workspaces;
222248
}
249+
// `shared:true` also returns workspaces we own and shared out; drop them
250+
// to leave only those shared with us.
223251
return workspaces.filter(
224252
(workspace) => workspace.owner_id !== session.userId,
225253
);

test/mocks/testHelpers.ts

Lines changed: 110 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,10 +12,15 @@ import * as vscode from "vscode";
1212
import { createTestTelemetryService } from "./telemetry";
1313
import { window as vscodeWindow } from "./vscode.runtime";
1414

15-
import type { Experiment, User } from "coder/site/src/api/typesGenerated";
15+
import type {
16+
Experiment,
17+
User,
18+
Workspace,
19+
} from "coder/site/src/api/typesGenerated";
1620
import type { WebSocketEventType } from "coder/site/src/utils/OneWayWebSocket";
1721
import type { IncomingMessage } from "node:http";
1822

23+
import type { AgentMetadataEvent } from "@/api/api-helper";
1924
import type { CoderApi } from "@/api/coderApi";
2025
import type { CliCredentialManager } from "@/core/cliCredentialManager";
2126
import type { ServiceContainer } from "@/core/container";
@@ -31,6 +36,10 @@ import type {
3136
ParsedMessageEvent,
3237
UnidirectionalStream,
3338
} from "@/websocket/eventStreamConnection";
39+
import type {
40+
WorkspaceSessionSnapshot,
41+
WorkspaceSessionState,
42+
} from "@/workspace/session";
3443

3544
/**
3645
* Subset of `ContextManager`'s public API that mocks (e.g. `MockContextManager`)
@@ -461,6 +470,21 @@ export function createMockLogger(): Logger {
461470
};
462471
}
463472

473+
/** Resolve once pending microtasks and the macrotask queue have drained. */
474+
export async function flush(): Promise<void> {
475+
await new Promise((resolve) => setImmediate(resolve));
476+
}
477+
478+
/**
479+
* Drain only the microtask queue. Use instead of flush() under fake timers,
480+
* which leave the macrotask queue (setImmediate) untouched.
481+
*/
482+
export async function flushPromises(): Promise<void> {
483+
await Promise.resolve();
484+
await Promise.resolve();
485+
await Promise.resolve();
486+
}
487+
464488
/**
465489
* Backdate a file's atime/mtime by `daysAgo` days so tests can exercise the
466490
* support-bundle age cutoff without waiting.
@@ -1184,3 +1208,88 @@ export class MockTerminalOutputChannel {
11841208
this.write.mockClear();
11851209
}
11861210
}
1211+
1212+
/** Default user id reported by MockWorkspaceSessionState's signed-in snapshot. */
1213+
export const TEST_CURRENT_USER_ID = "current-user";
1214+
1215+
/** In-memory WorkspaceSessionState double with sign-in/out controls. */
1216+
export class MockWorkspaceSessionState implements WorkspaceSessionState {
1217+
private revision = 0;
1218+
private readonly emitter =
1219+
new vscode.EventEmitter<WorkspaceSessionSnapshot>();
1220+
private snapshot: WorkspaceSessionSnapshot = {
1221+
kind: "signedIn",
1222+
revision: 0,
1223+
userId: TEST_CURRENT_USER_ID,
1224+
};
1225+
1226+
readonly onDidChange = this.emitter.event;
1227+
1228+
getSnapshot(): WorkspaceSessionSnapshot {
1229+
return this.snapshot;
1230+
}
1231+
1232+
signIn(userId = TEST_CURRENT_USER_ID): void {
1233+
this.revision += 1;
1234+
this.snapshot = { kind: "signedIn", revision: this.revision, userId };
1235+
this.emitter.fire(this.snapshot);
1236+
}
1237+
1238+
signOut(): void {
1239+
this.revision += 1;
1240+
this.snapshot = { kind: "signedOut", revision: this.revision };
1241+
this.emitter.fire(this.snapshot);
1242+
}
1243+
}
1244+
1245+
interface WorkspacesResponse {
1246+
workspaces: readonly Workspace[];
1247+
count: number;
1248+
}
1249+
1250+
/**
1251+
* Stands in for CoderApi at the boundaries WorkspaceProvider touches: the
1252+
* workspaces query and the per-agent metadata socket. Program responses with
1253+
* respondOnce()/pending(); metadata flows through a real MockEventStream so
1254+
* tests drive the production watcher rather than a stub.
1255+
*/
1256+
export class MockWorkspacesClient {
1257+
baseURL: string | undefined = "https://coder.example.com";
1258+
readonly metadataStreams = new Map<
1259+
string,
1260+
MockEventStream<{ data: AgentMetadataEvent[] }>
1261+
>();
1262+
1263+
readonly getWorkspaces = vi.fn(
1264+
(_req: { q: string }): Promise<WorkspacesResponse> =>
1265+
Promise.resolve({ workspaces: [], count: 0 }),
1266+
);
1267+
1268+
watchAgentMetadata(agentId: string) {
1269+
const stream = new MockEventStream<{ data: AgentMetadataEvent[] }>();
1270+
this.metadataStreams.set(agentId, stream);
1271+
return Promise.resolve(stream);
1272+
}
1273+
1274+
getAxiosInstance() {
1275+
return { defaults: { baseURL: this.baseURL } };
1276+
}
1277+
1278+
/** Resolve the next getWorkspaces call with these workspaces. */
1279+
respondOnce(workspaces: readonly Workspace[]): void {
1280+
this.getWorkspaces.mockResolvedValueOnce({
1281+
workspaces,
1282+
count: workspaces.length,
1283+
});
1284+
}
1285+
1286+
/** Make the next getWorkspaces call hang until the returned resolve() runs. */
1287+
pending(): { resolve: (workspaces: readonly Workspace[]) => void } {
1288+
const { promise, resolve } = Promise.withResolvers<WorkspacesResponse>();
1289+
this.getWorkspaces.mockReturnValueOnce(promise);
1290+
return {
1291+
resolve: (workspaces) =>
1292+
resolve({ workspaces, count: workspaces.length }),
1293+
};
1294+
}
1295+
}

0 commit comments

Comments
 (0)