Skip to content
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

Add Buildkite #1349

Merged
merged 9 commits into from
Feb 10, 2022
Merged

Add Buildkite #1349

merged 9 commits into from
Feb 10, 2022

Conversation

jkmassel
Copy link
Contributor

@jkmassel jkmassel commented Feb 5, 2022

Adds Buildkite support.

In order to make that work, this PR also:

  • Disables some tests that don't work on iOS 15.
  • Fixes Swift 5 AnyObject declarations.

We can either fix the tests, or wait for a future CI image that has an iOS 14 runtime. Might be worth discussing?

@jkmassel jkmassel self-assigned this Feb 5, 2022
@jkmassel jkmassel force-pushed the add/buildkite branch 6 times, most recently from 40c61c6 to c4b4451 Compare February 5, 2022 04:51
@jkmassel jkmassel changed the title Add/buildkite Add Buildkite Feb 5, 2022
Test

Add runtime validation

Make Rubocop Happy
@@ -56,6 +56,17 @@
BlueprintName = "AztecTests"
ReferencedContainer = "container:Aztec.xcodeproj">
</BuildableReference>
<SkippedTests>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

These tests don't work under iOS 15, so they're disabled for now

Copy link
Contributor

Choose a reason for hiding this comment

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

I was going to open a PR similar to wordpress-mobile/MediaEditor-iOS#29 here, but I'll wait for input on my question regarding the future of this library.

end
end

def create_simulator(runtime, device)
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 was surprised that this wasn't already a fastlane action – might be worth moving this into the release toolkit in the future (in order to be able to guarantee that we run tests on the desired runtime).

@jkmassel jkmassel marked this pull request as ready for review February 5, 2022 05:12
Copy link
Contributor

@mokagio mokagio left a comment

Choose a reason for hiding this comment

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

We can either fix the tests, or wait for a future CI image that has an iOS 14 runtime. Might be worth discussing?

My vote is for fixing the tests.

I'm also not sure what the state of Aztec is, but I think it's getting closer and closer to be removable? Or do we plan to leave it around for those who want to switch back to the "classic editor" in the app?

If it's days are numbered, we might as well carry on like this and direct effort to decommissioning it.

#################
# Build and Test
#################
- label: "🧪 Build and Test"
Copy link
Contributor

Choose a reason for hiding this comment

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

Muscle memory was going to make me suggest to use 🔬 but I don't know whether that's appropriate for a "Build and Test" step 🤔 😅

Copy link
Contributor

Choose a reason for hiding this comment

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

"🛠️ " ? 😇

@@ -1456,7 +1456,7 @@
isa = PBXProject;
attributes = {
LastSwiftUpdateCheck = 0800;
LastUpgradeCheck = 1230;
LastUpgradeCheck = 1310;
Copy link
Contributor

Choose a reason for hiding this comment

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

Time to upgrade to Xcode 13.2 @jkmassel 😝

buildlog_path: File.join(__dir__, '.build', 'logs'),
derived_data_path: File.join(__dir__, '.build', 'derived-data'),
ensure_devices_found: true
)
Copy link
Contributor

Choose a reason for hiding this comment

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

Have you considered splitting these in two lanes and running them in parallel in CI?

I'm guessing because the tests themselves don't take that long to run (the lane runs less than a minute with no DerivedData on my 2019 Intel) doing this might actually result in more total CI setup time for the parallel agents, which would be wasteful. 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Considered, but the overhead is a lot – I'd be open to splitting them if this repo starts doing a lot of builds though? 🤷‍♂️

@jkmassel jkmassel merged commit 1586d07 into develop Feb 10, 2022
@jkmassel jkmassel deleted the add/buildkite branch February 10, 2022 23:25
@jkmassel jkmassel mentioned this pull request Jul 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants