-
Notifications
You must be signed in to change notification settings - Fork 260
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.
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
FIX: Allow Maestro to work on Android API levels 25 and lower (Android 7.1.1 and lower) #1527
FIX: Allow Maestro to work on Android API levels 25 and lower (Android 7.1.1 and lower) #1527
Conversation
I tried to run this on an API 25 virtual device and the advanced flow fails on the second step with the following exception
|
Thanks for checking this, I admit I only tested this with the simple flow. I will try to find some more time to dig into this. |
@NyCodeGHG I've made an additional tweak. It seems AccessibilityNodeInfo I don't have my API 25 device on hand at the moment, so if you get a chance it would be great if you could test again with the advanced flow. Many thanks. |
d3f4e1b
to
116a96d
Compare
Now it just crashes on API level 25, not even launching the app
edit: seems like you accidentally reverted your changes? |
Thanks, I did accidentally revert my previous changes. Can I ask how you are building? In Logcat I see this exception...
but my change has made it use |
ff66758
to
1d47a58
Compare
I just used the maestro script in the project root |
So I do not think the
line 123 with my changes does not refer to the hintText line, it refers to the one above, so the error is definitely talking about ViewHierarchy without my changes in it. I am having a bit of a hard time running with my latest changes in @axelniklasson do you know how I can build and test this locally with my changes in There are very little / no docs on building locally. Thanks. It looks like (and a comment is included) that |
@artem888 can you please work with @testifyqa and help him test his changes out? Thanks! |
Hello there @testifyqa. If you need help getting an android emulator for api level 25 (or others) running so that your work on this can be made easier, please let us know. It will be a pleasure to help. This way you would not be depending on having a physical device with said version at hand. |
Hi there, I have a physical device with API 25 on hand now and also have emulators set up. My specific issue is that my changes made in |
I got it working with Also got a lot of warnings because of gradle task dependency issues, seems like this should be improved so the APKs in the resources folder actually get rebuild on changes when using the maestro script |
Excellent!! Thanks so much @NyCodeGHG. I went too deep and over looked simply running This would be really great to get merged in, not only for my team where I work, but also it will allow many more users to make use of Maestro on lower Android versions. Edit: Confirmed working for me too with advanced sample flow :) |
I've tried this also on API 24 which worked fine, but it starts to fail on 23 with this exception Stacktrace
|
Interesting, for my work I only need it working for API level 25 as the lowest. Can look at fixing it for API 23 as a separate PR possibly and rename the title/description of this one. I would really love to get this one merged as is to unblock us on part of my test strategy. Again for API 23 and lower seems like a straightforward fix though, probably a slightly different ADB shell command again. |
In case of this being merged as is, this line in build.gradle should also be changed to correctly reflect the minimum android api level in which maestro is supposed to work. And yes, the title have to be corrected. |
Great point, I'm away this weekend but when back home I can change that. If it's an easy fix for API level 23, and API 22 and 21 are the same commands as API 23 then I can see about including that in here possibly. |
This seems to be related: mobile-dev-inc/dadb#50 |
Just had a look, this is indeed the issue for API 23 (Marshmallow) and below. For this PR, I've updated the |
This would be great to add more support for older Android versions! 🏆 |
Any updates on getting approved and merged please? Would do wonders for my test teams. |
@testifyqa Can you also generate and commit the apks with:
As of now we don't do it automatically. After that we'll run a few tests to make sure existing flows don't break and then we can merge. |
Thanks @ArthurSav will do that now EDIT: Done! |
@testifyqa I'm getting the following error when trying to run
|
@ArthurSav I can have a look a bit later, I think the PR is old and needs a rebase as I think new src files were added in to |
2f3bbfd
to
0a59b88
Compare
@ArthurSav I rebased against this original repo as upstream and reassembled the apks and ran the same command on my API 25 device and it ran successfully, can you try again please? Unable to repro locally at the minute. |
Ignore that, able to repro after cleaning the project also I think, investigating |
|
Able to repro on more recent API versions locally now, investigating |
@ArthurSav Fixed!! I tested on API 25, 30 and 33. The fix was to fallback to empty string if hintText is null on api level 26+ also as not everything has a hintText value. |
c5fddde
to
1fbbb10
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Alright everyone, after running some tests this looks ready to be merged. Thanks for you help and contributions!
Woohoo thanks everyone :D |
Sorry accidentally deleted this,
This allows Maestro to work for Android API 24 and above