Skip to content

Bulk refactor - increase test coverage and add integration tests, extract some difficult to test methods #541

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

Open
wants to merge 73 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
73 commits
Select commit Hold shift + click to select a range
7e1bce9
pretest working
jaggederest Jun 16, 2025
c693a46
enable vscode-test and bump tsconfig to modern settings
jaggederest Jun 16, 2025
240b649
Merge branch 'main' into jaggederest/integration_tests
jaggederest Jun 16, 2025
0e58a31
test: fix integration tests to run without Remote SSH extension
jaggederest Jun 16, 2025
872b7e8
Merge remote-tracking branch 'origin/jaggederest/integration_tests' i…
jaggederest Jun 16, 2025
01c2d80
autocorrect formatting
jaggederest Jun 16, 2025
8ddbf26
bump node version to 22
jaggederest Jun 16, 2025
9b74df4
fix: configure Vitest to properly exclude VS Code integration tests
jaggederest Jun 16, 2025
adec211
whitespace
jaggederest Jun 16, 2025
d9b543a
fix: update tsconfig.json and convert pretty-bytes imports to standar…
jaggederest Jun 17, 2025
a7afdd6
Remove testmode flag in favor of checking existence of remote ssh ext…
jaggederest Jun 17, 2025
3097d8f
remove superfluous async, enable lint rule
jaggederest Jun 18, 2025
a2d2bc8
fix: resolve ESLint @typescript-eslint/require-await errors
jaggederest Jun 18, 2025
32dfda4
Update configurations and remove pointless Promise.all
jaggederest Jun 18, 2025
12b0124
Tweak eslint config to better handle json/md, remove compile-tests sc…
jaggederest Jun 18, 2025
cf58040
feat: expand integration tests and add coverage analysis
jaggederest Jun 19, 2025
1be94c3
fix: update integration tests to match actual commands
jaggederest Jun 19, 2025
67c47e0
feat: switch to VS Code built-in coverage for integration tests
jaggederest Jun 19, 2025
62c88f4
feat: add comprehensive integration test framework
jaggederest Jun 20, 2025
907347e
feat: implement comprehensive integration tests for CLI, URI handler,…
jaggederest Jun 20, 2025
7563580
feat: add comprehensive unit test coverage for all source files
jaggederest Jun 20, 2025
379b9ee
fix: resolve all broken unit tests with proper vscode and API mocking
jaggederest Jun 20, 2025
f2863b5
feat: enhance api.ts test coverage to 95.52%
jaggederest Jun 20, 2025
ced86e9
feat: achieve 48.4% overall test coverage with incremental improvements
jaggederest Jun 21, 2025
1dbee30
docs: update TODO.md and CLAUDE.md to reflect test coverage progress
jaggederest Jun 21, 2025
6d93e76
test: add unit tests for commands.ts - improved coverage from 28% to …
jaggederest Jun 21, 2025
2471b72
test: improve unit test coverage from 48.4% to 60.11%
jaggederest Jun 21, 2025
3e38dca
test: improve remote.ts coverage from 32.61% to 49.21%
jaggederest Jun 22, 2025
797b656
test: improve test coverage for commands, storage, and workspacesProv…
jaggederest Jun 22, 2025
8f150cb
feat: add structured logging foundation with TDD approach
jaggederest Jun 22, 2025
07bffb8
docs: update TODO.md with testing synthesis and logging plan
jaggederest Jun 22, 2025
fe296b2
feat: integrate logger into Storage class with TDD approach
jaggederest Jun 22, 2025
32b4df0
refactor: create type-safe mock builders and clean up api-helper.test.ts
jaggederest Jun 22, 2025
bfd4b34
refactor: clean up type casting in test files
jaggederest Jun 22, 2025
d4af4ed
fix: update test-helpers to use proper Storage type
jaggederest Jun 22, 2025
6c947fe
docs: update TODO.md with test quality improvement progress
jaggederest Jun 22, 2025
f8b6dbb
test: add Logger factory and verify backward compatibility
jaggederest Jun 22, 2025
a57d676
fix: address ESLint errors in api.test.ts
jaggederest Jun 22, 2025
97ff5fb
docs: simplify TODO.md and update progress
jaggederest Jun 22, 2025
8021706
docs: update CLAUDE.md with TDD patterns and techniques from session
jaggederest Jun 22, 2025
b624614
feat: integrate Logger into remote.ts with TDD approach
jaggederest Jun 22, 2025
2629397
feat: integrate Logger into extension.ts with initialization
jaggederest Jun 22, 2025
53272c1
test: add Logger integration tests for headers.ts
jaggederest Jun 22, 2025
92c548a
test: add Logger integration tests for workspaceMonitor
jaggederest Jun 22, 2025
9e8ed6d
test: add Logger integration tests for inbox
jaggederest Jun 22, 2025
3595bda
docs: update TODO.md with Logger integration progress
jaggederest Jun 22, 2025
9aac82d
test: add Logger integration tests for error.ts
jaggederest Jun 22, 2025
341cc67
test: add Logger integration tests for workspacesProvider
jaggederest Jun 23, 2025
dfd55e1
test: add Logger integration tests for commands.ts
jaggederest Jun 23, 2025
6285043
docs: mark Logger integration as complete in TODO.md
jaggederest Jun 23, 2025
eca919f
refactor: extract helper functions from monolithic activate() in exte…
jaggederest Jun 23, 2025
8505c4f
refactor: complete TDD extraction of all functions from activate()
jaggederest Jun 23, 2025
8b8edc7
test: consolidate test mocks into reusable factories
jaggederest Jun 23, 2025
1a43dd3
docs: update TODO.md and CLAUDE.md with test improvements
jaggederest Jun 23, 2025
a1af9cb
test: improve integration tests from 86 to 100 passing
jaggederest Jun 23, 2025
eaee610
test: clean up pointless integration tests and enable 3 more
jaggederest Jun 23, 2025
c4a2156
test: remove 9 more pointless integration tests
jaggederest Jun 23, 2025
1a9f34f
test: remove all skipped integration tests for fresh start
jaggederest Jun 24, 2025
6291f7f
refactor: improve testability through dependency injection and test s…
jaggederest Jun 26, 2025
a72e943
test: remove flaky UI tests and improve test stability
jaggederest Jun 26, 2025
eb787f8
test: simplify test files and reduce verbosity
jaggederest Jun 27, 2025
fa1f576
test: reduce test verbosity by 41% while maintaining 84% coverage
jaggederest Jun 27, 2025
c73c742
test: simplify test suite by removing low-value tests
jaggederest Jun 27, 2025
9a3b37d
fix: resolve Uri type errors in extension tests
jaggederest Jun 27, 2025
89ac9ee
refactor: dramatically simplify extension activation flow
jaggederest Jun 27, 2025
d84ee78
more test cleanup
jaggederest Jun 27, 2025
c17f927
tweak api-helper tests
jaggederest Jun 27, 2025
85293fe
refactor: extract classes into separate files for better organization
jaggederest Jun 27, 2025
a9db00f
remove .DS_Store
jaggederest Jun 27, 2025
33b5142
Delete COVERAGE.md
jaggederest Jun 27, 2025
a71d9fc
Delete TODO.md
jaggederest Jun 27, 2025
af14ff3
Merge main and move integration tests to src/test
jaggederest Jun 27, 2025
7af665b
Merge remote-tracking branch 'origin/jaggederest/refactor_extension' …
jaggederest Jun 27, 2025
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
4 changes: 4 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -6,3 +6,7 @@
/coverage/
*.vsix
yarn-error.log
/reports/
/.stryker-tmp/
stryker.log
.DS_Store
54 changes: 29 additions & 25 deletions CLAUDE.md
Original file line number Diff line number Diff line change
@@ -1,27 +1,31 @@
# Coder Extension Development Guidelines

