Skip to content

Conversation

@ahoppen
Copy link
Member

@ahoppen ahoppen commented Jun 5, 2024

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 #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)
}

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 swiftlang#1412 and keeping the following snippet to fix the underlying issue

```swift
// 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 ahoppen requested review from bnbarham and hamishknight June 5, 2024 21:24
@ahoppen ahoppen requested a review from benlangmuir as a code owner June 5, 2024 21:24
@ahoppen
Copy link
Member Author

ahoppen commented Jun 5, 2024

@swift-ci Please test

@ahoppen ahoppen merged commit da84c30 into swiftlang:main Jun 6, 2024
@ahoppen ahoppen deleted the no-sourcekitd-test-hooks branch June 6, 2024 04:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant