Add speedtest command by yazan-abu-obaideh · Pull Request #864 · coder/vscode-coder

yazan-abu-obaideh · 2026-03-27T13:57:03Z

Closes #750

EhabY

Thanks for contributing to this repo, appreciated ♥️

Also make sure to run format/lint:fix (I think the pipeline would fail now)

EhabY · 2026-03-29T11:15:34Z

src/core/cliUtils.ts

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


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

test/fixtures/scripts/echo-args.bash

src/commands.ts

EhabY · 2026-03-29T11:33:28Z

src/commands.ts

+				const configDir = this.pathResolver.getGlobalConfigDir(safeHost);
+				const configs = vscode.workspace.getConfiguration();
+				const auth = resolveCliAuth(configs, featureSet, baseUrl, configDir);
+				const workspaceName = createWorkspaceIdentifier(this.workspace!);


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)

Done, and I'd like to contribute the eslint rule too 👀 (is it null assertions in general or something more specific?)

null assertions (https://typescript-eslint.io/rules/no-non-null-assertion/) but in this case TS detect something was wrong (we could reset the variable in the meantime)

src/core/cliUtils.ts

EhabY · 2026-03-29T11:39:28Z

src/commands.ts

+					const doc = await vscode.workspace.openTextDocument({
+						content: stdout,
+						language: "json",
+					});
+					await vscode.window.showTextDocument(doc);
+				} catch (error) {


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

Addressed, but I'm not sure if I prefer this UX 🤔

Isn't it better to just show the results immediately, and only save if the user explicitly chooses to? Sometimes the user just want to see the results.

I'm basing this off this comment from Asher #750 (comment)

I suppose we can let the user copy it vs. save it, or opening the file and they can choose to copy it vs. save it 🤔

I don't like having to make the user choose since it adds friction, like we want results fast and easy, so perhaps opening them in an untitled file and letting the user save it might work, but in that case, can we suggest a good filename based on the timestamp so it's still easy to save?

@code-asher Thoughts?

Current flow is this BTW:

Execute command

Input the time (by default it's populted with 5s) and press Enter

Speed test runs and then shows a save dialog

User has to choose where to save it

A notification appears that offers to open the file

This does seem a lot now yeah...

My completely biased guess is that the most common thing will be for people to look at the results and discard them, or copy/paste them to Slack or GitHub or somewhere like that for debugging something, so saving will be the least used workflow. But I could be completely wrong about that.

On a personal level I would vote to open it in a new, unsaved file (a default title would indeed be nice) because it feels like how Emacs would do it. 😆

Ah except the JSON output is not particularly human readable is it? So viewing it may not be very useful if you always have to pipe it into some graphing tool anyway...

Edit: I tried the JSON, I suppose it is not that bad to read.

EhabY · 2026-03-29T11:51:28Z

src/commands.ts

+		await withProgress(
+			{
+				location: vscode.ProgressLocation.Notification,
+				title: "Running speed test...",
+			},
+			async () => {


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

Done, it's cancellable now, and the user chooses the time through the command dialog (5s default), but I'm not sure what you mean by 'mention that in the title'.

The Running speed test... could be something like Running ${duration} speed test... (or something along those lines)

yazan-abu-obaideh · 2026-04-02T10:39:39Z

@EhabY happy to contribute 😉

(should pass now 🙏)

EhabY

Love the tests! I think we'll have to await Asher's response for the UX but the rest are smaller issues!

EhabY · 2026-04-02T11:14:47Z

src/cliConfig.ts

+function buildGlobalFlags(
+	configs: Pick<WorkspaceConfiguration, "get">,
+	auth: CliAuth,
+	escapeAuth: boolean,


I would prefer we pass the escape method itself here instead of a boolean

EhabY · 2026-04-02T11:15:28Z

src/cliConfig.ts

+ * Returns global configuration flags for Coder CLI commands with raw auth
+ * values suitable for `execFile` (no shell escaping).
+ */
+export function getGlobalFlagsForExec(


How about we rename those to be clearer to something like this:

getGlobalFlags → raw args array (unescaped), the default, used by execFile

getGlobalShellFlags → shell-escaped args, used by spawn({ shell: true }), terminal.sendText, and SSH ProxyCommand

EhabY · 2026-04-02T11:20:08Z

src/commands.ts

+				return /^\d+[sm]$/.test(v.trim())
+					? null
+					: "Enter a duration like 5s, 10s, or 1m";


Does this only take seconds and minutes 🤔 ?

EhabY · 2026-04-02T11:25:30Z

src/commands.ts

+		const action = await vscode.window.showInformationMessage(
+			"Speed test results saved.",
+			"Open File",
+		);
+		if (action === "Open File") {


This might be removed but we should never await a thenable that is not blocking. We had a bug a while back that caused the notification to show, then hide after 10s and this promise was left hanging (VS Code API uses a thenable so it does not adhere to promises and as such might never throw)

EhabY · 2026-04-02T11:25:57Z

src/core/cliUtils.ts

+	binPath: string,
+	globalFlags: string[],
+	workspaceName: string,
+	options?: { signal?: AbortSignal; duration?: string },


Options are always passed no?

EhabY · 2026-04-02T11:26:49Z

test/unit/core/cliUtils.test.ts

+			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");
+			}


Do we need this? Can't we execute the JS file the same way?

yazan-abu-obaideh added 2 commits March 27, 2026 16:41

feat: coder#750

740b196

feat: coder#750

d167396

EhabY assigned yazan-abu-obaideh Mar 29, 2026

EhabY self-requested a review March 29, 2026 11:27

EhabY requested changes Mar 29, 2026

View reviewed changes

feat: coder#750

7671617

EhabY reviewed Apr 2, 2026

View reviewed changes

Conversation

yazan-abu-obaideh commented Mar 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

EhabY left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

code-asher Apr 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

code-asher Apr 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yazan-abu-obaideh commented Apr 2, 2026

Uh oh!

EhabY left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

yazan-abu-obaideh commented Mar 27, 2026 •

edited

Loading

code-asher Apr 2, 2026 •

edited

Loading

code-asher Apr 2, 2026 •

edited

Loading