Skip to content
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

Adding UI testing of APRSdroid #316

Draft
wants to merge 34 commits into
base: master
Choose a base branch
from
Draft

Conversation

penguin359
Copy link
Contributor

@penguin359 penguin359 commented Jan 4, 2022

This adds in the first set of UI tests for APRSdroid. This PR depends on PR #315. I may commit a few more UI tests before completing this PR, but I thought you might like a first look at what the UI testing looks like. Also, while I am currently writing them in Java, I wanted to check with @gr0rg and see what language you would prefer for new code, in particular, code used outside the main app such as this testing code. I could try changing this to either Kotlin or Scala if preferred.

  • Switched to AndroidX for test support library
  • Implemented some basic tests with the first-run dialog

@penguin359
Copy link
Contributor Author

Well, UI testing with such a wide array of API levels is proving a little problematic. From my original push above, I found that API level 15 is failing due to a Java SecurityException:

Injecting to another application requires INJECT_EVENTS permission

Apparently, something is missing from the Android Test framework, however, testing on API levels 21 and above have all passed as seen here:

https://github.com/penguin359/aprsdroid/runs/4700101382?check_suite_focus=true

Baring one failure on API 31 with Google APIs that I have not triaged yet. Hopefully, this can be fixed and made a little more reliable.

@penguin359
Copy link
Contributor Author

Inspired by the bug report in issue #314, I have added a few more integration tests around the Manual Maps location API and have confirmed his bug report. I have pushed several tests in MapModeTest.java that test the map functionality and coordinates that it reports. Currently, the BAT had 4 failing tests all centered around verifying the coordinates shown on the screen. The issue matches exactly what the OP discussed and is now being found by the automated tests.

You will see two different failures in there if you run it, longitude tests in the east report a large difference of about 180 degrees because they are reporting as west. Other tests are failing because the longitude or latitude is reporting as negative. You can download the test reports as HTML files from the build artifacts on this build:

https://github.com/penguin359/aprsdroid/actions/runs/1652985741

You probably want to look at API 21 or 24 as 31 has been flaky at running. I can take a look this week at getting an actual fix into APRSdroid, but I'm out of time for today.

@penguin359
Copy link
Contributor Author

BTW, this PR is still in draft because it's waiting on PR #315. Then it can be rebased onto master as most of the commits reported above come from that PR.

@penguin359
Copy link
Contributor Author

penguin359 commented Jan 4, 2022

Hmm, something appears to have gone wrong, all the reports from GitHub CI are claiming that they can't find the TextView with the coordinates. It's working on my local system and reporting exactly the issue I'm expecting to find. I'll have to investigate further...

@penguin359
Copy link
Contributor Author

Oi, getting Android instrumented testing to work on a remote CI system is challenging. I have no idea why Github is having so much trouble. I do realize that I modify the Google Maps-related tests to be ignored when not on a Google APIs ROM, but they are still failing even when I follow the recommendations to disable all animations, send the unlock key press event, and use the full Google APIs with Play services images.

Well, I've added a good number of maps UI tests as well as back-end tests for the string formatting code. I was hoping to those later tests with the unit tests portion of the framework so the basic string formatting can be tested locally on the host without downloading to an Android emulator, but the code uses Location.convert() which is only available in the Android API so I had to add it to the instrumented tests. They still run a lot faster than the full Google Maps UI tests.

I'll have to pull out the commits included in the PR #317 I just created, but otherwise, this branch should be in good shape.

@penguin359 penguin359 force-pushed the ui-testing branch 2 times, most recently from 4a6f5af to f8051fa Compare January 5, 2022 17:03
@ge0rg
Copy link
Owner

ge0rg commented Jan 5, 2022

Thanks very much for the contributions. They are for sure merge-worthy. I have no strong preference regarding the language. In the long term, I'd like to move away from Scala due to a decade worth of build chain issues that only got worse and worse, and with "incremental" build times ranging north of 3 minutes on a high-end machine.

That said, both Java and Kotlin are perfectly fine for unit tests, so you can keep them as is.

I've only had a brief look at the code so far, and of course going for full coverage is a very long shot, but I like the start and this is already improving the project and giving me a good feeling, so please continue!

One minor thing. You've added implementation 'androidx.preference:preference:1.1.1' as a build dependency, should that be a testImplementation instead?

@penguin359
Copy link
Contributor Author

Hmm, that was accidental. I remember it wanting to pull that in, but I thought I reverted it to using only the built-in preferences API. If it is used, then it would actually be androidTestImplementation as it's part of the instrumentation tests that run on a real platform. testImplementation is just for tests that can run hosted locally on the dev machine which means no Android.

I think most of the benefits of that library are actually for improved UI widgets so I'll probably see if I can remove that unless you plan to use any of those widgets anytime soon.

