-
Notifications
You must be signed in to change notification settings - Fork 10.5k
[build-script] XCTest CI builds on OS X #2036
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
Conversation
@swift-ci test |
Looks like some Swift package tests are failing due to something related to SwiftPM. Doesn't seem related to this PR. @kostiakoval @aciidb0mb3r are you aware of any ongoing CI issues in SwiftPM? |
Also, do we have ci tests in GitHub for swiftpm? How did these breakages escape at least this sort of testing? Sent from my iPhone
|
@gottesmm I believe the problem is that package tests are only run for apple/swift CI. The preset used by apple/swift-package-manager CI doesn't include them. That may be reasonable--they are quite expensive. /cc @shahmishal for CI wisdom. |
SWIFT_LINK_OBJC_RUNTIME=YES \ | ||
SKIP_INSTALL=NO \ | ||
DEPLOYMENT_LOCATION=YES \ | ||
DSTROOT="${XCTEST_BUILD_DIR}" \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you drop DSTROOT
, where would xcodebuild
place the build products?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The idea here is that the Swift build script no longer minds where the build products are placed. I'd like for the build script to be able to build and test XCTest to make it easier for newcomers to contribute (everyone uses the same invocation to test). But attempting to have it install build products in a specific location was a misstep on my part: we don't support installing corelibs-xctest on OS X, so we shouldn't try to place build products in a specific directory either.
With this change, build products are placed in the usual Xcode location: the derived data folder.
Alternatively, it might be possible to respect DSTROOT
here, but I just can't get the build to work when doing so... 😅
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One good reason to continue to pass DSTROOT
is to allow us to reliably clean the build directory on CI.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I could invoke xcodebuild clean build
here, if you'd prefer.
@modocache We are trying to move towards a model where all commits have some sort of CI testing. SwiftPM is no exception to this rule. If the tests are too expensive then the tests should be sped up or a reasonable subset should be provided that can be run. |
@gottesmm I can definitely sympathize with your argument, especially because I've accidentally broken the package tests before in #1704. Again, I'll leave it up to @shahmishal to determine what's the best approach here. @shahmishal Any thoughts on @gribozavr's concerns about derived data above? I'm excited to get this merged, it'll be a big improvement in XCTest's CI. Also, if someone could try the tests again, that'd be much appreciated! 🙇 |
@modocache I agree with @gribozavr we should specify derived data dir, so we can reliably clean the workspace. What is the error are you seeing with |
@swift-ci Please test |
Fix the OS X XCTest CI preset: 1. Installation of XCTest isn't supported on OS X. After all, the library is meant to be used on Linux, and not intended to be used or installed on OS X. Therefore, no longer pass an `INSTALL_PATH` to the `xcodebuild` invocation on OS X. 2. `xcodebuild` expects `swift-stdlib-tool` to be present in the same directory as the `SWIFT_EXEC` it's given. Re-use some shellscript from the toolchain installation script to copy the stdlib-tool substitute. This allows the XCTest CI to work on all platforms.
7f00119
to
32730fa
Compare
Oops, sorry, the error I was seeing must have been due to I believe this is now good to merge? The CI passed last time it was run. |
@shahmishal Friendly ping! I think this is a big improvement for corelibs-xctest development. |
@swift-ci Please test and merge |
Just my luck, two CI failures and both appear unrelated:
|
Excellent!! Thanks! |
What's in this pull request?
Fix the OS X XCTest CI preset:
INSTALL_PATH
to thexcodebuild
invocation on OS X.xcodebuild
expectsswift-stdlib-tool
to be present in the same directory as theSWIFT_EXEC
it's given. Re-use some shellscript from the toolchain installation script to copy the stdlib-tool substitute.This allows the XCTest CI to work on all platforms, and should unblock swiftlang/swift-corelibs-xctest#77. That pull request will make it much easier for new contributors to start working on swift-corelibs-xctest.
Resolved bug number: None
Before merging this pull request to apple/swift repository:
Triggering Swift CI
The swift-ci is triggered by writing a comment on this PR addressed to the GitHub user @swift-ci. Different tests will run depending on the specific comment that you use. The currently available comments are:
Smoke Testing
Validation Testing
Note: Only members of the Apple organization can trigger swift-ci.