Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -313,6 +313,11 @@
"category": "Coder",
"icon": "$(refresh)"
},
{
"command": "coder.speedTest",
"title": "Run Speed Test",
"category": "Coder"
},
{
"command": "coder.viewLogs",
"title": "Coder: View Logs",
Expand Down Expand Up @@ -371,6 +376,10 @@
"command": "coder.createWorkspace",
"when": "coder.authenticated"
},
{
"command": "coder.speedTest",
"when": "coder.workspace.connected"
},
{
"command": "coder.navigateToWorkspace",
"when": "coder.workspace.connected"
Expand Down
47 changes: 47 additions & 0 deletions src/commands.ts
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,53 @@ export class Commands {
this.logger.debug("Login complete to deployment:", url);
}

/**
* Run a speed test against the currently connected workspace and display the
* results in a new editor document.
*/
public async speedTest(): Promise<void> {
if (!this.workspace) {
vscode.window.showInformationMessage("No workspace connected.");
return;
}

await withProgress(
{
location: vscode.ProgressLocation.Notification,
title: "Running speed test...",
},
async () => {
Comment on lines +159 to +164
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we pass the cancellation token to the CLI here? Also possibly use the -t argument and mention that in the title: https://coder.com/docs/reference/cli/speedtest#-t---time

const baseUrl = this.requireExtensionBaseUrl();
const safeHost = toSafeHost(baseUrl);
const binary = await this.cliManager.fetchBinary(this.extensionClient);
const version = semver.parse(await cliUtils.version(binary));
const featureSet = featureSetForVersion(version);
const configDir = this.pathResolver.getGlobalConfigDir(safeHost);
const configs = vscode.workspace.getConfiguration();
const auth = resolveCliAuth(configs, featureSet, baseUrl, configDir);
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here we only pass the auth but not the global flags which might break if users have proxy rules or headers. See openAppStatus and how it uses getGlobalFlags

const workspaceName = createWorkspaceIdentifier(this.workspace!);
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should avoid these kinds of null assertions (I thought we had a rule for that actually 🤔). Also might make sense to capture this.workspace in a variable in case it changes during one of the await operations (and would make the null assertion unnecessary as well)


try {
const stdout = await cliUtils.speedtest(
binary,
auth,
workspaceName,
);
const doc = await vscode.workspace.openTextDocument({
content: stdout,
language: "json",
});
await vscode.window.showTextDocument(doc);
} catch (error) {
Comment on lines +181 to +186
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO I think we should show a save dialog instead of opening an untitled window.

We can show the save dialog then an info dialog that confirms it's been saved with a button to open the file in the editor

this.logger.error("Speed test failed", error);
vscode.window.showErrorMessage(
`Speed test failed: ${error instanceof Error ? error.message : String(error)}`,
);
}
},
);
}

/**
* View the logs for the currently connected workspace.
*/
Expand Down
30 changes: 30 additions & 0 deletions src/core/cliUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ import os from "node:os";
import path from "node:path";
import { promisify } from "node:util";

import type { CliAuth } from "../cliConfig";

/**
* Custom error thrown when a binary file is locked (typically on Windows).
*/
Expand Down Expand Up @@ -72,6 +74,34 @@ export async function version(binPath: string): Promise<string> {
return json.version;
}

/**
* Run a speed test against the specified workspace and return the JSON output.
* Throw if unable to execute the binary or parse the output.
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This here does not parse the output so this is inaccurate

*/
export async function speedtest(
binPath: string,
auth: CliAuth,
workspaceName: string,
): Promise<string> {
const result = await promisify(execFile)(binPath, [
...authArgs(auth),
"speedtest",
workspaceName,
"--output",
"json",
]);
return result.stdout;
}

/**
* Build CLI auth flags for execFile (no shell escaping).
*/
function authArgs(auth: CliAuth): string[] {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm pretty sure that this function exists elsewhere already, getGlobalFlags in the cli settings

return auth.mode === "url"
? ["--url", auth.url]
: ["--global-config", auth.configDir];
}

export interface RemovalResult {
fileName: string;
error: unknown;
Expand Down
4 changes: 4 additions & 0 deletions src/extension.ts
Original file line number Diff line number Diff line change
Expand Up @@ -281,6 +281,10 @@ export async function activate(ctx: vscode.ExtensionContext): Promise<void> {
void myWorkspacesProvider.fetchAndRefresh();
void allWorkspacesProvider.fetchAndRefresh();
}),
vscode.commands.registerCommand(
"coder.speedTest",
commands.speedTest.bind(commands),
),
vscode.commands.registerCommand(
"coder.viewLogs",
commands.viewLogs.bind(commands),
Expand Down
6 changes: 6 additions & 0 deletions test/fixtures/scripts/echo-args.bash
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
#!/usr/bin/env bash
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe make this a JS file and run it using node so it's cross-platform


# Prints each argument on its own line, so tests can verify exact args.
for arg in "$@"; do
echo "$arg"
done
57 changes: 57 additions & 0 deletions test/unit/core/cliUtils.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,63 @@ describe("CliUtils", () => {
]);
});

describe("speedtest", () => {
const echoArgsBin = path.join(tmp, "echo-args");

beforeAll(async () => {
const tmpl = await fs.readFile(
getFixturePath("scripts", "echo-args.bash"),
"utf8",
);
await fs.writeFile(echoArgsBin, tmpl);
await fs.chmod(echoArgsBin, "755");
});

it("passes global-config auth flags", async () => {
const result = await cliUtils.speedtest(
echoArgsBin,
{ mode: "global-config", configDir: "/tmp/test-config" },
"owner/workspace",
);
const args = result.trim().split("\n");
expect(args).toEqual([
"--global-config",
"/tmp/test-config",
"speedtest",
"owner/workspace",
"--output",
"json",
]);
});

it("passes url auth flags", async () => {
const result = await cliUtils.speedtest(
echoArgsBin,
{ mode: "url", url: "http://localhost:3000" },
"owner/workspace",
);
const args = result.trim().split("\n");
expect(args).toEqual([
"--url",
"http://localhost:3000",
"speedtest",
"owner/workspace",
"--output",
"json",
]);
});

it("throws when binary does not exist", async () => {
await expect(
cliUtils.speedtest(
"/nonexistent/binary",
{ mode: "global-config", configDir: "/tmp" },
"owner/workspace",
),
).rejects.toThrow("ENOENT");
});
});

it("ETag", async () => {
const binPath = path.join(tmp, "hash");

Expand Down