If you haven't had a chance to see instrumented tests in action, you should fire up an emulator and try running them from this branch. I was thoroughly impressed the first time I saw a live demo and even more so when I saw how straight-forward they can be to write with Espresso.

As for what remains to be completed on this branch before pulling it, I mainly want to see if I can fix the builds on GitHub as this branch has been failing since I added the Google Maps tests. It's frustrating as I can run all unit tests with ./gradlew test and all instrumented tests with ./gradlew connectedCheck on my personal workstation and they are now passing (most of the time), but they fail to find the TextView when run on GitHub or throw security exceptions about injecting events. I might add a category to some of the flaky tests and just keep them out of the standard run on GitHub for now. Also, I should add some JUnit Truths to the Google Maps tests so they are ignored unless the assumption about Google APIs being available is true. That way, the matrix with the APIs will run more tests than the other half.

And one final caveat, as I mentioned, they sometimes still fail on my workstation on occasion. That's because the swipe events that I inject are not 100% precise and don't always move the map close enough to the same spot after the swipe left and right. That's also why I have the floating-point precision for those assertions to be rather large (0.05, I think?). I probably either need to increase the zoom level or reduce the required precision so they will never fail under working conditions.

@penguin359
Copy link
Contributor Author

Oi, introducing Kotlin into APRSdroid might be a little more difficult than previously expected. I was going to try to convert a few of these files from Java to Kotlin before finalizing this PR, but I found that there's only a narrow range of Kotlin plugin versions that seem to be both compatible with Android and this specific older release of the plugin. I am using version 1.3.10 now.

Once I got it to load in Gradle, I then hit some errors from the compileDebugScala task which looks like the Scala compiler itself is trying to parse the Kotlin source code or something funny like that. It was a couple of rather short Kotlin files that it was erroring on. I tried copying them to an online compiler and both files compile together without error using a similar version of Kotlin. I might try starting over and doing it one file at a time, but it's looking more and more like I'll just want to get a more up-to-date Scala plugin working for the build before introducing Kotlin code.

One of my simple Kotlin interfaces:

package org.aprsdroid.app

import java.net.DatagramSocket

interface ServiceLocator {
     fun provideDatagramSocket(): DatagramSocket
}

which produces this error:

> Task :compileDebugScala
Pruning sources from previous analysis, due to incompatible CompileSetup.
C:\Users\loren\AndroidStudioProjects\aprsdroid\src\ServiceLocator.kt:5: expected class or object definition
interface ServiceLocator {
^

@ge0rg
Copy link
Owner

ge0rg commented Jan 11, 2022

ARGH! This is related to Scala trying to compile my *.swp files from VIM, and I've given up beating it to only accept *.scala as its input corpus.

Different versions of gradle-android-scala-plugin don't support scala.srcDirs or scala.excludes, and I never found a way to tell it to only look for src/*.scala and src/*/*.scala, so eventually I just started running vim -n instead. It seems to ignore Java files on its own, because otherwise it would just always explode ;)

The latest SDK Commandline tools starting with release 9.0 require Java
17 to execute. These are used by the Android Emulator GitHub Action so
it will fail to run if the default Java environment is older than 17.
Need JRE 17 to be installed and default, but not the full JDK. However,
the build toolchain for Scala still requires to be run under Java 11 so
we need both installed. We just need to make sure JAVA_HOME is set
appropriately for JDK 11 when calling Gradle.

Set JAVA_HOME to JDK 11 for gradle on instrumented tests.
Hardware acceleration is now possible with Linux runners on GitHub and
have much better performance so no need for macOS anymore.
Currently, some tests are failing and reveal a specific bug in the
longitude values. Values in the Eastern hemisphere are shows as West.
This is referred to in issue ge0rg#314.
This is finding two different bugs that will need to be fixed. First off,
western longitudes and southern latitudes are shown as negative in
addition to the direction marker changing making it a bit of a double
negative.

Second, the longitude marker for East/West appears to be backwards. Western
longitudes show as negative east, and eastern longitudes as positive west.
…uilds

The Dialog tests are currently failing on Android API 15 due to a Java
SecurityException related to a missing permission of INJECT_EVENTS.
Hopefully, hiding the keyboard will avoid the exception.
The play store and Google services might be required for full testing
of the Google Maps embedded in the app.
This code is finding the same bug already detected by the MapModeTests
albeit in a much more direct way without needing to instantiate the
user interface. Ideally, this should be a unit test that can be executed
on the host-side, but the code depends on the Location.convert() for
the actual coordinate conversions which is only available on the
Android platform itself.
…o be available

This will mark any tests that require Google Play Services to be
installed and up-to-date as ignored if that is not true. With
that, all instrumented tests are now passing under API level 15.
Some of the map mode tests are a little unreliable and
can fail if the simulated finger gestures are not
perfectly precise.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants