test(vscode): actually run the autocomplete tests in CI#9555
Open
kilo-code-bot[bot] wants to merge 12 commits intomainfrom
Open
test(vscode): actually run the autocomplete tests in CI#9555kilo-code-bot[bot] wants to merge 12 commits intomainfrom
kilo-code-bot[bot] wants to merge 12 commits intomainfrom
Conversation
The file sat under src/**/__tests__/ and was not covered by `bun test tests/unit/`, so none of the AutocompleteModel tests were running in CI. Port to bun:test and move next to the rest of the unit suite.
Same story as the AutocompleteModel port: the spec under src/__tests__/ was not picked up by `bun test tests/unit/`. Move to tests/unit/ and swap vi.mock for mock.module so the suite exercises the new validation rules on every PR.
Move the spec from src/agent-manager/__tests__/ to tests/unit/ so `bun run test:unit` picks it up. Mechanical port — only swap the vitest import for bun:test and re-anchor the source import.
Move the spec from src/__tests__/ to tests/unit/ so `bun run test:unit` picks it up.
Move the spec from src/__tests__/ to tests/unit/ so `bun run test:unit` picks it up.
Move the spec from src/__tests__/ to tests/unit/ so `bun run test:unit` picks it up. The original vitest mock omitted `workspace.asRelativePath` which the tracker calls on every editor; add it so the happy-path test actually exercises the relative-path fallback.
Rewrite the spec for bun:test: drive a hand-rolled vscode stub via mock.module (the shared preload omits ProgressLocation) and grab the command callback from the registerCommand spy instead of relying on vitest's hoisted vi.mock transform. Behavior coverage is preserved — all 12 original scenarios still run.
Port the spec from src/__tests__/ to tests/unit/ using bun:test. Since mock.module isn't hoisted like vitest's vi.mock, the mocks are declared before the dynamic import of AgentManagerProvider. Had to flesh out the git-import, WorktreeStateManager, and format-keybinding stubs with the named exports the provider actually imports — the original mocks only covered a subset and relied on vitest leaving missing exports undefined.
Rewrite for bun:test. The original spec wired in a `__setState` / `__resetState` harness around a `core/config/ContextProxy` module that no longer exists in the tree — those mocks were vestigial since `AutocompleteServiceManager` reads settings from vscode.workspace directly. Drop the dead setup and keep the codeSuggestion, snooze, and inline-provider registration assertions.
The helper imports `../../mocking/MockTextDocument`, which was deleted long ago — the spec has been un-runnable in any runner since. The only consumer of MockWorkspace / MockWorkspaceEdit was this spec, so remove the whole shim along with it.
When several freshly-ported suites called `mock.module("vscode", ...)`
at file scope, the last registered factory won globally — so running
e.g. autocomplete-visible-code-tracker before autocomplete-settings
erased `ConfigurationTarget`, and mocking agent-manager collaborators
clobbered setup-script-service's own import.
Fix:
- Expand the shared preload (tests/setup/vscode-mock.ts) with the bits
our suites need (`languages`, `ProgressLocation`,
`InlineCompletionTriggerKind`, `showErrorMessage`, `withProgress`,
`CancellationTokenSource`) so no one needs to replace the module.
- Swap the per-file `mock.module("vscode", ...)` overrides for
`spyOn` + direct property assignment on the preload shape.
- Drop the agent-manager dependency mocks — the provider is stubbed out
via `Object.create`, so real module imports don't run constructors
and the mocks only existed to pre-empt deps that load fine.
All 119 tests in the ported files now pass in any order, and
`bun run test:unit` stays green at 2090 / 2090.
Contributor
Author
Code Review SummaryStatus: 2 Issues Found | Recommendation: Address before merge Overview
Fix these issues in Kilo Cloud Issue Details (click to expand)WARNING
Other Observations (not in diff)No file content changed since commit Files Reviewed (0 files)
Reviewed by gpt-5.4-2026-03-05 · 645,461 tokens |
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
| ).mockImplementation(() => ({ dispose: mock() })) | ||
|
|
||
| let activeTextEditor: unknown = null | ||
| Object.defineProperty(vscode.window, "activeTextEditor", { |
Contributor
Author
There was a problem hiding this comment.
WARNING: Getter-only activeTextEditor leaks into sibling tests
Object.defineProperty replaces the shared preload's writable vscode.window.activeTextEditor field with an accessor that has no setter and is never restored. Files like packages/kilo-vscode/tests/unit/autocomplete-visible-code-tracker.test.ts assign to that property directly; if this spec runs first, those assignments start throwing TypeError, so the suite becomes order-dependent.
| get: () => activeTextEditor, | ||
| }) | ||
|
|
||
| mock.module("../../src/services/autocomplete/AutocompleteModel", () => ({ |
Contributor
Author
There was a problem hiding this comment.
WARNING: These module mocks are process-global under Bun
mock.module() here shadows the real module for later test files in the same run. That makes specs such as packages/kilo-vscode/tests/unit/autocomplete-model.test.ts order-dependent, because they can end up importing this stubbed AutocompleteModel instead of the production implementation they are meant to verify.
`Object.defineProperty(..., { get })` turned activeTextEditor into a
read-only property, so when another ported file (sorted earlier alpha,
e.g. autocomplete-visible-code-tracker) later tried to assign to it
under the shared preload, Bun threw "Attempted to assign to readonly
property" on CI.
Swap it for a plain setter helper that writes through the preload's
existing writable slot — keeps the tests working regardless of file
load order.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacks on top of #9310. The autocomplete model/settings tests added there live under
src/**/__tests__/*.spec.ts, butbun run test:unitonly globstests/unit/, so CI never executed them. Move both specs totests/unit/and switchvitestAPIs tobun:testso every PR actually exercises the new validation and FIM-streaming behavior.