Skip to content

Configure CI via Buildkite #6

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.

Sign up for GitHub

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
mokagio merged 6 commits into from
Aug 26, 2021
Merged

Configure CI via Buildkite #6

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
7d045fd
Configure ScreenObject shared scheme to run the UI tests
mokagio Aug 23, 2021
16d4e0d
Add Buildkite pipeline running the tests
mokagio Aug 23, 2021
ef19a80
Add parallel step to build and test on iPad Pro
mokagio Aug 23, 2021
2c8ff49
Extract syntax to run tests in Buildkite command
mokagio Aug 25, 2021
2bebe59
DRY Buildkite pipeline syntax
mokagio Aug 25, 2021
a0680cd
Add note on the hardcoded iOS version in the Builkite tests command
mokagio Aug 26, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 21 additions & 0 deletions .buildkite/commands/run-tests.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
#!/bin/bash -eu

# Notice that:
#
# - We are using raw `xcodebuild` without extra tooling such as Fastlane,
# `xcpretty`, or `xcbeautify`. This is an intentional compromise to keep the
# setup lean, as adding any of those would require extras in CI such as
# chating for what, right now, looks like merely a log readability gain.
#
# - The iOS Simulator version is hardcoded in the command call, unlike the
# Simulator device, so we have a single source of truth. The downside is that
# the syntax in the pipeline is less clear: a reader might as where's the
# iOS version set. As long as we only need to test against the latest iOS
# version, that seems like a reasonable tradeoff to make it easier to move to
# newer versions as they are released.
SIMULATOR_NAME=$1

xcodebuild clean test \
-project 'ScreenObject.xcodeproj' \
-scheme 'ScreenObject' \
-destination "platform=iOS Simulator,name=$SIMULATOR_NAME,OS=14.5"
Copy link
Contributor Author

@mokagio mokagio Aug 25, 2021

Choose a reason for hiding this comment

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

I decided to:

  1. Keep this simple and don't integrate Fastlane just to run the tests, nor xcpretty or xcbeautify to format the output. All of those options would require extra dependency and CI setup that doesn't seem granted at this time
  2. Hardcode the iOS version here, rather than pass it to the script, so we have a single source of truth. This makes the syntax a bit less clear in the pipeline, though. Given the scope of this project, I think we can deal with it for the moment. I can also see a future in which we might want to run tests against multiple iOS version, if we get to it, we can start passing the value in the same way as we do with $SIMULATOR_NAME.

Copy link

@AliSoftware AliSoftware Aug 26, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Same feedback as in the companion XCUITestHelpers PR here

11 changes: 11 additions & 0 deletions .buildkite/pipeline.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
env:
IMAGE_ID: xcode-12.5.1
agents:
queue: 'mac'
Comment on lines +1 to +4
Copy link
Contributor Author

@mokagio mokagio Aug 25, 2021

Choose a reason for hiding this comment

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

@jkmassel @AliSoftware TIL that Buildkite offers a native way to specify default env and agents. I think we should adopt this approach in our other pipelines instead of the custom anchor for the env value.

I haven't tested this, but I expect that if we define a default env with a FOO variable and then define a step-specific env with a BAR variable, the step will have access to both FOO and BAR.

Copy link

@AliSoftware AliSoftware Aug 25, 2021

Choose a reason for hiding this comment

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

I like the pipeline level default and agree it makes sense to use it.

But unlike you my guess would be that any step-specific env would completely override the default. Otherwise there wouldn't be a way to undefine FOO in some steps… so in any case definitively worth checking the behavior in such a case.

Copy link

@AliSoftware AliSoftware Aug 25, 2021

Choose a reason for hiding this comment

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

@mokagio Huh, actually this might not behave like either of us expect, as according to the BuildKite documentation, the env set at the top level… always takes precedence over the ones set on each step?!

Top level pipeline environment variables will override what is set in the env attribute of an individual step.

Source: BuildKite docs

Which is a strange choice from them imho, and iiuc means that we wouldn't be able to provide e.g. a different IMAGE_ID just for a specific step while using the top-level-defined one as a default, because the top-level one would always override them all… 😒

Copy link
Contributor Author

@mokagio mokagio Aug 26, 2021

Choose a reason for hiding this comment

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

🤔 I'm guessing that's a typo in the docs. I run a test and the results is inline with my expectation.

Copy link
Contributor Author

@mokagio mokagio Aug 26, 2021

Choose a reason for hiding this comment

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

I opened a PR in their docs to suggest an update.

This option works fine for this pipeline, but we should wait to see what Buildkite replies in terms of what the expected behavior is before considering updating our other pipelines.


steps:
- label: "🧪 Build and Test (iPhone 12 Simulator)"
command: .buildkite/commands/run-tests.sh 'iPhone 12'

- label: "🧪 Build and Test (iPad Pro (12.9-inch) (5th generation) Simulator)"
command: .buildkite/commands/run-tests.sh 'iPad Pro (12.9-inch) (5th generation)'
10 changes: 10 additions & 0 deletions ScreenObject.xcodeproj/xcshareddata/xcschemes/ScreenObject.xcscheme
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,16 @@
selectedLauncherIdentifier = "Xcode.DebuggerFoundation.Launcher.LLDB"
shouldUseLaunchSchemeArgsEnv = "YES">
<Testables>
<TestableReference
skipped = "NO">
<BuildableReference
BuildableIdentifier = "primary"
BlueprintIdentifier = "3FF14351266E2C7500138163"
BuildableName = "TestAppUITests.xctest"
BlueprintName = "TestAppUITests"
ReferencedContainer = "container:ScreenObject.xcodeproj">
</BuildableReference>
</TestableReference>
</Testables>
</TestAction>
<LaunchAction
Expand Down