Set up Android Github Action pipeline. Fixes #2892 #2893

reta · 2023-01-31T00:59:53Z

Signed-off-by: Andriy Redko drreta@gmail.com

Set up Android Github Action pipeline.
Fixes #2892

Checklist

Read the contributing guide
PR should be motivated, i.e. what does it fix, why, and if relevant how
If possible / relevant include an example in the description, that could help all readers
including project members to get a better picture of the change
Avoid other runtime dependencies
Meaningful commit history ; intention is important please rebase your commit history so that each
commit is meaningful and help the people that will explore a change in 2 years
The pull request follows coding style
Mention Fixes #<issue number> in the description if relevant
At least one commit should mention Fixes #<issue number> if relevant

codecov-commenter · 2023-01-31T14:42:22Z

Codecov Report

Base: 85.65% // Head: 85.65% // No change to project coverage 👍

Coverage data is based on head (3878f87) compared to base (50b21cf).
Patch has no changes to coverable lines.

Additional details and impacted files

@@            Coverage Diff            @@
##               main    #2893   +/-   ##
=========================================
  Coverage     85.65%   85.65%           
  Complexity     2847     2847           
=========================================
  Files           325      325           
  Lines          8623     8623           
  Branches       1060     1060           
=========================================
  Hits           7386     7386           
  Misses          964      964           
  Partials        273      273

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

TWiStErRob

Should work :)

TWiStErRob · 2023-01-31T15:22:42Z

.github/workflows/ci.yml

+    - name: 3. Setup Android SDK
+      uses: android-actions/setup-android@v2
+
+    - name: 4. Install Android SDK components
+      run: |
+        sdkmanager --install "platforms;android-31"


You probably don't even need these two steps, because Android Gradle Plugin will download everything automatically (what is needed) and GitHub Actions hosted runners are set up with Android SDK already.

3. is only necessary if you use self-hosted runners.
4. is not necessary if you use Gradle.

👍 thanks! still figuring out how to make it work :)

TWiStErRob · 2023-01-31T15:25:27Z

.github/workflows/ci.yml

+    - name: 5. Run Android tests on Java ${{ matrix.java }} with ${{ matrix.entry.mock-maker }} and ${{ matrix.entry.member-accessor }}
+      uses: reactivecircus/android-emulator-runner@v2
+      with:
+        api-level: 29
+        script: ./gradlew :androidTest:connectedCheck
+      env:
+        MOCK_MAKER: ${{ matrix.entry.mock-maker }}
+        MEMBER_ACCESSOR: ${{ matrix.entry.member-accessor }}


Could you please add a step (in the same job) for running gradlew test too so that unit tests with mocks are also tested. These should be just normal JVM tests, but we never know what can break :)

ah never mind, that would be covered by normal CI, nice 👏

(iff all the conditions in settings.gradle are met in normal CI)

TWiStErRob · 2023-01-31T15:26:07Z

.github/workflows/ci.yml

@@ -70,7 +70,7 @@ jobs:
        MOCK_MAKER: ${{ matrix.entry.mock-maker }}
        MEMBER_ACCESSOR: ${{ matrix.entry.member-accessor }}

-    - name: 7. Upload coverage report
+    - name: 10. Upload coverage report


:D yeah, adding and removing steps, will fix that 😄

TWiStErRob · 2023-01-31T15:26:55Z

.github/workflows/ci.yml

+      matrix:
+        java: [ 11 ]
+        entry:
+          - { mock-maker: 'mock-maker-default', member-accessor: 'member-accessor-default' }
+          - { mock-maker: 'mock-maker-inline', member-accessor: 'member-accessor-module' }
+          - { mock-maker: 'mock-maker-subclass', member-accessor: 'member-accessor-module' }
+          - { mock-maker: 'mock-maker-subclass', member-accessor: 'member-accessor-reflection' }
+          - { mock-maker: 'mock-maker-inline', member-accessor: 'member-accessor-reflection' }


I have a feeling matrix won't be necessary like this, because mockito-android has its own mockmaker, right?

It might be the case indeed, I will check it shortly

TWiStErRob · 2023-01-31T16:19:44Z

.github/workflows/ci.yml

@@ -85,6 +85,52 @@ jobs:
        chmod +x codecov
        ./codecov

+  #
+  # Android build job


FYI #2894
(I don't think it's necessary for this pipeline PR, but was inspired :) )

Signed-off-by: Andriy Redko <drreta@gmail.com>

reta · 2023-01-31T17:45:39Z

@TWiStErRob @TimvdLippe I think we nailed it :)

...ects/androidTest/src/androidTest/java/org/mockitousage/androidtest/BasicInstrumentedTests.kt

reta · 2023-01-31T18:57:34Z

