-
Notifications
You must be signed in to change notification settings - Fork 41
Add speedtest command #864
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -20,17 +20,37 @@ 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 getGlobalFlags( | ||
| configs: Pick<WorkspaceConfiguration, "get">, | ||
| auth: CliAuth, | ||
| ): string[] { | ||
| return buildGlobalFlags(configs, auth, true); | ||
| } | ||
|
|
||
| /** | ||
| * Returns global configuration flags for Coder CLI commands with raw auth | ||
| * values suitable for `execFile` (no shell escaping). | ||
| */ | ||
| export function getGlobalFlagsForExec( | ||
| configs: Pick<WorkspaceConfiguration, "get">, | ||
| auth: CliAuth, | ||
| ): string[] { | ||
| return buildGlobalFlags(configs, auth, false); | ||
| } | ||
|
|
||
| function buildGlobalFlags( | ||
| configs: Pick<WorkspaceConfiguration, "get">, | ||
| auth: CliAuth, | ||
| escapeAuth: boolean, | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would prefer we pass the escape method itself here instead of a boolean |
||
| ): string[] { | ||
| const esc = escapeAuth ? escapeCommandArg : (s: string) => s; | ||
| 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); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,13 +3,18 @@ import { | |
| type WorkspaceAgent, | ||
| } from "coder/site/src/api/typesGenerated"; | ||
| import * as fs from "node:fs/promises"; | ||
| import * as os from "node:os"; | ||
| import * as path from "node:path"; | ||
| import * as semver from "semver"; | ||
| import * as vscode from "vscode"; | ||
|
|
||
| import { createWorkspaceIdentifier, extractAgents } from "./api/api-helper"; | ||
| import { type CoderApi } from "./api/coderApi"; | ||
| import { getGlobalFlags, resolveCliAuth } from "./cliConfig"; | ||
| import { | ||
| getGlobalFlags, | ||
| getGlobalFlagsForExec, | ||
| resolveCliAuth, | ||
| } from "./cliConfig"; | ||
| import { type CliManager } from "./core/cliManager"; | ||
| import * as cliUtils from "./core/cliUtils"; | ||
| import { type ServiceContainer } from "./core/container"; | ||
|
|
@@ -22,7 +27,7 @@ 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, | ||
|
|
@@ -146,6 +151,91 @@ export class Commands { | |
| this.logger.debug("Login complete to deployment:", url); | ||
| } | ||
|
|
||
| /** | ||
| * Run a speed test against the currently connected workspace and save the | ||
| * results to a file chosen by the user. | ||
| */ | ||
| public async speedTest(): Promise<void> { | ||
| 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+[sm]$/.test(v.trim()) | ||
| ? null | ||
| : "Enter a duration like 5s, 10s, or 1m"; | ||
|
Comment on lines
+170
to
+172
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Does this only take seconds and minutes 🤔 ? |
||
| }, | ||
| }); | ||
| 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); | ||
yazan-abu-obaideh marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| const globalFlags = getGlobalFlagsForExec(configs, auth); | ||
| const workspaceName = createWorkspaceIdentifier(workspace); | ||
|
|
||
| return cliUtils.speedtest(binary, globalFlags, workspaceName, { | ||
| signal, | ||
| duration: duration.trim(), | ||
| }); | ||
| }, | ||
| { | ||
| location: vscode.ProgressLocation.Notification, | ||
| title: `Running speed test (${duration.trim()})...`, | ||
| 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 defaultName = `speedtest-${workspace.name}-${new Date().toISOString().slice(0, 10)}.json`; | ||
| const defaultUri = vscode.Uri.joinPath( | ||
| vscode.workspace.workspaceFolders?.[0]?.uri ?? | ||
| vscode.Uri.file(os.homedir()), | ||
| defaultName, | ||
| ); | ||
| const uri = await vscode.window.showSaveDialog({ | ||
| defaultUri, | ||
| filters: { JSON: ["json"] }, | ||
| }); | ||
| if (!uri) { | ||
| return; | ||
| } | ||
|
|
||
| await vscode.workspace.fs.writeFile(uri, Buffer.from(result.value)); | ||
| const action = await vscode.window.showInformationMessage( | ||
| "Speed test results saved.", | ||
| "Open File", | ||
| ); | ||
| if (action === "Open File") { | ||
|
Comment on lines
+229
to
+233
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This might be removed but we should never |
||
| const doc = await vscode.workspace.openTextDocument(uri); | ||
| await vscode.window.showTextDocument(doc); | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * View the logs for the currently connected workspace. | ||
| */ | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -72,6 +72,26 @@ export async function version(binPath: string): Promise<string> { | |
| 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 }, | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Options are always passed no? |
||
| ): Promise<string> { | ||
| 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; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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)); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -142,6 +142,87 @@ 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"); | ||
| } | ||
|
Comment on lines
+152
to
+158
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we need this? Can't we execute the JS file the same way? |
||
| }); | ||
|
|
||
| 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"); | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about we rename those to be clearer to something like this:
execFilespawn({ shell: true }),terminal.sendText, and SSHProxyCommand