Skip to content

Commit 1877d47

Browse files
committed
Use pull diagnostics by default in tests
Currently, all tests send publish diagnostics notifications, which is noise in the logs for most tests. Change the tests to use the pull diagnostics model by default and make the push diagnostic model opt-in. rdar://123241539
1 parent 5d8ebc3 commit 1877d47

14 files changed

+161
-94
lines changed

Sources/LanguageServerProtocol/SupportTypes/ClientCapabilities.swift

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -633,6 +633,11 @@ public struct TextDocumentClientCapabilities: Hashable, Codable {
633633

634634
/// Whether the clients supports related documents for document diagnostic pulls.
635635
public var relatedDocumentSupport: Bool?
636+
637+
public init(dynamicRegistration: Bool? = nil, relatedDocumentSupport: Bool? = nil) {
638+
self.dynamicRegistration = dynamicRegistration
639+
self.relatedDocumentSupport = relatedDocumentSupport
640+
}
636641
}
637642

638643
// MARK: Properties

Sources/SKTestSupport/MultiFileTestWorkspace.swift

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,7 @@ public class MultiFileTestWorkspace {
7070
public init(
7171
files: [RelativeFileLocation: String],
7272
workspaces: (URL) -> [WorkspaceFolder] = { [WorkspaceFolder(uri: DocumentURI($0))] },
73+
usePullDiagnostics: Bool = true,
7374
testName: String = #function
7475
) async throws {
7576
scratchDirectory = try testScratchDir(testName: testName)
@@ -103,6 +104,7 @@ public class MultiFileTestWorkspace {
103104
self.fileData = fileData
104105

105106
self.testClient = try await TestSourceKitLSPClient(
107+
usePullDiagnostics: usePullDiagnostics,
106108
workspaceFolders: workspaces(scratchDirectory),
107109
cleanUp: { [scratchDirectory] in
108110
if cleanScratchDirectories {

Sources/SKTestSupport/SwiftPMTestWorkspace.swift

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ public class SwiftPMTestWorkspace: MultiFileTestWorkspace {
4040
manifest: String = SwiftPMTestWorkspace.defaultPackageManifest,
4141
workspaces: (URL) -> [WorkspaceFolder] = { [WorkspaceFolder(uri: DocumentURI($0))] },
4242
build: Bool = false,
43+
usePullDiagnostics: Bool = true,
4344
testName: String = #function
4445
) async throws {
4546
var filesByPath: [RelativeFileLocation: String] = [:]
@@ -60,6 +61,7 @@ public class SwiftPMTestWorkspace: MultiFileTestWorkspace {
6061
try await super.init(
6162
files: filesByPath,
6263
workspaces: workspaces,
64+
usePullDiagnostics: usePullDiagnostics,
6365
testName: testName
6466
)
6567

Sources/SKTestSupport/TestSourceKitLSPClient.swift

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,13 @@ public final class TestSourceKitLSPClient: MessageHandler {
4646
/// The server that handles the requests.
4747
public let server: SourceKitServer
4848

49+
/// Whether pull or push-model diagnostics should be used.
50+
///
51+
/// This is used to fail the `nextDiagnosticsNotification` function early in case the pull-diagnostics model is used
52+
/// to avoid a fruitful debug for why no diagnostic request is being sent push diagnostics have been explicitly
53+
/// disabled.
54+
private let usePullDiagnostics: Bool
55+
4956
/// The connection via which the server sends requests and notifications to us.
5057
private let serverToClientConnection: LocalConnection
5158

@@ -74,6 +81,7 @@ public final class TestSourceKitLSPClient: MessageHandler {
7481
/// `true` by default
7582
/// - initializationOptions: Initialization options to pass to the SourceKit-LSP server.
7683
/// - capabilities: The test client's capabilities.
84+
/// - usePullDiagnostics: Whether to use push diagnostics or use push-based diagnostics
7785
/// - workspaceFolders: Workspace folders to open.
7886
/// - cleanUp: A closure that is called when the `TestSourceKitLSPClient` is destructed.
7987
/// This allows e.g. a `IndexedSingleSwiftFileWorkspace` to delete its temporary files when they are no longer
@@ -84,6 +92,7 @@ public final class TestSourceKitLSPClient: MessageHandler {
8492
initialize: Bool = true,
8593
initializationOptions: LSPAny? = nil,
8694
capabilities: ClientCapabilities = ClientCapabilities(),
95+
usePullDiagnostics: Bool = true,
8796
workspaceFolders: [WorkspaceFolder]? = nil,
8897
cleanUp: @escaping () -> Void = {}
8998
) async throws {
@@ -115,8 +124,28 @@ public final class TestSourceKitLSPClient: MessageHandler {
115124
)
116125

117126
self.cleanUp = cleanUp
127+
self.usePullDiagnostics = usePullDiagnostics
118128
self.serverToClientConnection.start(handler: WeakMessageHandler(self))
119129

130+
var capabilities = capabilities
131+
if usePullDiagnostics {
132+
if capabilities.textDocument == nil {
133+
capabilities.textDocument = TextDocumentClientCapabilities()
134+
}
135+
guard capabilities.textDocument!.diagnostic == nil else {
136+
struct ConflictingDiagnosticsError: Error, CustomStringConvertible {
137+
var description: String {
138+
"usePushDiagnostics = false is not supported if capabilities already contain diagnostic options"
139+
}
140+
}
141+
throw ConflictingDiagnosticsError()
142+
}
143+
capabilities.textDocument!.diagnostic = .init(dynamicRegistration: true)
144+
self.handleNextRequest { (request: RegisterCapabilityRequest) in
145+
XCTAssertEqual(request.registrations.only?.method, DocumentDiagnosticsRequest.method)
146+
return VoidResponse()
147+
}
148+
}
120149
if initialize {
121150
_ = try await self.send(
122151
InitializeRequest(
@@ -201,6 +230,12 @@ public final class TestSourceKitLSPClient: MessageHandler {
201230
public func nextDiagnosticsNotification(
202231
timeout: TimeInterval = defaultTimeout
203232
) async throws -> PublishDiagnosticsNotification {
233+
guard !usePullDiagnostics else {
234+
struct PushDiagnosticsError: Error, CustomStringConvertible {
235+
var description = "Client is using the diagnostics and will thus never receive a diagnostics notification"
236+
}
237+
throw PushDiagnosticsError()
238+
}
204239
return try await nextNotification(ofType: PublishDiagnosticsNotification.self, timeout: timeout)
205240
}
206241

Tests/SourceKitDTests/CrashRecoveryTests.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ final class CrashRecoveryTests: XCTestCase {
4848
try SkipUnless.platformIsDarwin("Linux and Windows use in-process sourcekitd")
4949
try SkipUnless.longTestsEnabled()
5050

51-
let testClient = try await TestSourceKitLSPClient()
51+
let testClient = try await TestSourceKitLSPClient(usePullDiagnostics: false)
5252
let uri = DocumentURI.for(.swift)
5353

5454
let positions = testClient.openDocument(

Tests/SourceKitLSPTests/BuildSystemTests.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,7 @@ final class BuildSystemTests: XCTestCase {
9090
private var haveClangd: Bool = false
9191

9292
override func setUp() async throws {
93-
testClient = try await TestSourceKitLSPClient()
93+
testClient = try await TestSourceKitLSPClient(usePullDiagnostics: false)
9494
buildSystem = TestBuildSystem()
9595

9696
let server = testClient.server

Tests/SourceKitLSPTests/CodeActionTests.swift

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -340,7 +340,10 @@ final class CodeActionTests: XCTestCase {
340340
}
341341

342342
func testCodeActionsRemovePlaceholders() async throws {
343-
let testClient = try await TestSourceKitLSPClient(capabilities: clientCapabilitiesWithCodeActionSupport())
343+
let testClient = try await TestSourceKitLSPClient(
344+
capabilities: clientCapabilitiesWithCodeActionSupport(),
345+
usePullDiagnostics: false
346+
)
344347
let uri = DocumentURI.for(.swift)
345348

346349
let positions = testClient.openDocument(

Tests/SourceKitLSPTests/DependencyTrackingTests.swift

Lines changed: 20 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,8 @@ final class DependencyTrackingTests: XCTestCase {
4141
.target(name: "LibB", dependencies: ["LibA"]),
4242
]
4343
)
44-
"""
44+
""",
45+
usePullDiagnostics: false
4546
)
4647

4748
let (libBUri, _) = try ws.openDocument("LibB.swift")
@@ -69,21 +70,24 @@ final class DependencyTrackingTests: XCTestCase {
6970
}
7071

7172
func testDependenciesUpdatedCXX() async throws {
72-
let ws = try await MultiFileTestWorkspace(files: [
73-
"lib.c": """
74-
int libX(int value) {
75-
return value ? 22 : 0;
76-
}
77-
""",
78-
"main.c": """
79-
#include "lib-generated.h"
80-
81-
int main(int argc, const char *argv[]) {
82-
return libX(argc);
83-
}
84-
""",
85-
"compile_flags.txt": "",
86-
])
73+
let ws = try await MultiFileTestWorkspace(
74+
files: [
75+
"lib.c": """
76+
int libX(int value) {
77+
return value ? 22 : 0;
78+
}
79+
""",
80+
"main.c": """
81+
#include "lib-generated.h"
82+
83+
int main(int argc, const char *argv[]) {
84+
return libX(argc);
85+
}
86+
""",
87+
"compile_flags.txt": "",
88+
],
89+
usePullDiagnostics: false
90+
)
8791

8892
let generatedHeaderURL = try ws.uri(for: "main.c").fileURL!.deletingLastPathComponent()
8993
.appendingPathComponent("lib-generated.h", isDirectory: false)

Tests/SourceKitLSPTests/LocalClangTests.swift

Lines changed: 61 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -160,7 +160,7 @@ final class LocalClangTests: XCTestCase {
160160
}
161161

162162
func testCodeAction() async throws {
163-
let testClient = try await TestSourceKitLSPClient()
163+
let testClient = try await TestSourceKitLSPClient(usePullDiagnostics: false)
164164
let uri = DocumentURI.for(.cpp)
165165
let positions = testClient.openDocument(
166166
"""
@@ -220,16 +220,19 @@ final class LocalClangTests: XCTestCase {
220220
// Note: tests generally should avoid including system headers
221221
// to keep them fast and portable. This test is specifically
222222
// ensuring clangd can find libc++ and builtin headers.
223-
let ws = try await MultiFileTestWorkspace(files: [
224-
"main.cpp": """
225-
#include <cstdint>
223+
let ws = try await MultiFileTestWorkspace(
224+
files: [
225+
"main.cpp": """
226+
#include <cstdint>
226227
227-
void test() {
228-
uint64_t 1️⃣b2️⃣;
229-
}
230-
""",
231-
"compile_flags.txt": "-Wunused-variable",
232-
])
228+
void test() {
229+
uint64_t 1️⃣b2️⃣;
230+
}
231+
""",
232+
"compile_flags.txt": "-Wunused-variable",
233+
],
234+
usePullDiagnostics: false
235+
)
233236

234237
let (_, positions) = try ws.openDocument("main.cpp")
235238

@@ -243,41 +246,44 @@ final class LocalClangTests: XCTestCase {
243246
}
244247

245248
func testClangModules() async throws {
246-
let ws = try await MultiFileTestWorkspace(files: [
247-
"ClangModuleA.h": """
248-
#ifndef ClangModuleA_h
249-
#define ClangModuleA_h
249+
let ws = try await MultiFileTestWorkspace(
250+
files: [
251+
"ClangModuleA.h": """
252+
#ifndef ClangModuleA_h
253+
#define ClangModuleA_h
250254
251-
void func_ClangModuleA(void);
255+
void func_ClangModuleA(void);
252256
253-
#endif
254-
""",
255-
"ClangModules_main.m": """
256-
@import ClangModuleA;
257+
#endif
258+
""",
259+
"ClangModules_main.m": """
260+
@import ClangModuleA;
257261
258-
void test(void) {
259-
func_ClangModuleA();
260-
}
261-
""",
262-
"module.modulemap": """
263-
module ClangModuleA {
264-
header "ClangModuleA.h"
265-
}
266-
""",
267-
"compile_flags.txt": """
268-
-I
269-
$TEST_DIR
270-
-fmodules
271-
""",
272-
])
262+
void test(void) {
263+
func_ClangModuleA();
264+
}
265+
""",
266+
"module.modulemap": """
267+
module ClangModuleA {
268+
header "ClangModuleA.h"
269+
}
270+
""",
271+
"compile_flags.txt": """
272+
-I
273+
$TEST_DIR
274+
-fmodules
275+
""",
276+
],
277+
usePullDiagnostics: false
278+
)
273279
_ = try ws.openDocument("ClangModules_main.m")
274280

275281
let diags = try await ws.testClient.nextDiagnosticsNotification()
276282
XCTAssertEqual(diags.diagnostics.count, 0)
277283
}
278284

279285
func testSemanticHighlighting() async throws {
280-
let testClient = try await TestSourceKitLSPClient()
286+
let testClient = try await TestSourceKitLSPClient(usePullDiagnostics: false)
281287
let uri = DocumentURI.for(.c)
282288

283289
testClient.openDocument(
@@ -298,23 +304,26 @@ final class LocalClangTests: XCTestCase {
298304
}
299305

300306
func testDocumentDependenciesUpdated() async throws {
301-
let ws = try await MultiFileTestWorkspace(files: [
302-
"Object.h": """
303-
struct Object {
304-
int field;
305-
};
306-
307-
struct Object * newObject();
308-
""",
309-
"main.c": """
310-
#include "Object.h"
311-
312-
int main(int argc, const char *argv[]) {
313-
struct Object *obj = 1️⃣newObject();
314-
}
315-
""",
316-
"compile_flags.txt": "",
317-
])
307+
let ws = try await MultiFileTestWorkspace(
308+
files: [
309+
"Object.h": """
310+
struct Object {
311+
int field;
312+
};
313+
314+
struct Object * newObject();
315+
""",
316+
"main.c": """
317+
#include "Object.h"
318+
319+
int main(int argc, const char *argv[]) {
320+
struct Object *obj = 1️⃣newObject();
321+
}
322+
""",
323+
"compile_flags.txt": "",
324+
],
325+
usePullDiagnostics: false
326+
)
318327

319328
let (mainUri, _) = try ws.openDocument("main.c")
320329
let headerUri = try ws.uri(for: "Object.h")

0 commit comments

Comments
 (0)