Skip to content

Run tests on either macOS 12 or macOS 13 #5089

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
merged 4 commits into from
Oct 10, 2023
Merged

Run tests on either macOS 12 or macOS 13 #5089

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
e76652d
update test scripts and ci.yaml for macOS 13
vashworth Oct 9, 2023
6fb70b7
create and delete simulator for pigeon tests, add some comments and n…
vashworth Oct 9, 2023
eef0513
Merge remote-tracking branch 'upstream/main' into macos_12_or_13
vashworth Oct 9, 2023
9d8d5c1
fix nits, run on macOS 12 or 13
vashworth Oct 9, 2023
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
4 changes: 2 additions & 2 deletions .ci.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ platform_properties:
[
{"dependency": "gems", "version": "v3.3.14"}
]
os: Mac-12
os: "Mac-12|Mac-13"
device_type: none
cpu: arm64
$flutter/osx_sdk : >-
Expand All @@ -77,7 +77,7 @@ platform_properties:
[
{"dependency": "gems", "version": "v3.3.14"}
]
os: Mac-12
os: "Mac-12|Mac-13"
device_type: none
cpu: x86
$flutter/osx_sdk : >-
Expand Down
14 changes: 14 additions & 0 deletions .ci/scripts/boot_simulator.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
#!/bin/bash
# Copyright 2013 The Flutter Authors. All rights reserved.
# Use of this source code is governed by a BSD-style license that can be
# found in the LICENSE file.
set -e

# The name here must match create_simulator.sh
readonly DEVICE_NAME=Flutter-iPhone

# Allow boot to fail; cases like "Unable to boot device in current state: Booted"
# exit with failure.
xcrun simctl boot "$DEVICE_NAME" || :
echo -e ""
xcrun simctl list
15 changes: 14 additions & 1 deletion .ci/scripts/create_simulator.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,5 +11,18 @@ readonly DEVICE_NAME=Flutter-iPhone
readonly DEVICE=com.apple.CoreSimulator.SimDeviceType.iPhone-14
readonly OS=com.apple.CoreSimulator.SimRuntime.iOS-16-4

xcrun simctl list
# Delete any existing devices named Flutter-iPhone. Having more than one may
# cause issues when builds target the 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.

For reference, here's an example of this problem in action: https://ci.chromium.org/ui/p/flutter/builders/try/Mac_arm64%20custom_package_tests%20master/7040/overview

Unclear if it's a problem new to macOS 13

echo -e "Deleting any existing devices names $DEVICE_NAME..."
RESULT=0
while [[ $RESULT == 0 ]]; do
xcrun simctl delete "$DEVICE_NAME" || RESULT=1
if [ $RESULT == 0 ]; then
echo -e "Deleted $DEVICE_NAME"
fi
done
echo -e ""

echo -e "\nCreating $DEVICE_NAME $DEVICE $OS ...\n"
xcrun simctl create "$DEVICE_NAME" "$DEVICE" "$OS" | xargs xcrun simctl boot
xcrun simctl list
7 changes: 6 additions & 1 deletion .ci/targets/ios_platform_tests.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,12 @@ tasks:
args: ["xcode-analyze", "--ios", "--ios-min-version=13.0"]
- name: native test
script: script/tool_runner.sh
args: ["native-test", "--ios", "--ios-destination", "platform=iOS Simulator,name=iPhone 14,OS=latest"]
# Simulator name must match name in create_simulator.sh
args: ["native-test", "--ios", "--ios-destination", "platform=iOS Simulator,name=Flutter-iPhone,OS=16.4"]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

As we upgrade, there's no guarantee that there will be an iPhone 14 simulator. Better to use the simulator we create in the "create simulator" step (same for the pigeon test_suites.dart).

Copy link
Contributor

Choose a reason for hiding this comment

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

Pigeon's test_suites is a weird case; we need it to run locally as well as in CI. So we don't want to hard-code it to a simulator that only exists in 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.

Okay, so I updated Pigeon's test_suites to create and delete it's own simulator so that it'll work in CI and locally

FYI couple reasons I think that's the best approach:

  1. Due to new way Simulator runtimes are handled, when you use OS=latest or you omit the OS (which defaults to latest), it may use a newer simulator runtime that's not the default for the Xcode version. For example, if iOS 17 runtime is mounted, it'll use that instead of iOS 16.4 even if you're using Xcode 14.3. So even if there's an "iPhone 14" device with OS 16.4, it won't match against it since it's looking for iOS 17.
  2. There's no guarantee to be a Simulator named "iPhone 14" unless we create it beforehand (both in CI and locally)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also btw, I ran dart run tool/run_tests.dart in packages/packages/pigeon locally to make sure it passed locally

- name: boot simulator
# Ensure simulator is still booted
Copy link
Contributor Author

Choose a reason for hiding this comment

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

While I was testing, sometimes the simulator would shutdown after "native test" and before "drive examples"

See https://luci-milo.appspot.com/ui/p/flutter/builders/try/Mac_arm64%20ios_platform_tests_shard_4%20master/7162/overview for example (I added logs to drive example, where you'll see the Flutter-iPhone simulator is shutdown)

Copy link
Contributor

Choose a reason for hiding this comment

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

That's really weird. Is there any indication of why it's shutting down?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, the "native test" step shows the simulator is booted and works as expected. There's no logs that indicate it would be triggered to be shutdown. It also doesn't seem to happen every time.

script: .ci/scripts/boot_simulator.sh
infra_step: true # Note infra steps failing prevents "always" from running.
- name: drive examples
# `drive-examples` contains integration tests, which changes the UI of the application.
# This UI change sometimes affects `xctest`.
Expand Down
45 changes: 44 additions & 1 deletion packages/pigeon/tool/shared/test_suites.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -294,11 +294,54 @@ Future<int> _runIOSPluginUnitTests(String testPluginPath) async {
return compileCode;
}

const String deviceName = 'Pigeon-Test-iPhone';
const String deviceType = 'com.apple.CoreSimulator.SimDeviceType.iPhone-14';
const String deviceRuntime = 'com.apple.CoreSimulator.SimRuntime.iOS-16-4';
const String deviceOS = '16.4';
await _createSimulator(deviceName, deviceType, deviceRuntime);
return runXcodeBuild(
'$examplePath/ios',
sdk: 'iphonesimulator',
destination: 'platform=iOS Simulator,name=iPhone 14',
destination: 'platform=iOS Simulator,name=$deviceName,OS=$deviceOS',
extraArguments: <String>['test'],
).whenComplete(() => _deleteSimulator(deviceName));
}

Future<int> _createSimulator(
String deviceName,
String deviceType,
String deviceRuntime,
) async {
// Delete any existing simulators with the same name until it fails. It will
// fail once there are no simulators with the name. Having more than one may
// cause issues when builds target the device.
int deleteResult = 0;
while (deleteResult == 0) {
deleteResult = await _deleteSimulator(deviceName);
}
return runProcess(
'xcrun',
<String>[
'simctl',
'create',
deviceName,
deviceType,
deviceRuntime,
],
streamOutput: false,
logFailure: true,
);
}

Future<int> _deleteSimulator(String deviceName) async {
return runProcess(
'xcrun',
<String>[
'simctl',
'delete',
deviceName,
],
streamOutput: false,
);
}

Expand Down