-
-
Notifications
You must be signed in to change notification settings - Fork 29
test: E2E instrumented testing with basic screenshot test #88
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
Conversation
- specify jdk 1.8 - make jni debuggable
- Min targetSDK must be >=30 for playstore now
* Test Screenshot ci (#2) - grant test app permissions in advance - fix SettingsWatcher to properly get Premissions if allowed
6a61d59
to
a3b55db
Compare
Rebased on master |
cf95b49
to
780db09
Compare
d76ecc0
to
3ddf622
Compare
3ddf622
to
1ac31df
Compare
b284558
to
e5fa4d2
Compare
@ShootingKing-AM What do you think about this? Almost there! I can't seem to get reports to work with Couldn't figure out how to use Gradle in the e2e job without having to setup Java+SDK+NDK + rebuild the whole thing, but maybe possible somehow. How does it look to you otherwise? Any thoughts? :) |
Reports are not necessary for now i guess. Agree that #85 is re-doing many things for test which needs to be optimized (wrote as todo) I feel that it's better to use gradle to run instrumentation,
|
190dc70
to
9c76704
Compare
@ShootingKing-AM Using gradle is fine, but as I said, we need a way to run it without rebuilding the entire thing and duplicating tons of steps. Do you know a way?
Not sure what you mean, testing using adb+apk directly seems pretty straight-forward the way I do it here?
Easy to fix, can be retrieved with
Feels excessive at this point. Let's get it working first.
It's fine as long as the tests pass every time (unlikely), it's not fine when they fail! (waiting 30min for CI is not acceptable imo) It's also a mess and very bug-prone to have to maintain two almost identical build-jobs, we should not have to build the artifacts twice. |
@ShootingKing-AM I think this is good enough to merge soon. Still some work left to do, but at least it runs successfully! Remaining:
|
No I don't know a way to use gradle to install and test already built apk
Those are for basic tests, Screenshot test needs some services/helpers.
#85 is not intended to replace local emulator-based or device-based instrumentation testing. Its just to catch and alert devs that aw-android fails to run for eg. in cases like ActivityWatch/aw-server-rust#255 or ActivityWatch/aw-server-rust#324.
I feel that we are doing unnecessary work :( Why to even look up and maintain it for future, if there are some changes in espressor/uiautomator as to where the screenshot() output will go, that would be compensated by simultaneous gradle version updates and we will still be getting the screenshots into
True, but its just a compile and test process, also felt that this is much easier and maintainable, since, gradle has to build and emu has macos-requirements(for now) Overall, at the end of day, need a e2e ci-friendly test which would have minimum maintenance work. I still strongly feel that adb based instrumentation is not the way-forward, rest of course is your wish :) |
@ShootingKing-AM Ah fair enough! Those are some very valid points, I'm still learning a lot about all of this. That being said, I'm probably going to go ahead and merge this for now, since it's definitely an improvement over nothing at all and a minimal almost-working setup that is easier to continue working on. Please feel free to keep working on this in a new PR. Thank you so much for getting this much-needed work started in #85! :) |
6fb0d50
to
ed39a73
Compare
Continuing work by @ShootingKing-AM in #85
Playing around, just making a new PR to not mess up changes in #85.Made it reuse apks from previous build step, along with a bunch of other changes/fixes.