From b81afc6fcc675558a16993f59a1a6545722996a6 Mon Sep 17 00:00:00 2001 From: Yazan Amer Date: Sun, 5 Apr 2026 14:01:08 +0300 Subject: [PATCH] feat: #750 Add speed test command + tests. Runs the speedtest command through the Coder Cli and opens a json for the user to view. --- eslint.config.mjs | 4 ++ package.json | 9 ++++ src/api/authInterceptor.ts | 6 ++- src/api/workspace.ts | 4 +- src/commands.ts | 77 +++++++++++++++++++++++++-- src/core/cliUtils.ts | 20 +++++++ src/extension.ts | 4 ++ src/remote/remote.ts | 8 +-- src/settings/cli.ts | 27 ++++++++-- src/util.ts | 2 +- test/fixtures/scripts/echo-args.js | 3 ++ test/unit/cliConfig.test.ts | 56 +++++++++++++++++--- test/unit/core/cliUtils.test.ts | 84 ++++++++++++++++++++++++++++++ 13 files changed, 282 insertions(+), 22 deletions(-) create mode 100644 test/fixtures/scripts/echo-args.js diff --git a/eslint.config.mjs b/eslint.config.mjs index 0c01a374..4cfcd965 100644 --- a/eslint.config.mjs +++ b/eslint.config.mjs @@ -19,6 +19,7 @@ export default defineConfig( "**/vite.config*.ts", "**/createWebviewConfig.ts", ".vscode-test/**", + "test/fixtures/scripts/**", ]), // Base ESLint recommended rules (for JS/TS/TSX files only) @@ -62,6 +63,7 @@ export default defineConfig( "error", { considerDefaultExhaustiveForUnions: true }, ], + "@typescript-eslint/no-non-null-assertion": "error", "@typescript-eslint/no-unused-vars": [ "error", { varsIgnorePattern: "^_", argsIgnorePattern: "^_" }, @@ -136,6 +138,8 @@ export default defineConfig( "@typescript-eslint/unbound-method": "off", // Empty callbacks are common in test stubs "@typescript-eslint/no-empty-function": "off", + // Test assertions often use non-null assertions for brevity + "@typescript-eslint/no-non-null-assertion": "off", // Test mocks often have loose typing - relax unsafe rules "@typescript-eslint/no-unsafe-assignment": "off", "@typescript-eslint/no-unsafe-call": "off", diff --git a/package.json b/package.json index bad26661..3719b9d8 100644 --- a/package.json +++ b/package.json @@ -319,6 +319,11 @@ "category": "Coder", "icon": "$(refresh)" }, + { + "command": "coder.speedTest", + "title": "Run Speed Test", + "category": "Coder" + }, { "command": "coder.viewLogs", "title": "Coder: View Logs", @@ -383,6 +388,10 @@ "command": "coder.createWorkspace", "when": "coder.authenticated" }, + { + "command": "coder.speedTest", + "when": "coder.workspace.connected" + }, { "command": "coder.navigateToWorkspace", "when": "coder.workspace.connected" diff --git a/src/api/authInterceptor.ts b/src/api/authInterceptor.ts index f1b0d386..ec181481 100644 --- a/src/api/authInterceptor.ts +++ b/src/api/authInterceptor.ts @@ -123,8 +123,12 @@ export class AuthInterceptor implements vscode.Disposable { return this.authRequiredPromise; } + if (!this.onAuthRequired) { + return false; + } + this.logger.debug("Triggering re-authentication"); - this.authRequiredPromise = this.onAuthRequired!(hostname); + this.authRequiredPromise = this.onAuthRequired(hostname); try { return await this.authRequiredPromise; diff --git a/src/api/workspace.ts b/src/api/workspace.ts index 95122cb9..698d212a 100644 --- a/src/api/workspace.ts +++ b/src/api/workspace.ts @@ -8,7 +8,7 @@ import { spawn } from "node:child_process"; import * as vscode from "vscode"; import { type FeatureSet } from "../featureSet"; -import { type CliAuth, getGlobalFlags } from "../settings/cli"; +import { getGlobalShellFlags, type CliAuth } from "../settings/cli"; import { escapeCommandArg } from "../util"; import { type UnidirectionalStream } from "../websocket/eventStreamConnection"; @@ -65,7 +65,7 @@ export async function startWorkspaceIfStoppedOrFailed( return new Promise((resolve, reject) => { const startArgs = [ - ...getGlobalFlags(vscode.workspace.getConfiguration(), auth), + ...getGlobalShellFlags(vscode.workspace.getConfiguration(), auth), "start", "--yes", createWorkspaceIdentifier(workspace), diff --git a/src/commands.ts b/src/commands.ts index 1791b3a7..81fc8a9b 100644 --- a/src/commands.ts +++ b/src/commands.ts @@ -21,13 +21,17 @@ import { toError } from "./error/errorUtils"; import { featureSetForVersion } from "./featureSet"; import { type Logger } from "./logging/logger"; import { type LoginCoordinator } from "./login/loginCoordinator"; -import { withProgress } from "./progress"; +import { withCancellableProgress, withProgress } from "./progress"; import { maybeAskAgent, maybeAskUrl } from "./promptUtils"; import { RECOMMENDED_SSH_SETTINGS, applySettingOverrides, } from "./remote/sshOverrides"; -import { getGlobalFlags, resolveCliAuth } from "./settings/cli"; +import { + getGlobalFlags, + getGlobalShellFlags, + resolveCliAuth, +} from "./settings/cli"; import { escapeCommandArg, toRemoteAuthority, toSafeHost } from "./util"; import { vscodeProposed } from "./vscodeProposed"; import { @@ -162,6 +166,73 @@ 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 { + const workspace = this.workspace; + if (!workspace) { + vscode.window.showInformationMessage("No workspace connected."); + return; + } + + const duration = await vscode.window.showInputBox({ + title: "Speed Test Duration", + prompt: "Duration for the speed test (e.g., 5s, 10s, 1m)", + value: "5s", + validateInput: (v) => { + return /^\d+[smh]$/.test(v.trim()) + ? null + : "Enter a duration like 5s, 10s, or 1m"; + }, + }); + if (duration === undefined) { + return; + } + + const result = await withCancellableProgress( + async ({ signal }) => { + 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); + const globalFlags = getGlobalFlags(configs, auth); + const workspaceName = createWorkspaceIdentifier(workspace); + + return cliUtils.speedtest(binary, globalFlags, workspaceName, { + signal, + duration: duration.trim(), + }); + }, + { + location: vscode.ProgressLocation.Notification, + title: `Running ${duration.trim()} speed test...`, + cancellable: true, + }, + ); + + if (!result.ok) { + if (!result.cancelled) { + this.logger.error("Speed test failed", result.error); + vscode.window.showErrorMessage( + `Speed test failed: ${result.error instanceof Error ? result.error.message : String(result.error)}`, + ); + } + return; + } + + const doc = await vscode.workspace.openTextDocument({ + content: result.value, + language: "json", + }); + vscode.window.showTextDocument(doc); + } + /** * View the logs for the currently connected workspace. */ @@ -505,7 +576,7 @@ export class Commands { const configDir = this.pathResolver.getGlobalConfigDir(safeHost); const configs = vscode.workspace.getConfiguration(); const auth = resolveCliAuth(configs, featureSet, baseUrl, configDir); - const globalFlags = getGlobalFlags(configs, auth); + const globalFlags = getGlobalShellFlags(configs, auth); terminal.sendText( `${escapeCommandArg(binary)} ${globalFlags.join(" ")} ssh ${app.workspace_name}`, ); diff --git a/src/core/cliUtils.ts b/src/core/cliUtils.ts index 4d2f7c55..db0283a1 100644 --- a/src/core/cliUtils.ts +++ b/src/core/cliUtils.ts @@ -72,6 +72,26 @@ export async function version(binPath: string): Promise { return json.version; } +/** + * Run a speed test against the specified workspace and return the raw output. + * Throw if unable to execute the binary. + */ +export async function speedtest( + binPath: string, + globalFlags: string[], + workspaceName: string, + options: { signal?: AbortSignal; duration?: string }, +): Promise { + const args = [...globalFlags, "speedtest", workspaceName, "--output", "json"]; + if (options.duration) { + args.push("-t", options.duration); + } + const result = await promisify(execFile)(binPath, args, { + signal: options.signal, + }); + return result.stdout; +} + export interface RemovalResult { fileName: string; error: unknown; diff --git a/src/extension.ts b/src/extension.ts index 7f84ff08..fbb02482 100644 --- a/src/extension.ts +++ b/src/extension.ts @@ -289,6 +289,10 @@ export async function activate(ctx: vscode.ExtensionContext): Promise { void myWorkspacesProvider.fetchAndRefresh(); void allWorkspacesProvider.fetchAndRefresh(); }), + vscode.commands.registerCommand( + "coder.speedTest", + commands.speedTest.bind(commands), + ), vscode.commands.registerCommand( "coder.viewLogs", commands.viewLogs.bind(commands), diff --git a/src/remote/remote.ts b/src/remote/remote.ts index e482725a..cdf71a0c 100644 --- a/src/remote/remote.ts +++ b/src/remote/remote.ts @@ -36,8 +36,8 @@ import { type LoginCoordinator } from "../login/loginCoordinator"; import { OAuthSessionManager } from "../oauth/sessionManager"; import { type CliAuth, - getGlobalFlags, getGlobalFlagsRaw, + getGlobalShellFlags, getSshFlags, resolveCliAuth, } from "../settings/cli"; @@ -674,7 +674,7 @@ export class Remote { const vscodeConfig = vscode.workspace.getConfiguration(); const escapedBinaryPath = escapeCommandArg(binaryPath); - const globalConfig = getGlobalFlags(vscodeConfig, cliAuth); + const globalConfig = getGlobalShellFlags(vscodeConfig, cliAuth); const logArgs = await this.getLogArgs(logDir); if (useWildcardSSH) { @@ -863,7 +863,9 @@ export class Remote { const titleMap = new Map(settings.map((s) => [s.setting, s.title])); return watchConfigurationChanges(settings, (changedSettings) => { - const changedTitles = changedSettings.map((s) => titleMap.get(s)!); + const changedTitles = changedSettings + .map((s) => titleMap.get(s)) + .filter((t) => t !== undefined); const message = changedTitles.length === 1 diff --git a/src/settings/cli.ts b/src/settings/cli.ts index 2dd8c275..23185139 100644 --- a/src/settings/cli.ts +++ b/src/settings/cli.ts @@ -21,17 +21,36 @@ export function getGlobalFlagsRaw( } /** - * Returns global configuration flags for Coder CLI commands. - * Includes either `--global-config` or `--url` depending on the auth mode. + * Returns global configuration flags for Coder CLI commands with auth values + * escaped for shell use (e.g., `terminal.sendText`, `spawn({ shell: true })`). + */ +export function getGlobalShellFlags( + configs: Pick, + auth: CliAuth, +): string[] { + return buildGlobalFlags(configs, auth, escapeCommandArg); +} + +/** + * Returns global configuration flags for Coder CLI commands with raw auth + * values suitable for `execFile` (no shell escaping). */ export function getGlobalFlags( configs: Pick, auth: CliAuth, +): string[] { + return buildGlobalFlags(configs, auth, (s) => s); +} + +function buildGlobalFlags( + configs: Pick, + auth: CliAuth, + esc: (s: string) => string, ): string[] { const authFlags = auth.mode === "url" - ? ["--url", escapeCommandArg(auth.url)] - : ["--global-config", escapeCommandArg(auth.configDir)]; + ? ["--url", esc(auth.url)] + : ["--global-config", esc(auth.configDir)]; const raw = getGlobalFlagsRaw(configs); const filtered = stripManagedFlags(raw); diff --git a/src/util.ts b/src/util.ts index f9aa549d..a0909f38 100644 --- a/src/util.ts +++ b/src/util.ts @@ -34,7 +34,7 @@ export function findPort(text: string): number | null { } // Get the last match, which is the most recent port. - const lastMatch = allMatches.at(-1)!; + const lastMatch = allMatches[allMatches.length - 1]; // Each capture group corresponds to a different Remote SSH extension log format: // [0] full match, [1] and [2] ms-vscode-remote.remote-ssh, // [3] windsurf/open-remote-ssh/antigravity, [4] anysphere.remote-ssh diff --git a/test/fixtures/scripts/echo-args.js b/test/fixtures/scripts/echo-args.js new file mode 100644 index 00000000..328b7bda --- /dev/null +++ b/test/fixtures/scripts/echo-args.js @@ -0,0 +1,3 @@ +/* eslint-env node */ +// Prints each argument on its own line, so tests can verify exact args. +process.argv.slice(2).forEach((arg) => console.log(arg)); diff --git a/test/unit/cliConfig.test.ts b/test/unit/cliConfig.test.ts index c8eded86..8013eac3 100644 --- a/test/unit/cliConfig.test.ts +++ b/test/unit/cliConfig.test.ts @@ -7,6 +7,7 @@ import { type CliAuth, getGlobalFlags, getGlobalFlagsRaw, + getGlobalShellFlags, getSshFlags, isKeyringEnabled, resolveCliAuth, @@ -23,7 +24,7 @@ const globalConfigAuth: CliAuth = { }; describe("cliConfig", () => { - describe("getGlobalFlags", () => { + describe("getGlobalShellFlags", () => { const urlAuth: CliAuth = { mode: "url", url: "https://dev.coder.com" }; interface AuthFlagsCase { @@ -47,7 +48,9 @@ describe("cliConfig", () => { "should return auth flags for $scenario", ({ auth, expectedAuthFlags }) => { const config = new MockConfigurationProvider(); - expect(getGlobalFlags(config, auth)).toStrictEqual(expectedAuthFlags); + expect(getGlobalShellFlags(config, auth)).toStrictEqual( + expectedAuthFlags, + ); }, ); @@ -58,7 +61,7 @@ describe("cliConfig", () => { "--disable-direct-connections", ]); - expect(getGlobalFlags(config, globalConfigAuth)).toStrictEqual([ + expect(getGlobalShellFlags(config, globalConfigAuth)).toStrictEqual([ "--verbose", "--disable-direct-connections", "--global-config", @@ -76,7 +79,7 @@ describe("cliConfig", () => { "--disable-direct-connections", ]); - expect(getGlobalFlags(config, globalConfigAuth)).toStrictEqual([ + expect(getGlobalShellFlags(config, globalConfigAuth)).toStrictEqual([ "--verbose", "--disable-direct-connections", "--global-config", @@ -112,12 +115,12 @@ describe("cliConfig", () => { const config = new MockConfigurationProvider(); config.set("coder.globalFlags", flags); - expect(getGlobalFlags(config, globalConfigAuth)).toStrictEqual([ + expect(getGlobalShellFlags(config, globalConfigAuth)).toStrictEqual([ "-v", "--global-config", '"/config/dir"', ]); - expect(getGlobalFlags(config, urlAuth)).toStrictEqual([ + expect(getGlobalShellFlags(config, urlAuth)).toStrictEqual([ "-v", "--url", '"https://dev.coder.com"', @@ -129,7 +132,7 @@ describe("cliConfig", () => { const config = new MockConfigurationProvider(); config.set("coder.globalFlags", ["--global-configs", "--use-keyrings"]); - expect(getGlobalFlags(config, globalConfigAuth)).toStrictEqual([ + expect(getGlobalShellFlags(config, globalConfigAuth)).toStrictEqual([ "--global-configs", "--use-keyrings", "--global-config", @@ -160,7 +163,7 @@ describe("cliConfig", () => { "--no-feature-warning", ]); - expect(getGlobalFlags(config, auth)).toStrictEqual([ + expect(getGlobalShellFlags(config, auth)).toStrictEqual([ "-v", "--header-command custom", // ignored by CLI "--no-feature-warning", @@ -172,6 +175,43 @@ describe("cliConfig", () => { ); }); + describe("getGlobalFlags", () => { + const urlAuth: CliAuth = { mode: "url", url: "https://dev.coder.com" }; + + it("should not escape auth flags", () => { + const config = new MockConfigurationProvider(); + expect(getGlobalFlags(config, globalConfigAuth)).toStrictEqual([ + "--global-config", + "/config/dir", + ]); + expect(getGlobalFlags(config, urlAuth)).toStrictEqual([ + "--url", + "https://dev.coder.com", + ]); + }); + + it("should still escape header-command flags", () => { + const config = new MockConfigurationProvider(); + config.set("coder.headerCommand", "echo test"); + expect(getGlobalFlags(config, globalConfigAuth)).toStrictEqual([ + "--global-config", + "/config/dir", + "--header-command", + quoteCommand("echo test"), + ]); + }); + + it("should include user global flags", () => { + const config = new MockConfigurationProvider(); + config.set("coder.globalFlags", ["--verbose"]); + expect(getGlobalFlags(config, globalConfigAuth)).toStrictEqual([ + "--verbose", + "--global-config", + "/config/dir", + ]); + }); + }); + describe("getGlobalFlagsRaw", () => { it("returns empty array when no global flags configured", () => { const config = new MockConfigurationProvider(); diff --git a/test/unit/core/cliUtils.test.ts b/test/unit/core/cliUtils.test.ts index dd1c56f0..2347b8b7 100644 --- a/test/unit/core/cliUtils.test.ts +++ b/test/unit/core/cliUtils.test.ts @@ -142,6 +142,90 @@ describe("CliUtils", () => { ]); }); + describe("speedtest", () => { + const echoArgsBin = isWindows() + ? path.join(tmp, "echo-args.cmd") + : path.join(tmp, "echo-args"); + + beforeAll(async () => { + const scriptPath = getFixturePath("scripts", "echo-args.js"); + if (isWindows()) { + await fs.writeFile(echoArgsBin, `@node "${scriptPath}" %*\r\n`); + } else { + const content = await fs.readFile(scriptPath, "utf8"); + await fs.writeFile(echoArgsBin, `#!/usr/bin/env node\n${content}`); + await fs.chmod(echoArgsBin, "755"); + } + }); + + it("passes global flags", async () => { + const result = await cliUtils.speedtest( + echoArgsBin, + ["--global-config", "/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 flags", async () => { + const result = await cliUtils.speedtest( + echoArgsBin, + ["--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("passes duration flag", async () => { + const result = await cliUtils.speedtest( + echoArgsBin, + ["--url", "http://localhost:3000"], + "owner/workspace", + { duration: "10s" }, + ); + const args = result.trim().split("\n"); + expect(args).toEqual([ + "--url", + "http://localhost:3000", + "speedtest", + "owner/workspace", + "--output", + "json", + "-t", + "10s", + ]); + }); + + it("throws when binary does not exist", async () => { + await expect( + cliUtils.speedtest( + "/nonexistent/binary", + ["--global-config", "/tmp"], + "owner/workspace", + {}, + ), + ).rejects.toThrow("ENOENT"); + }); + }); + it("ETag", async () => { const binPath = path.join(tmp, "hash");