Add setOrientation command
#2121
Conversation
I do not have an android setup and have not tested this yet. This implementation is based off conventions in `AndroidDriver.kt` for interacting with `adb` and this reference for what commands should be run https://stackoverflow.com/q/25864385.
johntmcintosh
force-pushed
the
feature/set-orientation
branch
from
abcf6ea
to
90b4677
Compare
|
👋 Hi there :) I'm just a random guy with no prior knowledge on
I do have access to an android setup, and I have made a first test: so far, so good! 
(Tested from Do not hesitate to tell me if I can be of any help 💪 [Edit 2024-11-07]: I have also tested on a real device (not an emulator) and it's also working like a charm. |
| @@ -601,6 +601,18 @@ class AndroidDriver( | |||
| } | |||
| } | |||
|
|
|||
| override fun setOrientation(orientation: DeviceOrientation) { | |||
| // Disable accelerometer based rotation before overriding orientation | |||
| dadb.shell("settings put system accelerometer_rotation 0") | |||
There was a problem hiding this comment.
Random thought - not sure it's something we should care about - but from my understanding the "auto-rotate" that is disabled here is never re-enabled.
That also seems to be true for the user_rotation system settings: the last setOrientation call will "persist" the chosen user_rotation system setting beyond the current running flow.
If someone is reusing the same android device to run multiple flows one after the other, it might introduce some side effect between them: if one flow is using setOrientation commands while another expect the orientation to be in "auto-rotate" mode (or expect a "default" orientation).
Edit: if this "side effect" is not something desired, it might be possible to mimic what is done for the setProxy: if set, proxy is "reset" when AndroidDriver is closed:
(
setProxy is also using settings under the hood)
There was a problem hiding this comment.
Hey @vbarthel-fr 👋 first off, thanks so much pulling and performing an android test for me!
You bring up a good topic here that I hadn't thought about yet. (And thanks for the pointer to resetProxy as an example being called from close().)
Do you foresee any holes in doing something like this:
- The first time that
setOrientationis called for a flow we store the current values ofaccelerometer_rotationanduser_rotation(and for iOS just theXCUIDevice.shared.orientation) - From the
close(), if those were saved we reset them to the original values.
There was a problem hiding this comment.
Yep, that sounds good to me :)
But I'm not knowledgeable enough to know for sure that is OK to rely on AndroidDriver.close(). Tomorrow, I will try to have a closer look at the source code, starting by understanding when close() is called.
|
I tried to implement an end-to-end test for Android : https://github.com/vbarthel-fr/maestro-e2e-set-orientation I tried my best to have the smallest APK as possible (8Kb in debug) that might even be added to this repo if it seems valuable. The app observes the ║
║ > Flow: test-set-orientation-flow
║
║ ✅ Launch app "com.example.maestro.orientation"
║ ✅ Set orientation LANDSCAPE_LEFT
║ ✅ Assert that "LANDSCAPE_LEFT" is visible
║ ✅ Set orientation LANDSCAPE_RIGHT
║ ✅ Assert that "LANDSCAPE_RIGHT" is visible
║ ✅ Set orientation UPSIDE_DOWN
║ ✅ Assert that "UPSIDE_DOWN" is visible
║ ✅ Set orientation PORTRAIT
║ ✅ Assert that "PORTRAIT" is visible
║hopefully it might be useful at some point :) |
Proposed changes
Introduce a
setOrientationcommand, accepting orientation values ofportrait, upsideDown, landscapeLeft, landscapeRight.If we're onboard with the syntax of this command and argument names, we should also make an addition to the public documentation repo to merge when this is ready to ship.
Background
After starting to build some flows thanks to the recent addition of landscape support on iOS, I found that it worked locally if I manually rotated the simulator into landscape orientation, but was unable to run on CI due to the simulator always launching in portrait.
#1221 started some discussion about adding the capability to force rotation, but at the time was blocked pending the baseline landscape support that was recently added.
Alternatives
Without adding this capability to maestro, I think the only way to run flows in landscape orientation on CI (at least on iOS simulators) is to expect users to rotate the simulator after it launches before starting the flows. Rotating the simulator is not available through
xcrun simctl, which leaves AppleScript as the alternative. AppleScript requires accessibility permissions be granted to the CI runner. While an AppleScript approach should be feasible, it add complexity to all users CI setups by requiring accessibility permissions to run.Testing
Unit tests: added new unit tests matching conventions from similar commands
iOS: verified locally using my own flows that without this command landscape tests fail, but with the addition of a call to
setOrientation: landscapeLeftafter launching the app, the tests succeed. The simulator frame stays in portrait orientation, but the test runner is now able to interact with the content correctly. This is the same behavior I see when running XCUITests on the landscape orientation app directly from Xcode.Android: I do not have an android setup available and have NOT tested this implementation on Android. Android testing support requested!
CI
I get one failure from CI related not committing the compiled iOS changes. I tried including a commit of that, but got the same error, so I could use some guidance on how to proceed there (or if that needs to be re-compiled by a maintainer for consistent provisioning or similar).
Issues fixed
#1221