Skip to content

Fix test explorer tests not updating on document modification #1663

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.

Sign up for GitHub

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 3 commits into
base: main
Choose a base branch
from
Open

Fix test explorer tests not updating on document modification #1663

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d8bbf35
Fix test explorer tests not updating on document modification
plemarquand Jun 27, 2025
2dfbc54
Add tests
plemarquand Jun 28, 2025
b55c3d1
Skip modifying tests < 6.0
plemarquand Jun 28, 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
88 changes: 39 additions & 49 deletions src/TestExplorer/TestExplorer.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -133,16 +133,31 @@ export class TestExplorer {
*/
static observeFolders(workspaceContext: WorkspaceContext): vscode.Disposable {
const tokenSource = new vscode.CancellationTokenSource();
const disposable = workspaceContext.onDidChangeFolders(
({ folder, operation, workspace }) => {
switch (operation) {
case FolderOperation.add:
if (folder) {
void folder.swiftPackage.getTargets(TargetType.test).then(targets => {
if (targets.length === 0) {
return;
}
const disposable = workspaceContext.onDidChangeFolders(({ folder, operation }) => {
switch (operation) {
case FolderOperation.add:
if (folder) {
void folder.swiftPackage.getTargets(TargetType.test).then(targets => {
if (targets.length === 0) {
return;
}

folder.addTestExplorer();
// discover tests in workspace but only if disableAutoResolve is not on.
// discover tests will kick off a resolve if required
if (!configuration.folder(folder.workspaceFolder).disableAutoResolve) {
void folder.testExplorer?.discoverTestsInWorkspace(
tokenSource.token
);
}
});
}
break;
case FolderOperation.packageUpdated:
if (folder) {
void folder.swiftPackage.getTargets(TargetType.test).then(targets => {
const hasTestTargets = targets.length > 0;
if (hasTestTargets && !folder.hasTestExplorer()) {
folder.addTestExplorer();
// discover tests in workspace but only if disableAutoResolve is not on.
// discover tests will kick off a resolve if required
Expand All @@ -153,43 +168,15 @@ export class TestExplorer {
tokenSource.token
);
}
});
}
break;
case FolderOperation.packageUpdated:
if (folder) {
void folder.swiftPackage.getTargets(TargetType.test).then(targets => {
const hasTestTargets = targets.length > 0;
if (hasTestTargets && !folder.hasTestExplorer()) {
folder.addTestExplorer();
// discover tests in workspace but only if disableAutoResolve is not on.
// discover tests will kick off a resolve if required
if (
!configuration.folder(folder.workspaceFolder)
.disableAutoResolve
) {
void folder.testExplorer?.discoverTestsInWorkspace(
tokenSource.token
);
}
} else if (!hasTestTargets && folder.hasTestExplorer()) {
folder.removeTestExplorer();
} else if (folder.hasTestExplorer()) {
folder.refreshTestExplorer();
}
});
}
break;
case FolderOperation.focus:
if (folder) {
const languageClientManager =
workspace.languageClientManager.get(folder);
languageClientManager.documentSymbolWatcher = (document, symbols) =>
TestExplorer.onDocumentSymbols(folder, document, symbols);
}
}
} else if (!hasTestTargets && folder.hasTestExplorer()) {
folder.removeTestExplorer();
} else if (folder.hasTestExplorer()) {
folder.refreshTestExplorer();
}
});
}
}
);
});
return {
dispose: () => {
tokenSource.dispose();
Expand All @@ -212,7 +199,7 @@ export class TestExplorer {
/**
* Called whenever we have new document symbols
*/
private static onDocumentSymbols(
static onDocumentSymbols(
folder: FolderContext,
document: vscode.TextDocument,
symbols: vscode.DocumentSymbol[] | null | undefined
Expand All @@ -225,14 +212,17 @@ export class TestExplorer {
if (target && target.type === "test") {
testExplorer.lspTestDiscovery
.getDocumentTests(folder.swiftPackage, uri)
.then(tests =>
.then(tests => {
TestDiscovery.updateTestsForTarget(
testExplorer.controller,
{ id: target.c99name, label: target.name },
tests,
uri
)
)
);
testExplorer.onTestItemsDidChangeEmitter.fire(
testExplorer.controller
);
})
// Fallback to parsing document symbols for XCTests only
.catch(() => {
const tests = parseTestsFromDocumentSymbols(
Expand Down
7 changes: 6 additions & 1 deletion src/WorkspaceContext.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ import { isValidWorkspaceFolder, searchForPackages } from "./utilities/workspace
import { SwiftPluginTaskProvider } from "./tasks/SwiftPluginTaskProvider";
import { SwiftTaskProvider } from "./tasks/SwiftTaskProvider";
import { LLDBDebugConfigurationProvider } from "./debugger/debugAdapterFactory";
import { TestExplorer } from "./TestExplorer/TestExplorer";

/**
* Context for whole workspace. Holds array of contexts for each workspace folder
Expand Down Expand Up @@ -79,7 +80,11 @@ export class WorkspaceContext implements vscode.Disposable {
) {
this.statusItem = new StatusItem();
this.buildStatus = new SwiftBuildStatus(this.statusItem);
this.languageClientManager = new LanguageClientToolchainCoordinator(this);
this.languageClientManager = new LanguageClientToolchainCoordinator(this, {
onDocumentSymbols: (folder, document, symbols) => {
TestExplorer.onDocumentSymbols(folder, document, symbols);
},
});
this.tasks = new TaskManager(this);
this.diagnostics = new DiagnosticsManager(this);
this.taskProvider = new SwiftTaskProvider(this);
Expand Down
16 changes: 15 additions & 1 deletion src/sourcekit-lsp/LanguageClientManager.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,17 @@ import { LSPActiveDocumentManager } from "./didChangeActiveDocument";
import { DidChangeActiveDocumentNotification } from "./extensions/DidChangeActiveDocumentRequest";
import { lspClientOptions } from "./LanguageClientConfiguration";

interface LanguageClientManageOptions {
/**
* Options for the LanguageClientManager
*/
onDocumentSymbols?: (
folder: FolderContext,
document: vscode.TextDocument,
symbols: vscode.DocumentSymbol[] | null | undefined
) => void;
}

/**
* Manages the creation and destruction of Language clients as we move between
* workspace folders
Expand Down Expand Up @@ -101,6 +112,7 @@ export class LanguageClientManager implements vscode.Disposable {

constructor(
public folderContext: FolderContext,
private options: LanguageClientManageOptions = {},
private languageClientFactory: LanguageClientFactory = new LanguageClientFactory()
) {
this.namedOutputChannels.set(
Expand Down Expand Up @@ -427,7 +439,9 @@ export class LanguageClientManager implements vscode.Disposable {
workspaceFolder,
this.activeDocumentManager,
errorHandler,
this.documentSymbolWatcher
(document, symbols) => {
this.options.onDocumentSymbols?.(this.folderContext, document, symbols);
}
);

return {
Expand Down
9 changes: 8 additions & 1 deletion src/sourcekit-lsp/LanguageClientToolchainCoordinator.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,13 @@ export class LanguageClientToolchainCoordinator implements vscode.Disposable {

public constructor(
workspaceContext: WorkspaceContext,
private options: {
onDocumentSymbols?: (
folder: FolderContext,
document: vscode.TextDocument,
symbols: vscode.DocumentSymbol[] | null | undefined
) => void;
} = {},
languageClientFactory: LanguageClientFactory = new LanguageClientFactory() // used for testing only
) {
this.subscriptions.push(
Expand Down Expand Up @@ -124,7 +131,7 @@ export class LanguageClientToolchainCoordinator implements vscode.Disposable {
const versionString = folder.swiftVersion.toString();
let client = this.clients.get(versionString);
if (!client) {
client = new LanguageClientManager(folder, languageClientFactory);
client = new LanguageClientManager(folder, this.options, languageClientFactory);
this.clients.set(versionString, client);
// Callers that must restart when switching folders will call setLanguageClientFolder themselves.
if (singleServerSupport) {
Expand Down
122 changes: 122 additions & 0 deletions test/integration-tests/testexplorer/TestExplorerIntegration.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import {
assertContainsTrimmed,
assertTestControllerHierarchy,
assertTestResults,
buildStateFromController,
eventPromise,
gatherTests,
runTest,
Expand Down Expand Up @@ -50,6 +51,7 @@ import { executeTaskAndWaitForResult } from "../../utilities/tasks";
import { createBuildAllTask } from "../../../src/tasks/SwiftTaskProvider";
import { FolderContext } from "../../../src/FolderContext";
import { lineBreakRegex } from "../../../src/utilities/tasks";
import { randomString } from "../../../src/utilities/utilities";

suite("Test Explorer Suite", function () {
const MAX_TEST_RUN_TIME_MINUTES = 6;
Expand Down Expand Up @@ -81,6 +83,126 @@ suite("Test Explorer Suite", function () {
requiresDebugger: true,
});

suite("Modifying", function () {
let sourceFile: string;
let originalSource: string;

suiteSetup(function () {
if (
(process.platform === "win32" &&
workspaceContext.globalToolchainSwiftVersion.isLessThan(
new Version(6, 1, 0)
)) ||
workspaceContext.globalToolchainSwiftVersion.isLessThan(new Version(6, 0, 2))
) {
this.skip();
}

// FIXME: Both Linux and Windows aren't triggering the onTestItemsDidChange event
// at the expected time for these tests.
this.skip();
});

beforeEach(() => {
sourceFile = path.join(
folderContext.folder.fsPath,
"Tests",
"PackageTests",
"PackageTests.swift"
);
originalSource = fs.readFileSync(sourceFile, "utf8");
});

async function appendSource(newContent: string) {
const document = await vscode.workspace.openTextDocument(sourceFile);
await vscode.window.showTextDocument(document);
const edit = new vscode.WorkspaceEdit();
const lastLine = document.lineAt(document.lineCount - 1);
edit.insert(document.uri, lastLine.range.end, newContent);
await vscode.workspace.applyEdit(edit);
return document;
}

async function setSource(content: string) {
const document = await vscode.workspace.openTextDocument(sourceFile);
await vscode.window.showTextDocument(document);
const edit = new vscode.WorkspaceEdit();
edit.replace(
document.uri,
document.validateRange(new vscode.Range(0, 0, 10000000, 0)),
content
);
await vscode.workspace.applyEdit(edit);
return document;
}

type TestHierarchy = string | TestHierarchy[];

// Because we're at the whim of how often VS Code/the LSP provide document symbols
// we can't assume that changes to test items will be reflected in the next onTestItemsDidChange
// so poll until the condition is met.
async function validate(validator: (testItems: TestHierarchy) => boolean) {
let testItems: TestHierarchy = [];
const startTime = Date.now();
while (Date.now() - startTime < 5000) {
testItems = buildStateFromController(testExplorer.controller.items);
if (validator(testItems)) {
return;
}
await new Promise(resolve => setTimeout(resolve, 100));
}
assert.fail("Expected test items to be updated, but they were not: " + testItems);
}

test("Test explorer updates when a test is added and removed", async () => {
const testName = `newTest${randomString()}()`;
const newTest = `\n@Test func ${testName} {\n #expect(1 == 1)\n}\n`;

console.log(
">>> Appending new test to the source file and waiting for test items to change"
);
await Promise.all([
eventPromise(testExplorer.onTestItemsDidChange),
appendSource(newTest),
]);

console.log(">>> Validating that the new test appears in the test items");
await validate(testItems => testItems[1].includes(testName));

console.log(
">>> Restoring the original source file and waiting for test items to change"
);
await Promise.all([
eventPromise(testExplorer.onTestItemsDidChange),
setSource(originalSource),
]);

console.log(">>> Validating that the new test no longer appears in the test items");
await validate(testItems => !testItems[1].includes(testName));
});

test("Test explorer updates when a suite is added and removed", async () => {
const suiteName = `newSuite${randomString()}`;
const newSuite = `\n@Suite\nstruct ${suiteName} {\n @Test\n func testPassing() throws {\n #expect(1 == 1)\n }\n}\n`;
await Promise.all([
eventPromise(testExplorer.onTestItemsDidChange),
appendSource(newSuite),
]);
await validate(testItems => testItems[1].includes(suiteName));

await Promise.all([
eventPromise(testExplorer.onTestItemsDidChange),
setSource(originalSource),
]);
await validate(testItems => !testItems[1].includes(suiteName));
});

afterEach(async () => {
const document = await setSource(originalSource);
await document.save();
});
});

suite("Debugging", function () {
async function runXCTest() {
const suiteId = "PackageTests.PassingXCTestSuite";
Expand Down
23 changes: 13 additions & 10 deletions test/integration-tests/testexplorer/utilities.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,19 @@ export function testExplorerFor(

type TestHierarchy = string | TestHierarchy[];

/**
* Builds a tree of text items from a TestItemCollection
*/
export const buildStateFromController = (items: vscode.TestItemCollection): TestHierarchy =>
reduceTestItemChildren(
items,
(acc, item) => {
const children = buildStateFromController(item.children);
return [...acc, item.label, ...(children.length ? [children] : [])];
},
[] as TestHierarchy
);

/**
* Asserts that the test item hierarchy matches the description provided by a collection
* of `TestControllerState`s.
Expand All @@ -88,16 +101,6 @@ export function assertTestControllerHierarchy(
controller: vscode.TestController,
state: TestHierarchy
) {
const buildStateFromController = (items: vscode.TestItemCollection): TestHierarchy =>
reduceTestItemChildren(
items,
(acc, item) => {
const children = buildStateFromController(item.children);
return [...acc, item.label, ...(children.length ? [children] : [])];
},
[] as TestHierarchy
);

assert.deepEqual(
buildStateFromController(controller.items),
state,
Expand Down
Loading