Skip to content

Simplify tests #896

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
ahoppen merged 6 commits into from
Oct 13, 2023
Merged

Simplify tests #896

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
c4ff9d0
Fix race conditions in code completion
ahoppen Oct 12, 2023
efd55c2
Simplify `TestSourceKitServer`
ahoppen Oct 12, 2023
0e85e54
Remove unused functions from `TestClient`
ahoppen Oct 12, 2023
2120d12
Rename `TestSourceKitServer` to `TestSourceKitClient`
ahoppen Oct 12, 2023
64ba40e
Make `TestSourceKitLSPClient.serverOptions` a static member on `Sourc…
ahoppen Oct 12, 2023
6679652
Remove `awaitTask`
ahoppen Oct 12, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion Sources/SourceKitLSP/SourceKitServer.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -535,7 +535,11 @@ extension SourceKitServer: MessageHandler {
// completion session but it only makes sense for the client to request
// more results for this completion session after it has received the
// initial results.
messageHandlingQueue.async(barrier: false) {
// FIXME: (async) We need more granular request handling. Completion requests
// to the same file depend on each other because we only have one global
// code completion session in sourcekitd but they don't need to be full
// barriers to any other request.
Comment on lines +538 to +541
Copy link
Contributor

@bnbarham bnbarham Oct 12, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If there's only the one session, doesn't that mean all completion requests (even across files) have to depend on each other? What happens today if multiple are sent?

We could also just make it an error to request completion when there's already a completion happening.

Copy link
Member Author

@ahoppen ahoppen Oct 12, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let’s discuss any of the code completion scheduling on #899.

messageHandlingQueue.async(barrier: R.self is CompletionRequest.Type) {
let cancellationToken = CancellationToken()

let request = Request(
Expand Down
36 changes: 14 additions & 22 deletions Sources/SourceKitLSP/Swift/CodeCompletionSession.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,6 @@ import SourceKitD
/// `codecomplete.close` requests.
actor CodeCompletionSession {
private unowned let server: SwiftLanguageServer
/// The queue on which `update` and `close` are executed to ensure in-order
/// execution.
private let queue: AsyncQueue = AsyncQueue(.serial)
private let snapshot: DocumentSnapshot
let utf8StartOffset: Int
private let position: Position
Expand Down Expand Up @@ -72,16 +69,13 @@ actor CodeCompletionSession {
in snapshot: DocumentSnapshot,
options: SKCompletionOptions
) async throws -> CompletionList {
let task = queue.asyncThrowing {
switch self.state {
case .closed:
self.state = .open
return try await self.open(filterText: filterText, position: position, in: snapshot, options: options)
case .open:
return try await self.updateImpl(filterText: filterText, position: position, in: snapshot, options: options)
}
switch self.state {
case .closed:
self.state = .open
return try await self.open(filterText: filterText, position: position, in: snapshot, options: options)
case .open:
return try await self.updateImpl(filterText: filterText, position: position, in: snapshot, options: options)
}
return try await task.value
}

private func open(
Expand Down Expand Up @@ -185,19 +179,17 @@ actor CodeCompletionSession {
_ = try? server.sourcekitd.sendSync(req)
}

func close() {
func close() async {
// Temporary back-reference to server to keep it alive during close().
let server = self.server

queue.async {
switch self.state {
case .closed:
// Already closed, nothing to do.
break
case .open:
self.sendClose(server)
self.state = .closed
}
switch self.state {
case .closed:
// Already closed, nothing to do.
break
case .open:
self.sendClose(server)
self.state = .closed
}
}
}
Expand Down