From 5202a8fc1ca93ccccc7cbc2f6fd94ee48e1cdafc Mon Sep 17 00:00:00 2001 From: Alex Hoppen Date: Thu, 19 Sep 2024 12:47:06 -0700 Subject: [PATCH] Add test cases that launch a SourceKitLSP server using a BSP server --- .gitignore | 1 + .../BuiltInBuildSystemAdapter.swift | 5 +- Sources/BuildSystemIntegration/CMakeLists.txt | 2 +- ...ift => LegacyBuildServerBuildSystem.swift} | 27 +++-- .../BuildServerTestProject.swift | 69 +++++++++++ .../INPUTS/AbstractBuildServer.py | 39 ++++--- .../server.py | 3 +- .../server.py | 3 +- .../SourceKitLSP/DetermineBuildSystem.swift | 2 +- ...ildServerBuildSystemIntegrationTests.swift | 110 ++++++++++++++++++ ...> LegacyBuildServerBuildSystemTests.swift} | 6 +- 11 files changed, 239 insertions(+), 28 deletions(-) rename Sources/BuildSystemIntegration/{BuildServerBuildSystem.swift => LegacyBuildServerBuildSystem.swift} (93%) create mode 100644 Sources/SKTestSupport/BuildServerTestProject.swift create mode 100644 Tests/BuildSystemIntegrationTests/LegacyBuildServerBuildSystemIntegrationTests.swift rename Tests/BuildSystemIntegrationTests/{BuildServerBuildSystemTests.swift => LegacyBuildServerBuildSystemTests.swift} (96%) diff --git a/.gitignore b/.gitignore index 2abd0e14..7bbe02da 100644 --- a/.gitignore +++ b/.gitignore @@ -9,3 +9,4 @@ Package.resolved /*.sublime-workspace /.swiftpm .*.sw? +__pycache__ diff --git a/Sources/BuildSystemIntegration/BuiltInBuildSystemAdapter.swift b/Sources/BuildSystemIntegration/BuiltInBuildSystemAdapter.swift index 6a8be415..c2ea71fc 100644 --- a/Sources/BuildSystemIntegration/BuiltInBuildSystemAdapter.swift +++ b/Sources/BuildSystemIntegration/BuiltInBuildSystemAdapter.swift @@ -48,7 +48,10 @@ private func createBuildSystem( ) async -> BuiltInBuildSystem? { switch buildSystemKind { case .buildServer(let projectRoot): - return await BuildServerBuildSystem(projectRoot: projectRoot, connectionToSourceKitLSP: connectionToSourceKitLSP) + return await LegacyBuildServerBuildSystem( + projectRoot: projectRoot, + connectionToSourceKitLSP: connectionToSourceKitLSP + ) case .compilationDatabase(let projectRoot): return CompilationDatabaseBuildSystem( projectRoot: projectRoot, diff --git a/Sources/BuildSystemIntegration/CMakeLists.txt b/Sources/BuildSystemIntegration/CMakeLists.txt index 1cee4a0a..8cf2d81f 100644 --- a/Sources/BuildSystemIntegration/CMakeLists.txt +++ b/Sources/BuildSystemIntegration/CMakeLists.txt @@ -1,6 +1,5 @@ add_library(BuildSystemIntegration STATIC - BuildServerBuildSystem.swift BuildSettingsLogger.swift BuildSystemManager.swift BuildSystemManagerDelegate.swift @@ -14,6 +13,7 @@ add_library(BuildSystemIntegration STATIC FallbackBuildSettings.swift FileBuildSettings.swift Language+InferredFromFileExtension.swift + LegacyBuildServerBuildSystem.swift MainFilesProvider.swift PathPrefixMapping.swift PrefixMessageWithTaskEmoji.swift diff --git a/Sources/BuildSystemIntegration/BuildServerBuildSystem.swift b/Sources/BuildSystemIntegration/LegacyBuildServerBuildSystem.swift similarity index 93% rename from Sources/BuildSystemIntegration/BuildServerBuildSystem.swift rename to Sources/BuildSystemIntegration/LegacyBuildServerBuildSystem.swift index a448cd88..27214581 100644 --- a/Sources/BuildSystemIntegration/BuildServerBuildSystem.swift +++ b/Sources/BuildSystemIntegration/LegacyBuildServerBuildSystem.swift @@ -41,11 +41,15 @@ func executable(_ name: String) -> String { #endif } -/// A `BuildSystem` based on communicating with a build server +#if compiler(>=6.3) +#warning("We have had a one year transition period to the pull based build server. Consider removing this build server") +#endif + +/// A `BuildSystem` based on communicating with a build server using the old push-based settings model. /// -/// Provides build settings from a build server launched based on a -/// `buildServer.json` configuration file provided in the repo root. -package actor BuildServerBuildSystem: MessageHandler { +/// This build server should be phased out in favor of the pull-based settings model described in +/// https://forums.swift.org/t/extending-functionality-of-build-server-protocol-with-sourcekit-lsp/74400 +package actor LegacyBuildServerBuildSystem: MessageHandler { package let projectRoot: AbsolutePath let serverConfig: BuildServerConfig @@ -252,7 +256,7 @@ private func readResponseDataKey(data: LSPAny?, key: String) -> String? { return nil } -extension BuildServerBuildSystem: BuiltInBuildSystem { +extension LegacyBuildServerBuildSystem: BuiltInBuildSystem { static package func projectRoot(for workspaceFolder: AbsolutePath, options: SourceKitLSPOptions) -> AbsolutePath? { guard localFileSystem.isFile(workspaceFolder.appending(component: "buildServer.json")) else { return nil @@ -267,7 +271,7 @@ extension BuildServerBuildSystem: BuiltInBuildSystem { return WorkspaceBuildTargetsResponse(targets: [ BuildTarget( id: .dummy, - displayName: "Compilation database", + displayName: "BuildServer", baseDirectory: nil, tags: [.test], capabilities: BuildTargetCapabilities(), @@ -279,10 +283,18 @@ extension BuildServerBuildSystem: BuiltInBuildSystem { } package func buildTargetSources(request: BuildTargetSourcesRequest) async throws -> BuildTargetSourcesResponse { + guard request.targets.contains(.dummy) else { + return BuildTargetSourcesResponse(items: []) + } // BuildServerBuildSystem does not support syntactic test discovery or background indexing. // (https://github.com/swiftlang/sourcekit-lsp/issues/1173). // TODO: (BSP migration) Forward this request to the BSP server - return BuildTargetSourcesResponse(items: []) + return BuildTargetSourcesResponse(items: [ + SourcesItem( + target: .dummy, + sources: [SourceItem(uri: DocumentURI(self.projectRoot.asURL), kind: .directory, generated: false)] + ) + ]) } package func didChangeWatchedFiles(notification: OnWatchedFilesDidChangeNotification) {} @@ -304,6 +316,7 @@ extension BuildServerBuildSystem: BuiltInBuildSystem { // which renders this code path dead. let uri = request.textDocument.uri if !urisRegisteredForChanges.contains(uri) { + urisRegisteredForChanges.insert(uri) let request = RegisterForChanges(uri: uri, action: .register) _ = self.buildServer?.send(request) { result in if let error = result.failure { diff --git a/Sources/SKTestSupport/BuildServerTestProject.swift b/Sources/SKTestSupport/BuildServerTestProject.swift new file mode 100644 index 00000000..4748dfbf --- /dev/null +++ b/Sources/SKTestSupport/BuildServerTestProject.swift @@ -0,0 +1,69 @@ +//===----------------------------------------------------------------------===// +// +// This source file is part of the Swift.org open source project +// +// Copyright (c) 2014 - 2024 Apple Inc. and the Swift project authors +// Licensed under Apache License v2.0 with Runtime Library Exception +// +// See https://swift.org/LICENSE.txt for license information +// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors +// +//===----------------------------------------------------------------------===// + +import Foundation +import ISDBTestSupport +import XCTest + +/// The path to the INPUTS directory of shared test projects. +private let skTestSupportInputsDirectory: URL = { + #if os(macOS) + var resources = + productsDirectory + .appendingPathComponent("SourceKitLSP_SKTestSupport.bundle") + .appendingPathComponent("Contents") + .appendingPathComponent("Resources") + if !FileManager.default.fileExists(atPath: resources.path) { + // Xcode and command-line swiftpm differ about the path. + resources.deleteLastPathComponent() + resources.deleteLastPathComponent() + } + #else + let resources = XCTestCase.productsDirectory + .appendingPathComponent("SourceKitLSP_SKTestSupport.resources") + #endif + guard FileManager.default.fileExists(atPath: resources.path) else { + fatalError("missing resources \(resources.path)") + } + return resources.appendingPathComponent("INPUTS", isDirectory: true).standardizedFileURL +}() + +/// Creates a project that uses a BSP server to provide build settings. +/// +/// The build server is implemented in Python on top of the code in `AbstractBuildServer.py`. +package class BuildServerTestProject: MultiFileTestProject { + package init(files: [RelativeFileLocation: String], buildServer: String) async throws { + var files = files + files["buildServer.json"] = """ + { + "name": "client name", + "version": "10", + "bspVersion": "2.0", + "languages": ["a", "b"], + "argv": ["server.py"] + } + """ + files["server.py"] = """ + import sys + from typing import Dict, List, Optional + + sys.path.append("\(skTestSupportInputsDirectory.path)") + + from AbstractBuildServer import AbstractBuildServer + + \(buildServer) + + BuildServer().run() + """ + try await super.init(files: files) + } +} diff --git a/Sources/SKTestSupport/INPUTS/AbstractBuildServer.py b/Sources/SKTestSupport/INPUTS/AbstractBuildServer.py index f25f306e..ba1313dd 100644 --- a/Sources/SKTestSupport/INPUTS/AbstractBuildServer.py +++ b/Sources/SKTestSupport/INPUTS/AbstractBuildServer.py @@ -1,6 +1,6 @@ import json import sys -from typing import Optional +from typing import Dict, List, Optional class RequestError(Exception): @@ -39,14 +39,14 @@ class AbstractBuildServer: try: result = self.handle_message(message) if result: - response_message: dict[str, object] = { + response_message: Dict[str, object] = { "jsonrpc": "2.0", "id": message["id"], "result": result, } self.send_raw_message(response_message) except RequestError as e: - error_response_message: dict[str, object] = { + error_response_message: Dict[str, object] = { "jsonrpc": "2.0", "id": message["id"], "error": { @@ -56,12 +56,12 @@ class AbstractBuildServer: } self.send_raw_message(error_response_message) - def handle_message(self, message: dict[str, object]) -> Optional[dict[str, object]]: + def handle_message(self, message: Dict[str, object]) -> Optional[Dict[str, object]]: """ Dispatch handling of the given method, received from SourceKit-LSP to the message handling function. """ method: str = str(message["method"]) - params: dict[str, object] = message["params"] # type: ignore + params: Dict[str, object] = message["params"] # type: ignore if method == "build/exit": return self.exit(params) elif method == "build/initialize": @@ -77,7 +77,7 @@ class AbstractBuildServer: if "id" in message: raise RequestError(code=-32601, message=f"Method not found: {method}") - def send_raw_message(self, message: dict[str, object]): + def send_raw_message(self, message: Dict[str, object]): """ Send a raw message to SourceKit-LSP. The message needs to have all JSON-RPC wrapper fields. @@ -89,24 +89,37 @@ class AbstractBuildServer: ) sys.stdout.flush() - def send_notification(self, method: str, params: dict[str, object]): + def send_notification(self, method: str, params: Dict[str, object]): """ Send a notification with the given method and parameters to SourceKit-LSP. """ - message: dict[str, object] = { + message: Dict[str, object] = { "jsonrpc": "2.0", "method": method, "params": params, } self.send_raw_message(message) + def send_sourcekit_options_changed(self, uri: str, options: List[str]): + """ + Send a `build/sourceKitOptionsChanged` notification to SourceKit-LSP, informing it about new build settings + using the old push-based settings model. + """ + self.send_notification( + "build/sourceKitOptionsChanged", + { + "uri": uri, + "updatedOptions": {"options": options}, + }, + ) + # Message handling functions. # Subclasses should override these to provide functionality. - def exit(self, notification: dict[str, object]) -> None: + def exit(self, notification: Dict[str, object]) -> None: pass - def initialize(self, request: dict[str, object]) -> dict[str, object]: + def initialize(self, request: Dict[str, object]) -> Dict[str, object]: return { "displayName": "test server", "version": "0.1", @@ -119,11 +132,11 @@ class AbstractBuildServer: }, } - def initialized(self, notification: dict[str, object]) -> None: + def initialized(self, notification: Dict[str, object]) -> None: pass - def register_for_changes(self, notification: dict[str, object]): + def register_for_changes(self, notification: Dict[str, object]): pass - def shutdown(self, notification: dict[str, object]) -> None: + def shutdown(self, notification: Dict[str, object]) -> None: pass diff --git a/Sources/SKTestSupport/INPUTS/BuildServerBuildSystemTests.testBuildTargetsChanged/server.py b/Sources/SKTestSupport/INPUTS/BuildServerBuildSystemTests.testBuildTargetsChanged/server.py index a9eb10eb..01702e11 100755 --- a/Sources/SKTestSupport/INPUTS/BuildServerBuildSystemTests.testBuildTargetsChanged/server.py +++ b/Sources/SKTestSupport/INPUTS/BuildServerBuildSystemTests.testBuildTargetsChanged/server.py @@ -1,5 +1,6 @@ from pathlib import Path import sys +from typing import Dict sys.path.append(str(Path(__file__).parent.parent)) @@ -7,7 +8,7 @@ from AbstractBuildServer import AbstractBuildServer class BuildServer(AbstractBuildServer): - def register_for_changes(self, notification: dict[str, object]): + def register_for_changes(self, notification: Dict[str, object]): if notification["action"] == "register": self.send_notification( "buildTarget/didChange", diff --git a/Sources/SKTestSupport/INPUTS/BuildServerBuildSystemTests.testFileRegistration/server.py b/Sources/SKTestSupport/INPUTS/BuildServerBuildSystemTests.testFileRegistration/server.py index 874d138b..06edf1fd 100755 --- a/Sources/SKTestSupport/INPUTS/BuildServerBuildSystemTests.testFileRegistration/server.py +++ b/Sources/SKTestSupport/INPUTS/BuildServerBuildSystemTests.testFileRegistration/server.py @@ -1,5 +1,6 @@ from pathlib import Path import sys +from typing import Dict sys.path.append(str(Path(__file__).parent.parent)) @@ -7,7 +8,7 @@ from AbstractBuildServer import AbstractBuildServer class BuildServer(AbstractBuildServer): - def register_for_changes(self, notification: dict[str, object]): + def register_for_changes(self, notification: Dict[str, object]): if notification["action"] == "register": self.send_notification( "build/sourceKitOptionsChanged", diff --git a/Sources/SourceKitLSP/DetermineBuildSystem.swift b/Sources/SourceKitLSP/DetermineBuildSystem.swift index 5d8b0640..8b1f311d 100644 --- a/Sources/SourceKitLSP/DetermineBuildSystem.swift +++ b/Sources/SourceKitLSP/DetermineBuildSystem.swift @@ -22,7 +22,7 @@ import struct TSCBasic.RelativePath fileprivate extension WorkspaceType { var buildSystemType: BuiltInBuildSystem.Type { switch self { - case .buildServer: return BuildServerBuildSystem.self + case .buildServer: return LegacyBuildServerBuildSystem.self case .compilationDatabase: return CompilationDatabaseBuildSystem.self case .swiftPM: return SwiftPMBuildSystem.self } diff --git a/Tests/BuildSystemIntegrationTests/LegacyBuildServerBuildSystemIntegrationTests.swift b/Tests/BuildSystemIntegrationTests/LegacyBuildServerBuildSystemIntegrationTests.swift new file mode 100644 index 00000000..fcced013 --- /dev/null +++ b/Tests/BuildSystemIntegrationTests/LegacyBuildServerBuildSystemIntegrationTests.swift @@ -0,0 +1,110 @@ +//===----------------------------------------------------------------------===// +// +// This source file is part of the Swift.org open source project +// +// Copyright (c) 2014 - 2019 Apple Inc. and the Swift project authors +// Licensed under Apache License v2.0 with Runtime Library Exception +// +// See https://swift.org/LICENSE.txt for license information +// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors +// +//===----------------------------------------------------------------------===// + +import BuildServerProtocol +import BuildSystemIntegration +import Foundation +import ISDBTestSupport +import LanguageServerProtocol +import SKSupport +import SKTestSupport +import TSCBasic +import XCTest + +let sdkArgs = + if let defaultSDKPath { + """ + "-sdk", "\(defaultSDKPath)", + """ + } else { + "" + } + +final class LegacyBuildServerBuildSystemIntegrationTests: XCTestCase { + func testBuildSettingsFromBuildServer() async throws { + let project = try await BuildServerTestProject( + files: [ + "Test.swift": """ + #if DEBUG + #error("DEBUG SET") + #else + #error("DEBUG NOT SET") + #endif + """ + ], + buildServer: """ + class BuildServer(AbstractBuildServer): + def register_for_changes(self, notification: Dict[str, object]): + if notification["action"] == "register": + self.send_notification( + "build/sourceKitOptionsChanged", + { + "uri": notification["uri"], + "updatedOptions": { + "options": [ + "$TEST_DIR/Test.swift", + "-DDEBUG", + \(sdkArgs) + ] + }, + }, + ) + """ + ) + + let (uri, _) = try project.openDocument("Test.swift") + try await repeatUntilExpectedResult { + let diags = try await project.testClient.send( + DocumentDiagnosticsRequest(textDocument: TextDocumentIdentifier(uri)) + ) + return diags.fullReport?.items.map(\.message) == ["DEBUG SET"] + } + } + + func testBuildSettingsFromBuildServerChanged() async throws { + let project = try await BuildServerTestProject( + files: [ + "Test.swift": """ + #if DEBUG + #error("DEBUG SET") + #else + #error("DEBUG NOT SET") + #endif + """ + ], + buildServer: """ + import threading + + class BuildServer(AbstractBuildServer): + def send_delayed_options_changed(self, uri: str): + self.send_sourcekit_options_changed(uri, ["$TEST_DIR/Test.swift", "-DDEBUG", \(sdkArgs)]) + + def register_for_changes(self, notification: Dict[str, object]): + if notification["action"] != "register": + return + self.send_sourcekit_options_changed( + notification["uri"], + ["$TEST_DIR/Test.swift", \(sdkArgs)] + ) + threading.Timer(1, self.send_delayed_options_changed, [notification["uri"]]).start() + """ + ) + + let (uri, _) = try project.openDocument("Test.swift") + try await repeatUntilExpectedResult { + let diags = try await project.testClient.send( + DocumentDiagnosticsRequest(textDocument: TextDocumentIdentifier(uri)) + ) + return diags.fullReport?.items.map(\.message) == ["DEBUG SET"] + } + } +} diff --git a/Tests/BuildSystemIntegrationTests/BuildServerBuildSystemTests.swift b/Tests/BuildSystemIntegrationTests/LegacyBuildServerBuildSystemTests.swift similarity index 96% rename from Tests/BuildSystemIntegrationTests/BuildServerBuildSystemTests.swift rename to Tests/BuildSystemIntegrationTests/LegacyBuildServerBuildSystemTests.swift index f3c1511b..35ddaa57 100644 --- a/Tests/BuildSystemIntegrationTests/BuildServerBuildSystemTests.swift +++ b/Tests/BuildSystemIntegrationTests/LegacyBuildServerBuildSystemTests.swift @@ -54,7 +54,7 @@ final class BuildServerBuildSystemTests: XCTestCase { let buildFolder = try! AbsolutePath(validating: NSTemporaryDirectory()) func testServerInitialize() async throws { - let buildSystem = try await BuildServerBuildSystem( + let buildSystem = try await LegacyBuildServerBuildSystem( projectRoot: root, connectionToSourceKitLSP: LocalConnection(receiverName: "Dummy SourceKit-LSP") ) @@ -75,7 +75,7 @@ final class BuildServerBuildSystemTests: XCTestCase { let testMessageHandler = TestMessageHandler(targetExpectations: [ (OnBuildTargetDidChangeNotification(changes: nil), expectation) ]) - let buildSystem = try await BuildServerBuildSystem( + let buildSystem = try await LegacyBuildServerBuildSystem( projectRoot: root, connectionToSourceKitLSP: testMessageHandler.connection ) @@ -109,7 +109,7 @@ final class BuildServerBuildSystemTests: XCTestCase { // BuildSystemManager has a weak reference to delegate. Keep it alive. _fixLifetime(testMessageHandler) } - let buildSystem = try await BuildServerBuildSystem( + let buildSystem = try await LegacyBuildServerBuildSystem( projectRoot: root, connectionToSourceKitLSP: testMessageHandler.connection )