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

Basic Android instrumented and unit tests (closes #2341) #2360

Merged
merged 6 commits into from Jul 30, 2021

Conversation

robpridham-bbc
Copy link
Contributor

@robpridham-bbc robpridham-bbc commented Jul 16, 2021

See #2341

Most of this PR is project overhead associated with a new Android project.

The primary benefit is providing basic instrumented tests which replicate failures seen in #2316, #2327 and #2340. Currently it only performs the most rudimentary establishment of some mocks; however, this is enough to produce these failures on the problematic versions.

I have included basic JUnit 4/5 non-instrumented tests which is perhaps unnecessary, given that more extensive tests exist in the non-Android subprojects, and there is nothing Android-specific about them. However I thought it might be valuable to have them exist within the context of the Android toolchain to provide basic assurance.

There is a README included with this, which may need further development to inform non-Android developers.

@TimvdLippe
Copy link
Contributor

@robpridham-bbc Thank you so much for getting this of the ground! I refactored the PR slightly to make sure it ingrates with ./gradlew build. Now, if you run the build locally, it should run the unit tests and use the locally built version of Mockito.

Unfortunately, I wasn't able to figure out how to run the instrumentation themselves from the command line. As I understand the, the instrumentation tests were the actual tests that were failing. I tried downgrading ByteBuddy to check that the tests are failing as expected, but none of them were.

Based on https://spin.atomicobject.com/2016/03/10/android-test-script/ I understand that we would need to have a script that can boot up an emulator on CI and then run the corresponding scripts? At least that's what it told me when I ran ./gradlew :androidTest:connectedCheck locally.

I am fine if we land this as-is if we need more infrastructure changes to support these emulators. We probably want to add a separate GitHub action task to run these. However, it would be good to first verify that approach will work before we land this PR. WDYT?

@TimvdLippe
Copy link
Contributor

Interesting. Locally it builds just fine, but it claims that on JDK 11, the :androidTest:lint task failed. I don't really know how we can tackle that, especially as the GitHub action doesn't provide any logs as to what exactly failed.

@codecov-commenter
Copy link

codecov-commenter commented Jul 16, 2021

Codecov Report

Merging #2360 (92d3ed0) into main (af521c8) will increase coverage by 0.02%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff              @@
##               main    #2360      +/-   ##
============================================
+ Coverage     84.88%   84.90%   +0.02%     
  Complexity     2794     2794              
============================================
  Files           328      328              
  Lines          8480     8480              
  Branches       1025     1025              
============================================
+ Hits           7198     7200       +2     
  Misses         1004     1004              
+ Partials        278      276       -2     
Impacted Files Coverage Δ
...to/internal/util/concurrent/WeakConcurrentMap.java 39.39% <0.00%> (+2.02%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update af521c8...92d3ed0. Read the comment docs.

@TimvdLippe
Copy link
Contributor

Ah. Seems like the memory increase worked and we got a green build 🎉

@robpridham-bbc
Copy link
Contributor Author

@robpridham-bbc Thank you so much for getting this of the ground! I refactored the PR slightly to make sure it ingrates with ./gradlew build. Now, if you run the build locally, it should run the unit tests and use the locally built version of Mockito.

Unfortunately, I wasn't able to figure out how to run the instrumentation themselves from the command line. As I understand the, the instrumentation tests were the actual tests that were failing. I tried downgrading ByteBuddy to check that the tests are failing as expected, but none of them were.

Based on https://spin.atomicobject.com/2016/03/10/android-test-script/ I understand that we would need to have a script that can boot up an emulator on CI and then run the corresponding scripts? At least that's what it told me when I ran ./gradlew :androidTest:connectedCheck locally.

I am fine if we land this as-is if we need more infrastructure changes to support these emulators. We probably want to add a separate GitHub action task to run these. However, it would be good to first verify that approach will work before we land this PR. WDYT?

Subject to confirmation, ./gradlew :androidTest:connectedCheck sounds right. I'm a little bit out of touch with the scripting requirements because (a) our normal dev work is IDE-driven where it handles this and (b) we do our device/emulator automation testing on Firebase Test Lab, which involves a Gradle plugin (fladle) uploading the output APK there and invoking the tests.

Firebase might actually be a good option for this project. For us in our day jobs it costs money, but a very small amount as there's a free quota of daily execution minutes that we rarely exceed.

@TimvdLippe
Copy link
Contributor

Hm, I would prefer not to add any dependencies on external services such as Firebase. I will look into making a simple bash script that can do not. Not sure if that will work, but based on the blog post that maybe is feasible?

@robpridham-bbc
Copy link
Contributor Author

robpridham-bbc commented Jul 16, 2021

The bash script article looks OK although it seems to assume an already created emulator image. You can create one at the command line too but it's a bit more involved.

By the way, I feel I should probably point out that the migration into the top level project probably works fine from a Gradle point of view but it does mean it's not maintainable as a normal Android project any more, e.g. in Android Studio or as far as I'm aware IntelliJ. I'm not averse to what you've done; this is a balance between convenient integration into the main project (rather than being a weird Android-specific outlier) and making authoring any future extensions more difficult.

However it might also have unwanted consequences. I think this is why you were seeing no exception messages for Lint. Similarly, in its current form it doesn't pass for me, and its output is totally opaque, although I haven't dug into why.

> Task :androidTest:connectedDebugAndroidTest
Starting 6 tests on SM-G998B - 11

org.mockitousage.androidtest.BasicInstrumentedTests > mockAndUseBasicClassUsingAnnotatedMock[SM-G998B - 11] FAILED .21.jar
        java.lang.Exception: java

> Task :androidTest:connectedDebugAndroidTest FAILED

This is not normal - when I go back to my original commit and test against 3.11.2, I get proper output for the failure:

> Task :app:connectedDebugAndroidTest
Starting 6 tests on SM-G998B - 11

org.mockitousage.androidtest.BasicInstrumentedTests > mockAndUseBasicClassWithVerify[SM-G998B - 11] FAILED 
        java.lang.IllegalStateException: Could not initialize plugin: interface org.mockito.plugins.MockMaker (alternate: null)
        at org.mockito.internal.configuration.plugins.PluginLoader$1.invoke(PluginLoader.java:84)

org.mockitousage.androidtest.BasicInstrumentedTests > mockAndUseBasicClassUsingLocalMock[SM-G998B - 11] FAILED 
        java.lang.IllegalStateException: Could not initialize plugin: interface org.mockito.plugins.MockMaker (alternate: null)
        at org.mockito.internal.configuration.plugins.PluginLoader$1.invoke(PluginLoader.java:84)

org.mockitousage.androidtest.BasicInstrumentedTests > mockAndUseBasicInterfaceUsingAnnotatedMock[SM-G998B - 11] FAILED 
        java.lang.IllegalStateException: Could not initialize plugin: interface org.mockito.plugins.MockMaker (alternate: null)
        at org.mockito.internal.configuration.plugins.PluginLoader$1.invoke(PluginLoader.java:84)

org.mockitousage.androidtest.BasicInstrumentedTests > mockAndUseBasicClassUsingAnnotatedMock[SM-G998B - 11] FAILED 
        java.lang.IllegalStateException: Could not initialize plugin: interface org.mockito.plugins.MockMaker (alternate: null)
        at org.mockito.internal.configuration.plugins.PluginLoader$1.invoke(PluginLoader.java:84)

org.mockitousage.androidtest.BasicInstrumentedTests > mockAndUseBasicInterfaceAndVerify[SM-G998B - 11] FAILED 
        java.lang.IllegalStateException: Could not initialize plugin: interface org.mockito.plugins.MockMaker (alternate: null)
        at org.mockito.internal.configuration.plugins.PluginLoader$1.invoke(PluginLoader.java:84)

org.mockitousage.androidtest.BasicInstrumentedTests > mockAndUseBasicInterfaceUsingLocalMock[SM-G998B - 11] FAILED 
        java.lang.IllegalStateException: Could not initialize plugin: interface org.mockito.plugins.MockMaker (alternate: null)
        at org.mockito.internal.configuration.plugins.PluginLoader$1.invoke(PluginLoader.java:84)

> Task :app:connectedDebugAndroidTest FAILED

FAILURE: Build failed with an exception.

@TimvdLippe
Copy link
Contributor

I wasn't able to work on this in the past two weeks. That said, I am inclined to merge this PR as-is and keep iterating on the bash script in a follow-up. Preferably we get at least some coverage for Android, although the real value will be in the instrumentation tests that will require an emulator to run.

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.

None yet

3 participants