Skip to content

Commit 13cadb9

Browse files
committed
Wait for request to be sent to sourcekitd before cancelling it in SwiftSourceKitPluginTests.testCancellation
Otherwise, the cancellation could get handled before the request actually gets sent to sourcekitd, which is not what we want to test here. There appears to be a second underlying issue that causes unexpected results when we cancel the fast completion request after sending the slow completion request. I’ll look at that in a follow-up PR.
1 parent 4220b7f commit 13cadb9

File tree

4 files changed

+59
-13
lines changed

4 files changed

+59
-13
lines changed

Sources/SourceKitD/DynamicallyLoadedSourceKitD.swift

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,9 @@ package actor DynamicallyLoadedSourceKitD: SourceKitD {
108108
/// List of notification handlers that will be called for each notification.
109109
private var notificationHandlers: [WeakSKDNotificationHandler] = []
110110

111+
/// List of hooks that should be executed for every request sent to sourcekitd.
112+
private var requestHandlingHooks: [UUID: (SKDRequestDictionary) -> Void] = [:]
113+
111114
package static func getOrCreate(
112115
dylibPath: URL,
113116
pluginPaths: PluginPaths?
@@ -204,6 +207,29 @@ package actor DynamicallyLoadedSourceKitD: SourceKitD {
204207
notificationHandlers.removeAll(where: { $0.value == nil || $0.value === handler })
205208
}
206209

210+
/// Execute `body` and invoke `hook` for every sourcekitd request that is sent during the execution time of `body`.
211+
///
212+
/// Note that `hook` will not only be executed for requests sent *by* body but this may also include sourcekitd
213+
/// requests that were sent by other clients of the same `DynamicallyLoadedSourceKitD` instance that just happen to
214+
/// send a request during that time.
215+
///
216+
/// This is intended for testing only.
217+
package func withRequestHandlingHook(
218+
body: () async throws -> Void,
219+
hook: @escaping (SKDRequestDictionary) -> Void
220+
) async rethrows {
221+
let id = UUID()
222+
requestHandlingHooks[id] = hook
223+
defer { requestHandlingHooks[id] = nil }
224+
try await body()
225+
}
226+
227+
package func didSend(request: SKDRequestDictionary) {
228+
for hook in requestHandlingHooks.values {
229+
hook(request)
230+
}
231+
}
232+
207233
package nonisolated func log(request: SKDRequestDictionary) {
208234
logger.info(
209235
"""

Sources/SourceKitD/SourceKitD.swift

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,10 @@ package protocol SourceKitD: AnyObject, Sendable {
8383
/// Removes a previously registered notification handler.
8484
func removeNotificationHandler(_ handler: SKDNotificationHandler) async
8585

86+
/// A function that gets called after the request has been sent to sourcekitd but but does not wait for results to be
87+
/// received. This can be used by clients to implement hooks that should be executed for every sourcekitd request.
88+
func didSend(request: SKDRequestDictionary) async
89+
8690
/// Log the given request.
8791
///
8892
/// This log call is issued during normal operation. It is acceptable for the logger to truncate the log message
@@ -146,6 +150,9 @@ extension SourceKitD {
146150
self.api.send_request(request.dict, &handle) { response in
147151
continuation.resume(returning: SKDResponse(response!, sourcekitd: self))
148152
}
153+
Task {
154+
await self.didSend(request: request)
155+
}
149156
if let handle {
150157
return SourceKitDRequestHandle(handle: handle)
151158
}

Tests/SourceKitDTests/SourceKitDRegistryTests.swift

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,7 @@ final class FakeSourceKitD: SourceKitD {
7474
var values: sourcekitd_api_values { fatalError() }
7575
func addNotificationHandler(_ handler: SKDNotificationHandler) { fatalError() }
7676
func removeNotificationHandler(_ handler: SKDNotificationHandler) { fatalError() }
77+
func didSend(request: SKDRequestDictionary) {}
7778
private init() {
7879
token = nextToken.fetchAndIncrement()
7980
}

Tests/SwiftSourceKitPluginTests/SwiftSourceKitPluginTests.swift

Lines changed: 25 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -311,22 +311,34 @@ final class SwiftSourceKitPluginTests: XCTestCase {
311311
compilerArguments: [path]
312312
)
313313

314+
let slowCompletionRequestSent = self.expectation(description: "slow completion result sent")
314315
let slowCompletionResultReceived = self.expectation(description: "slow completion")
315-
let slowCompletionTask = Task {
316-
do {
317-
_ = try await sourcekitd.completeOpen(
318-
path: path,
319-
position: positions["1️⃣"],
320-
filter: ""
321-
)
322-
XCTFail("Expected completion to be cancelled")
323-
} catch {
324-
XCTAssert(error is CancellationError, "Expected completion to be cancelled, failed with \(error)")
316+
let dynamicallyLoadedSourceKitd = try XCTUnwrap(sourcekitd as? DynamicallyLoadedSourceKitD)
317+
try await dynamicallyLoadedSourceKitd.withRequestHandlingHook {
318+
let slowCompletionTask = Task {
319+
do {
320+
_ = try await sourcekitd.completeOpen(
321+
path: path,
322+
position: positions["1️⃣"],
323+
filter: ""
324+
)
325+
XCTFail("Expected completion to be cancelled")
326+
} catch {
327+
XCTAssert(error is CancellationError, "Expected completion to be cancelled, failed with \(error)")
328+
}
329+
slowCompletionResultReceived.fulfill()
325330
}
326-
slowCompletionResultReceived.fulfill()
331+
// Wait for the slow completion request to actually be sent to sourcekitd. Otherwise, we might hit a cancellation
332+
// check somewhere during request sending and we aren't actually sending the completion request to sourcekitd.
333+
try await fulfillmentOfOrThrow([slowCompletionRequestSent])
334+
335+
slowCompletionTask.cancel()
336+
try await fulfillmentOfOrThrow([slowCompletionResultReceived], timeout: 30)
337+
} hook: { request in
338+
// Check that we aren't matching against a request sent by something else that has handle to the same sourcekitd.
339+
XCTAssert(request.description.contains(path), "Received unexpected request: \(request)")
340+
slowCompletionRequestSent.fulfill()
327341
}
328-
slowCompletionTask.cancel()
329-
try await fulfillmentOfOrThrow([slowCompletionResultReceived], timeout: 30)
330342

331343
let fastCompletionStarted = Date()
332344
let result = try await sourcekitd.completeOpen(

0 commit comments

Comments
 (0)