Support tvOS and watchOS simulators #203

simlay · 2023-12-28T17:39:01Z

Closes #194 and #193.

It's unclear if we want test these targets in CI. For one, these targets require the build-std nightly feature and for another, all targets for watchOS are tier 3 and tvOS are tier 3. They might sometimes break (as with tier 3 targets).

At this point, the thing I dislike about this implementation is that some parts imply that there are device targets for tvOS and watchOS and that's kinda not in planned (to me).

I elected to test watchOS and tvOS using just the test-ws/test-bin because it's simpler and I ended up with different errors in the test-ws/test-app pass tests.

I partially started this task with the goal of testing aarch64-apple-tvos-sim. I feel like I'm going crazy but I swear it was working but at moment of this writing, it fails on my machine. Yet, targets such as aarch64-apple-watchos-sim, x86_64-apple-watchos-sim and x86_64-apple-tvos all work. Free macOS github runners aren't apple silicon so we cannot test the aarch64 targets at the moment in CI. I think the existing tests should suffice.

test-ws/test-bin/.cargo/config.toml

briansmith

I suggest we merge PR #206 and then rebase this on top of that PR. Then we can also add CI testing for this support, to ensure it works for both x86_64 and ARM64 hosts.

briansmith · 2024-02-20T22:22:32Z

.travis.sh


 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)


I agree it's wrong. better to handle it in a separate PR.

briansmith · 2024-02-20T22:24:59Z

.travis.sh

+     export WATCHOS_SIM_ID=$(xcrun simctl create My-apple-watch com.apple.CoreSimulator.SimDeviceType.Apple-Watch-SE-44mm-2nd-generation $WATCHOS_RUNTIME_ID)
+
+     xcrun simctl boot $WATCHOS_SIM_ID
+     tests_sequence_unstable_target x86_64-apple-watchos-sim


Similar to the M1 support PR, I suggest that we (automatically) use aarch64-apple-watchos-sim if the host is aarch64 and x86_64-apple-watchos-sim if the host is x86_64.

Perhaps we could extend the auto- mechanism to do this?

I wonder what it will do if we use -d sim or '-d watchos-sim' actually. -d allows partial match. And if we are lucky the ios discovery code will only create an auto device for the matching architecture simulator...

And if it does not it may be desirable/easy to make it do so.

briansmith · 2024-02-20T22:29:06Z

test-ws/test-bin/.cargo/config.toml

+runner = "cargo run --manifest-path ../../Cargo.toml --bin cargo-dinghy -- -p auto-ios-aarch64-sim runner"
+
+[target.x86_64-apple-ios]
+runner = "cargo run --manifest-path ../../Cargo.toml --bin cargo-dinghy -- -p auto-ios-x86_64 runner"


I do agree this is very useful to document and many people will want to copy-paste something like this into their own .cargo/config.toml. But perhaps we should have a separate PR for adding this example/documentation?

.travis.sh

briansmith · 2024-02-20T23:57:22Z

.travis.sh

+
+     title "••••• Darwin: watchvos simulator tests •••••"
+     title "boot a simulator"
+     rustup toolchain add nightly --component rust-src;


This is the second time that the command is run.

briansmith · 2024-02-21T00:05:48Z

dinghy-lib/src/apple/device.rs

+    Ok(build_bundle)
+}
+
+fn make_watchos_app(


It seems like these two functions really duplicate a lot of make_ios_app. I think it would be easier to maintain if most of the commonality were factored out or if make_ios_app were renamed and parameterized over AppleSimulatorType so that it could handle all three cases.

I kinda agree here, unless there is something I can't see.

Yeah, I originally thought I might have to do more changes per simulator target so I just duplicated the functions. The bulk of the difference is now in the plist addition.

briansmith · 2024-02-21T00:09:24Z

dinghy-lib/src/apple/xcode.rs

@@ -70,6 +70,63 @@ pub fn add_plist_to_app(bundle: &BuildBundle, arch: &str, app_bundle_id: &str) -
    Ok(())
 }

+pub fn add_plist_to_tvos_app(bundle: &BuildBundle, arch: &str, app_bundle_id: &str) -> Result<()> {


Ditto regarding DRY. it is quite difficult to spot the differences between the three add_plist_to_*_app functions to understand what's different, and it would be easier to maintain this is we could have the common logic shared.

* Added CI tests for watchOS and tvOS

simlay · 2024-02-22T02:24:33Z

.travis.sh

+tests_sequence_unstable_target() {
+    # There's something odd with using the .cargo/config runner attribute and
+    # workspaces when the runner uses `cargo run --manifest-path ../Cargo.toml
+    # --bin cargo-dinghy ...`
+    title "testing from project directory for rust target $1 on device $2"
+    title "testing from workspace directory"
+    ( \
+        cd test-ws \
+        && cargo clean \
+        && $CARGO_DINGHY   -d $1 -p $2 test -Zbuild-std pass \
+        && ! $CARGO_DINGHY -d $1 -p $2 test -Zbuild-std fails \
+        && ! $CARGO_DINGHY -d $1 -p $2 test -Zbuild-std \
+    )
+
+    title "testing from project directory"
+    ( \
+        cd test-ws/test-app \
+        && cargo clean \
+        && $CARGO_DINGHY   -d $1 -p $2 test pass \
+        && ! $CARGO_DINGHY -d $1 -p $2 test fails \
+        && ! $CARGO_DINGHY -d $1 -p $2 test \
+    )
+
+    title "test from workspace directory with project filter"
+    ( \
+        cd test-ws \
+        && cargo clean \
+        && $CARGO_DINGHY   -d $1 -p $2 test -p test-app pass \
+        && ! $CARGO_DINGHY -d $1 -p $2 test -p test-app fails \
+        && ! $CARGO_DINGHY -d $1 -p $2 test -p test-app \
+    )
+}
+


This additional function makes test_sequence, tests_sequence_aarch64_ios_sim, and tests_sequence_unstable_target which is pretty gross. Any suggestions?

simlay · 2024-02-22T03:06:40Z

dinghy-lib/src/apple/xcode.rs

+        Some(AppleSimulatorType::Ios) | Some(AppleSimulatorType::Tvos) | None => {
+            writeln!(plist, "<key>UIRequiredDeviceCapabilities</key>")?;
+            writeln!(plist, "<array><string>{}</string></array>", arch)?;
+            writeln!(plist, "<key>UILaunchStoryboardName</key>")?;
+            writeln!(plist, "<string></string>")?;
+        },


In the future, these will diverge more I think. Today, I spent a bit of time diving into an audio microphone issue on cpal. I manually added micrphone to the UIRequiredDeviceCapabilities.

It's out of the scope of this PR but it'd be nice to have some possible permission options or maybe a custom Info.plist.

kali · 2024-02-22T07:45:35Z

OK, I think we're good here. Thanks for working on this and being patient with the review :)

simlay changed the title ~~Rename ios module to apple~~ Support tvOS and watchOS simulators Dec 28, 2023

simlay marked this pull request as ready for review January 2, 2024 00:41

kali reviewed Jan 15, 2024

View reviewed changes

test-ws/test-bin/.cargo/config.toml Outdated Show resolved Hide resolved

pixlwave mentioned this pull request Jan 17, 2024

Add support for watchOS. briansmith/ring#1914

Closed

briansmith reviewed Feb 20, 2024

View reviewed changes

briansmith reviewed Feb 21, 2024

View reviewed changes

Rename ios module to apple

37cd7b7

* Added CI tests for watchOS and tvOS

simlay force-pushed the add-tvos-and-watchos branch from a8ec5b0 to 37cd7b7 Compare February 21, 2024 20:27

briansmith mentioned this pull request Feb 21, 2024

Support cargo-dinghy as a cargo test runner #210

Closed

Make things DRYer, updates from comments

678a0eb

simlay commented Feb 22, 2024

View reviewed changes

simlay added 3 commits February 21, 2024 21:30

Hopefully fix ci

cbfd44c

More refactors, hopefully fix tests

102c713

More refactoring to make it clear what the differences are in the plists

df767d2

simlay commented Feb 22, 2024

View reviewed changes

simlay added 3 commits February 21, 2024 22:18

Add build-std to unstable calls

0307a4d

Hopefully fix ci

562b233

Fix bug with x86 watchOS simulator

9c24cf4

kali merged commit 121354f into sonos:main Feb 22, 2024
6 checks passed

simlay deleted the add-tvos-and-watchos branch February 22, 2024 15:50

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support tvOS and watchOS simulators #203

Support tvOS and watchOS simulators #203

simlay commented Dec 28, 2023 •

edited

Loading

briansmith left a comment

briansmith Feb 20, 2024

briansmith Feb 20, 2024

kali Feb 21, 2024

briansmith Feb 20, 2024

kali Feb 21, 2024

briansmith Feb 20, 2024

briansmith Feb 21, 2024

kali Feb 21, 2024

simlay Feb 22, 2024

briansmith Feb 21, 2024

simlay Feb 22, 2024

simlay Feb 22, 2024

kali commented Feb 22, 2024

Support tvOS and watchOS simulators #203

Support tvOS and watchOS simulators #203

Conversation

simlay commented Dec 28, 2023 • edited Loading

briansmith left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kali commented Feb 22, 2024

simlay commented Dec 28, 2023 •

edited

Loading