Jacoco reports low coverage with Compose tests
Steps to reproduce
- JaCoCo version: 0.8.7
- Operating system: macOS Big Sur 11.4
- Tool integration: Gradle
- Complete executable reproducer: https://github.com/arcadefire/JacocoComposeSample
- Steps:
- Checkout to the repo above and execute the command:
./gradlew clean JacocoTestReport
- Checkout to the repo above and execute the command:
Expected behaviour
Creating a simple composable function displaying a text and a test covering it that checks that the text was displayed should produce 100% test coverage.
Actual behaviour
The coverage for such a simple case is particularly low, especially the conditional coverage, even though the function contains no conditional logic.
Also, looking at the function's coverage details, the yellow diamonds report a number of untested conditional branches.
@arcadefire Quoting https://stackoverflow.com/questions/53485360/incorrect-jacoco-code-coverage-for-kotlin-coroutine/53568092#53568092 as example, many other similar posts, similar issues in this repository and our talks about JaCoCo
JaCoCo performs analysis of bytecode, not source code
And if you'll have a look at bytecode of these functions, you'll see all these branches, which are generated by compiler plugin for Compose. And also for example one of the comments in the above link states
* @Composable fun A(x: Int) { * f(x) * } * * gets transformed into * * @Composable fun A(x: Int, $composer: Composer<*>, $changed: Int) { * var $dirty = $changed * if ($changed and 0b0110 === 0) { * $dirty = $dirty or if ($composer.changed(x)) 0b0010 else 0b0100 * } * if (%dirty and 0b1011 xor 0b1010 !== 0 \|\| !$composer.skipping) { * f(x) * } else { * $composer.skipToGroupEnd() * } * }
So in such sense there is no bug and JaCoCo report is absolutely correct about presence of branches in bytecode.
While we try to filter similar things generated by compiler, it is already quite complex task, not even taking into account such compiler plugins, and our resources are quite limited. So question arises, especially after reading https://developer.android.com/jetpack/compose/mental-model that describes functions with @Composable as declarative programming paradigm - does it really make sense to measure coverage for such functions? or you'd better instead focus on testing (and hence measuring coverage) of business logic of your application that is not located in these declarative functions? i.e. I'm not at all sure about ROI of implementing in JaCoCo filters for Compose compiler plugin.
@Godin Thanks for looking into this. :)
Using Jacoco with Compose's testing library (in combination with Robolectric) would've been a fast way to ensure tests coverage without resorting to other libraries or different strategies - especially using additional tooling. Yes, business logic shouldn't be part of any view. With declarative UI (or well, even without) you can have simple branches that could be covered by tests too.
Something like:
if (state.isButtonVisible) {
Button(...)
}
I agree with you that constantly supporting filters for a specific library might not be worth it, especially for a small team, but I'd expect this library to gain a lot of traction in the Android community, so perhaps it would be a nice improvement.
The case @arcadefire described is a real world problem and we also face the same problem.
It is definitely better to have unit tests for the Fragments on Android with Robolectric running them. I even don't understand the https://developer.android.com/jetpack/compose/mental-model model shared here. It doesn't make so much sense to us.
We rely on Jacoco to produce code quality reports and on our newer modules we opted to use Compose library. Our coverage dramatically dropped after those changes as it seems Compose generate a lot of hidden conditions behind the scenes, and there is no way to test them.
We can potentially add the Fragment tests to the exclude list. However, we lose quality control when we do that.
@Godin could you please share the plans if this issue will be addressed or any alternative solutions will be provided? E.g. class level exclusions via annotations etc. or any other solution?
Can Jacoco automatically exclude methods annotated with androidx.compose.ui.tooling.preview.Preview ?
@headsvk No, JaCoCo is framework agnostic.
@marchof How about a feature that would allows us to specify excluded annotations? The current functionality based on hardcoded Generated string in the annotation name is too rigid.
@headsvk As additional configuration options add more complexity and make problems by users more difficult to reproduce we decided to add no configurable filters.
Any alternative way to filter this? Or get a proper coverage report with composables?
Thanks in advance!! โกโก๐๐
For us, the @Preview composables are a big drag on our code coverage metric. We are able to use the ExcludeFromJacocoGeneratedReport annotation to exclude other classes, functions etc. But it does not work on @Preview @Composable functions.
I understand Jacoco is Framework/Library agnostic. But unable to exclude blocks of code, which are either annotated or delimited by some other means seems like a serious limitation of the tool.
It would be great if we can have some mechanism to help out everywhere who is struggling with this issue. Compose is future for Android and not a niche anymore.
The problem is not with the @Preview annotation itself. Its the lambdas inside the preview functions.
For e.g.
@ExcludeFromJacocoGeneratedReport
@Preview(uiMode = UI_MODE_NIGHT_YES)
@Composable
private fun AddDataLinkPreviewDark() {
XyzComponentDevTheme {
AddDataLink(
buttonText = "buttonText1",
description = "description",
onClick = {},
)
}
}
@Composable
@ExcludeFromJacocoGeneratedReport
fun XyzComponentDevTheme(
content: @Composable BoxScope.() -> Unit,
) {
XyzTheme {
Box(
modifier = Modifier.background(MaterialTheme.colors.background),
content = content,
)
}
}
In the above example, we have the wrapper "XyzComponentDevTheme", which is used to apply some shared config to all our previews. Anything which is passed in as a content to XyzComponentDevTheme shows up as uncovered.
We prefer to keep all our previews in the same file as the Composable itself and hence there is no way to exclude these preview functions. which we have a lot in our modules.
@Godin @marchof Any idea why this is not working when the functions themselves are annotated with the Generated annotation? Any workarounds? Thanks.
@vvictor10 The lambda is compiled to a separate method. This is a known limitation of annotations. To fix this we would need to analyze the call hierarchy, like "if a method is called from a method with a specific annotation this should also be ignored".
@marchof Makes sense. Is analyzing the call stack impossible to do? Wondering wht the downsides to doing such a thing would be? Previews existing along with the source code is super helpful for us, but at the same time, accurate code coverage metrics is also very important for us :)
@vvictor10 We don't collect call stacks at runtime (that would be way too much runtime overhead). Theoretically this would be possible also with static analysis.
Here's a tip for how we solved it
project.afterEvaluate {
// Hack to exclude all source files which has Compose related code
excludeComposeCode()
...
}
def excludeComposeCode() {
fileTree('../')
.filter {
// Find all kotlin files which contains 'import androidx.compose.runtime.Composable'
it.name.endsWith('.kt') && it.text.contains('import androidx.compose.runtime.Composable')
}
.forEach {
// Translate each file path to '**/ui/**/<filename>.class' + '**/ui/**/<filename>Kt.class' and add to ext.excludes
def name = it.name.replace('.kt', '')
ext.excludes.add('**/ui/**/'+name+'.class')
ext.excludes.add('**/ui/**/'+name+'Kt.class')
}
}
Is this something that should be reported to Google then? So that they try to not generate code that cannot be covered by tests?
@vvictor10 As for lambdas, something like this partially works (only partially)
XyzTheme(body = @ExcludeFromJacocoGeneratedReport {
Box(
modifier = Modifier.background(MaterialTheme.colors.background),
content = content,
)
}
)