Reland iOS landscape mode support #1974

bartekpacia · 2024-08-29T13:53:54Z

Context for the reviewer:

This PR is an attmept to reland Fix iOS landscape mode #1809
Fix iOS landscape mode #1809 was reverted in Revert "Fix iOS landscape mode (#1809)" #1916 because of a critical breakage in An unknown network error occurred while communicating with the XCUITest server #1914
The cause of critical breakage was this, and it's fixed in this PR
additionally, I found a bigger problem Driver.deviceInfo() is being called too many times #2031 which I don't think is worth solving (1) right now and (2) in this specific PR

github-actions · 2024-08-29T13:54:10Z

Make sure to run ./maestro-ios-xctest-runner/build-maestro-ios-runner.sh with every swift change

bartekpacia · 2024-08-30T13:12:39Z

The problem is that XCUIDevice.shared.orientation returns UIDeviceOrientation.unknown when the iOS simulator is first started. This effectively makes it impossible to query the iOS simulator for its orientation.

Plot twist

When the iOS simulator's orientation is manually changed (screenshot below), then [XCUIDevice.shared.orientation] starts returning correct value:

[XCUIDevice.shared.orientation] also starts returning correct value when it's assigned a value:

XCUIDevice.shared.orientation = .portrait

Possible fix 1

If XCUIDevice.shared.orientation returns UIDeviceOrientation.unknown, let's assume that orientation is UIDeviceOrientation.portrait .

Possible fix 2

Introduce a new setOrientation command that will set orientation programatically, as shown above. Educate users that they have to call that command to change their app to a different orientation.

bartekpacia · 2024-08-30T13:38:30Z

I asked a question on StackOverflow

bartekpacia · 2024-09-09T08:44:16Z

Good thing 1 is that (from my testing) all iOS (and iPadOS) simulator always startup in portrait orientation, so defaulting to portrait when XCUIDevice.shared.orientation returns unknown seems like a good-enough workaround.

Good thing 2 is that even after I changed orientation of the simulator to portrait and then closed them (and killed Simulator.app), they all start up again in portrait.

bartekpacia · 2024-09-09T09:15:37Z

Demo:

demo_2x.mp4

This reverts commit 096a1d4.

…on.unknown`

…s rerun

…message

This was a source of bugs when device orientation changed. AndroidDriver has never cached values, instead it was always requesting them for each new command that used them. Now iOS driver does the same.

…Info() many times

herval · 2024-09-09T13:49:02Z

The problem is that XCUIDevice.shared.orientation returns UIDeviceOrientation.unknown when the iOS simulator is first started. This effectively makes it impossible to query the iOS simulator for its orientation.

Plot twist

When the iOS simulator's orientation is manually changed (screenshot below), then [XCUIDevice.shared.orientation] starts returning correct value:
`[XCUIDevice.shared.orientation]` also starts returning correct value when it's assigned a value:
XCUIDevice.shared.orientation = .portrait
Possible fix 1

If XCUIDevice.shared.orientation returns UIDeviceOrientation.unknown, let's assume that orientation is UIDeviceOrientation.portrait .

Possible fix 2

Introduce a new setOrientation command that will set orientation programatically, as shown above. Educate users that they have to call that command to change their app to a different orientation.

how about option 3 - if it returns unknown, set it to portrait (the difference from that to option 1 is it would correct the behavior from there on, so if there's anything else that requires device orientation, it'd receive the correct value)

herval · 2024-09-09T13:51:51Z

maestro-client/src/main/java/maestro/Maestro.kt

+    val deviceName: String
+        get() = driver.name()
+
+    private var _cachedDeviceInfo: DeviceInfo? = null


aren't these two (this and the original code) functionally the same? My understanding is this is what by lazy does behind the scenes

It is.

I changed it away from using lazy because I needed _cachedDeviceInfo to be a var instead of val for $reasons which are no longer valid because I changed the code and forgot to bring it back.

herval · 2024-09-09T13:53:08Z

maestro-client/src/main/java/maestro/Maestro.kt

@@ -160,8 +163,10 @@ class Maestro(
    }

    fun swipeFromCenter(swipeDirection: SwipeDirection, durationMs: Long) {
+        val deviceInfo = deviceInfo()


this doesnt seem to use the cached info - is that the intent?

Yes.

If we use the cached version, then swipe will completely break if the device is rotated mid-flow (or when running maestro test in --continuous mode).

herval · 2024-09-09T13:54:37Z

maestro-client/src/main/java/maestro/drivers/IOSDriver.kt

@@ -529,3 +507,14 @@ class IOSDriver(
        private const val SCREEN_SETTLE_TIMEOUT_MS: Long = 3000
    }
 }
+
+private fun Double.asPercentOf(total: Int): Int {


what's the advantage of this over the regular private functions from before?

Conciseness, mainly.

If I were to keep using widthPercentToPoint(percent: Double) I'd have to refactor it to widthPercentToPoint(percent: Double, width: Double) which becomes a bit long and repetitive.

herval · 2024-09-09T13:55:46Z

maestro-ios-xctest-runner/maestro-driver-iosUITests/Routes/Helpers/ScreenSizeHelper.swift

+        if orientation == .unknown {
+            // If orientation is "unknown", we assume it is "portrait" to
+            // work around https://stackoverflow.com/q/78932288/7009800
+            return UIDeviceOrientation.portrait


what would be the implications of setting it to portrait as a side-effect here, when it's unknown?

As of now, I'm not aware of any. See "good things" above.

bartekpacia · 2024-09-12T11:49:13Z

Updated caveats

Consider this older list of caveats to be out-of-date and not true.

Here's the actual list of caveats, as of now:

(iOS) breaks when simulator is "physically" in landscape, but app is locked to portrait. I consider this a rare edge case and suggest we are fine with it.

What exactly happens?
- taps made in Maestro Studio don't work (likely cause: coordinates aren't translated correctly)
- preview in Maestro Studio is broken (see example below)
Example

possible fix: find iOS/XCUITest APIs that allow us to detect this situation and handle accordingly
~~(Android) Maestro Studio is broken when the device and app are in horizontal orientation~~ Fixed in fix landscape mode in Maestro Studio on both Android and iOS #2050

Example

bartekpacia force-pushed the feature/ios-landscape branch 2 times, most recently from b654f84 to 8f2a2d6 Compare August 29, 2024 22:21

bartekpacia changed the title ~~Reapply "Fix iOS landscape mode, PR #1809" (#1916)~~ Reland iOS landscape mode support Aug 29, 2024

bartekpacia force-pushed the feature/ios-landscape branch from 1cbab3e to e9a3b44 Compare September 9, 2024 07:39

bartekpacia mentioned this pull request Sep 9, 2024

A small refactor to LocalXCTestInstaller #2028

Merged

bartekpacia force-pushed the feature/ios-landscape branch 2 times, most recently from 45a17aa to 806c533 Compare September 9, 2024 08:30

bartekpacia force-pushed the feature/ios-landscape branch from a467fdd to 55a2ce8 Compare September 9, 2024 10:55

bartekpacia mentioned this pull request Sep 9, 2024

Orchestra.buildFilter(): remove unused DeviceInfo parameter #2030

Merged

bartekpacia added 6 commits September 9, 2024 13:18

Reapply "Fix iOS landscape mode, PR #1809" (#1916)

cdaab6f

This reverts commit 096a1d4.

remove redundant changes

9ae06fe

don't throw fatalError, throw instead

1c0228e

default to UIDeviceOrientation.portrait if it is `UIDeviceOrientati…

addac1e

…on.unknown`

maestro.client.Maestro: reset cachedDeviceInfo when continuous flow i…

9b929e7

…s rerun

DeviceSizeHelper and DeviceInfoHandler: refactor and add helpful log …

d75bd93

…message

bartekpacia force-pushed the feature/ios-landscape branch from 55a2ce8 to d75bd93 Compare September 9, 2024 11:20

remove screen size caching in IOSDriver

6920c11

This was a source of bugs when device orientation changed. AndroidDriver has never cached values, instead it was always requesting them for each new command that used them. Now iOS driver does the same.

bartekpacia mentioned this pull request Sep 9, 2024

Driver.deviceInfo() is being called too many times #2031

Open

always use the latest up-to-date orientation by calling Driver.device…

7f13ca9

…Info() many times

bartekpacia marked this pull request as ready for review September 9, 2024 13:37

bartekpacia requested review from rwtnb and axelniklasson September 9, 2024 13:37

bartekpacia added 2 commits September 9, 2024 15:41

update zip files

8bab954

delete clearCachedDeviceInfo function, it is no longer needed

3cfb908

herval reviewed Sep 9, 2024

View reviewed changes

Maestro.cachedDeviceInfo: use lazy for it (again)

dae0fe6

bartekpacia merged commit 92c1984 into main Sep 9, 2024
7 checks passed

bartekpacia deleted the feature/ios-landscape branch September 9, 2024 15:04

This was referenced Sep 12, 2024

fix landscape mode in Maestro Studio on both Android and iOS #2050

Merged

Landscape support for iOS #565

Closed

johntmcintosh mentioned this pull request Nov 2, 2024

Add setOrientation command #2121

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Reland iOS landscape mode support #1974

Reland iOS landscape mode support #1974

bartekpacia commented Aug 29, 2024 •

edited

Loading

github-actions bot commented Aug 29, 2024

bartekpacia commented Aug 30, 2024 •

edited

Loading

bartekpacia commented Aug 30, 2024

bartekpacia commented Sep 9, 2024 •

edited

Loading

bartekpacia commented Sep 9, 2024

herval commented Sep 9, 2024

herval Sep 9, 2024

bartekpacia Sep 9, 2024 •

edited

Loading

herval Sep 9, 2024

bartekpacia Sep 9, 2024

herval Sep 9, 2024

bartekpacia Sep 9, 2024

herval Sep 9, 2024

bartekpacia Sep 9, 2024

bartekpacia commented Sep 12, 2024 •

edited

Loading

Reland iOS landscape mode support #1974

Reland iOS landscape mode support #1974

Conversation

bartekpacia commented Aug 29, 2024 • edited Loading

github-actions bot commented Aug 29, 2024

bartekpacia commented Aug 30, 2024 • edited Loading

bartekpacia commented Aug 30, 2024

bartekpacia commented Sep 9, 2024 • edited Loading

bartekpacia commented Sep 9, 2024

herval commented Sep 9, 2024

herval Sep 9, 2024

Choose a reason for hiding this comment

bartekpacia Sep 9, 2024 • edited Loading

Choose a reason for hiding this comment

herval Sep 9, 2024

Choose a reason for hiding this comment

bartekpacia Sep 9, 2024

Choose a reason for hiding this comment

herval Sep 9, 2024

Choose a reason for hiding this comment

bartekpacia Sep 9, 2024

Choose a reason for hiding this comment

herval Sep 9, 2024

Choose a reason for hiding this comment

bartekpacia Sep 9, 2024

Choose a reason for hiding this comment

bartekpacia commented Sep 12, 2024 • edited Loading

Updated caveats

bartekpacia commented Aug 29, 2024 •

edited

Loading

bartekpacia commented Aug 30, 2024 •

edited

Loading

bartekpacia commented Sep 9, 2024 •

edited

Loading

bartekpacia Sep 9, 2024 •

edited

Loading

bartekpacia commented Sep 12, 2024 •

edited

Loading