Readd code coverage merging #2657
Conversation
b3e3c3b to
f8f5fad
Compare
There was a problem hiding this comment.
Summary of Changes
Hello @alexvanyo, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
This pull request reintroduces the functionality to merge code coverage reports generated by Jacoco from various subprojects and test types (unit and Android tests) into unified reports. This is achieved by adding a new Gradle convention plugin and implementing the necessary configuration logic to collect coverage data and class files from the relevant test tasks across the project's modules.
Highlights
- New Convention Plugin: Introduces a new Gradle convention plugin named
mergeJacocospecifically designed to handle the configuration for merging Jacoco code coverage reports. - Jacoco Merge Configuration: Adds a new function
configureJacocoMergewithin the build logic to set up Jacoco tasks that collect and merge coverage data from unit tests (per variant like debug, release, staging) and Android tests across all subprojects. - Aggregate Coverage Reports: Defines aggregate Gradle tasks (
jacocoUnitTestCoverageReport,jacocoTestCoverageReport) that depend on the variant-specific and Android test merge tasks, providing convenient entry points to generate combined HTML and XML coverage reports.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configureGemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Code Review
This pull request re-adds the functionality to merge JaCoCo code coverage reports across subprojects. It introduces a new Gradle convention plugin (mergeJacoco) and associated logic in Jacoco.kt to collect coverage data and class files from individual module reports and generate merged reports. The changes correctly register and apply the new plugin and define the core merging tasks. Potential areas for improvement include making the task identification and variant handling more robust by avoiding hardcoded names and lists where possible, and ensuring consistency in how file collections are processed.
| private fun Project.getUnitTestReportTasks(variant: String) = | ||
| getTasksByName("create${variant.capitalizeForTaskName()}UnitTestCoverageReport", false) | ||
| .filterIsInstance<JacocoReportTask>() |
There was a problem hiding this comment.
Using getTasksByName can be brittle as it relies on exact task name matching and doesn't leverage Gradle's task configuration avoidance APIs. If the task naming convention changes elsewhere, this logic will break. While accessing tasks created by other plugins can be challenging, consider if there's a more robust way to identify these tasks, perhaps by type or properties, or using tasks.named with a provider if the task names can be reliably constructed.
| private fun Project.getAndroidTestReportTasks() = | ||
| variants.flatMap { variant -> | ||
| getTasksByName("createManagedDevice${variant.capitalizeForTaskName()}AndroidTestCoverageReport", false) | ||
| .filterIsInstance<JacocoReportTask>() | ||
| } |
There was a problem hiding this comment.
Similar to getUnitTestReportTasks, using getTasksByName here makes the code dependent on specific task naming conventions (createManagedDevice${variant.capitalizeForTaskName()}AndroidTestCoverageReport). This could be a point of failure if task names change. Explore if task configuration avoidance or identification by type/properties is feasible.
| private val variants = | ||
| listOf( | ||
| "debug", | ||
| "release", | ||
| "staging", | ||
| ) |
There was a problem hiding this comment.
The variants list is hardcoded. This means the merging logic will only work for projects with these exact variants ("debug", "release", "staging"). If a project has different or additional variants, this will need manual updates. Can the list of variants be derived dynamically from the Android extension of the subprojects, or is this hardcoding necessary due to the cross-project configuration context?
f8f5fad to
96de7f0
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
✅ All tests successful. No failed tests found. Additional details and impacted files@@ Coverage Diff @@
## main #2657 +/- ##
=============================================
+ Coverage 54.13% 67.68% +13.55%
- Complexity 514 578 +64
=============================================
Files 264 264
Lines 13667 13667
Branches 1801 1796 -5
=============================================
+ Hits 7398 9250 +1852
+ Misses 5707 3638 -2069
- Partials 562 779 +217
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
No description provided.