Skip to content

Commit a2d2909

Browse files
ahoppenahoppen
committed
Fix test discovery for Objective-C XCTests
There were two issues with Objective-C XCTest discovery: 1. We were relying on syntactic test discovery after a document is edited. But since we don't have syntactic test discovery for Objective-C tests, this meant that all tests would disappear as a document got edited. Until we get syntactic test discovery for Objective-C, use the semantic index to discover tests, even if they are out-of-date. 2. We were assuming that the `DocumentSymbols` request returned `[DocumentSymbol]` to find the ranges of tests. But clangd returns the alternative `[SymbolInformation]`, which meant that we were only returning the position of a test function’s name instead of the test function’s range. rdar://126810202
1 parent 3fb3a99 commit a2d2909

File tree

4 files changed

+309
-20
lines changed

4 files changed

+309
-20
lines changed

‎Sources/SourceKitLSP/LanguageService.swift

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -198,7 +198,9 @@ public protocol LanguageService: AnyObject {
198198
/// Perform a syntactic scan of the file at the given URI for test cases and test classes.
199199
///
200200
/// This is used as a fallback to show the test cases in a file if the index for a given file is not up-to-date.
201-
func syntacticDocumentTests(for uri: DocumentURI, in workspace: Workspace) async throws -> [TestItem]
201+
///
202+
/// A return value of `nil` indicates that this language service does not support syntactic test discovery.
203+
func syntacticDocumentTests(for uri: DocumentURI, in workspace: Workspace) async throws -> [TestItem]?
202204

203205
/// Crash the language server. Should be used for crash recovery testing only.
204206
func _crash() async

‎Sources/SourceKitLSP/TestDiscovery.swift

Lines changed: 44 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -41,22 +41,26 @@ fileprivate extension SymbolOccurrence {
4141
/// Find the innermost range of a document symbol that contains the given position.
4242
private func findInnermostSymbolRange(
4343
containing position: Position,
44-
documentSymbols documentSymbolsResponse: DocumentSymbolResponse
44+
documentSymbolsResponse: DocumentSymbolResponse
4545
) -> Range<Position>? {
46-
guard case .documentSymbols(let documentSymbols) = documentSymbolsResponse else {
47-
// Both `ClangLanguageService` and `SwiftLanguageService` return `documentSymbols` so we don't need to handle the
48-
// .symbolInformation case.
49-
logger.fault(
50-
"""
51-
Expected documentSymbols response from language service to resolve test ranges but got \
52-
\(documentSymbolsResponse.forLogging)
53-
"""
54-
)
55-
return nil
46+
switch documentSymbolsResponse {
47+
case .documentSymbols(let documentSymbols):
48+
return findInnermostSymbolRange(containing: position, documentSymbols: documentSymbols)
49+
case .symbolInformation(let symbolInformation):
50+
return findInnermostSymbolRange(containing: position, symbolInformation: symbolInformation)
5651
}
52+
}
53+
54+
private func findInnermostSymbolRange(
55+
containing position: Position,
56+
documentSymbols: [DocumentSymbol]
57+
) -> Range<Position>? {
5758
for documentSymbol in documentSymbols where documentSymbol.range.contains(position) {
5859
if let children = documentSymbol.children,
59-
let rangeOfChild = findInnermostSymbolRange(containing: position, documentSymbols: .documentSymbols(children))
60+
let rangeOfChild = findInnermostSymbolRange(
61+
containing: position,
62+
documentSymbolsResponse: .documentSymbols(children)
63+
)
6064
{
6165
// If a child contains the position, prefer that because it's more specific.
6266
return rangeOfChild
@@ -66,6 +70,21 @@ private func findInnermostSymbolRange(
6670
return nil
6771
}
6872

73+
/// Return the smallest range in `symbolInformation` containing `position`.
74+
private func findInnermostSymbolRange(
75+
containing position: Position,
76+
symbolInformation symbolInformationArray: [SymbolInformation]
77+
) -> Range<Position>? {
78+
var bestRange: Range<Position>? = nil
79+
for symbolInformation in symbolInformationArray where symbolInformation.location.range.contains(position) {
80+
let range = symbolInformation.location.range
81+
if bestRange == nil || (bestRange!.lowerBound < range.lowerBound && range.upperBound < bestRange!.upperBound) {
82+
bestRange = range
83+
}
84+
}
85+
return bestRange
86+
}
87+
6988
extension SourceKitLSPServer {
7089
/// Converts a flat list of test symbol occurrences to a hierarchical `TestItem` array, inferring the hierarchical
7190
/// structure from `childOf` relations between the symbol occurrences.
@@ -263,9 +282,15 @@ extension SourceKitLSPServer {
263282

264283
let syntacticTests = try await languageService.syntacticDocumentTests(for: req.textDocument.uri, in: workspace)
265284

266-
if let index = workspace.index(checkedFor: .inMemoryModifiedFiles(documentManager)) {
285+
// We `syntacticDocumentTests` returns `nil`, it indicates that it doesn't support syntactic test discovery.
286+
// In that case, the semantic index is the only source of tests we have and we thus want to show tests from the
287+
// semantic index, even if they are out-of-date. The alternative would be showing now tests after an edit to a file.
288+
let indexCheckLevel: IndexCheckLevel =
289+
syntacticTests == nil ? .deletedFiles : .inMemoryModifiedFiles(documentManager)
290+
291+
if let index = workspace.index(checkedFor: indexCheckLevel) {
267292
var syntacticSwiftTestingTests: [TestItem] {
268-
syntacticTests.filter { $0.style == TestStyle.swiftTesting }
293+
syntacticTests?.filter { $0.style == TestStyle.swiftTesting } ?? []
269294
}
270295

271296
let testSymbols =
@@ -283,7 +308,7 @@ extension SourceKitLSPServer {
283308
for: testSymbols,
284309
resolveLocation: { uri, position in
285310
if uri == snapshot.uri, let documentSymbols,
286-
let range = findInnermostSymbolRange(containing: position, documentSymbols: documentSymbols)
311+
let range = findInnermostSymbolRange(containing: position, documentSymbolsResponse: documentSymbols)
287312
{
288313
return Location(uri: uri, range: range)
289314
}
@@ -298,7 +323,7 @@ extension SourceKitLSPServer {
298323
}
299324
}
300325
// We don't have any up-to-date semantic index entries for this file. Syntactically look for tests.
301-
return syntacticTests
326+
return syntacticTests ?? []
302327
}
303328
}
304329

@@ -437,7 +462,7 @@ extension TestItem {
437462
}
438463

439464
extension SwiftLanguageService {
440-
public func syntacticDocumentTests(for uri: DocumentURI, in workspace: Workspace) async throws -> [TestItem] {
465+
public func syntacticDocumentTests(for uri: DocumentURI, in workspace: Workspace) async throws -> [TestItem]? {
441466
let snapshot = try documentManager.latestSnapshot(uri)
442467
let semanticSymbols = workspace.index(checkedFor: .deletedFiles)?.symbols(inFilePath: snapshot.uri.pseudoPath)
443468
let xctestSymbols = await SyntacticSwiftXCTestScanner.findTestSymbols(
@@ -453,7 +478,7 @@ extension SwiftLanguageService {
453478
}
454479

455480
extension ClangLanguageService {
456-
public func syntacticDocumentTests(for uri: DocumentURI, in workspace: Workspace) async -> [TestItem] {
457-
return []
481+
public func syntacticDocumentTests(for uri: DocumentURI, in workspace: Workspace) async -> [TestItem]? {
482+
return nil
458483
}
459484
}

‎Tests/SourceKitLSPTests/DocumentTestDiscoveryTests.swift

Lines changed: 132 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1065,4 +1065,136 @@ final class DocumentTestDiscoveryTests: XCTestCase {
10651065
XCTAssertFalse(testsAfterEdit.contains { $0.label == "NotQuiteTest" })
10661066
XCTAssertTrue(testsAfterEdit.contains { $0.label == "OtherNotQuiteTest" })
10671067
}
1068+
1069+
func testObjectiveCTestFromSemanticIndex() async throws {
1070+
let project = try await SwiftPMTestProject(
1071+
files: [
1072+
"Tests/MyLibraryTests/Test.m": """
1073+
#import <XCTest/XCTest.h>
1074+
1075+
@interface MyTests : XCTestCase
1076+
@end
1077+
1078+
1️⃣@implementation MyTests
1079+
2️⃣- (void)testSomething {
1080+
}3️⃣
1081+
@4️⃣end
1082+
"""
1083+
],
1084+
manifest: """
1085+
// swift-tools-version: 5.7
1086+
1087+
import PackageDescription
1088+
1089+
let package = Package(
1090+
name: "MyLibrary",
1091+
targets: [.testTarget(name: "MyLibraryTests")]
1092+
)
1093+
""",
1094+
build: true
1095+
)
1096+
1097+
let (uri, positions) = try project.openDocument("Test.m")
1098+
1099+
let tests = try await project.testClient.send(DocumentTestsRequest(textDocument: TextDocumentIdentifier(uri)))
1100+
1101+
XCTAssertEqual(
1102+
tests,
1103+
[
1104+
TestItem(
1105+
id: "MyTests",
1106+
label: "MyTests",
1107+
disabled: false,
1108+
style: TestStyle.xcTest,
1109+
location: Location(uri: uri, range: positions["1️⃣"]..<positions["4️⃣"]),
1110+
children: [
1111+
TestItem(
1112+
id: "MyTests/testSomething",
1113+
label: "testSomething",
1114+
disabled: false,
1115+
style: TestStyle.xcTest,
1116+
location: Location(uri: uri, range: positions["2️⃣"]..<positions["3️⃣"]),
1117+
children: [],
1118+
tags: []
1119+
)
1120+
],
1121+
tags: []
1122+
)
1123+
]
1124+
)
1125+
}
1126+
1127+
func testObjectiveCTestsAfterInMemoryEdit() async throws {
1128+
try SkipUnless.platformIsDarwin("Non-Darwin platforms don't support Objective-C")
1129+
let project = try await SwiftPMTestProject(
1130+
files: [
1131+
"Tests/MyLibraryTests/Test.m": """
1132+
#import <XCTest/XCTest.h>
1133+
1134+
@interface MyTests : XCTestCase
1135+
@end
1136+
1137+
1️⃣@implementation MyTests
1138+
2️⃣- (void)testSomething {}3️⃣
1139+
0️⃣
1140+
@4️⃣end
1141+
"""
1142+
],
1143+
manifest: """
1144+
// swift-tools-version: 5.7
1145+
1146+
import PackageDescription
1147+
1148+
let package = Package(
1149+
name: "MyLibrary",
1150+
targets: [.testTarget(name: "MyLibraryTests")]
1151+
)
1152+
""",
1153+
build: true
1154+
)
1155+
1156+
let (uri, positions) = try project.openDocument("Test.m")
1157+
1158+
project.testClient.send(
1159+
DidChangeTextDocumentNotification(
1160+
textDocument: VersionedTextDocumentIdentifier(uri, version: 2),
1161+
contentChanges: [
1162+
TextDocumentContentChangeEvent(
1163+
range: Range(positions["0️⃣"]),
1164+
text: """
1165+
- (void)testSomethingElse {}
1166+
"""
1167+
)
1168+
]
1169+
)
1170+
)
1171+
1172+
let tests = try await project.testClient.send(DocumentTestsRequest(textDocument: TextDocumentIdentifier(uri)))
1173+
// Since we don't have syntactic test discovery for clang-languages, we don't discover `testSomethingElse` as a
1174+
// test method until we perform a build
1175+
XCTAssertEqual(
1176+
tests,
1177+
[
1178+
TestItem(
1179+
id: "MyTests",
1180+
label: "MyTests",
1181+
disabled: false,
1182+
style: TestStyle.xcTest,
1183+
location: Location(uri: uri, range: positions["1️⃣"]..<positions["4️⃣"]),
1184+
children: [
1185+
TestItem(
1186+
id: "MyTests/testSomething",
1187+
label: "testSomething",
1188+
disabled: false,
1189+
style: TestStyle.xcTest,
1190+
location: Location(uri: uri, range: positions["2️⃣"]..<positions["3️⃣"]),
1191+
children: [],
1192+
tags: []
1193+
)
1194+
],
1195+
tags: []
1196+
)
1197+
]
1198+
)
1199+
}
10681200
}

‎Tests/SourceKitLSPTests/WorkspaceTestDiscoveryTests.swift

Lines changed: 130 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -908,4 +908,134 @@ final class WorkspaceTestDiscoveryTests: XCTestCase {
908908
XCTAssertFalse(testsAfterEdit.contains { $0.label == "NotQuiteTest" })
909909
XCTAssertTrue(testsAfterEdit.contains { $0.label == "OtherNotQuiteTest" })
910910
}
911+
912+
func testObjectiveCTestFromSemanticIndex() async throws {
913+
let project = try await SwiftPMTestProject(
914+
files: [
915+
"Tests/MyLibraryTests/Test.m": """
916+
#import <XCTest/XCTest.h>
917+
918+
@interface MyTests : XCTestCase
919+
@end
920+
921+
@implementation 1️⃣MyTests
922+
- (void)2️⃣testSomething {
923+
}
924+
@end
925+
"""
926+
],
927+
manifest: """
928+
// swift-tools-version: 5.7
929+
930+
import PackageDescription
931+
932+
let package = Package(
933+
name: "MyLibrary",
934+
targets: [.testTarget(name: "MyLibraryTests")]
935+
)
936+
""",
937+
build: true
938+
)
939+
940+
let tests = try await project.testClient.send(WorkspaceTestsRequest())
941+
942+
XCTAssertEqual(
943+
tests,
944+
[
945+
TestItem(
946+
id: "MyTests",
947+
label: "MyTests",
948+
disabled: false,
949+
style: TestStyle.xcTest,
950+
location: try project.location(from: "1️⃣", to: "1️⃣", in: "Test.m"),
951+
children: [
952+
TestItem(
953+
id: "MyTests/testSomething",
954+
label: "testSomething",
955+
disabled: false,
956+
style: TestStyle.xcTest,
957+
location: try project.location(from: "2️⃣", to: "2️⃣", in: "Test.m"),
958+
children: [],
959+
tags: []
960+
)
961+
],
962+
tags: []
963+
)
964+
]
965+
)
966+
}
967+
968+
func testObjectiveCTestsAfterInMemoryEdit() async throws {
969+
try SkipUnless.platformIsDarwin("Non-Darwin platforms don't support Objective-C")
970+
let project = try await SwiftPMTestProject(
971+
files: [
972+
"Tests/MyLibraryTests/Test.m": """
973+
#import <XCTest/XCTest.h>
974+
975+
@interface MyTests : XCTestCase
976+
@end
977+
978+
1️⃣@implementation MyTests
979+
2️⃣- (void)testSomething {}3️⃣
980+
0️⃣
981+
@4️⃣end
982+
"""
983+
],
984+
manifest: """
985+
// swift-tools-version: 5.7
986+
987+
import PackageDescription
988+
989+
let package = Package(
990+
name: "MyLibrary",
991+
targets: [.testTarget(name: "MyLibraryTests")]
992+
)
993+
""",
994+
build: true
995+
)
996+
997+
let (uri, positions) = try project.openDocument("Test.m")
998+
999+
project.testClient.send(
1000+
DidChangeTextDocumentNotification(
1001+
textDocument: VersionedTextDocumentIdentifier(uri, version: 2),
1002+
contentChanges: [
1003+
TextDocumentContentChangeEvent(
1004+
range: Range(positions["0️⃣"]),
1005+
text: """
1006+
- (void)testSomethingElse {}
1007+
"""
1008+
)
1009+
]
1010+
)
1011+
)
1012+
1013+
let tests = try await project.testClient.send(WorkspaceTestsRequest())
1014+
// Since we don't have syntactic test discovery for clang-languages, we don't discover `testSomethingElse` as a
1015+
// test method until we perform a build
1016+
XCTAssertEqual(
1017+
tests,
1018+
[
1019+
TestItem(
1020+
id: "MyTests",
1021+
label: "MyTests",
1022+
disabled: false,
1023+
style: TestStyle.xcTest,
1024+
location: Location(uri: uri, range: positions["1️⃣"]..<positions["4️⃣"]),
1025+
children: [
1026+
TestItem(
1027+
id: "MyTests/testSomething",
1028+
label: "testSomething",
1029+
disabled: false,
1030+
style: TestStyle.xcTest,
1031+
location: Location(uri: uri, range: positions["2️⃣"]..<positions["3️⃣"]),
1032+
children: [],
1033+
tags: []
1034+
)
1035+
],
1036+
tags: []
1037+
)
1038+
]
1039+
)
1040+
}
9111041
}

0 commit comments

Comments
 (0)