From c4749e67502e7bf40d231650d2cf5d2324a5e1ba Mon Sep 17 00:00:00 2001 From: Adam Ward Date: Wed, 4 Jun 2025 08:20:46 -0400 Subject: [PATCH 1/4] Try to trigger intermittent failure Looks like LSP was not started for these tests --- .vscode-test.js | 2 +- test/integration-tests/commands/build.test.ts | 139 +++++++++--------- 2 files changed, 72 insertions(+), 69 deletions(-) diff --git a/.vscode-test.js b/.vscode-test.js index 56aa8ef89..5521fca6f 100644 --- a/.vscode-test.js +++ b/.vscode-test.js @@ -74,7 +74,7 @@ module.exports = defineConfig({ ui: "tdd", color: true, timeout, - forbidOnly: isCIBuild, + forbidOnly: false, grep: isFastTestRun ? "@slow" : undefined, invert: isFastTestRun, slow: 10000, diff --git a/test/integration-tests/commands/build.test.ts b/test/integration-tests/commands/build.test.ts index 00730d114..1e5b3bc12 100644 --- a/test/integration-tests/commands/build.test.ts +++ b/test/integration-tests/commands/build.test.ts @@ -25,84 +25,87 @@ import { continueSession, waitForDebugAdapterRequest } from "../../utilities/deb import { activateExtensionForSuite, folderInRootWorkspace } from "../utilities/testutilities"; import { Version } from "../../../src/utilities/version"; -suite("Build Commands @slow", function () { - // Default timeout is a bit too short, give it a little bit more time - this.timeout(3 * 60 * 1000); +for (let i = 0; i < 25; ++i) { + suite.only("Build Commands @slow " + i, function () { + // Default timeout is a bit too short, give it a little bit more time + this.timeout(3 * 60 * 1000); - let folderContext: FolderContext; - let workspaceContext: WorkspaceContext; - const uri = testAssetUri("defaultPackage/Sources/PackageExe/main.swift"); - const breakpoints = [ - new vscode.SourceBreakpoint(new vscode.Location(uri, new vscode.Position(2, 0))), - ]; + let folderContext: FolderContext; + let workspaceContext: WorkspaceContext; + const uri = testAssetUri("defaultPackage/Sources/PackageExe/main.swift"); + const breakpoints = [ + new vscode.SourceBreakpoint(new vscode.Location(uri, new vscode.Position(2, 0))), + ]; - activateExtensionForSuite({ - async setup(ctx) { - // The description of this package is crashing on Windows with Swift 5.9.x and below - if ( - process.platform === "win32" && - ctx.globalToolchain.swiftVersion.isLessThan(new Version(5, 10, 0)) - ) { - this.skip(); - } - // A breakpoint will have not effect on the Run command. - vscode.debug.addBreakpoints(breakpoints); + activateExtensionForSuite({ + async setup(ctx) { + // The description of this package is crashing on Windows with Swift 5.9.x and below + if ( + process.platform === "win32" && + ctx.globalToolchain.swiftVersion.isLessThan(new Version(5, 10, 0)) + ) { + this.skip(); + } + // A breakpoint will have not effect on the Run command. + vscode.debug.addBreakpoints(breakpoints); - workspaceContext = ctx; - await waitForNoRunningTasks(); - folderContext = await folderInRootWorkspace("defaultPackage", workspaceContext); - await workspaceContext.focusFolder(folderContext); - }, - requiresDebugger: true, - }); + workspaceContext = ctx; + await waitForNoRunningTasks(); + folderContext = await folderInRootWorkspace("defaultPackage", workspaceContext); + await workspaceContext.focusFolder(folderContext); + }, + requiresDebugger: true, + requiresLSP: true, + }); - suiteTeardown(async () => { - vscode.debug.removeBreakpoints(breakpoints); - }); + suiteTeardown(async () => { + vscode.debug.removeBreakpoints(breakpoints); + }); - test("Swift: Run Build", async () => { - const result = await vscode.commands.executeCommand(Commands.RUN, "PackageExe"); - expect(result).to.be.true; - }); + test("Swift: Run Build", async () => { + const result = await vscode.commands.executeCommand(Commands.RUN, "PackageExe"); + expect(result).to.be.true; + }); - test("Swift: Debug Build", async () => { - // Promise used to indicate we hit the break point. - // NB: "stopped" is the exact command when debuggee has stopped due to break point, - // but "stackTrace" is the deterministic sync point we will use to make sure we can execute continue - const bpPromise = waitForDebugAdapterRequest( - "Debug PackageExe (defaultPackage)", - workspaceContext.globalToolchain.swiftVersion, - "stackTrace" - ); + test("Swift: Debug Build", async () => { + // Promise used to indicate we hit the break point. + // NB: "stopped" is the exact command when debuggee has stopped due to break point, + // but "stackTrace" is the deterministic sync point we will use to make sure we can execute continue + const bpPromise = waitForDebugAdapterRequest( + "Debug PackageExe (defaultPackage)", + workspaceContext.globalToolchain.swiftVersion, + "stackTrace" + ); - const resultPromise: Thenable = vscode.commands.executeCommand( - Commands.DEBUG, - "PackageExe" - ); + const resultPromise: Thenable = vscode.commands.executeCommand( + Commands.DEBUG, + "PackageExe" + ); - await bpPromise; - let succeeded = false; - void resultPromise.then(s => (succeeded = s)); - while (!succeeded) { - await continueSession(); - await new Promise(r => setTimeout(r, 500)); - } - await expect(resultPromise).to.eventually.be.true; - }); + await bpPromise; + let succeeded = false; + void resultPromise.then(s => (succeeded = s)); + while (!succeeded) { + await continueSession(); + await new Promise(r => setTimeout(r, 500)); + } + await expect(resultPromise).to.eventually.be.true; + }); - test("Swift: Clean Build", async () => { - let result = await vscode.commands.executeCommand(Commands.RUN, "PackageExe"); - expect(result).to.be.true; + test("Swift: Clean Build", async () => { + let result = await vscode.commands.executeCommand(Commands.RUN, "PackageExe"); + expect(result).to.be.true; - const buildPath = path.join(folderContext.folder.fsPath, ".build"); - const beforeItemCount = (await fs.readdir(buildPath)).length; + const buildPath = path.join(folderContext.folder.fsPath, ".build"); + const beforeItemCount = (await fs.readdir(buildPath)).length; - result = await vscode.commands.executeCommand(Commands.CLEAN_BUILD); - expect(result).to.be.true; + result = await vscode.commands.executeCommand(Commands.CLEAN_BUILD); + expect(result).to.be.true; - const afterItemCount = (await fs.readdir(buildPath)).length; - // .build folder is going to be filled with built artifacts after Commands.RUN command - // After executing the clean command the build directory is guranteed to have less entry. - expect(afterItemCount).to.be.lessThan(beforeItemCount); + const afterItemCount = (await fs.readdir(buildPath)).length; + // .build folder is going to be filled with built artifacts after Commands.RUN command + // After executing the clean command the build directory is guranteed to have less entry. + expect(afterItemCount).to.be.lessThan(beforeItemCount); + }); }); -}); +} From 2aebcd46b9dddf6a495132e9560f3057cea4ee0e Mon Sep 17 00:00:00 2001 From: Adam Ward Date: Wed, 4 Jun 2025 09:13:28 -0400 Subject: [PATCH 2/4] Make sure LSP is ready before adding folder Make sure we're not racing with LSP startup --- src/sourcekit-lsp/LanguageClientToolchainCoordinator.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/sourcekit-lsp/LanguageClientToolchainCoordinator.ts b/src/sourcekit-lsp/LanguageClientToolchainCoordinator.ts index 1097cb6f8..1f6f8f4d2 100644 --- a/src/sourcekit-lsp/LanguageClientToolchainCoordinator.ts +++ b/src/sourcekit-lsp/LanguageClientToolchainCoordinator.ts @@ -62,7 +62,10 @@ export class LanguageClientToolchainCoordinator implements vscode.Disposable { case FolderOperation.add: { const client = await this.create(folder, singleServer, languageClientFactory); await (singleServer - ? client.addFolder(folder) + ? client.useLanguageClient(async () => { + // Just want to wait for client to be ready + await client.addFolder(folder); + }) : client.setLanguageClientFolder(folder)); break; } From cf8430976514a67e9adb8ed469dad2f2e12eb2b2 Mon Sep 17 00:00:00 2001 From: Adam Ward Date: Wed, 4 Jun 2025 10:01:15 -0400 Subject: [PATCH 3/4] Revert debugging changes --- .vscode-test.js | 2 +- test/integration-tests/commands/build.test.ts | 139 +++++++++--------- 2 files changed, 69 insertions(+), 72 deletions(-) diff --git a/.vscode-test.js b/.vscode-test.js index 5521fca6f..56aa8ef89 100644 --- a/.vscode-test.js +++ b/.vscode-test.js @@ -74,7 +74,7 @@ module.exports = defineConfig({ ui: "tdd", color: true, timeout, - forbidOnly: false, + forbidOnly: isCIBuild, grep: isFastTestRun ? "@slow" : undefined, invert: isFastTestRun, slow: 10000, diff --git a/test/integration-tests/commands/build.test.ts b/test/integration-tests/commands/build.test.ts index 1e5b3bc12..00730d114 100644 --- a/test/integration-tests/commands/build.test.ts +++ b/test/integration-tests/commands/build.test.ts @@ -25,87 +25,84 @@ import { continueSession, waitForDebugAdapterRequest } from "../../utilities/deb import { activateExtensionForSuite, folderInRootWorkspace } from "../utilities/testutilities"; import { Version } from "../../../src/utilities/version"; -for (let i = 0; i < 25; ++i) { - suite.only("Build Commands @slow " + i, function () { - // Default timeout is a bit too short, give it a little bit more time - this.timeout(3 * 60 * 1000); +suite("Build Commands @slow", function () { + // Default timeout is a bit too short, give it a little bit more time + this.timeout(3 * 60 * 1000); - let folderContext: FolderContext; - let workspaceContext: WorkspaceContext; - const uri = testAssetUri("defaultPackage/Sources/PackageExe/main.swift"); - const breakpoints = [ - new vscode.SourceBreakpoint(new vscode.Location(uri, new vscode.Position(2, 0))), - ]; + let folderContext: FolderContext; + let workspaceContext: WorkspaceContext; + const uri = testAssetUri("defaultPackage/Sources/PackageExe/main.swift"); + const breakpoints = [ + new vscode.SourceBreakpoint(new vscode.Location(uri, new vscode.Position(2, 0))), + ]; - activateExtensionForSuite({ - async setup(ctx) { - // The description of this package is crashing on Windows with Swift 5.9.x and below - if ( - process.platform === "win32" && - ctx.globalToolchain.swiftVersion.isLessThan(new Version(5, 10, 0)) - ) { - this.skip(); - } - // A breakpoint will have not effect on the Run command. - vscode.debug.addBreakpoints(breakpoints); + activateExtensionForSuite({ + async setup(ctx) { + // The description of this package is crashing on Windows with Swift 5.9.x and below + if ( + process.platform === "win32" && + ctx.globalToolchain.swiftVersion.isLessThan(new Version(5, 10, 0)) + ) { + this.skip(); + } + // A breakpoint will have not effect on the Run command. + vscode.debug.addBreakpoints(breakpoints); - workspaceContext = ctx; - await waitForNoRunningTasks(); - folderContext = await folderInRootWorkspace("defaultPackage", workspaceContext); - await workspaceContext.focusFolder(folderContext); - }, - requiresDebugger: true, - requiresLSP: true, - }); + workspaceContext = ctx; + await waitForNoRunningTasks(); + folderContext = await folderInRootWorkspace("defaultPackage", workspaceContext); + await workspaceContext.focusFolder(folderContext); + }, + requiresDebugger: true, + }); - suiteTeardown(async () => { - vscode.debug.removeBreakpoints(breakpoints); - }); + suiteTeardown(async () => { + vscode.debug.removeBreakpoints(breakpoints); + }); - test("Swift: Run Build", async () => { - const result = await vscode.commands.executeCommand(Commands.RUN, "PackageExe"); - expect(result).to.be.true; - }); + test("Swift: Run Build", async () => { + const result = await vscode.commands.executeCommand(Commands.RUN, "PackageExe"); + expect(result).to.be.true; + }); - test("Swift: Debug Build", async () => { - // Promise used to indicate we hit the break point. - // NB: "stopped" is the exact command when debuggee has stopped due to break point, - // but "stackTrace" is the deterministic sync point we will use to make sure we can execute continue - const bpPromise = waitForDebugAdapterRequest( - "Debug PackageExe (defaultPackage)", - workspaceContext.globalToolchain.swiftVersion, - "stackTrace" - ); + test("Swift: Debug Build", async () => { + // Promise used to indicate we hit the break point. + // NB: "stopped" is the exact command when debuggee has stopped due to break point, + // but "stackTrace" is the deterministic sync point we will use to make sure we can execute continue + const bpPromise = waitForDebugAdapterRequest( + "Debug PackageExe (defaultPackage)", + workspaceContext.globalToolchain.swiftVersion, + "stackTrace" + ); - const resultPromise: Thenable = vscode.commands.executeCommand( - Commands.DEBUG, - "PackageExe" - ); + const resultPromise: Thenable = vscode.commands.executeCommand( + Commands.DEBUG, + "PackageExe" + ); - await bpPromise; - let succeeded = false; - void resultPromise.then(s => (succeeded = s)); - while (!succeeded) { - await continueSession(); - await new Promise(r => setTimeout(r, 500)); - } - await expect(resultPromise).to.eventually.be.true; - }); + await bpPromise; + let succeeded = false; + void resultPromise.then(s => (succeeded = s)); + while (!succeeded) { + await continueSession(); + await new Promise(r => setTimeout(r, 500)); + } + await expect(resultPromise).to.eventually.be.true; + }); - test("Swift: Clean Build", async () => { - let result = await vscode.commands.executeCommand(Commands.RUN, "PackageExe"); - expect(result).to.be.true; + test("Swift: Clean Build", async () => { + let result = await vscode.commands.executeCommand(Commands.RUN, "PackageExe"); + expect(result).to.be.true; - const buildPath = path.join(folderContext.folder.fsPath, ".build"); - const beforeItemCount = (await fs.readdir(buildPath)).length; + const buildPath = path.join(folderContext.folder.fsPath, ".build"); + const beforeItemCount = (await fs.readdir(buildPath)).length; - result = await vscode.commands.executeCommand(Commands.CLEAN_BUILD); - expect(result).to.be.true; + result = await vscode.commands.executeCommand(Commands.CLEAN_BUILD); + expect(result).to.be.true; - const afterItemCount = (await fs.readdir(buildPath)).length; - // .build folder is going to be filled with built artifacts after Commands.RUN command - // After executing the clean command the build directory is guranteed to have less entry. - expect(afterItemCount).to.be.lessThan(beforeItemCount); - }); + const afterItemCount = (await fs.readdir(buildPath)).length; + // .build folder is going to be filled with built artifacts after Commands.RUN command + // After executing the clean command the build directory is guranteed to have less entry. + expect(afterItemCount).to.be.lessThan(beforeItemCount); }); -} +}); From eaa9961107f773edb02eeb4536b39a9415f99f82 Mon Sep 17 00:00:00 2001 From: Adam Ward Date: Wed, 4 Jun 2025 10:52:58 -0400 Subject: [PATCH 4/4] Be more explicit waiting for running state Was still a race on windows --- src/sourcekit-lsp/LanguageClientManager.ts | 24 ++++++++++++------- .../LanguageClientToolchainCoordinator.ts | 5 +--- 2 files changed, 17 insertions(+), 12 deletions(-) diff --git a/src/sourcekit-lsp/LanguageClientManager.ts b/src/sourcekit-lsp/LanguageClientManager.ts index 146cd2f1a..c3ef2e079 100644 --- a/src/sourcekit-lsp/LanguageClientManager.ts +++ b/src/sourcekit-lsp/LanguageClientManager.ts @@ -439,14 +439,21 @@ export class LanguageClientManager implements vscode.Disposable { } private async startClient(client: LanguageClient, errorHandler: SourceKitLSPErrorHandler) { - client.onDidChangeState(e => { - // if state is now running add in any sub-folder workspaces that - // we have cached. If this is the first time we are starting then - // we won't have any sub folder workspaces, but if the server crashed - // or we forced a restart then we need to do this - if (e.oldState === State.Starting && e.newState === State.Running) { - void this.addSubFolderWorkspaces(client); - } + const runningPromise = new Promise((res, rej) => { + const disposable = client.onDidChangeState(e => { + // if state is now running add in any sub-folder workspaces that + // we have cached. If this is the first time we are starting then + // we won't have any sub folder workspaces, but if the server crashed + // or we forced a restart then we need to do this + if (e.oldState === State.Starting && e.newState === State.Running) { + res(); + disposable.dispose(); + void this.addSubFolderWorkspaces(client); + } else if (e.oldState === State.Starting && e.newState === State.Stopped) { + rej("SourceKit-LSP failed to start"); + disposable.dispose(); + } + }); }); if (client.clientOptions.workspaceFolder) { this.folderContext.workspaceContext.outputChannel.log( @@ -465,6 +472,7 @@ export class LanguageClientManager implements vscode.Disposable { // start client this.clientReadyPromise = client .start() + .then(() => runningPromise) .then(() => { // Now that we've started up correctly, start the error handler to auto-restart // if sourcekit-lsp crashes during normal operation. diff --git a/src/sourcekit-lsp/LanguageClientToolchainCoordinator.ts b/src/sourcekit-lsp/LanguageClientToolchainCoordinator.ts index 1f6f8f4d2..1097cb6f8 100644 --- a/src/sourcekit-lsp/LanguageClientToolchainCoordinator.ts +++ b/src/sourcekit-lsp/LanguageClientToolchainCoordinator.ts @@ -62,10 +62,7 @@ export class LanguageClientToolchainCoordinator implements vscode.Disposable { case FolderOperation.add: { const client = await this.create(folder, singleServer, languageClientFactory); await (singleServer - ? client.useLanguageClient(async () => { - // Just want to wait for client to be ready - await client.addFolder(folder); - }) + ? client.addFolder(folder) : client.setLanguageClientFolder(folder)); break; }