## Build and Test Commands

- Build: `yarn build`
- Watch mode: `yarn watch`
- Package: `yarn package`
- Lint: `yarn lint`
- Lint with auto-fix: `yarn lint:fix`
- Run all tests: `yarn test`
- Run specific test: `vitest ./src/filename.test.ts`
- CI test mode: `yarn test:ci`
- Integration tests: `yarn test:integration`

## Code Style Guidelines

- TypeScript with strict typing
- No semicolons (see `.prettierrc`)
- Trailing commas for all multi-line lists
- 120 character line width
- Use ES6 features (arrow functions, destructuring, etc.)
- Use `const` by default; `let` only when necessary
- Prefix unused variables with underscore (e.g., `_unused`)
- Sort imports alphabetically in groups: external → parent → sibling
- Error handling: wrap and type errors appropriately
- Use async/await for promises, avoid explicit Promise construction where possible
- Test files must be named `*.test.ts` and use Vitest
## Core Philosophy

**First-Principles + KISS**: Question every assumption aggressively, then build the simplest solution from fundamental truths. If there's a straight line, take it, otherwise ask questions and gather any information necessary to determine the right path forward.

## Commands

```bash
yarn lint:fix # Lint with auto-fix
yarn test:ci --coverage # Run ALL unit tests (ALWAYS use this)
yarn pretest && yarn test:integration # Integration tests
yarn mutate # Mutation testing (may take up to 180s - run occasionally)
```

