Skip to content

[BuildPlan] Fix traverseModules to account for tests with direct ma… #8940

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
Jul 16, 2025

Conversation

xedin
Copy link
Contributor

@xedin xedin commented Jul 16, 2025

…cro dependencies

Motivation:

Currently there is a difference in behavior between BuildPlan.computeDestinations and BuildPlan.traverseModules
where the latter always returns test targets as built for the "target". This is incorrect if i.e. targets have direct macro
dependencies.

Modifications:

This change aligns traverseModules with computeDestinations in regard to test targets that are built for the host.
And let's test targets be discovered through an aggregate produce that also informs their destination.

Result:

The fix helps sourcekit-lsp to correctly identify test targets.

Resolves: swiftlang/sourcekit-lsp#2205

…cro dependencies

This change aligns `traverseModules` with `computeDestinations`
in regard to test targets that have direct macro dependencies:
if at least one test target directly depends on a macro, all of
the tests are built for the "host".

Resolves: swiftlang/sourcekit-lsp#2205
@xedin
Copy link
Contributor Author

xedin commented Jul 16, 2025

@swift-ci please test

@xedin
Copy link
Contributor Author

xedin commented Jul 16, 2025

@swift-ci please test Windows platform

@bnbarham
Copy link
Contributor

Thanks @xedin. I can confirm this fixes the issue 🙇

@xedin xedin merged commit b10658b into swiftlang:main Jul 16, 2025
6 checks passed
xedin added a commit that referenced this pull request Jul 17, 2025
…ct ma… (#8943)

…cro dependencies

- Explanation:

This change aligns `traverseModules` with `computeDestinations` in
regard to test targets that have direct macro dependencies: if at least
one test target directly depends on a macro, all of the tests are built
for the "host".

- Resolves: swiftlang/sourcekit-lsp#2205 
- Resolves: rdar://155814321 

- Main Branch PR:
#8940

- Risk: Low. This functionality is only used by sourcekit-lsp at the
moment and shouldn't have any impact on swiftpm operation.
 
- Reviewed By: -

- Testing: Added new test-cases to the suite.

(cherry picked from commit a3e78e2)
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.

Test discovery partially fails with Swift 6.1 toolchain
3 participants