Skip to content

[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

Merged
merged 1 commit into from
Apr 7, 2016

Conversation

modocache
Copy link
Contributor

What's in this pull request?

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, 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:

  • Test pull request on Swift continuous integration.

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

Platform Comment
All supported platforms @swift-ci Please smoke test
OS X platform @swift-ci Please smoke test OS X platform
Linux platform @swift-ci Please smoke test Linux platform

Validation Testing

Platform Comment
All supported platforms @swift-ci Please test
OS X platform @swift-ci Please test OS X platform
Linux platform @swift-ci Please test Linux platform

Note: Only members of the Apple organization can trigger swift-ci.

@tkremenek
Copy link
Member

@swift-ci test

@modocache
Copy link
Contributor Author

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?

@gottesmm
Copy link
Contributor

gottesmm commented Apr 2, 2016

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

On Apr 2, 2016, at 2:07 PM, Brian Gesiak notifications@github.com wrote:

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?


You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub

@modocache
Copy link
Contributor Author

@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}" \
Copy link
Contributor

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?

Copy link
Contributor Author

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... 😅

Copy link
Contributor

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.

Copy link
Contributor Author

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.

@gottesmm
Copy link
Contributor

gottesmm commented Apr 3, 2016

@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.

@modocache
Copy link
Contributor Author

@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! 🙇

@shahmishal
Copy link
Member

@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 DSTROOT="${XCTEST_BUILD_DIR}"?

@shahmishal
Copy link
Member

@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.
@modocache modocache force-pushed the xctest-build-presets branch from 7f00119 to 32730fa Compare April 6, 2016 01:47
@modocache
Copy link
Contributor Author

What is the error are you seeing with DSTROOT="${XCTEST_BUILD_DIR}"?

Oops, sorry, the error I was seeing must have been due to INSTALL_PATH and the other parameters I removed. I added DSTROOT and everything works fine. Thanks!!

I believe this is now good to merge? The CI passed last time it was run.

@modocache
Copy link
Contributor Author

@shahmishal Friendly ping! I think this is a big improvement for corelibs-xctest development.

@gribozavr
Copy link
Contributor

@swift-ci Please test and merge

@modocache
Copy link
Contributor Author

Just my luck, two CI failures and both appear unrelated:

  • OS X #782: The XCTAssert failure fixed in 2d50e81.
  • Linux #1003: ClangModules/attr-swift_name_renaming.swift test failure, seems unrelated to this.

@gribozavr gribozavr merged commit 893a1a9 into swiftlang:master Apr 7, 2016
@modocache modocache deleted the xctest-build-presets branch April 8, 2016 02:57
@modocache
Copy link
Contributor Author

Excellent!! Thanks!

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.

5 participants