Repository navigation
Report unit test coverage across modules - #5891
Conversation
Per-module JaCoCo reports credit a class only with the tests of its own module, so classes tested from another module show as uncovered. For example core's CalciteRelNodeVisitor is tested almost only by ppl's CalcitePPL*Test, and its report showed 11 of 2,622 lines covered, which made codecov/patch report about 0% for any change to it. Add a root jacocoAggregatedReport task that merges the unit test execution data of every module that applies jacoco and reports it against the same classes, with the same excludes, as the per-module reports that CI uploaded. CI now uploads this one report to Codecov. Line coverage of those classes goes from 27,094 / 41,033 (66.0%) to 32,135 / 41,033 (78.3%); core alone from 56.1% to 80.9%, and CalciteRelNodeVisitor to 2,271 of 2,622 lines. The per-module reports and jacocoTestCoverageVerification rules are unchanged. Signed-off-by: Lantao Jin <ltjin@amazon.com>
PR Reviewer Guide 🔍(Review updated until commit e2fdc20)Here are some key observations to aid the review process:
|
PR Code Suggestions ✨Latest suggestions up to e2fdc20
Previous suggestionsSuggestions up to commit 4c9ce10
|
LantaoJin
added
the
infrastructure
jacocoAggregatedReport depended on every unit test task, so with --continue a single failing test skipped the report, and the CI coverage upload failed with no file to upload. The per-module reports it replaces are finalizers of the test task and are still generated when a test fails, because JaCoCo writes the execution data anyway. Order the report after the test tasks instead of depending on them, as each module's jacocoTestReport does. It no longer runs the tests by itself, so the developer guide now says ./gradlew test jacocoAggregatedReport. The CI command already runs build first. Signed-off-by: Lantao Jin <ltjin@amazon.com>
Codecov Report✅ All modified and coverable lines are covered by tests. ❌ Your project check has failed because the head coverage (75.15%) is below the target coverage (99.00%). You can increase the head coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #5891 +/- ##
=============================================
+ Coverage 63.87% 75.15% +11.27%
- Complexity 9091 10328 +1237
=============================================
Files 962 962
Lines 41033 41048 +15
Branches 4668 4669 +1
=============================================
+ Hits 26211 30848 +4637
+ Misses 13944 8904 -5040
- Partials 878 1296 +418
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
Persistent review updated to latest commit e2fdc20 |
Non-blocking: Next is to change the target coverage. This PR increase the coverage to 75%+ from 63%. How about 75% as short term target and 85% for long term? |
2 participants
Description
JaCoCo coverage is reported per Gradle module, so a class gets credit only for the tests in its own module. Much of
coreis tested from other modules:CalciteRelNodeVisitoris tested almost only byppl'sCalcitePPL*Test, yetcore's report shows 11 of its 2,622 lines covered. As a result,codecov/patchreports about 0% for any PR that changes such a class, however well the PR tests it (for example #5783).This PR adds a root task,
jacocoAggregatedReport. It merges the unit test execution data of every module that appliesjacoco(includingopensearch-sql-pluginandinteg-test:test, whose tests also run code in other modules) and reports it against the classes of the 12 modules whose reports CI uploaded so far. It reuses each module's ownjacocoTestReportclass directories, so the per-module excludes (for examplecore's**/ast/**) still apply, and the set of reported classes does not change. The CI unit job now runs the task and uploads this one XML to Codecov instead of the 12 per-module files.Line coverage of the same 41,033 lines, after
./gradlew teston JDK 21:coreasync-query-corepplapiCalciteRelNodeVisitorgoes from 11 to 2,271 of 2,622 lines, andJoinAndLookupUtilsfrom 14 to 55 of 55. Branch coverage goes from 55.2% to 67.2%.What does not change:
jacocoTestReportreports and the per-modulejacocoTestCoverageVerificationrules. They still measure each module's own tests, so no threshold changes meaning.legacy,commonandopensearch-sql-pluginclasses stay out, as before.Related Issues
Resolves #5890
Check List
--signoffor-s.By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.