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.

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

Test on macOS-13 (x86) and macOS-14 (arm) #206

Merged
merged 1 commit into from
Feb 21, 2024
Merged

Test on macOS-13 (x86) and macOS-14 (arm) #206

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
5 changes: 4 additions & 1 deletion .github/workflows/test.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,10 @@ jobs:
run: ./.travis.sh

macos:
runs-on: macOS-latest
strategy:
matrix:
os: [ macOS-13, macOS-14 ]
runs-on: ${{matrix.os}}

steps:
- uses: actions/checkout@v1
Expand Down
52 changes: 48 additions & 4 deletions .travis.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,19 +60,63 @@ tests_sequence() {
)
}

tests_sequence_aarch64_ios_sim() {
title "testing from workspace directory"
( \
cd test-ws \
&& cargo clean \
&& $CARGO_DINGHY -d $1 -p auto-ios-aarch64-sim test pass \
&& ! $CARGO_DINGHY -d $1 -p auto-ios-aarch64-sim test fails \
&& ! $CARGO_DINGHY -d $1 -p auto-ios-aarch64-sim test \
Copy link

Choose a reason for hiding this comment

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

I am new to this tool so I don't know what -p does or why it seems to be needed here. I think maybe we're working around a bug where cargo-dinghy isn't automatically choosing -sim when the host is aarch64, even though it does automatically choose the right thing for x86_64?

if this is indeed a bug, perhaps we should fix that bug so that we can DRY these two functions?

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 just wondering about that. It's a bug. To verify locally, I ran rustup target remove x86_64-appl-ios (on my m1), then cargo dinghy -d <sim id> test and it failed because dinghy was compiling for x86_64-apple-ios rather than aarch64-apple-ios-sim

Copy link

Choose a reason for hiding this comment

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

cargo dinghy -d test and it failed because dinghy was compiling for x86_64-apple-ios rather than aarch64-apple-ios-sim

Maybe the best thing to do by default is query the target device and choose the architecture that matches the host, if available, and then choose the default target triple based on what's compatible for that device? If the target device is a real device then it would be aarch64-apple-ios and if it is a simulator then either aarch64-apple-ios-sim or x86_64-apple-ios? And do analogously if the target device is a tvos (simulator) or watchos (simulator).

Copy link

Choose a reason for hiding this comment

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

Also, I think the kind of improvement I'm suggesting in this thread could be postponed to a future PR. The duplication here is unfortunate but this is good progress that would unblock work on several issues.

)

title "testing from project directory"
( \
cd test-ws/test-app \
&& cargo clean \
&& $CARGO_DINGHY -d $1 -p auto-ios-aarch64-sim test pass \
&& ! $CARGO_DINGHY -d $1 -p auto-ios-aarch64-sim test fails \
&& ! $CARGO_DINGHY -d $1 -p auto-ios-aarch64-sim test \
)

title "test from workspace directory with project filter"
( \
cd test-ws \
&& cargo clean \
&& $CARGO_DINGHY -d $1 -p auto-ios-aarch64-sim test -p test-app pass \
&& ! $CARGO_DINGHY -d $1 -p auto-ios-aarch64-sim test -p test-app fails \
&& ! $CARGO_DINGHY -d $1 -p auto-ios-aarch64-sim test -p test-app \
)
}


if [ `uname` = Darwin ]
then
title "••••• Darwin: ios simulator tests •••••"
title "boot a simulator"
rustup target add x86_64-apple-ios;
RUNTIME_ID=$(xcrun simctl list runtimes | grep iOS | cut -d ' ' -f 7 | tail -1)
export SIM_ID=$(xcrun simctl create My-iphone7 com.apple.CoreSimulator.SimDeviceType.iPhone-8 $RUNTIME_ID)

# Installed simulators on github runners differ depending on the version
# of macos. When the simulator device type ID needs to be updated, select
# a new one:
# https://github.com/actions/runner-images/blob/main/images/macos/macos-12-Readme.md#installed-simulators
# https://github.com/actions/runner-images/blob/main/images/macos/macos-13-Readme.md#installed-simulators
# https://github.com/actions/runner-images/blob/main/images/macos/macos-14-arm64-Readme.md#installed-simulators
export SIM_ID=$(xcrun simctl create My-iphone-se com.apple.CoreSimulator.SimDeviceType.iPhone-SE-3rd-generation $RUNTIME_ID)
Copy link

Choose a reason for hiding this comment

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

Please add a comment on why we are using this specific choice of iPhone to simulate. If iPhone 8 works when running on M1 then maybe this is better done in a separate PR?

xcrun simctl boot $SIM_ID
tests_sequence $SIM_ID

# The x86_64-apple-ios target seems to not work on an ARM64 host,
# and the aarch64-apple-ios-sim target doesn't work on an x86-64 host.
if [ "$(uname -m)" = "arm64" ]; then
simlay marked this conversation as resolved.
Show resolved Hide resolved
simlay marked this conversation as resolved.
Show resolved Hide resolved
rustup target add aarch64-apple-ios-sim;
tests_sequence_aarch64_ios_sim $SIM_ID
else
simlay marked this conversation as resolved.
Show resolved Hide resolved
rustup target add x86_64-apple-ios;
tests_sequence $SIM_ID
fi

xcrun simctl delete $SIM_ID

if ios-deploy -c -t 1 > /tmp/ios_devices
then
device=$(grep "Found" /tmp/ios_devices | head -1 | cut -d " " -f 3)
Expand Down