diff --git a/CHANGELOG.md b/CHANGELOG.md index 1e7fe047d..d939de2fe 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,14 @@ ### Fixed +- Fix test explorer tests not updating on document modification ([#1663](https://github.com/swiftlang/vscode-swift/pull/1663)) +- Fix improper parenting of tests w/ identical names in explorer ([#1664](https://github.com/swiftlang/vscode-swift/pull/1664)) +- Ensure document symbols are provided for folders in multi root workspaces ([#1668](https://github.com/swiftlang/vscode-swift/pull/1668)) + +## 2.6.1 - 2025-06-27 + +### Fixed + - Cleanup Swift diagnostics when the source file is moved or deleted ([#1653](https://github.com/swiftlang/vscode-swift/pull/1653)) - Make sure newline starts with /// when splitting doc comment ([#1651](https://github.com/swiftlang/vscode-swift/pull/1651)) - Capture diagnostics with `Swift: Capture Diagnostic Bundle` to a .zip file ([#1656](https://github.com/swiftlang/vscode-swift/pull/1656)) diff --git a/src/FolderContext.ts b/src/FolderContext.ts index d5fde8e21..1d0b32c50 100644 --- a/src/FolderContext.ts +++ b/src/FolderContext.ts @@ -177,9 +177,9 @@ export class FolderContext implements vscode.Disposable { } /** Refresh the tests in the test explorer for this folder */ - refreshTestExplorer() { + async refreshTestExplorer() { if (this.testExplorer?.controller.resolveHandler) { - void this.testExplorer.controller.resolveHandler(undefined); + return this.testExplorer.controller.resolveHandler(undefined); } } @@ -211,6 +211,25 @@ export class FolderContext implements vscode.Disposable { }); return target; } + + /** + * Called whenever we have new document symbols + */ + onDocumentSymbols( + document: vscode.TextDocument, + symbols: vscode.DocumentSymbol[] | null | undefined + ) { + const uri = document?.uri; + if ( + this.testExplorer && + symbols && + uri && + uri.scheme === "file" && + isPathInsidePath(uri.fsPath, this.folder.fsPath) + ) { + void this.testExplorer.getDocumentTests(this, uri, symbols); + } + } } export interface EditedPackage { diff --git a/src/TestExplorer/TestExplorer.ts b/src/TestExplorer/TestExplorer.ts index bf2ff6d78..dee218828 100644 --- a/src/TestExplorer/TestExplorer.ts +++ b/src/TestExplorer/TestExplorer.ts @@ -15,7 +15,6 @@ import * as vscode from "vscode"; import { FolderContext } from "../FolderContext"; import { getErrorDescription } from "../utilities/utilities"; -import { isPathInsidePath } from "../utilities/filesystem"; import { FolderOperation, WorkspaceContext } from "../WorkspaceContext"; import { TestRunProxy, TestRunner } from "./TestRunner"; import { LSPTestDiscovery } from "./LSPTestDiscovery"; @@ -136,45 +135,11 @@ export class TestExplorer { 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 - 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(); - } - }); + void this.setupTestExplorerForFolder(folder, tokenSource.token); } + break; } }); return { @@ -185,6 +150,38 @@ export class TestExplorer { }; } + /** + * Configures a test explorer for the given folder. + * If the folder has test targets, and there is no existing test explorer, + * it will create a test explorer and discover tests. + * If the folder has no test targets, it will remove any existing test explorer. + * If the folder has test targets and an existing test explorer, it will refresh the tests. + */ + private static async setupTestExplorerForFolder( + folder: FolderContext, + token: vscode.CancellationToken + ) { + const targets = await folder.swiftPackage.getTargets(TargetType.test); + const hasTestTargets = targets.length > 0; + if (hasTestTargets && !folder.hasTestExplorer()) { + const testExplorer = folder.addTestExplorer(); + if ( + configuration.folder(folder.workspaceFolder).disableAutoResolve && + process.platform === "win32" && + folder.swiftVersion.isLessThan(new Version(5, 10, 0)) + ) { + // On Windows 5.9 and earlier discoverTestsInWorkspace kicks off a build, + // which will perform a resolve. + return; + } + await testExplorer.discoverTestsInWorkspace(token); + } else if (hasTestTargets && folder.hasTestExplorer()) { + await folder.refreshTestExplorer(); + } else if (!hasTestTargets && folder.hasTestExplorer()) { + folder.removeTestExplorer(); + } + } + /** * Sets the `swift.tests` context variable which is used by commands * to determine if the test item belongs to the Swift extension. @@ -196,45 +193,29 @@ export class TestExplorer { }); } - /** - * Called whenever we have new document symbols - */ - static onDocumentSymbols( + async getDocumentTests( folder: FolderContext, - document: vscode.TextDocument, - symbols: vscode.DocumentSymbol[] | null | undefined - ) { - const uri = document?.uri; - const testExplorer = folder?.testExplorer; - if (testExplorer && symbols && uri && uri.scheme === "file") { - if (isPathInsidePath(uri.fsPath, folder.folder.fsPath)) { - void folder.swiftPackage.getTarget(uri.fsPath).then(target => { - if (target && target.type === "test") { - testExplorer.lspTestDiscovery - .getDocumentTests(folder.swiftPackage, uri) - .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( - target.name, - symbols, - uri - ); - testExplorer.updateTests(testExplorer.controller, tests, uri); - }); - } - }); - } + uri: vscode.Uri, + symbols: vscode.DocumentSymbol[] + ): Promise { + const target = await folder.swiftPackage.getTarget(uri.fsPath); + if (!target || target.type !== "test") { + return; + } + + try { + const tests = await this.lspTestDiscovery.getDocumentTests(folder.swiftPackage, uri); + TestDiscovery.updateTestsForTarget( + this.controller, + { id: target.c99name, label: target.name }, + tests, + uri + ); + this.onTestItemsDidChangeEmitter.fire(this.controller); + } catch { + // Fallback to parsing document symbols for XCTests only + const tests = parseTestsFromDocumentSymbols(target.name, symbols, uri); + this.updateTests(this.controller, tests, uri); } } diff --git a/src/WorkspaceContext.ts b/src/WorkspaceContext.ts index eb270b21b..e565daa68 100644 --- a/src/WorkspaceContext.ts +++ b/src/WorkspaceContext.ts @@ -37,7 +37,6 @@ 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 @@ -82,7 +81,7 @@ export class WorkspaceContext implements vscode.Disposable { this.buildStatus = new SwiftBuildStatus(this.statusItem); this.languageClientManager = new LanguageClientToolchainCoordinator(this, { onDocumentSymbols: (folder, document, symbols) => { - TestExplorer.onDocumentSymbols(folder, document, symbols); + folder.onDocumentSymbols(document, symbols); }, }); this.tasks = new TaskManager(this); diff --git a/src/sourcekit-lsp/LanguageClientManager.ts b/src/sourcekit-lsp/LanguageClientManager.ts index 4219dfa52..d764e0734 100644 --- a/src/sourcekit-lsp/LanguageClientManager.ts +++ b/src/sourcekit-lsp/LanguageClientManager.ts @@ -97,7 +97,8 @@ export class LanguageClientManager implements vscode.Disposable { private singleServerSupport: boolean; // used by single server support to keep a record of the project folders // that are not at the root of their workspace - public subFolderWorkspaces: vscode.Uri[]; + public subFolderWorkspaces: FolderContext[] = []; + private addedFolders: FolderContext[] = []; private namedOutputChannels: Map = new Map(); private swiftVersion: Version; private activeDocumentManager = new LSPActiveDocumentManager(); @@ -122,7 +123,6 @@ export class LanguageClientManager implements vscode.Disposable { this.swiftVersion = folderContext.swiftVersion; this.singleServerSupport = this.swiftVersion.isGreaterThanOrEqual(new Version(5, 7, 0)); this.subscriptions = []; - this.subFolderWorkspaces = []; // on change config restart server const onChangeConfig = vscode.workspace.onDidChangeConfiguration(event => { @@ -244,9 +244,9 @@ export class LanguageClientManager implements vscode.Disposable { async addFolder(folderContext: FolderContext) { if (!folderContext.isRootFolder) { await this.useLanguageClient(async client => { - const uri = folderContext.folder; - this.subFolderWorkspaces.push(folderContext.folder); + this.subFolderWorkspaces.push(folderContext); + const uri = folderContext.folder; const workspaceFolder = { uri: client.code2ProtocolConverter.asUri(uri), name: FolderContext.uriName(uri), @@ -256,13 +256,16 @@ export class LanguageClientManager implements vscode.Disposable { }); }); } + this.addedFolders.push(folderContext); } async removeFolder(folderContext: FolderContext) { if (!folderContext.isRootFolder) { await this.useLanguageClient(async client => { const uri = folderContext.folder; - this.subFolderWorkspaces = this.subFolderWorkspaces.filter(item => item !== uri); + this.subFolderWorkspaces = this.subFolderWorkspaces.filter( + item => item.folder !== uri + ); const workspaceFolder = { uri: client.code2ProtocolConverter.asUri(uri), @@ -273,13 +276,14 @@ export class LanguageClientManager implements vscode.Disposable { }); }); } + this.addedFolders = this.addedFolders.filter(item => item.folder !== folderContext.folder); } private async addSubFolderWorkspaces(client: LanguageClient) { - for (const uri of this.subFolderWorkspaces) { + for (const folderContext of this.subFolderWorkspaces) { const workspaceFolder = { - uri: client.code2ProtocolConverter.asUri(uri), - name: FolderContext.uriName(uri), + uri: client.code2ProtocolConverter.asUri(folderContext.folder), + name: FolderContext.uriName(folderContext.folder), }; await client.sendNotification(DidChangeWorkspaceFoldersNotification.type, { event: { added: [workspaceFolder], removed: [] }, @@ -440,7 +444,17 @@ export class LanguageClientManager implements vscode.Disposable { this.activeDocumentManager, errorHandler, (document, symbols) => { - this.options.onDocumentSymbols?.(this.folderContext, document, symbols); + const documentFolderContext = [this.folderContext, ...this.addedFolders].find( + folderContext => document.uri.fsPath.startsWith(folderContext.folder.fsPath) + ); + if (!documentFolderContext) { + this.languageClientOutputChannel?.log( + "Unable to find folder for document: " + document.uri.fsPath, + "WARN" + ); + return; + } + this.options.onDocumentSymbols?.(documentFolderContext, document, symbols); } ); diff --git a/test/integration-tests/ExtensionActivation.test.ts b/test/integration-tests/ExtensionActivation.test.ts index 3bb3bc8a8..4081c9c83 100644 --- a/test/integration-tests/ExtensionActivation.test.ts +++ b/test/integration-tests/ExtensionActivation.test.ts @@ -116,7 +116,9 @@ suite("Extension Activation/Deactivation Tests", () => { assert(folder); const languageClient = workspaceContext.languageClientManager.get(folder); - const lspWorkspaces = languageClient.subFolderWorkspaces.map(({ fsPath }) => fsPath); + const lspWorkspaces = languageClient.subFolderWorkspaces.map( + ({ folder }) => folder.fsPath + ); assertContains(lspWorkspaces, testAssetUri("cmake").fsPath); }); @@ -125,7 +127,9 @@ suite("Extension Activation/Deactivation Tests", () => { assert(folder); const languageClient = workspaceContext.languageClientManager.get(folder); - const lspWorkspaces = languageClient.subFolderWorkspaces.map(({ fsPath }) => fsPath); + const lspWorkspaces = languageClient.subFolderWorkspaces.map( + ({ folder }) => folder.fsPath + ); assertContains(lspWorkspaces, testAssetUri("cmake-compile-flags").fsPath); }); }); diff --git a/test/integration-tests/commands/runTestMultipleTimes.test.ts b/test/integration-tests/commands/runTestMultipleTimes.test.ts index 2473d0d94..49c9022c4 100644 --- a/test/integration-tests/commands/runTestMultipleTimes.test.ts +++ b/test/integration-tests/commands/runTestMultipleTimes.test.ts @@ -25,7 +25,7 @@ suite("Test Multiple Times Command Test Suite", () => { activateExtensionForSuite({ async setup(ctx) { - folderContext = await folderInRootWorkspace("diagnostics", ctx); + folderContext = await folderInRootWorkspace("defaultPackage", ctx); folderContext.addTestExplorer(); const item = folderContext.testExplorer?.controller.createTestItem( diff --git a/test/integration-tests/testexplorer/TestExplorerIntegration.test.ts b/test/integration-tests/testexplorer/TestExplorerIntegration.test.ts index 5c5e1adbc..7dcbdb2a2 100644 --- a/test/integration-tests/testexplorer/TestExplorerIntegration.test.ts +++ b/test/integration-tests/testexplorer/TestExplorerIntegration.test.ts @@ -83,126 +83,6 @@ 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"; @@ -991,4 +871,112 @@ suite("Test Explorer Suite", function () { }); }); }); + + 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(); + } + }); + + 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`; + + await Promise.all([ + eventPromise(testExplorer.onTestItemsDidChange), + appendSource(newTest), + ]); + + await validate(testItems => testItems[1].includes(testName)); + + await Promise.all([ + eventPromise(testExplorer.onTestItemsDidChange), + setSource(originalSource), + ]); + + 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(); + }); + }); });