swiftlang / sourcekit-lsp

Language Server Protocol implementation for Swift and C-based languages
Apache License 2.0
3.3k stars 273 forks source link

Remove sourcekitd test hooks #1435

Closed ahoppen closed 4 months ago

ahoppen commented 4 months ago

Turns out that sourcekitd test hooks were a bad idea because of the following comment that I wrote:

`testHooks` are only considered when an instance is being created. If a sourcekitd instance at the given path already exists, its test hooks will be used.

During test execution in Xcode, we generate a bunch of SourceKitServer instances in the same process that all call DynamicallyLoadedSourceKitD.getOrCreate. Now, if testDontReturnEmptyDiagnosticsIfDiagnosticRequestIsCancelled is not the first test being executed in the process (which usually it is not), the test hooks in it won’t get used.

Switch back to using the preparation hooks, essentially reverting https://github.com/apple/sourcekit-lsp/pull/1412 and keeping the following snippet to fix the underlying issue

// Poll until the `CancelRequestNotification` has been propagated to the request handling.
for _ in 0..<Int(defaultTimeout * 100) {
  if Task.isCancelled {
    break
  }
  usleep(10_000)
}
ahoppen commented 4 months ago

@swift-ci Please test