google / dagger

A fast dependency injector for Android and Java.
https://dagger.dev
Apache License 2.0
17.43k stars 2.01k forks source link

Hilt: No JaCoCo test coverage on @AndroidEntryPoint classes. #1982

Open goldy1992 opened 4 years ago

goldy1992 commented 4 years ago

Given a class

@AndroidEntryPoint
MyActivity : AppCompatActivity()

and a test class

@HiltAndroidTest
@RunWith(RobolectricTestRunner::class)
@LooperMode(LooperMode.Mode.PAUSED)
class MyActivityTest() {
    private lateinit var scenario : ActivityScenario<MainActivity>

    @Rule
    @JvmField
    val rule : HiltAndroidRule = HiltAndroidRule(this)

    @Before
    fun setup() {
        rule.inject()
        scenario = ActivityScenario.launch(MyActivity::class.java)
    }

    @Test
    fun myActivityTest() {
        scenario.onActivity { activity: MyActivity ->
        // test code
        }
   }
  // .. rest of test class
}

and given JaCoCo Gradle Plugin version 0.8.5

Expected: Some code coverage on MyActivity class. Actual:: 0% coverage

Notes:

This is surely to do with the fact that Hilt is adding an injecting class between MyActivity and it's super class.

This is a Generic example, please tag me if you require any more information.

danysantiago commented 4 years ago

I'm sorry, I'm not familiar with JaCoCo. Would you mind filling an issue against the AGP team here: https://issuetracker.google.com/issues/new?component=192708&template=840533

My guess is same as yours, Hilt's transform must be throwing off the instrumentation made by JaCoCo which is needed generate a coverage report. In that issue if you can provide a sample app that would be perfect. Hilt's Gradle Plugin uses AGP Transform APIs so one important question is if JaCoCo coverage is aware of the transform pipeline such that it takes into account the result of other transforms.

goldy1992 commented 4 years ago

Thank you @danysantiago this issue has been recorded here: https://issuetracker.google.com/issues/161300933

goldy1992 commented 4 years ago

For reference, the issue has been raised here.

goldy1992 commented 4 years ago

@danysantiago https://issuetracker.google.com/issues/161300933 I have provided more information on this issue to the android gradle team.

The incorrect coverage is caused by the enableTransformForLocalTests = true flag. I think this transformation causes inconsistencies between the code that jacoco expects and the transformed code.

goldy1992 commented 4 years ago

Tested on 2.29.1-alpha and this is still an issue

OlegNovosad commented 4 years ago

This sounds like a critical issues. How is it possible to use Hilt in production without getting coverage? I observe the same issue. Are there any workarounds here?

goldy1992 commented 4 years ago

@OlegNovosad There are no workarounds to this using JaCoCo. It is likely to need a major change from the hilt team or the JaCoCo team, since the issue is to do with bytecode transformation in order for hilt to inject the dependencies.

The issue https://issuetracker.google.com/issues/161300933 has been labelled as a P3 from the google team.

Chang-Eric commented 4 years ago

A workaround you can probably use is to write the code as if you were not able to use the bytecode transformation and specify the base class manually. https://dagger.dev/hilt/gradle-setup#why-use-the-plugin

This comes with its own drawbacks though since then the generated code will affect IDE suggestions.

goldy1992 commented 4 years ago

@Chang-Eric this does indeed provide a workaround... Due to the vague explanation in the link you provided, I will demonstrate this with an example for an activity:

@AndroidEntryPoint(AppCompatActivity::class)
class MainActivity : Hilt_MainActivity() {
 // contents of class
}

@HiltAndroidTest
@RunWith(RobolectricTestRunner::class)
@LooperMode(LooperMode.Mode.PAUSED)
class MainActivityTest {

    private lateinit var scenario : ActivityScenario<MainActivity>

    @Rule
    @JvmField
    val rule : HiltAndroidRule = HiltAndroidRule(this)

    @Before
    fun setup() {
        rule.inject()
        scenario = ActivityScenario.launch(MainActivity::class.java)
    }

    @Test
    fun myTest() {
    }
}

This declaration allows JaCoCo to know of the existence of the generated Hilt_MainActivity class... This is a workaround as we end up having to refer to generated code from our real code, which doesn't seem to cause to much trouble on my Android Studio 4.0.1 however will probably cause havoc on previous versions and/or other IDEs.

Chang-Eric commented 3 years ago

Thanks for the clear example! So that I can fix it, which part of the docs were vague? Is it that we should say directly that the plugin will swap your base class or something else?

goldy1992 commented 3 years ago

@Chang-Eric I guess things are much clearer now that I have a better overview of what the gradle plugin does. I guess the thing that was not immediately clear is that the gradle plugin is optional.

As for the original issue... is it even possible for the hilt gradle plugin to run without direct bytecode transformation, so that unit tests are not effected?

Furthermore I see some instances in your solution that do not seem to work without the gradle plugin:

@AndroidEntryPoint(MediaBrowserServiceCompat::class)
open class MediaPlaybackService : Hilt_MediaBrowserCompat(), SomeInterface

and the generated source directory is

@kotlin.Metadata(mv = {1, 4, 0}, bv = {1, 0, 3}, k = 1, d1 = {"someMetadata...;", ..."})
@dagger.hilt.android.AndroidEntryPoint(value = androidx.media.MediaBrowserServiceCompat.class)
public class MediaPlaybackService implements SomeInterface

I will try to reproduce this issue on a more simple simple project and share it.

Chang-Eric commented 3 years ago

The gradle plugin won't do any bytecode transformation if all of the source code is already directly using the right base class.

The optional bit is confusing admittedly, we should add a note about that. The plugin sets up some other things for you like flags and stuff (and soon maybe some other classpath-y things), so it is really the bytecode transformation aspect is optional.

That last issue is interesting. Actually, I think we just ran into something similar to that internally this week. I wouldn't be surprised if there is some other kapt issue there that we need to debug... I think we should split that out into a separate issue though.

Chang-Eric commented 3 years ago

Btw, do you have correctErrorTypes set to true?

https://dagger.dev/hilt/gradle-setup#using-hilt-with-kotlin

kapt {
 correctErrorTypes true
}
goldy1992 commented 3 years ago

Yes I have this option set to true

goldy1992 commented 3 years ago

A useful note for anyone having trouble with this using Android Gradle Plugin (AGP) 4.1. In order to get any unit test code coverage with JaCoCo you need to testCoverageEnabled false so that AGP's compiled classes for use on device are not used instead of the local instrumented classes.

See https://issuetracker.google.com/issues/171125857 and AGP 4.1 release notes on unit tests https://developer.android.com/studio/releases/gradle-plugin