ytai / ioio

Software, firmware and hardware of the IOIO - I/O for Android
Apache License 2.0
746 stars 355 forks source link

Updated all references for Activity to AppCompatActivity #221

Closed topherbuckley closed 3 years ago

topherbuckley commented 3 years ago

Sorry, had quite a few other things at work to do before working on this again. Looks like some changes remained while others were reverted based on what we discussed in #167, #175, #176, and #183.

I was able to build locally using the wrapper without error, but lets see what the CI tests show.

topherbuckley commented 3 years ago

Looking through the CI logs I see this error reoccuring in the various sample apps:

Error 1:

> Configure project :HelloIOIO
Warning: The 'kotlin-android-extensions' Gradle plugin is deprecated. Please use this migration guide (https://goo.gle/kotlin-android-extensions-deprecation) to start working with View Binding (https://developer.android.com/topic/libraries/view-binding) and the 'kotlin-parcelize' plugin.

Error 2:

ioio.examples.hello.HelloIOIOSmokeTest > smokeTestSimplyStart[emulator(AVD) - 9] FAILED 
    java.lang.RuntimeException: Waited for the root of the view hierarchy to have window focus and not request layout for 10 seconds. If you specified a non default root matcher, it may be picking a root that never takes focus. Root:
    Root{application-window-token=android.view.ViewRootImpl$W@b4de454, window-token=android.view.ViewRootImpl$W@b4de454, has-window-focus=false, layout-params-type=1, layout-params-string={(0,0)(fillxfill) sim={forwardNavigation} ty=BASE_APPLICATION wanim=0x10302f8

Error 3:

eglMakeCurrent failed in binding subwindow!

Some possible solutions here.

I will look into this next week as I'm off until Monday.

hannesa2 commented 3 years ago

Issue 1 is solved by https://github.com/ytai/ioio/pull/222 The others are more difficult

topherbuckley commented 3 years ago

Issue 1 is solved by #222 The others are more difficult

Thanks for taking care of #1. I'll look into the remainder today.

topherbuckley commented 3 years ago

As a starting point, I tested the HelloIOIO apk on my Nexus 3a (Android 10) and Nexus 6 (Android 9) and there were no errors in the LogCat specific to the app, and none with keyword window anywhere outside the app. Thus it appears there is something wrong not with the app itself, but with the CI tests. Having a look at your https://github.com/hannesa2/action-android now.

topherbuckley commented 3 years ago

Confused...All I did was rebase onto ytai/master with your fix for issue #1. Looks like this was enough?

hannesa2 commented 3 years ago

Hmmm, maybe the kotlin-extension caused this together with AppCompatActivity ? No clue why.

What I've observed, the layout changed concerning actionbar. Any chance to get this fixed ?

image

image

image

topherbuckley commented 3 years ago

d concerning actionbar. Any chance to get t

Happy to fix, but I'm not clear as to which column of images refers to which commit. Could you clarify? Obviously the middle one is bad, but is the left or right hand images the "correct" one? Can you better clarify what is the desired layout, then I can make the necessary changes to bring things back to "normal" :)

hannesa2 commented 3 years ago

Having now an actionbar like in second row is fine ! But eg when you look at last and first picture, there the label is overlapped with some other control.

For the desired layout I've no strong opinion. But when you ask me, I would like the see a correct actionbar and no overlapping label

topherbuckley commented 3 years ago
* left is master

* middle is the pixel diff

* right is your pull request

Having now an actionbar like in second row is fine ! But eg when you look at last and first picture, there the label is overlapped with some other control.

For the desired layout I've no strong opinion. But when you ask me, I would like the see a correct actionbar and no overlapping label

Got it. Thanks for the clarification. I'll work on this tomorrow and let you know how it goes.

Where do you pull these screenshots from in order to generate the pixel diffs btw? Never seen that. I see where I can download a zip of the screenshots in each CI run, but are they stored in the repo somewhere as well?

hannesa2 commented 3 years ago

I pulled it only from CI.

Here the images are not that important, but I've an idea in my head to store it and compare it during each pull request automatically as gradle plugin. But it will cause a lot of other tasks. But the first task is done: we generate images. I will see, if I find time to do this for Github actions as well

I did this for Zuul-CI in the past

hannesa2 commented 3 years ago

I'm not sure about the result

HelloIOIOI : miss the button image

HolidayIOIOI : miss the seekbar image

    fun smokeTestSimplyStart() {
        Espresso.onView(ViewMatchers.withId(R.id.frequencySeekBar)).check(ViewAssertions.matches(ViewMatchers.isDisplayed()))
        Screenshot.takeScreenshot("smoke")
    }

As you see, the screenshot is produced after the visibility check, but where it is ?

How does it looks like, when you start it manually ?

topherbuckley commented 3 years ago

I'm not sure about the result

HelloIOIOI : miss the button image

HolidayIOIOI : miss the seekbar image

    fun smokeTestSimplyStart() {
        Espresso.onView(ViewMatchers.withId(R.id.frequencySeekBar)).check(ViewAssertions.matches(ViewMatchers.isDisplayed()))
        Screenshot.takeScreenshot("smoke")
    }

As you see, the screenshot is produced after the visibility check, but where is it ?

How does it looks like, when you start it manually ?

Interesting. Everything looks fine when I run it on my two physical devices and on my virtual Pixel3a. Maybe it is an issue with timing? Are the screenshots taken at a set time or after some callback? I wonder if it is actually generating everything after the screenshots are taken. I'll take another look now.

topherbuckley commented 3 years ago

I made a test branch to hack away at the github actions stuff and it seems to work without me doing anything to it:

https://github.com/topherbuckley/ioio/actions

I first tried reducing the workload by only building and testing on HelloIOIO rather than all the apps. The screenshot looked fine.

I then reverted back to testing all the modules, and the HelloIOIO screenshot turned out strange again. I then tried reverting to the malinskiy repo to see if any changes you had made might be creating this, though again the screenshots appeared strange.

Without knowing much JS, the only thing that strikes me as odd at this point from a quick skim through is that the default defaultSdKUrl is used and is SDK 26 (a fair bit outdated) in the install-sdk action. Also we appear to be using API 28 in the emulator-run-cmd action, which is also a bit outdated.

Another possibility you may know more about is something I considered after reading through the github action logs. As per default it seems all the tests and builds on github labs are running in parallel and maybe things that need to be run async are being run synchronously?

e.g here:

> Task :HelloIOIO:connectedDebugAndroidTest
Starting 1 tests on emulator(AVD) - 9

> Task :HelloIOIO:createScreenshotDirectory

> Task :HelloIOIO:fetchScreenshots
/storage/emulated/0/Download/./: 1 file pulled, 0 skipped. 0.2 MB/s (2641 bytes in 0.012s)

> Task :HelloIOIO:connectedAndroidTest

Does it not seem strange that the cAT, which has your screenshot code from applications/HelloIOIO/src/androidTest/HelloIOIOSmokeTest.kt runs AFTER the fetchScreenshots task? I'm a complete noobie when it comes to CI, espresso tests, and the like, so these are just ideas.

What I do know is that the apps all work fine on physical and emulated devices, and this is either a github action or espresso test issue.

A side questions, is there no way to test even a small change without waiting 10 min for a full run? Developing CI with github actions must drive one insane if so!

hannesa2 commented 3 years ago

I rebased, to get this https://github.com/Malinskiy/action-android/commit/4b538ec9108752cbd8878ede1a8008f7f67b1311

It looks like it solves the issue, please have a look here https://github.com/hannesa2/ioio/pull/17 There I did some tests

topherbuckley commented 3 years ago

Sounds good. Had a quick look at the diff. I wasn't well versed enough to know exactly what was going on in the typescript files, but appreciate the link as a point of reference if this ever comes up again.

Thanks for the merge and 6.1.0 release. I'll be using the release/jitpack in my project from here forward. I'll let you know if we run into any ioio related issues with it.