diff --git a/src/TestExplorer/TestExplorer.ts b/src/TestExplorer/TestExplorer.ts index 2ffabb3f6..bf2ff6d78 100644 --- a/src/TestExplorer/TestExplorer.ts +++ b/src/TestExplorer/TestExplorer.ts @@ -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 @@ -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(); @@ -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 @@ -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( diff --git a/src/WorkspaceContext.ts b/src/WorkspaceContext.ts index ae397eaa3..eb270b21b 100644 --- a/src/WorkspaceContext.ts +++ b/src/WorkspaceContext.ts @@ -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 @@ -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); diff --git a/src/sourcekit-lsp/LanguageClientManager.ts b/src/sourcekit-lsp/LanguageClientManager.ts index 143b79c86..4219dfa52 100644 --- a/src/sourcekit-lsp/LanguageClientManager.ts +++ b/src/sourcekit-lsp/LanguageClientManager.ts @@ -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 @@ -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( @@ -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 { diff --git a/src/sourcekit-lsp/LanguageClientToolchainCoordinator.ts b/src/sourcekit-lsp/LanguageClientToolchainCoordinator.ts index 1097cb6f8..5517ad9d5 100644 --- a/src/sourcekit-lsp/LanguageClientToolchainCoordinator.ts +++ b/src/sourcekit-lsp/LanguageClientToolchainCoordinator.ts @@ -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( @@ -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) { diff --git a/test/integration-tests/testexplorer/TestExplorerIntegration.test.ts b/test/integration-tests/testexplorer/TestExplorerIntegration.test.ts index 02fb76ac0..5c5e1adbc 100644 --- a/test/integration-tests/testexplorer/TestExplorerIntegration.test.ts +++ b/test/integration-tests/testexplorer/TestExplorerIntegration.test.ts @@ -23,6 +23,7 @@ import { assertContainsTrimmed, assertTestControllerHierarchy, assertTestResults, + buildStateFromController, eventPromise, gatherTests, runTest, @@ -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; @@ -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"; diff --git a/test/integration-tests/testexplorer/utilities.ts b/test/integration-tests/testexplorer/utilities.ts index ab6774efd..813a0c142 100644 --- a/test/integration-tests/testexplorer/utilities.ts +++ b/test/integration-tests/testexplorer/utilities.ts @@ -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. @@ -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, diff --git a/test/unit-tests/sourcekit-lsp/LanguageClientManager.test.ts b/test/unit-tests/sourcekit-lsp/LanguageClientManager.test.ts index 07038347f..0e531ca56 100644 --- a/test/unit-tests/sourcekit-lsp/LanguageClientManager.test.ts +++ b/test/unit-tests/sourcekit-lsp/LanguageClientManager.test.ts @@ -214,6 +214,7 @@ suite("LanguageClientManager Suite", () => { test("returns the same language client for the same folder", async () => { const factory = new LanguageClientToolchainCoordinator( instance(mockedWorkspace), + {}, languageClientFactoryMock ); @@ -239,6 +240,7 @@ suite("LanguageClientManager Suite", () => { mockedWorkspace.folders.push(instance(newFolder)); const factory = new LanguageClientToolchainCoordinator( instance(mockedWorkspace), + {}, languageClientFactoryMock ); @@ -264,6 +266,7 @@ suite("LanguageClientManager Suite", () => { mockedWorkspace.folders.push(instance(newFolder)); const factory = new LanguageClientToolchainCoordinator( instance(mockedWorkspace), + {}, languageClientFactoryMock ); @@ -278,6 +281,7 @@ suite("LanguageClientManager Suite", () => { test("launches SourceKit-LSP on startup", async () => { const factory = new LanguageClientToolchainCoordinator( instance(mockedWorkspace), + {}, languageClientFactoryMock ); @@ -301,6 +305,7 @@ suite("LanguageClientManager Suite", () => { mockedConfig.swiftSDK = "arm64-apple-ios"; const factory = new LanguageClientToolchainCoordinator( instance(mockedWorkspace), + {}, languageClientFactoryMock ); @@ -329,6 +334,7 @@ suite("LanguageClientManager Suite", () => { new LanguageClientToolchainCoordinator( instance(mockedWorkspace), + {}, languageClientFactoryMock ); await waitForReturnedPromises(languageClientMock.start); @@ -347,6 +353,7 @@ suite("LanguageClientManager Suite", () => { new LanguageClientToolchainCoordinator( instance(mockedWorkspace), + {}, languageClientFactoryMock ); await waitForReturnedPromises(languageClientMock.start); @@ -365,6 +372,7 @@ suite("LanguageClientManager Suite", () => { new LanguageClientToolchainCoordinator( instance(mockedWorkspace), + {}, languageClientFactoryMock ); await waitForReturnedPromises(languageClientMock.start); @@ -403,6 +411,7 @@ suite("LanguageClientManager Suite", () => { new LanguageClientToolchainCoordinator( instance(mockedWorkspace), + {}, languageClientFactoryMock ); await waitForReturnedPromises(languageClientMock.start); @@ -468,7 +477,11 @@ suite("LanguageClientManager Suite", () => { test("doesn't launch SourceKit-LSP if disabled by the user", async () => { mockedLspConfig.disable = true; - const sut = new LanguageClientManager(instance(mockedFolder), languageClientFactoryMock); + const sut = new LanguageClientManager( + instance(mockedFolder), + {}, + languageClientFactoryMock + ); await waitForReturnedPromises(languageClientMock.start); expect(sut.state).to.equal(State.Stopped); @@ -480,6 +493,7 @@ suite("LanguageClientManager Suite", () => { mockedLspConfig.serverPath = "/path/to/my/custom/sourcekit-lsp"; const factory = new LanguageClientToolchainCoordinator( instance(mockedWorkspace), + {}, languageClientFactoryMock ); @@ -531,6 +545,7 @@ suite("LanguageClientManager Suite", () => { new LanguageClientToolchainCoordinator( instance(mockedWorkspace), + {}, languageClientFactoryMock ); @@ -589,7 +604,7 @@ suite("LanguageClientManager Suite", () => { return { dispose: () => {} }; }); - new LanguageClientManager(instance(mockedFolder), languageClientFactoryMock); + new LanguageClientManager(instance(mockedFolder), {}, languageClientFactoryMock); await waitForReturnedPromises(languageClientMock.start); const activeDocumentManager = new LSPActiveDocumentManager(); @@ -621,7 +636,7 @@ suite("LanguageClientManager Suite", () => { document, }) ); - new LanguageClientManager(instance(mockedFolder), languageClientFactoryMock); + new LanguageClientManager(instance(mockedFolder), {}, languageClientFactoryMock); await waitForReturnedPromises(languageClientMock.start); const activeDocumentManager = new LSPActiveDocumentManager(); @@ -692,6 +707,7 @@ suite("LanguageClientManager Suite", () => { test("doesn't launch SourceKit-LSP on startup", async () => { const sut = new LanguageClientManager( instance(mockedFolder), + {}, languageClientFactoryMock ); await waitForReturnedPromises(languageClientMock.start); @@ -713,6 +729,7 @@ suite("LanguageClientManager Suite", () => { ); const factory = new LanguageClientToolchainCoordinator( instance(mockedWorkspace), + {}, languageClientFactoryMock ); @@ -750,6 +767,7 @@ suite("LanguageClientManager Suite", () => { ); const factory = new LanguageClientToolchainCoordinator( instance(mockedWorkspace), + {}, languageClientFactoryMock );