FYI, I tested the Github action on 5.1.0:

> Task :androidTest:connectedDebugAndroidTest
Starting 6 tests on test(AVD) - 10

org.mockitousage.androidtest.BasicInstrumentedTests > mockAndUseBasicClassWithVerify[test(AVD) - 10] FAILED 
	java.lang.NoClassDefFoundError: Failed resolution of: Ljava/lang/StackWalker$Option;
	at org.mockito.internal.debugging.LocationImpl.stackWalker(LocationImpl.java:138)

org.mockitousage.androidtest.BasicInstrumentedTests > mockAndUseBasicClassUsingLocalMock[test(AVD) - 10] FAILED 
	java.lang.NoClassDefFoundError: org.mockito.internal.debugging.LocationImpl
	at org.mockito.internal.debugging.LocationFactory.create(LocationFactory.java:17)

org.mockitousage.androidtest.BasicInstrumentedTests > mockAndUseBasicInterfaceUsingAnnotatedMock[test(AVD) - 10] FAILED 
	java.lang.NoClassDefFoundError: org.mockito.internal.debugging.LocationImpl
	at org.mockito.internal.debugging.LocationFactory.create(LocationFactory.java:17)

org.mockitousage.androidtest.BasicInstrumentedTests > mockAndUseBasicClassUsingAnnotatedMock[test(AVD) - 10] FAILED 
	java.lang.NoClassDefFoundError: org.mockito.internal.debugging.LocationImpl
	at org.mockito.internal.debugging.LocationFactory.create(LocationFactory.java:17)

org.mockitousage.androidtest.BasicInstrumentedTests > mockAndUseBasicInterfaceAndVerify[test(AVD) - 10] FAILED 
	java.lang.NoClassDefFoundError: org.mockito.internal.debugging.LocationImpl
	at org.mockito.internal.debugging.LocationFactory.create(LocationFactory.java:17)

org.mockitousage.androidtest.BasicInstrumentedTests > mockAndUseBasicInterfaceUsingLocalMock[test(AVD) - 10] FAILED 
	java.lang.NoClassDefFoundError: org.mockito.internal.debugging.LocationImpl
	at org.mockito.internal.debugging.LocationFactory.create(LocationFactory.java:17)

> Task :androidTest:connectedDebugAndroidTest FAILED

TWiStErRob

Nice, getting simpler! 👏

TWiStErRob · 2023-01-31T20:49:32Z

.github/workflows/ci.yml

+      matrix:
+        java: [ 11 ]
+        entry:
+          - { mock-maker: 'mock-maker-default', member-accessor: 'member-accessor-default' }


Do you want to keep a 1-element matrix?
It could be fully inlined to the env var values?

Actually might want to introduce more API levels in the future, because they have different JVMs. See my comment on the issue, API 21 fails, while API 31 passes.

Yeah, I think we could remove the mock-maker part

I think we settled on level 26 as the minimum: https://github.com/mockito/mockito/blob/main/build.gradle#L102

For reference the official docs only list some new APIs, so even Android 8 doesn't support Java 11. However, as I understand with desugaring Java 11+ APIs can be used as low as API 1. This is of course not practical, I think an API 21 minimum would be enough. That said official full Java 11 support is only in 33.

Anyway, based on the info, starting with android-api: [26,27,28,29,30,31,33] would work.
(32 has no Java changes, it was a "feature drop", not given a major version, if it works, it could be included though)

Just to clarify, the matrix should only change the emulator version, the compile/target/minSdk should be hardcoded to highest/lowest values as Google recommends it (TL;DR: we have ~1 year to use the latest target, otherwise cannot publish).

.github/workflows/ci.yml

...ects/androidTest/src/androidTest/java/org/mockitousage/androidtest/BasicInstrumentedTests.kt

.github/workflows/ci.yml

Signed-off-by: Andriy Redko <drreta@gmail.com>

TWiStErRob

I would include api-level: ${{ matrix.android-api }} for all the versions that pass right now to preserve compatibility.

reta · 2023-02-01T13:42:22Z

I would include api-level: ${{ matrix.android-api }} for all the versions that pass right now to preserve compatibility.

We may need to exclude some, I believe I have tried 31 first and the emulator part didn't work :(

reta · 2023-02-01T14:26:08Z

.github/workflows/ci.yml

+    strategy:
+      matrix:
+        java: [ 11 ]
+        android-api: [26, 27, 28, 29]


@TWiStErRob so I have to remove 30-31 API levels (for now), the emulator GA seems to incorrectly deduct the arch:

/bin/sh -c \sdkmanager --install 'system-images;android-31;default;x86' --channel=0 > /dev/null Warning: Failed to find package 'system-images;android-31;default;x86'

The result is that emulator is not installed and the test run fails. I think we could hack the arch parameters or just get to that later on when it is fixed. I think it looks good now. Thanks!

It just doesn't exist: sdkmanager --list | grep android- | grep default:

system-images;android-26;default;arm64-v8a | 1 | ARM 64 v8a System Image system-images;android-26;default;x86 | 1 | Intel x86 Atom System Image system-images;android-26;default;x86_64 | 1 | Intel x86 Atom_64 System Image system-images;android-27;default;arm64-v8a | 1 | ARM 64 v8a System Image system-images;android-27;default;x86 | 1 | Intel x86 Atom System Image system-images;android-27;default;x86_64 | 1 | Intel x86 Atom_64 System Image system-images;android-28;default;arm64-v8a | 1 | ARM 64 v8a System Image system-images;android-28;default;x86 | 4 | Intel x86 Atom System Image system-images;android-28;default;x86_64 | 4 | Intel x86 Atom_64 System Image system-images;android-29;default;arm64-v8a | 8 | ARM 64 v8a System Image system-images;android-29;default;x86 | 8 | Intel x86 Atom System Image system-images;android-29;default;x86_64 | 8 | Intel x86 Atom_64 System Image system-images;android-30;default;arm64-v8a | 1 | ARM 64 v8a System Image system-images;android-30;default;x86_64 | 10 | Intel x86 Atom_64 System Image system-images;android-31;default;arm64-v8a | 4 | ARM 64 v8a System Image system-images;android-31;default;x86_64 | 4 | Intel x86 Atom_64 System Image

I would be very surprised if the "latest" MacOS, that GitHub just released recently would be x86.

The action doesn't deduce the architecture, it's hardcoded to x86. Can you try adding the numbers and forcing the arch?

https://github.com/ReactiveCircus/android-emulator-runner/blob/e1248ae048e31ec2e171adc06be585d06c1f936e/action.yml#L14-L16

uses: reactivecircus/android-emulator-runner@v2 with: api-level: ${{ matrix.android-api }} + arch: x86_64 # or arm64-v8a (don't know if these Macs are M1/M2 or not)

x86_64 does not work for sure, I tried that before, let me try arm

What does "does not work for sure" means, because in their examples they also use that:
https://github.com/ReactiveCircus/android-emulator-runner#usage--examples

PANIC: Avd's CPU Architecture 'arm64' is not supported by the QEMU2 emulator on x86_64 host. sounds good.

It seems like going up to 31 is fine, updated the matrix, after 31 - failures :(

I think 33 would work, told you 32 is "special": #2893 (comment)

Notice that in the above table there's no 32 and 33, but there are emulators:

system-images;android-32;android-desktop;arm64-v8a | 5 | Desktop ARM 64 v8a System Image system-images;android-32;android-desktop;x86_64 | 5 | Desktop Intel x86 Atom_64 System Image system-images;android-32;google_apis;arm64-v8a | 3 | Google APIs ARM 64 v8a System Image system-images;android-32;google_apis;x86_64 | 3 | Google APIs Intel x86 Atom_64 System Image system-images;android-32;google_apis_playstore;x86_64 | 3 | Google Play Intel x86 Atom_64 System Image system-images;android-33;google_apis;arm64-v8a | 8 | Google APIs ARM 64 v8a System Image system-images;android-33;google_apis;x86_64 | 8 | Google APIs Intel x86 Atom_64 System Image system-images;android-33;google_apis_playstore;x86_64 | 7 | Google Play Intel x86 Atom_64 System Image

Let's merge it as is, this is a huge improvement over "nothing". We can figure out 32/33 later.

Oh sorry, I missed that the issue was only specific to 32

Let's see why https://issuetracker.google.com/issues/267458959

Signed-off-by: Andriy Redko <drreta@gmail.com>

TimvdLippe

Only 1 nit. Do I understand correctly we want to merge #2894 first?

TimvdLippe · 2023-02-02T19:23:14Z

.github/workflows/ci.yml

+    strategy:
+      matrix:
+        java: [ 11 ]
+        android-api: [26, 27, 28, 29, 30, 31]


I don't think we want to test so many Android releases, since we don't have a lot of CPU capacity on GitHub actions. Instead, I think only having 26 (which is the oldest version that we still support) is sufficient. If we notice gaps in Android support, we can always extend the matrix, but for now I think just 1 Android version is good enough.

Good point, very likely if it works on the oldest, it'll work elsewhere too.

That said, I would follow this up with a PR for running 33, because that has full on Java 11 support, which can mess things up. But this PR, even with 1 version is good enough.

Kept only 26

TWiStErRob · 2023-02-02T19:30:49Z

No, let's merge this first. There's no dependency, either one can go first, but this one is ready, mine still needs review.

Signed-off-by: Andriy Redko <drreta@gmail.com>

TimvdLippe

Great work, thank you!

TWiStErRob · 2023-02-03T09:47:53Z

Thank you for your patience @reta! Thanks for @TimvdLippe for taking this, it'll definitely help the Android community if this keeps passing on every release.

reta · 2023-02-03T12:29:18Z

Thank you for your patience @reta! Thanks for @TimvdLippe for taking this, it'll definitely help the Android community if this keeps passing on every release.

Thanks a for tremendous help and guidance @TWiStErRob !

reta force-pushed the issue-2892 branch 8 times, most recently from aea1c5b to f2e40ee Compare January 31, 2023 14:41

reta force-pushed the issue-2892 branch 2 times, most recently from 028f1c9 to 6410a71 Compare January 31, 2023 14:51

TWiStErRob reviewed Jan 31, 2023

View reviewed changes

reta force-pushed the issue-2892 branch 2 times, most recently from 8462aa0 to 1a29867 Compare January 31, 2023 16:46

Set up Android Github Action pipeline. Fixes mockito#2892

81d30af

Signed-off-by: Andriy Redko <drreta@gmail.com>

reta force-pushed the issue-2892 branch from 1a29867 to 81d30af Compare January 31, 2023 17:33

reta marked this pull request as ready for review January 31, 2023 17:45

reta commented Jan 31, 2023

View reviewed changes

...ects/androidTest/src/androidTest/java/org/mockitousage/androidtest/BasicInstrumentedTests.kt Show resolved Hide resolved

TWiStErRob reviewed Jan 31, 2023

View reviewed changes

Addressing code review comments

fb613e0

Signed-off-by: Andriy Redko <drreta@gmail.com>

TWiStErRob approved these changes Feb 1, 2023

View reviewed changes

reta force-pushed the issue-2892 branch 3 times, most recently from 7bfd06b to 47106ed Compare February 1, 2023 14:10

reta commented Feb 1, 2023

View reviewed changes

reta force-pushed the issue-2892 branch from 47106ed to e017c88 Compare February 1, 2023 14:59

reta force-pushed the issue-2892 branch 7 times, most recently from 10fb19f to 9e4bfa9 Compare February 1, 2023 16:25

Add Android API level matrix parameter to Github Action

69bdd38

Signed-off-by: Andriy Redko <drreta@gmail.com>

reta force-pushed the issue-2892 branch from 9e4bfa9 to 69bdd38 Compare February 1, 2023 16:47

TWiStErRob approved these changes Feb 1, 2023

View reviewed changes

TimvdLippe requested changes Feb 2, 2023

View reviewed changes

Keep only one Android API level for now (26)

3878f87

Signed-off-by: Andriy Redko <drreta@gmail.com>

reta requested a review from TimvdLippe February 2, 2023 19:46

TimvdLippe approved these changes Feb 2, 2023

View reviewed changes

TimvdLippe merged commit e978455 into mockito:main Feb 2, 2023

This was referenced Feb 3, 2023

CI on Android API level 33 for Java 11 compatibility testing #2899

Merged

StackWalker.Option not found on Mockito 5.1.0 #2890

Closed

Android CI improvements, improves #2892 #2903

Merged

Set up Android Github Action pipeline. Fixes #2892 #2893

Set up Android Github Action pipeline. Fixes #2892 #2893

Conversation

reta commented Jan 31, 2023 • edited

Checklist

codecov-commenter commented Jan 31, 2023 • edited

Codecov Report

TWiStErRob left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

reta commented Jan 31, 2023

reta commented Jan 31, 2023

TWiStErRob left a comment

Choose a reason for hiding this comment

TWiStErRob Jan 31, 2023 • edited

Choose a reason for hiding this comment

TWiStErRob Jan 31, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

TWiStErRob left a comment

Choose a reason for hiding this comment

reta commented Feb 1, 2023

Choose a reason for hiding this comment

TWiStErRob Feb 1, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

TWiStErRob Feb 1, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

TimvdLippe left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

TWiStErRob Feb 2, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

TWiStErRob commented Feb 2, 2023

TimvdLippe left a comment

Choose a reason for hiding this comment

TWiStErRob commented Feb 3, 2023

reta commented Feb 3, 2023

reta commented Jan 31, 2023 •

edited

codecov-commenter commented Jan 31, 2023 •

edited

TWiStErRob Jan 31, 2023 •

edited

TWiStErRob Jan 31, 2023 •

edited

TWiStErRob Feb 1, 2023 •

edited

TWiStErRob Feb 1, 2023 •

edited

TWiStErRob Feb 2, 2023 •

edited