Skip to content

Fix test discovery for Objective-C XCTests #1196

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.

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
merged 1 commit into from
May 7, 2024

Conversation

ahoppen
Copy link
Member

@ahoppen ahoppen commented Apr 21, 2024

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

@ahoppen ahoppen requested a review from benlangmuir as a code owner April 21, 2024 15:30
@ahoppen ahoppen force-pushed the tests-for-objc-xctest branch from bce982f to aff3a97 Compare April 24, 2024 15:55
@ahoppen ahoppen force-pushed the tests-for-objc-xctest branch from aff3a97 to 45185ff Compare May 3, 2024 14:25
@ahoppen ahoppen changed the title Fix test discovery for Objective-C XCTests 🚥#1195 Fix test discovery for Objective-C XCTests May 3, 2024
@ahoppen
Copy link
Member Author

ahoppen commented May 3, 2024

@swift-ci Please test

@ahoppen ahoppen force-pushed the tests-for-objc-xctest branch from 45185ff to a2d2909 Compare May 3, 2024 21:08
@ahoppen
Copy link
Member Author

ahoppen commented May 3, 2024

@swift-ci Please test

@ahoppen ahoppen force-pushed the tests-for-objc-xctest branch from a2d2909 to c9b0628 Compare May 4, 2024 15:40
@ahoppen
Copy link
Member Author

ahoppen commented May 4, 2024

@swift-ci Please test

@ahoppen
Copy link
Member Author

ahoppen commented May 5, 2024

@swift-ci Please test Windows

1 similar comment
@ahoppen
Copy link
Member Author

ahoppen commented May 6, 2024

@swift-ci Please test Windows

@ahoppen ahoppen force-pushed the tests-for-objc-xctest branch from c9b0628 to 8613f4f Compare May 6, 2024 15:10
@ahoppen
Copy link
Member Author

ahoppen commented May 6, 2024

@swift-ci Please test

@ahoppen
Copy link
Member Author

ahoppen commented May 6, 2024

@swift-ci Please test Windows

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
@ahoppen ahoppen force-pushed the tests-for-objc-xctest branch from 8613f4f to 6a9a95f Compare May 6, 2024 15:17
@ahoppen
Copy link
Member Author

ahoppen commented May 6, 2024

@swift-ci Please test

@ahoppen
Copy link
Member Author

ahoppen commented May 6, 2024

@swift-ci Please test Windows

1 similar comment
@ahoppen
Copy link
Member Author

ahoppen commented May 6, 2024

@swift-ci Please test Windows

@ahoppen ahoppen merged commit bdddc2b into swiftlang:main May 7, 2024
@ahoppen ahoppen deleted the tests-for-objc-xctest branch May 7, 2024 04:55
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.

2 participants