diff --git a/src/FolderContext.ts b/src/FolderContext.ts index 3a0d465c8..cc2578dfb 100644 --- a/src/FolderContext.ts +++ b/src/FolderContext.ts @@ -30,8 +30,10 @@ import { TestRunProxy } from "./TestExplorer/TestRunner"; export class FolderContext implements vscode.Disposable { public backgroundCompilation: BackgroundCompilation; public hasResolveErrors = false; - public testExplorer?: TestExplorer; public taskQueue: TaskQueue; + public testExplorer?: TestExplorer; + public resolvedTestExplorer: Promise; + private testExplorerResolver?: (testExplorer: TestExplorer) => void; private packageWatcher: PackageWatcher; private testRunManager: TestRunManager; @@ -49,10 +51,16 @@ export class FolderContext implements vscode.Disposable { public workspaceFolder: vscode.WorkspaceFolder, public workspaceContext: WorkspaceContext ) { - this.packageWatcher = new PackageWatcher(this, workspaceContext); + this.packageWatcher = new PackageWatcher(this, workspaceContext.logger); this.backgroundCompilation = new BackgroundCompilation(this); this.taskQueue = new TaskQueue(this); this.testRunManager = new TestRunManager(); + + // Tests often need to wait for the test explorer to be created before they can run. + // This promise resolves when the test explorer is created, allowing them to wait for it before starting. + this.resolvedTestExplorer = new Promise(resolve => { + this.testExplorerResolver = resolve; + }); } /** dispose of any thing FolderContext holds */ @@ -112,6 +120,10 @@ export class FolderContext implements vscode.Disposable { return folderContext; } + get languageClientManager() { + return this.workspaceContext.languageClientManager.get(this); + } + get name(): string { const relativePath = this.relativePath; if (relativePath.length === 0) { @@ -169,7 +181,13 @@ export class FolderContext implements vscode.Disposable { /** Create Test explorer for this folder */ addTestExplorer() { if (this.testExplorer === undefined) { - this.testExplorer = new TestExplorer(this); + this.testExplorer = new TestExplorer( + this, + this.workspaceContext.tasks, + this.workspaceContext.logger, + this.workspaceContext.onDidChangeSwiftFiles.bind(this.workspaceContext) + ); + this.testExplorerResolver?.(this.testExplorer); } return this.testExplorer; } diff --git a/src/PackageWatcher.ts b/src/PackageWatcher.ts index 5bf7a366a..1626df428 100644 --- a/src/PackageWatcher.ts +++ b/src/PackageWatcher.ts @@ -16,11 +16,12 @@ import * as path from "path"; import * as fs from "fs/promises"; import * as vscode from "vscode"; import { FolderContext } from "./FolderContext"; -import { FolderOperation, WorkspaceContext } from "./WorkspaceContext"; +import { FolderOperation } from "./WorkspaceContext"; import { BuildFlags } from "./toolchain/BuildFlags"; import { Version } from "./utilities/version"; import { fileExists } from "./utilities/filesystem"; import { showReloadExtensionNotification } from "./ui/ReloadExtension"; +import { SwiftLogger } from "./logging/SwiftLogger"; /** * Watches for changes to **Package.swift** and **Package.resolved**. @@ -39,7 +40,7 @@ export class PackageWatcher { constructor( private folderContext: FolderContext, - private workspaceContext: WorkspaceContext + private logger: SwiftLogger ) {} /** @@ -136,11 +137,8 @@ export class PackageWatcher { async handleSwiftVersionFileChange() { const version = await this.readSwiftVersionFile(); - if (version && version.toString() !== this.currentVersion?.toString()) { - await this.workspaceContext.fireEvent( - this.folderContext, - FolderOperation.swiftVersionUpdated - ); + if (version?.toString() !== this.currentVersion?.toString()) { + await this.folderContext.fireEvent(FolderOperation.swiftVersionUpdated); await showReloadExtensionNotification( "Changing the swift toolchain version requires the extension to be reloaded" ); @@ -155,9 +153,7 @@ export class PackageWatcher { return Version.fromString(contents.toString().trim()); } catch (error) { if ((error as NodeJS.ErrnoException).code !== "ENOENT") { - this.workspaceContext.logger.error( - `Failed to read .swift-version file at ${versionFile}: ${error}` - ); + this.logger.error(`Failed to read .swift-version file at ${versionFile}: ${error}`); } } return undefined; @@ -173,7 +169,7 @@ export class PackageWatcher { async handlePackageSwiftChange() { // Load SwiftPM Package.swift description await this.folderContext.reload(); - await this.workspaceContext.fireEvent(this.folderContext, FolderOperation.packageUpdated); + await this.folderContext.fireEvent(FolderOperation.packageUpdated); } /** @@ -186,10 +182,7 @@ export class PackageWatcher { await this.folderContext.reloadPackageResolved(); // if file contents has changed then send resolve updated message if (this.folderContext.swiftPackage.resolved?.fileHash !== packageResolvedHash) { - await this.workspaceContext.fireEvent( - this.folderContext, - FolderOperation.resolvedUpdated - ); + await this.folderContext.fireEvent(FolderOperation.resolvedUpdated); } } @@ -200,9 +193,6 @@ export class PackageWatcher { */ private async handleWorkspaceStateChange() { await this.folderContext.reloadWorkspaceState(); - await this.workspaceContext.fireEvent( - this.folderContext, - FolderOperation.workspaceStateUpdated - ); + await this.folderContext.fireEvent(FolderOperation.workspaceStateUpdated); } } diff --git a/src/TestExplorer/TestExplorer.ts b/src/TestExplorer/TestExplorer.ts index 4320f55ac..66b55dcce 100644 --- a/src/TestExplorer/TestExplorer.ts +++ b/src/TestExplorer/TestExplorer.ts @@ -13,21 +13,23 @@ //===----------------------------------------------------------------------===// import * as vscode from "vscode"; +import * as TestDiscovery from "./TestDiscovery"; import { FolderContext } from "../FolderContext"; -import { getErrorDescription } from "../utilities/utilities"; -import { FolderOperation, WorkspaceContext } from "../WorkspaceContext"; +import { compositeDisposable, getErrorDescription } from "../utilities/utilities"; +import { FolderOperation, SwiftFileEvent, WorkspaceContext } from "../WorkspaceContext"; import { TestRunProxy, TestRunner } from "./TestRunner"; import { LSPTestDiscovery } from "./LSPTestDiscovery"; import { Version } from "../utilities/version"; -import configuration from "../configuration"; import { buildOptions, getBuildAllTask } from "../tasks/SwiftTaskProvider"; import { SwiftExecOperation, TaskOperation } from "../tasks/TaskQueue"; -import * as TestDiscovery from "./TestDiscovery"; import { TargetType } from "../SwiftPackage"; import { parseTestsFromSwiftTestListOutput } from "./SPMTestDiscovery"; import { parseTestsFromDocumentSymbols } from "./DocumentSymbolTestDiscovery"; import { flattenTestItemCollection } from "./TestUtils"; import { TestCodeLensProvider } from "./TestCodeLensProvider"; +import { SwiftLogger } from "../logging/SwiftLogger"; +import { TaskManager } from "../tasks/TaskManager"; +import configuration from "../configuration"; /** Build test explorer UI */ export class TestExplorer { @@ -36,7 +38,7 @@ export class TestExplorer { public testRunProfiles: vscode.TestRunProfile[]; private lspTestDiscovery: LSPTestDiscovery; - private subscriptions: { dispose(): unknown }[]; + private subscriptions: vscode.Disposable[]; private tokenSource = new vscode.CancellationTokenSource(); // Emits after the `vscode.TestController` has been updated. @@ -46,24 +48,22 @@ export class TestExplorer { public onDidCreateTestRunEmitter = new vscode.EventEmitter(); public onCreateTestRun: vscode.Event; - private codeLensProvider?: TestCodeLensProvider; + private codeLensProvider: TestCodeLensProvider; - constructor(public folderContext: FolderContext) { + constructor( + public folderContext: FolderContext, + private tasks: TaskManager, + private logger: SwiftLogger, + private onDidChangeSwiftFiles: ( + listener: (event: SwiftFileEvent) => void + ) => vscode.Disposable + ) { this.onTestItemsDidChange = this.onTestItemsDidChangeEmitter.event; this.onCreateTestRun = this.onDidCreateTestRunEmitter.event; + this.lspTestDiscovery = this.configureLSPTestDiscovery(folderContext); this.codeLensProvider = new TestCodeLensProvider(this); - - this.controller = vscode.tests.createTestController( - folderContext.name, - `${folderContext.name} Tests` - ); - - this.controller.resolveHandler = async item => { - if (!item) { - await this.discoverTestsInWorkspace(this.tokenSource.token); - } - }; + this.controller = this.createController(folderContext); this.testRunProfiles = TestRunner.setupProfiles( this.controller, @@ -76,6 +76,7 @@ export class TestExplorer { this.controller, this.onTestItemsDidChangeEmitter, this.onDidCreateTestRunEmitter, + this.codeLensProvider, ...this.testRunProfiles, this.onTestItemsDidChange(() => this.updateSwiftTestContext()), this.discoverUpdatedTestsAfterBuild(folderContext), @@ -97,7 +98,7 @@ export class TestExplorer { symbols: vscode.DocumentSymbol[] ): Promise { const target = await folder.swiftPackage.getTarget(uri.fsPath); - if (!target || target.type !== "test") { + if (target?.type !== "test") { return; } @@ -117,13 +118,35 @@ export class TestExplorer { } } + public dispose() { + this.tokenSource.cancel(); + this.subscriptions.forEach(element => element.dispose()); + this.subscriptions = []; + } + /** * Creates an LSPTestDiscovery client for the given folder context. */ private configureLSPTestDiscovery(folderContext: FolderContext): LSPTestDiscovery { - const workspaceContext = folderContext.workspaceContext; - const languageClientManager = workspaceContext.languageClientManager.get(folderContext); - return new LSPTestDiscovery(languageClientManager); + return new LSPTestDiscovery(folderContext.languageClientManager); + } + + /** + * Creates a test controller for the given folder context. + */ + private createController(folderContext: FolderContext) { + const controller = vscode.tests.createTestController( + folderContext.name, + `${folderContext.name} Tests` + ); + + controller.resolveHandler = async item => { + if (!item) { + await this.discoverTestsInWorkspace(this.tokenSource.token); + } + }; + + return controller; } /** @@ -131,56 +154,40 @@ export class TestExplorer { */ private discoverUpdatedTestsAfterBuild(folderContext: FolderContext): vscode.Disposable { let testFileEdited = true; - const endProcessDisposable = folderContext.workspaceContext.tasks.onDidEndTaskProcess( - event => { - const task = event.execution.task; - const execution = task.execution as vscode.ProcessExecution; - if ( - task.scope === folderContext.workspaceFolder && - task.group === vscode.TaskGroup.Build && - execution?.options?.cwd === folderContext.folder.fsPath && - event.exitCode === 0 && - task.definition.dontTriggerTestDiscovery !== true && - testFileEdited - ) { - testFileEdited = false; - - // only run discover tests if the library has tests - void folderContext.swiftPackage.getTargets(TargetType.test).then(targets => { - if (targets.length > 0) { - void this.discoverTestsInWorkspace(this.tokenSource.token); - } - }); - } + const endProcessDisposable = this.tasks.onDidEndTaskProcess(event => { + const task = event.execution.task; + const execution = task.execution as vscode.ProcessExecution; + if ( + task.scope === folderContext.workspaceFolder && + task.group === vscode.TaskGroup.Build && + execution?.options?.cwd === folderContext.folder.fsPath && + event.exitCode === 0 && + task.definition.dontTriggerTestDiscovery !== true && + testFileEdited + ) { + testFileEdited = false; + + // only run discover tests if the library has tests + void folderContext.swiftPackage.getTargets(TargetType.test).then(targets => { + if (targets.length > 0) { + void this.discoverTestsInWorkspace(this.tokenSource.token); + } + }); } - ); + }); // add file watcher to catch changes to swift test files - const didChangeSwiftFileDisposable = folderContext.workspaceContext.onDidChangeSwiftFiles( - ({ uri }) => { - if (testFileEdited === false) { - void folderContext.getTestTarget(uri).then(target => { - if (target) { - testFileEdited = true; - } - }); - } + const didChangeSwiftFileDisposable = this.onDidChangeSwiftFiles(({ uri }) => { + if (testFileEdited === false) { + void folderContext.getTestTarget(uri).then(target => { + if (target) { + testFileEdited = true; + } + }); } - ); - - return { - dispose: () => { - endProcessDisposable.dispose(); - didChangeSwiftFileDisposable.dispose(); - }, - }; - } + }); - dispose() { - this.controller.refreshHandler = undefined; - this.controller.resolveHandler = undefined; - this.tokenSource.cancel(); - this.subscriptions.forEach(element => element.dispose()); + return compositeDisposable(endProcessDisposable, didChangeSwiftFileDisposable); } /** @@ -189,7 +196,7 @@ export class TestExplorer { * @param workspaceContext Workspace context for extension * @returns Observer disposable */ - static observeFolders(workspaceContext: WorkspaceContext): vscode.Disposable { + public static observeFolders(workspaceContext: WorkspaceContext): vscode.Disposable { const tokenSource = new vscode.CancellationTokenSource(); const disposable = workspaceContext.onDidChangeFolders(({ folder, operation }) => { switch (operation) { @@ -201,12 +208,7 @@ export class TestExplorer { break; } }); - return { - dispose: () => { - tokenSource.dispose(); - disposable.dispose(); - }, - }; + return compositeDisposable(tokenSource, disposable); } /** @@ -270,7 +272,7 @@ export class TestExplorer { // we fall back to discovering tests with SPM. await this.discoverTestsInWorkspaceLSP(token); } catch { - this.folderContext.workspaceContext.logger.debug( + this.logger.debug( "workspace/tests LSP request not supported, falling back to SPM to discover tests.", "Test Discovery" ); @@ -283,7 +285,7 @@ export class TestExplorer { * Uses `swift test --list-tests` to get the list of tests */ private async discoverTestsInWorkspaceSPM(token: vscode.CancellationToken) { - async function runDiscover(explorer: TestExplorer, firstTry: boolean) { + const runDiscover = async (explorer: TestExplorer, firstTry: boolean) => { try { // we depend on sourcekit-lsp to detect swift-testing tests so let the user know // that things won't work properly if sourcekit-lsp has been disabled for some reason @@ -300,7 +302,7 @@ export class TestExplorer { ) .then(selected => { if (selected === enable) { - explorer.folderContext.workspaceContext.logger.info( + this.logger.info( `Enabling SourceKit-LSP after swift-testing message` ); void vscode.workspace @@ -310,16 +312,18 @@ export class TestExplorer { /* Put in worker queue */ }); } else if (selected === ok) { - explorer.folderContext.workspaceContext.logger.info( + this.logger.info( `User acknowledged that SourceKit-LSP is disabled` ); } }); } const toolchain = explorer.folderContext.toolchain; + // get build options before build is run so we can be sure they aren't changed // mid-build const testBuildOptions = buildOptions(toolchain); + // normally we wouldn't run the build here, but you can suspend swiftPM on macOS // if you try and list tests while skipping the build if you are using a different // sanitizer settings @@ -399,13 +403,13 @@ export class TestExplorer { } else { explorer.setErrorTestItem(errorDescription); } - explorer.folderContext.workspaceContext.logger.error( + this.logger.error( `Test Discovery Failed: ${errorDescription}`, explorer.folderContext.name ); } } - } + }; await runDiscover(this, true); } @@ -439,9 +443,7 @@ export class TestExplorer { * @param errorDescription Error description to display */ private setErrorTestItem(errorDescription: string | undefined, title = "Test Discovery Error") { - this.folderContext.workspaceContext.logger.error( - `Test Discovery Error: ${errorDescription}` - ); + this.logger.error(`Test Discovery Error: ${errorDescription}`); this.controller.items.forEach(item => { this.controller.items.delete(item.id); }); diff --git a/src/WorkspaceContext.ts b/src/WorkspaceContext.ts index 252f67f65..6c48d7408 100644 --- a/src/WorkspaceContext.ts +++ b/src/WorkspaceContext.ts @@ -73,6 +73,9 @@ export class WorkspaceContext implements vscode.Disposable { public onDidStartBuild = this.buildStartEmitter.event; public onDidFinishBuild = this.buildFinishEmitter.event; + private observers = new Set<(listener: FolderEvent) => unknown>(); + private swiftFileObservers = new Set<(listener: SwiftFileEvent) => unknown>(); + public loggerFactory: SwiftLoggerFactory; constructor( @@ -627,9 +630,6 @@ export class WorkspaceContext implements vscode.Disposable { } return autoGenerate; } - - private observers = new Set<(listener: FolderEvent) => unknown>(); - private swiftFileObservers = new Set<(listener: SwiftFileEvent) => unknown>(); } /** Test events for test run begin/end */ diff --git a/src/utilities/utilities.ts b/src/utilities/utilities.ts index fde6f2e23..8d3cb1d08 100644 --- a/src/utilities/utilities.ts +++ b/src/utilities/utilities.ts @@ -443,3 +443,16 @@ export function destructuredPromise(): { return { promise: p, resolve: resolve!, reject: reject! }; } /* eslint-enable @typescript-eslint/no-explicit-any */ + +/** + * Creates a composite disposable from multiple disposables. + * @param disposables The disposables to include. + * @returns A composite disposable that disposes all included disposables. + */ +export function compositeDisposable(...disposables: vscode.Disposable[]): vscode.Disposable { + return { + dispose: () => { + disposables.forEach(d => d.dispose()); + }, + }; +} diff --git a/test/integration-tests/commands/runTestMultipleTimes.test.ts b/test/integration-tests/commands/runTestMultipleTimes.test.ts index bc82fbf12..dbf3e7a4f 100644 --- a/test/integration-tests/commands/runTestMultipleTimes.test.ts +++ b/test/integration-tests/commands/runTestMultipleTimes.test.ts @@ -27,12 +27,9 @@ suite("Test Multiple Times Command Test Suite", () => { activateExtensionForSuite({ async setup(ctx) { folderContext = await folderInRootWorkspace("defaultPackage", ctx); - folderContext.addTestExplorer(); + const testExplorer = await folderContext.resolvedTestExplorer; - const item = folderContext.testExplorer?.controller.createTestItem( - "testId", - "Test Item For Testing" - ); + const item = testExplorer.controller.createTestItem("testId", "Test Item For Testing"); expect(item).to.not.be.undefined; testItem = item!; diff --git a/test/integration-tests/testexplorer/TestExplorerIntegration.test.ts b/test/integration-tests/testexplorer/TestExplorerIntegration.test.ts index e49581d66..7de6cd84b 100644 --- a/test/integration-tests/testexplorer/TestExplorerIntegration.test.ts +++ b/test/integration-tests/testexplorer/TestExplorerIntegration.test.ts @@ -71,7 +71,7 @@ suite("Test Explorer Suite", function () { throw new Error("Unable to find test explorer"); } - testExplorer = folderContext.addTestExplorer(); + testExplorer = await folderContext.resolvedTestExplorer; await executeTaskAndWaitForResult(await createBuildAllTask(folderContext));