nowinandroid icon indicating copy to clipboard operation
nowinandroid copied to clipboard

[Bug]: JVM unit tests are not executed on CI

Open SimonMarquis opened this issue 1 year ago • 5 comments

Is there an existing issue for this?

  • [X] I have searched the existing issues

Is there a StackOverflow question about this issue?

  • [X] I have searched StackOverflow

What happened?

Since the recent merge of this PR

  • https://github.com/android/nowinandroid/pull/923

, the :core:common module is now a pure JVM module.

Unfortunately, the CI currently only runs Android tests:

https://github.com/android/nowinandroid/blob/50b13ecb21138b76e14ff0b2f4cf1dcfda9d0f43/.github/workflows/Build.yaml#L106

This also brought up this issue with missing test dependencies that were not visible on CI builds:

  • #1544

Relevant logcat output

No response

Code of Conduct

  • [X] I agree to follow this project's Code of Conduct

SimonMarquis avatar Jul 12 '24 11:07 SimonMarquis

How do you suggest we run all the JVM tests? I wouldn't want to specify every JVM module in the command.

JoseAlcerreca avatar Sep 05 '24 14:09 JoseAlcerreca

A solution to this could be to do something similar to slackhq/slack-gradle-plugin's UnitTest.kt that creates a global ciUnitTest task that executed the JVM default test tasks as well as the chosen test${variant}UnitTest Android test task. This worked for us with an even simpler setup on adevinta/spark-android.

SimonMarquis avatar Sep 05 '24 18:09 SimonMarquis

I would prefer switching over to a built-in umbrella task like check if we can, as that would avoid other cases like this in the future where there's a new type of check that we forget to add to CI.

If check runs too much, maybe we can look into something like switching to single-variant Android modules.

alexvanyo avatar Sep 20 '24 19:09 alexvanyo

What about add task with dependsOn at JVM module's gradle? https://docs.gradle.org/current/userguide/controlling_task_execution.html#sec:adding_dependencies_to_tasks

If the CI merely check demoDebugTest, you can make dependency with that.

Jaehwa-Noh avatar Sep 21 '24 08:09 Jaehwa-Noh

I would prefer switching over to a built-in umbrella task like check if we can, as that would avoid other cases like this in the future where there's a new type of check that we forget to add to CI.

I think this is a bad idea for this type of project. We have a wide variety of "checks" in Android projects: unit tests, instrumented tests, screenshot tests, lints checks, etc. Having a single "umbrella" for running all tests might seem a good idea, but in practice, we will never use it. Even the CI is already splitted in "logical" groups of checks. For memory, performance, parallelization and even clarity reasons. That is why I proposed we follow the solution to group similar checks into distinct umbrellas. ciUnitTests, ciAndroidTests, ciLint, etc.

SimonMarquis avatar Dec 14 '24 10:12 SimonMarquis