Skip to content

Commit fd12d9d

Browse files
committed
Add some logging for testRecomputeFileWorkspaceMembershipOnPackageSwiftChange
I just saw a failure of this test in CI, and can’t figure out what might have happened. Adding some logging that hopefully helps us if the failure happens again.
1 parent e2e5b7d commit fd12d9d

File tree

5 files changed

+23
-4
lines changed

5 files changed

+23
-4
lines changed

Package.swift

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -305,8 +305,20 @@ let package = Package(
305305
.testTarget(
306306
name: "SourceKitLSPTests",
307307
dependencies: [
308+
"BuildServerProtocol",
309+
"LSPLogging",
310+
"LSPTestSupport",
311+
"LanguageServerProtocol",
312+
"SKCore",
313+
"SKSupport",
308314
"SKTestSupport",
315+
"SourceKitD",
309316
"SourceKitLSP",
317+
.product(name: "IndexStoreDB", package: "indexstore-db"),
318+
.product(name: "ISDBTestSupport", package: "indexstore-db"),
319+
.product(name: "SwiftParser", package: "swift-syntax"),
320+
.product(name: "SwiftSyntax", package: "swift-syntax"),
321+
.product(name: "SwiftToolsSupport-auto", package: "swift-tools-support-core"),
310322
]
311323
),
312324
]

Sources/LSPLogging/Logging.swift

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,9 @@
1515
/// - Info: Helpful but not essential for troubleshooting (not persisted, logged to memory)
1616
/// - Notice/log (Default): Essential for troubleshooting
1717
/// - Error: Error seen during execution
18+
/// - Used eg. if the user sends an erroneous request or if a request fails
1819
/// - Fault: Bug in program
20+
/// - Used eg. if invariants inside sourcekit-lsp are broken and the error is not due to user-provided input
1921

2022
import Foundation
2123

Sources/SKSwiftPMWorkspace/SwiftPMWorkspace.swift

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -363,6 +363,7 @@ extension SwiftPMWorkspace: SKCore.BuildSystem {
363363

364364
public func filesDidChange(_ events: [FileEvent]) async {
365365
if events.contains(where: { self.fileEventShouldTriggerPackageReload(event: $0) }) {
366+
logger.log("Reloading package because of file change")
366367
await orLog("Reloading package") {
367368
// TODO: It should not be necessary to reload the entire package just to get build settings for one file.
368369
try await self.reloadPackage()

Sources/SourceKitLSP/SourceKitServer.swift

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1051,6 +1051,7 @@ extension SourceKitServer: BuildSystemDelegate {
10511051
}
10521052

10531053
public func fileHandlingCapabilityChanged() {
1054+
logger.log("Resetting URI to workspace cache because file handling capability of a workspace changed")
10541055
self.uriToWorkspaceCache = [:]
10551056
}
10561057
}

Tests/SourceKitLSPTests/WorkspaceTests.swift

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
//===----------------------------------------------------------------------===//
1212

1313
import Foundation
14+
import LSPLogging
1415
import LSPTestSupport
1516
import LanguageServerProtocol
1617
@_spi(Testing) import SKCore
@@ -457,7 +458,7 @@ final class WorkspaceTests: XCTestCase {
457458
DocumentURI(ws.scratchDirectory.appendingPathComponent("PackageA"))
458459
)
459460

460-
// Add the otherlib target to Package.swift
461+
// Add the MyExec target to PackageB/Package.swift
461462
let newPackageManifest = """
462463
// swift-tools-version: 5.7
463464
@@ -477,7 +478,7 @@ final class WorkspaceTests: XCTestCase {
477478
.appendingPathComponent("Package.swift")
478479
try newPackageManifest.write(
479480
to: packageBManifestPath,
480-
atomically: false,
481+
atomically: true,
481482
encoding: .utf8
482483
)
483484

@@ -490,7 +491,7 @@ final class WorkspaceTests: XCTestCase {
490491
// Ensure that the DidChangeWatchedFilesNotification is handled before we continue.
491492
_ = try await ws.testClient.send(BarrierRequest())
492493

493-
// After updating Package.swift in PackageB, PackageB can provide proper build settings for MyExec/main.swift and
494+
// After updating PackageB/Package.swift, PackageB can provide proper build settings for MyExec/main.swift and
494495
// thus workspace membership should switch to PackageB.
495496

496497
// Updating the build settings takes a few seconds. Send code completion requests every second until we receive correct results.
@@ -499,10 +500,12 @@ final class WorkspaceTests: XCTestCase {
499500
// Updating the build settings takes a few seconds. Send code completion requests every second until we receive correct results.
500501
let packageBRootUri = DocumentURI(ws.scratchDirectory.appendingPathComponent("PackageB"))
501502
for _ in 0..<30 {
502-
if await ws.testClient.server.workspaceForDocument(uri: mainUri)?.rootUri == packageBRootUri {
503+
let workspace = await ws.testClient.server.workspaceForDocument(uri: mainUri)
504+
if workspace?.rootUri == packageBRootUri {
503505
didReceiveCorrectWorkspaceMembership = true
504506
break
505507
}
508+
logger.log("Received incorrect workspace \(workspace?.rootUri?.pseudoPath ?? "<nil>"). Trying again in 1s")
506509
try await Task.sleep(nanoseconds: 1_000_000_000)
507510
}
508511

0 commit comments

Comments
 (0)