[Bug]: Test dependencies are included in release builds

SimonMarquis commented 1 year ago

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?

The runtime classpath of the release build type of module :app contains test dependencies and it definitely should not.

androidx.compose.ui:ui-test-junit4:1.3.0
androidx.compose.ui:ui-test:1.3.0
androidx.concurrent:concurrent-futures:1.1.0
androidx.multidex:multidex:2.0.1
androidx.test.espresso:espresso-core:3.4.0
androidx.test.espresso:espresso-idling-resource:3.4.0
androidx.test.ext:junit:1.1.3
androidx.test.services:storage:1.4.2-rc01
androidx.test:annotation:1.0.1-rc01
androidx.test:core:1.5.0-rc01
androidx.test:monitor:1.6.0-rc01
androidx.test:rules:1.4.0
androidx.test:runner:1.4.0
app.cash.turbine:turbine-jvm:0.12.1
app.cash.turbine:turbine:0.12.1
com.google.dagger:hilt-android-testing:2.44
com.squareup:javawriter:2.1.1
junit:junit:4.13.2
org.hamcrest:hamcrest-core:1.3
org.hamcrest:hamcrest-integration:1.3
org.hamcrest:hamcrest-library:1.3
org.jetbrains.kotlinx:kotlinx-coroutines-test-jvm:1.6.4
org.jetbrains.kotlinx:kotlinx-coroutines-test:1.6.4

The main side effects are:

slight increase in the apk output (~10 KiB thanks to R8)
increased build time because of transitive dependencies
unexpected AndroidManifest permission android.permission.REORDER_TASKS
3 exported test Activities in androidx.test.core.app.InstrumentationActivityInvoker

This seems to have been introduced ~1 month ago by this change https://github.com/android/nowinandroid/commit/c4debb74e7f1cffed9c3d6e07f5887044de0febb#diff-8cff73265af19c059547b76aca8882cbaa3209291406f52df1dafbbc78e80c46L91-R92. I'll create a PR to revert this change:

-    implementation(project(":sync:sync-test"))
+    androidTestImplementation(project(":sync:sync-test"))

===

Also, what do you think about adding an automated check to avoid these kind of issues in the future? I used dropbox/dependency-guard to get this data and it can perform checks to enforce no test dependencies are being used in these configurations.

For instance, adding this config

dependencyGuard {
    configuration("demoReleaseRuntimeClasspath") {
        allowedFilter = { "test" !in it && "junit" !in it }
    }
}

... would have lead to the following error output:

Execution failed for task ':app:dependencyGuard'.
> Disallowed Dependencies found in :app for the configuration "demoReleaseRuntimeClasspath" 
  ":core:testing",
  ":sync:sync-test",
  "androidx.compose.ui:ui-test-junit4:1.3.0",
  "androidx.compose.ui:ui-test:1.3.0",
  "androidx.test.espresso:espresso-core:3.4.0",
  "androidx.test.espresso:espresso-idling-resource:3.4.0",
  "androidx.test.ext:junit:1.1.3",
  "androidx.test.services:storage:1.4.2-rc01",
  "androidx.test:annotation:1.0.1-rc01",
  "androidx.test:core:1.5.0-rc01",
  "androidx.test:monitor:1.6.0-rc01",
  "androidx.test:rules:1.4.0",
  "androidx.test:runner:1.4.0",
  "com.google.dagger:hilt-android-testing:2.44",
  "junit:junit:4.13.2",
  "org.jetbrains.kotlinx:kotlinx-coroutines-test-jvm:1.6.4",
  "org.jetbrains.kotlinx:kotlinx-coroutines-test:1.6.4",

Relevant logcat output

No response

Code of Conduct

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

SimonMarquis commented 1 year ago

@mmoczkowski can you comment on the last part of the issue description?

Also, what do you think about adding an automated check to avoid these kind of issues in the future? I used dropbox/dependency-guard to get this data and it can perform checks to enforce no test dependencies are being used in these configurations.

SimonMarquis commented 10 months ago

DependencyGuard implementation in progress in #1035

android / nowinandroid