Skip to content

Ensure Xcode SDK paths are set when running test targets. #7040

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
Oct 31, 2023

Conversation

grynspan
Copy link
Contributor

Currently, some of our code paths that run test targets don't correctly set the DYLD_FRAMEWORK_PATH and DYLD_LIBRARY_PATH arguments on macOS. This hasn't been a problem in practice because the XCTest helper tool is able to sort it out for us, but my plan with swift-testing is to run an executable directly (like we do for corelibs-xctest.) That means we'll have a code path that could potentially rely on XCTest or other Apple SDK bits, but which is not calling through the XCTest helper tool.

This change hoists our setting of those arguments to a position where they are always set (on macOS only!) before we run a test target, regardless of build style.

Currently, some of our code paths that run test targets don't correctly set the `DYLD_FRAMEWORK_PATH` and `DYLD_LIBRARY_PATH` arguments on macOS. This hasn't been a problem in practice because the XCTest helper tool is able to sort it out for us, but my plan with swift-testing is to run an executable directly (like we do for corelibs-xctest.) That means we'll have a code path that could potentially rely on XCTest or other Apple SDK bits, but which is not calling through the XCTest helper tool.

This change hoists our setting of those arguments to a position where they are always set (on macOS only!) before we run a test target, regardless of build style.
@grynspan
Copy link
Contributor Author

@swift-ci please test

@tomerd
Copy link
Contributor

tomerd commented Oct 26, 2023

seems fine to me, but @neonichu to review

Copy link
Contributor

@MaxDesiatov MaxDesiatov left a comment

Choose a reason for hiding this comment

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

Would it be possible to add a test that exercises constructTestEnvironment and checks that it returns correct env vars when executed on macOS?

@grynspan
Copy link
Contributor Author

@MaxDesiatov We can write a test that does that, but it would be a bit tautological. I was planning to write some tests of broader test target behaviour in subsequent PRs that would cover these variables.

@grynspan grynspan merged commit bcad083 into main Oct 31, 2023
@grynspan grynspan deleted the jgrynspan/missing-dyld-args branch October 31, 2023 12:42
neonichu added a commit that referenced this pull request Nov 6, 2023
grynspan added a commit that referenced this pull request Nov 14, 2023
Currently, some of our code paths that run test targets don't correctly
set the `DYLD_FRAMEWORK_PATH` and `DYLD_LIBRARY_PATH` arguments on
macOS. This hasn't been a problem in practice because the XCTest helper
tool is able to sort it out for us, but my plan with swift-testing is to
run an executable directly (like we do for corelibs-xctest.) That means
we'll have a code path that could potentially rely on XCTest or other
Apple SDK bits, but which is not calling through the XCTest helper tool.

This change hoists our setting of those arguments to a position where
they are always set (on macOS only!) before we run a test target,
regardless of build style.

This change was previously landed as #7040 but was reverted while we investigated a toolchain regression. We do not believe #7040 is the root cause of that issue, so this PR reapplies it.
grynspan added a commit that referenced this pull request Nov 14, 2023
Currently, some of our code paths that run test targets don't correctly
set the `DYLD_FRAMEWORK_PATH` and `DYLD_LIBRARY_PATH` arguments on
macOS. This hasn't been a problem in practice because the XCTest helper
tool is able to sort it out for us, but my plan with swift-testing is to
run an executable directly (like we do for corelibs-xctest.) That means
we'll have a code path that could potentially rely on XCTest or other
Apple SDK bits, but which is not calling through the XCTest helper tool.

This change hoists our setting of those arguments to a position where
they are always set (on macOS only!) before we run a test target,
regardless of build style.

This change was previously landed as #7040 but was reverted (#7054)
while we investigated a toolchain regression. We do not believe #7040 is
the root cause of that issue, so this PR reapplies it.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants