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

Simplify and modernize Android Test module. #2894

Merged
merged 12 commits into from Feb 4, 2023
Merged

Simplify and modernize Android Test module. #2894

merged 12 commits into from Feb 4, 2023

Conversation

TWiStErRob
Copy link
Contributor

@TWiStErRob TWiStErRob commented Jan 31, 2023

Goal: bring Android Test module out of the dark ages and reduce maintenance cost of it by minimizing.

Changes:

  • Latest-ish versions
    (using latest 7.4.0, 7.4.1, 8.0 beta) is not possible, because IntelliJ IDEA has an older Android plugin, so even the latest stable 2022.3.2 can't open AGP 7.4.0 projects.

  • Fixed all deprecations and warnings I found related to AGP.

  • Removed unused dependencies, it's just maintenance burden:

  • Removed unused resources, there's no app UI, also fixes:

    > Task :androidTest:licenseAndroidDebug NO-SOURCE
    Skipping task ':androidTest:licenseAndroidDebug' as it has no source files and no previous output files.
    Resolve mutations for :androidTest:licenseAndroidMain (Thread[Execution worker Thread 31,5,main]) started.
    :androidTest:licenseAndroidMain (Thread[Execution worker Thread 31,5,main]) started.
    Unknown file extension: subprojects\androidTest\src\main\res\mipmap-hdpi\ic_launcher.png
    Unknown file extension: subprojects\androidTest\src\main\res\mipmap-hdpi\ic_launcher_round.png
    Unknown file extension: subprojects\androidTest\src\main\res\mipmap-mdpi\ic_launcher.png
    Unknown file extension: subprojects\androidTest\src\main\res\mipmap-mdpi\ic_launcher_round.png
    Unknown file extension: subprojects\androidTest\src\main\res\mipmap-xhdpi\ic_launcher.png
    Missing header in: subprojects\androidTest\src\main\res\mipmap-anydpi-v26\ic_launcher.xml
    Missing header in: subprojects\androidTest\src\main\res\drawable-v24\ic_launcher_foreground.xml
    Missing header in: subprojects\androidTest\src\main\res\mipmap-anydpi-v26\ic_launcher_round.xml
    Unknown file extension: subprojects\androidTest\src\main\res\mipmap-xxhdpi\ic_launcher.png
    Unknown file extension: subprojects\androidTest\src\main\res\mipmap-xhdpi\ic_launcher_round.png
    Unknown file extension: subprojects\androidTest\src\main\res\mipmap-xxhdpi\ic_launcher_round.png
    Missing header in: subprojects\androidTest\src\main\res\drawable\ic_launcher_background.xml
    Unknown file extension: subprojects\androidTest\src\main\res\mipmap-xxxhdpi\ic_launcher.png
    Unknown file extension: subprojects\androidTest\src\main\res\mipmap-xxxhdpi\ic_launcher_round.png
    Missing header in: subprojects\androidTest\src\main\res\values\colors.xml
    Missing header in: subprojects\androidTest\src\main\res\values\strings.xml
    Missing header in: subprojects\androidTest\src\main\res\values-night\themes.xml
    Missing header in: subprojects\androidTest\src\main\res\values\themes.xml
    

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

@TWiStErRob TWiStErRob marked this pull request as ready for review February 3, 2023 09:41
@TWiStErRob
Copy link
Contributor Author

@reta @TimvdLippe ready for review, please start conversations on lines if you need clarification, happy to explain anything.

build.gradle Outdated
@@ -14,7 +14,7 @@ buildscript {
classpath 'org.shipkit:shipkit-auto-version:1.2.2'

classpath 'com.google.googlejavaformat:google-java-format:1.15.0'
classpath 'com.android.tools.build:gradle:4.2.0'
classpath 'com.android.tools.build:gradle:7.4.0-beta02'
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewer, please make sure you meet the requirements in settings.gradle.kts and import this in your IDE. I'm using the latest stable, theoretically this can be lowered by a few minors without breaking anything.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The beta02 is a bit concerning for release (just being cautious)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Google highly encourages using even alpha versions. I think using 7.4.0 beta is better than the previous 7.3.1. Google's "minor" versions are usually major changes with a lot of changes internally. Beta is usually pretty stable already, and the way it's used here (build an empty-ish trivial application) is pretty safe. As soon as the next IntelliJ IDEA release removes the silly arbitrary restriction this can be updated to 7.4.1 stable.

Based on the above we could argue to go down to 7.3.1 and bump to 7.4.1 later when support arrives. Because of the same simplicity.

Note that it is also possible to only include this module if the IDE supports it by adding more conditions to settings.gradle.kts, like: if (System.getProperty("idea.version") ?: "" < "2022.3") { ... }, but using the latest AGP version is not that necessary.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 all set from my side :)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would prefer to use stable versions only, so let's go with 7.3.1 for now. When 7.4.0 has been published, we can indeed update our toolchain.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

7.4.1 is already stable and it would pass CI, but no-one using IDEA would be able to open the project. I'll try 7.3.1 for now then.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@TimvdLippe 7.3.1 (downgraded in 79d032f) looks OK apart from Android Lint being too old for Kotlin 1.8+:

> Task :androidTest:lintAnalyzeDebug
e: P:/caches/gradle/caches/modules-2/files-2.1/org.jetbrains.kotlin/kotlin-stdlib/1.8.10/6d5560a229477df9406943d5feda5618e98eb64c/kotlin-stdlib-1.8.10.jar!/META-INF/kotlin-stdlib.kotlin_module: Module was compiled with an incompatible version of Kotlin. The binary version of its metadata is 1.8.0, expected version is 1.6.0.
e: P:/caches/gradle/caches/modules-2/files-2.1/org.jetbrains.kotlin/kotlin-stdlib/1.8.10/6d5560a229477df9406943d5feda5618e98eb64c/kotlin-stdlib-1.8.10.jar!/META-INF/kotlin-stdlib-jdk8.kotlin_module: Module was compiled with an incompatible version of Kotlin. The binary version of its metadata is 1.8.0, expected version is 1.6.0.
e: P:/caches/gradle/caches/modules-2/files-2.1/org.jetbrains.kotlin/kotlin-stdlib/1.8.10/6d5560a229477df9406943d5feda5618e98eb64c/kotlin-stdlib-1.8.10.jar!/META-INF/kotlin-stdlib-jdk7.kotlin_module: Module was compiled with an incompatible version of Kotlin. The binary version of its metadata is 1.8.0, expected version is 1.6.0.
e: P:/caches/gradle/caches/modules-2/files-2.1/org.jetbrains.kotlin/kotlin-stdlib-common/1.8.10/a61b182458550492c12aee66157d7b524a63a5ec/kotlin-stdlib-common-1.8.10.jar!/META-INF/kotlin-stdlib-common.kotlin_module: Module was compiled with an incompatible version of Kotlin. The binary version of its metadata is 1.8.0, expected version is 1.6.0.
e: P:/projects/contrib/github-mockito/subprojects/androidTest/build/intermediates/compile_app_classes_jar/debug/classes.jar!/META-INF/androidTest_debug.kotlin_module: Module was compiled with an incompatible version of Kotlin. The binary version of its metadata is 1.8.0, expected version is 1.6.0.

but this is an acceptable temporary loss on this project.

@reta
Copy link
Contributor

reta commented Feb 3, 2023

@TWiStErRob I sadly have no Android expertise but simplifications are look really good

Copy link
Contributor

@TimvdLippe TimvdLippe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One nit with the minor version usage. Also, can you rebase on master to retrigger the GitHub action jobs now that we are also running on Android 33?

build.gradle Outdated
@@ -14,7 +14,7 @@ buildscript {
classpath 'org.shipkit:shipkit-auto-version:1.2.2'

classpath 'com.google.googlejavaformat:google-java-format:1.15.0'
classpath 'com.android.tools.build:gradle:4.2.0'
classpath 'com.android.tools.build:gradle:7.4.0-beta02'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would prefer to use stable versions only, so let's go with 7.3.1 for now. When 7.4.0 has been published, we can indeed update our toolchain.

@codecov-commenter
Copy link

codecov-commenter commented Feb 4, 2023

Codecov Report

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

Coverage data is based on head (79d032f) compared to base (b31b95a).
Patch has no changes to coverable lines.

Additional details and impacted files
@@            Coverage Diff            @@
##               main    #2894   +/-   ##
=========================================
  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.

@TimvdLippe TimvdLippe merged commit ebdca97 into mockito:main Feb 4, 2023
@TimvdLippe
Copy link
Contributor

Awesome, thanks!

@TWiStErRob TWiStErRob deleted the android-upgrade branch February 4, 2023 22:42
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

4 participants