## Key Rules

- **TypeScript strict mode**, no semicolons, 120 char lines
- **Test files**: `*.test.ts` (Vitest for unit, VS Code API for integration)
- **Use test-helpers.ts**: 30+ mock factories available - NEVER create inline mocks, instead create a new factory in that file and import it
- **TDD always**: Write test → implement → refactor
- **Never use any**: Always try to use at least a decently close Partial type or equivalent
- **Never delete tests**: Only delete or skip tests if directly asked, otherwise ask the user for help if fixing the tests does not work.

## Testing Approach

1. Use `yarn test:ci --coverage` before and after EVERY change
2. Import factories and mocks from test-helpers.ts (createMock* and *Factory)
3. Write a test, make sure it fails, and only then make it pass
4. Use proper types, NEVER use eslint-disable to make mocks work
5. If mocking is too complicated, consider whether the function under test needs a minor refactoring that passes existing tests first, to make it easier to test.
22 changes: 21 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
"fmt": "prettier --write .",
"lint": "eslint . --ext ts,md,json",
"lint:fix": "yarn lint --fix",
"mutate": "npx stryker run",
"package": "webpack --mode production --devtool hidden-source-map",
"package:prerelease": "npx vsce package --pre-release",
"pretest": "tsc -p . --outDir out && yarn run build && yarn run lint",
Expand Down Expand Up @@ -60,6 +61,16 @@
"type": "string",
"default": ""
},
"coder.binaryPath": {
"markdownDescription": "The full path to the Coder CLI binary. If not specified, the extension will attempt to download it automatically.",
"type": "string",
"default": ""
},
"coder.verbose": {
"markdownDescription": "Enable verbose logging for debugging purposes.",
"type": "boolean",
"default": false
},
"coder.enableDownloads": {
"markdownDescription": "Allow the plugin to download the CLI when missing or out of date.",
"type": "boolean",
Expand Down Expand Up @@ -109,6 +120,11 @@
"markdownDescription": "Automatically log into the default URL when the extension is activated. coder.defaultUrl is preferred, otherwise the CODER_URL environment variable will be used. This setting has no effect if neither is set.",
"type": "boolean",
"default": false
},
"coder.url": {
"markdownDescription": "The URL of the Coder deployment. This can be used to automatically configure the connection.",
"type": "string",
"default": ""
}
}
},
Expand Down Expand Up @@ -292,6 +308,8 @@
"zod": "^3.25.65"
},
"devDependencies": {
"@stryker-mutator/core": "^9.0.1",
"@stryker-mutator/vitest-runner": "^9.0.1",
"@types/eventsource": "^3.0.0",
"@types/glob": "^7.1.3",
"@types/node": "^22.14.1",
Expand All @@ -301,10 +319,12 @@
"@types/ws": "^8.18.1",
"@typescript-eslint/eslint-plugin": "^7.0.0",
"@typescript-eslint/parser": "^6.21.0",
"@vitest/coverage-v8": "^2.1.0",
"@vscode/test-cli": "^0.0.10",
"@vscode/test-electron": "^2.5.2",
"@vscode/vsce": "^2.21.1",
"bufferutil": "^4.0.9",
"c8": "^10.1.3",
"coder": "https://github.com/coder/coder#main",
"dayjs": "^1.11.13",
"eslint": "^8.57.1",
Expand All @@ -321,7 +341,7 @@
"tsc-watch": "^6.2.1",
"typescript": "^5.4.5",
"utf-8-validate": "^6.0.5",
"vitest": "^0.34.6",
"vitest": "^2.1.0",
"vscode-test": "^1.5.0",
"webpack": "^5.99.6",
"webpack-cli": "^5.1.4"
Expand Down
242 changes: 242 additions & 0 deletions src/api-helper.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,242 @@
import { ErrorEvent } from "eventsource";
import { describe, expect, it } from "vitest";
import * as apiHelper from "./api-helper";
import {
AgentMetadataEventSchema,
AgentMetadataEventSchemaArray,
errToStr,
extractAgents,
} from "./api-helper";
import {
createMockAgent,
createMockWorkspace,
createWorkspaceWithAgents,
} from "./test-helpers";

// Test helpers
const createMockResource = (
id: string,
agents?: ReturnType<typeof createMockAgent>[],
) => ({
id,
created_at: new Date().toISOString(),
job_id: "job-id",
workspace_transition: "start" as const,
type: "docker_container",
name: id,
hide: false,
icon: "",
agents,
metadata: [],
daily_cost: 0,
});

const createValidMetadataEvent = (overrides: Record<string, unknown> = {}) => ({
result: {
collected_at: "2024-01-01T00:00:00Z",
age: 60,
value: "test-value",
error: "",
...overrides,
},
description: {
display_name: "Test Metric",
key: "test_metric",
script: "echo 'test'",
interval: 30,
timeout: 10,
},
});

describe("api-helper", () => {
describe("errToStr", () => {
it.each([
["Error instance", new Error("Test error message"), "Test error message"],
["Error with empty message", new Error(""), ""],
["non-empty string", "String error message", "String error message"],
["empty string", "", "default"],
["whitespace-only string", " \n\t ", "default"],
["null", null, "default"],
["undefined", undefined, "default"],
["number", 42, "default"],
["object", { unknown: "object" }, "default"],
])("should handle %s", (_, input, expected) => {
expect(errToStr(input, "default")).toBe(expected);
});

it.each([
["with message", { message: "Connection failed" }, "Connection failed"],
["without message", {}, "default"],
])("should handle ErrorEvent %s", (_, eventInit, expected) => {
const errorEvent = new ErrorEvent("error", eventInit);
expect(errToStr(errorEvent, "default")).toBe(expected);
});

it("should handle API error response", () => {
const apiError = {
isAxiosError: true,
response: {
data: {
message: "API request failed",
detail: "API request failed",
},
},
};
expect(errToStr(apiError, "default")).toBe("API request failed");
});

it("should handle API error response object", () => {
const apiErrorResponse = {
detail: "Invalid authentication",
message: "Invalid authentication",
};
expect(errToStr(apiErrorResponse, "default")).toBe(
"Invalid authentication",
);
});
});

describe("extractAgents", () => {
it.each([
[
"multiple resources with agents",
[
createMockResource("resource-1", [
createMockAgent({ id: "agent1", name: "main" }),
createMockAgent({ id: "agent2", name: "secondary" }),
]),
createMockResource("resource-2", [
createMockAgent({ id: "agent3", name: "tertiary" }),
]),
],
3,
["agent1", "agent2", "agent3"],
],
["empty resources", [], 0, []],
[
"resources with undefined agents",
[createMockResource("resource-1", undefined)],
0,
[],
],
[
"resources with empty agents",
[createMockResource("resource-1", [])],
0,
[],
],
])("should handle %s", (_, resources, expectedCount, expectedIds) => {
const mockWorkspace = createMockWorkspace({
latest_build: {
...createMockWorkspace().latest_build,
resources,
},
});

const agents = extractAgents(mockWorkspace);
expect(agents).toHaveLength(expectedCount);
expect(agents.map((a) => a.id)).toEqual(expectedIds);
});
});

describe("extractAllAgents", () => {
it("should extract agents from multiple workspaces", () => {
const workspaces = [
createWorkspaceWithAgents([
createMockAgent({ id: "agent1", name: "main" }),
]),
createWorkspaceWithAgents([
createMockAgent({ id: "agent2", name: "secondary" }),
]),
];

const agents = apiHelper.extractAllAgents(workspaces);
expect(agents).toHaveLength(2);
expect(agents.map((a) => a.id)).toEqual(["agent1", "agent2"]);
});

it("should handle empty workspaces array", () => {
const agents = apiHelper.extractAllAgents([]);
expect(agents).toHaveLength(0);
expect(agents).toEqual([]);
});

it("should handle workspaces with no agents", () => {
const workspaces = [createMockWorkspace(), createMockWorkspace()];
const agents = apiHelper.extractAllAgents(workspaces);
expect(agents).toHaveLength(0);
});
});

describe("AgentMetadataEventSchema", () => {
it("should validate correct event", () => {
const validEvent = createValidMetadataEvent();
const result = AgentMetadataEventSchema.safeParse(validEvent);
expect(result.success).toBe(true);
if (result.success) {
expect(result.data.result.collected_at).toBe("2024-01-01T00:00:00Z");
expect(result.data.result.age).toBe(60);
expect(result.data.result.value).toBe("test-value");
expect(result.data.result.error).toBe("");
expect(result.data.description.display_name).toBe("Test Metric");
expect(result.data.description.key).toBe("test_metric");
expect(result.data.description.script).toBe("echo 'test'");
expect(result.data.description.interval).toBe(30);
expect(result.data.description.timeout).toBe(10);
}
});

it("should reject invalid event", () => {
const event = createValidMetadataEvent({ age: "invalid" });
const result = AgentMetadataEventSchema.safeParse(event);
expect(result.success).toBe(false);
if (!result.success) {
expect(result.error.issues[0].code).toBe("invalid_type");
expect(result.error.issues[0].path).toEqual(["result", "age"]);
}
});

it("should validate array of events", () => {
const events = [
createValidMetadataEvent(),
createValidMetadataEvent({ value: "different-value" }),
];
const result = AgentMetadataEventSchemaArray.safeParse(events);
expect(result.success).toBe(true);
if (result.success) {
expect(result.data).toHaveLength(2);
expect(result.data[0].result.value).toBe("test-value");
expect(result.data[1].result.value).toBe("different-value");
}
});

it("should reject array with invalid events", () => {
const events = [createValidMetadataEvent(), { invalid: "structure" }];
const result = AgentMetadataEventSchemaArray.safeParse(events);
expect(result.success).toBe(false);
});

it("should handle missing required fields", () => {
const incompleteEvent = {
result: {
collected_at: "2024-01-01T00:00:00Z",
// missing age, value, error
},
description: {
display_name: "Test",
// missing other fields
},
};
const result = AgentMetadataEventSchema.safeParse(incompleteEvent);
expect(result.success).toBe(false);
if (!result.success) {
const missingFields = result.error.issues.map(
(issue) => issue.path[issue.path.length - 1],
);
expect(missingFields).toContain("age");
expect(missingFields).toContain("value");
expect(missingFields).toContain("error");
}
});
});
});
Loading