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

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

Open
goldy1992 opened this issue Jul 11, 2020 · 15 comments
Open

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

goldy1992 opened this issue Jul 11, 2020 · 15 comments

Comments

@goldy1992
Copy link

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:

  • The coverage worked fine before the hilt plugin migration
  • Given the same scenario with
    @AndroidEntryPoint MyActivity : MyActivityBase() where class ÀctivityBase : AppCompatActivity() There is coverage reported on ActivityBase but not on MyActivity.

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.

goldy1992 added a commit to goldy1992/Mp3Player that referenced this issue Jul 11, 2020
Issue-125 - added the Viewmodel implementation with LiveData to key classes, preserving the ui state on interruptions such as screen rotations. Migrated to Android Hilt to remove unnecessary boiler plate code in the dagger packages. Code coverage and activity and service classes is depentant on Dagger issue 1982 google/dagger#1982
@danysantiago
Copy link
Member

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
Copy link
Author

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

@goldy1992
Copy link
Author

For reference, the issue has been raised here.

@goldy1992
Copy link
Author

@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
Copy link
Author

Tested on 2.29.1-alpha and this is still an issue

@OlegNovosad
Copy link

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
Copy link
Author

@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
Copy link
Member

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
Copy link
Author

@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
Copy link
Member

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
Copy link
Author

@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
Copy link
Member

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
Copy link
Member

Btw, do you have correctErrorTypes set to true?

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

kapt {
 correctErrorTypes true
}

@goldy1992
Copy link
Author

Yes I have this option set to true

@goldy1992
Copy link
Author

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

goldy1992 added a commit to goldy1992/Mp3Player that referenced this issue Nov 28, 2020
* Feature/issue 110/content observer (#113)

Issue-110 - added functionality to react to changes in audio files, i.e. the add/deletion of files and reacted by updating the search database accordingly. Also fixed bug viewing songs from folders.

* issue-112 - started to work on the client side migration

* Issue-110 - pushing latest changes

* issue-112 - fixed more front end errors

* Issue-112 - committing latest changes

* Issue-112 - committing latest changes

* issue-112 - fixed more front end errors

* Issue-112 - TODO: configure test support

* issue-112 - temporary commit, before attempting to modularise components, to aid unit testing

* issue-112 - modularised source code

* issue-112 - pushing latest changes

* issue-112 - continuing the module modularisation

* issue-112 - continuing the module modularisation

* issue-112 - pushing latest changes

* issue-112 - pushing latest changes

* issue-112 - pushing latest changes

* issue-112 - continuing the module modularisation

* issue-112 - continuing the module modularisation

* issue-112 - continuing the module modularisation

* issue-112 - continuing the module modularisation, fixing client unit tests

* issue-112 - continuing the module modularisation, fixing client unit tests

* issue-112 - pushing latest changes

* issue-112 - continuing the module modularisation, fixing client unit tests

* issue-112 - fixed client unit tests

* issue-112 - converted commons module to kotlin

* issue-112 - converted commons module to kotlin

* issue-112 - fixed test and updated sonar and travis config

* issue-112 - fixed test and updated sonar and travis config

* issue-112 - started work on migrating service to kotlin

* issue-112 - fixed compiler errors

* issue-112 - solved services compile issues

* issue-112 - committing latest changes, TODO: add coroutines to the client to try and include test compilation

* issue-112 - committing latest code

* issue-112 - committing latest code

* issue-112 - committing latest code

* issue-112 - committing latest code

* issue-112 - committing latest code

* issue-112 - committing latest code

* issue-112 - committing latest code

* issue-112 - committing latest code

* issue-112 - committing latest code

* issue-112 - committing latest code

* issue-112 - committing latest code, TODO: fix execution data error to hopefully fix coverage

* Issue-112 - pushing latest changes

* Issue-112 - resolved client coverage issue

* Issue-112 - started work on services coverage

* Issue-112 - tidied up the client gradle file

* Issue-112 - removed old dependencies on org.mockito.*

* Issue-112 - working on the services unit test fixes

* issue-112 - TODO: fix final failing test: SongFromFolderRetrieverTest

* Issue-112 - resolved coverage issues

* Issue-112 - tidied up jacoco gradle code and added better support for commons code coverage

* Issue-112 - reactivated ignored tests

* Issue-112 - updated kotlin to exclude LoggingUtils.kt

* Issue-112 - updated kotlin to exclude all res directory coverage

* Issue-112 - removed redundant code

* Issue-112 - TODO: remove the use of handlers and handler threads and use coroutines to determine on what thread code gets executed

* Issue-112 - removed handlers in service

* Issue-112 - removed duplicate init from PermissionsProcessor

* Issue-112 - TODO: fix unit tests

* Issue-112 - TODO: fix unit tests

* issue-112 - fixed failing unit tests

* Issue-112 - TODO: fix unit tests

* issue-112 - fixed failing unit tests

* Issue-112 - added an MediaPlaybackService onSearch test... TODO: fix the search bug

* Issue-112 - fixed the search bug

* Issue-112 - TODO: fix unit tests

* Issue-112 - added MyGenericItemTouchListener tests

* Issue-115 - committing latest changes

* Issue-115 - fixed compilation errors

* Issue-115 - removed handler dependencies

* Issue-115 - committing latest changes for source sets

* Issue-115 - resolved jacoco issues, TODO: configure the nhaarman plugin so that it is recognised in AndroidStudio

* issue-115 - added kotlin mockito dependency

* Issue-115 - added test app build configuration to align build flavours

* issue-115 - added more unit test coverage to media item utils

* issue-115 - added more unit test coverage to media item utils

* Feature/issue 117/cleanup gradle script (#119)

Issue-117 - cleaned up the Gradle script to remove unnecessary warnings and increased test coverage to above 80%

* Feature/issue 120/end to end tests (#121)

Issue-120 - refactored and added first end to end tests

* Feature/issue 120/end to end tests (#123)

Issue-120 - Organised integration and end to end tests into separate android flavors and moved test classes and test util classes to more appropriate locations. The commons module now has no flavors.

* Feature/issue 125/viewmodel and livedata (#126)

Issue-125 - added the Viewmodel implementation with LiveData to key classes, preserving the ui state on interruptions such as screen rotations. Migrated to Android Hilt to remove unnecessary boiler plate code in the dagger packages. Code coverage and activity and service classes is depentant on Dagger issue 1982 google/dagger#1982

* Feature/issue 127/mediaplayer bottom sheet (#129)

Pull request to align styles with Material Design library and clean up any obsolete xml layout files, the majority being present in the :app module

* Feature/issue 130/single activity implementation (#131)

Issue-130 - Migrated to the use of fragments instead of activities.

* Issue-132 - tried oraclejdk11 in travis.yml (#133)

Issue-132 - added a solution to get the latest JDK for sonar cloud

* Issue-135 - Updated MaterialDesign version and migrated to Slider. (#136)

Issue-135 - Updated MaterialDesign version and migrated to Slider.

* Feature/issue 50/color theme (#137)

Issue-50 - added dark mode support and a settings fragment .

* Feature/issue 50/color theme (#138)

- added palette image ready for use
- changed theme icon to palette and added placeholder for settings as well as refactoring menu names
- added fix to ensure the activity recreation does not throw an exception
- Removed junit5 library
- Upgraded to AGP 4.1
- re-organised build structure to accommodate AGP 4.1

Co-authored-by: Mike Goldsmith <michael.goldsmith@datalex.com>
JSunde added a commit to JSunde/ground-android that referenced this issue Feb 4, 2022
…ed covarege from being calculated for @androidentrypoint classes. See google/dagger#1982 for additional context.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants