Tianpok/add code coverage#48
Conversation
📝 WalkthroughWalkthroughThe pull request integrates JaCoCo code coverage reporting into the build pipeline. The Maven configuration adds JaCoCo plugin support, the CI workflow updates to run full verification and upload coverage reports as artifacts, and the README is restructured with badges, updated run instructions, and coverage documentation. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (3)
README.md (1)
12-13: Avoid hard-coded coverage numbers in docs/badges.Line 12 and Line 121 duplicate a fixed
53.9%value, which will go stale as tests evolve. Prefer run-generated reporting references over static percentages.Suggested README update
- <img alt="Line coverage" src="https://img.shields.io/badge/line%20coverage-53.9%25-yellow"> + <img alt="Line coverage" src="https://img.shields.io/badge/line%20coverage-see%20CI%20artifact-blue"> -Current JaCoCo line coverage from the latest local verification run is **53.9%** (`1,714 / 3,178` lines). +Current JaCoCo coverage is published per run in the `jacoco-coverage-report` CI artifact.Also applies to: 121-121
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@README.md` around lines 12 - 13, The README contains a hard-coded line coverage badge using the img tag with alt "Line coverage" and src set to a fixed 53.9% value; replace that static badge by pointing the src to a dynamic coverage badge (e.g., your CI/coverage provider or a shields.io dynamic endpoint) or remove the percentage so the badge is generated from real run-time coverage data; update the img tag's src attribute and keep the alt text ("Line coverage") so the README displays the live coverage badge instead of the fixed 53.9% string..github/workflows/unit-tests.yml (1)
26-31: Addif: always()to the coverage upload step to capture artifacts even when tests fail.The upload step at lines 26–31 currently skips execution when the
mvn verifycommand fails. Addingif: always()ensures coverage reports are available for debugging even on failed test runs.Suggested change
- name: Upload coverage report + if: always() uses: actions/upload-artifact@v4 with: name: jacoco-coverage-report path: target/site/jacoco if-no-files-found: error🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/unit-tests.yml around lines 26 - 31, The "Upload coverage report" step (the step named "Upload coverage report" that uses actions/upload-artifact@v4) currently runs only on success; update that step to include the GitHub Actions conditional "if: always()" so the artifact upload runs even when previous steps (like mvn verify) fail, ensuring the jacoco-coverage-report at path target/site/jacoco is captured for debugging.pom.xml (1)
45-64: Add a JaCoCocheckexecution to enforce coverage gates in CI.The current configuration (lines 46–64) only generates coverage reports via the
prepare-agentandreportexecutions. Coverage regressions won't fail the build. Add acheckexecution with rules to enforce a minimum coverage threshold.Suggested POM change
<properties> <maven.compiler.release>17</maven.compiler.release> <project.build.sourceEncoding>UTF-8</project.build.sourceEncoding> <junit.version>5.12.2</junit.version> <jacoco.version>0.8.14</jacoco.version> + <jacoco.minimum.line.coverage>0.50</jacoco.minimum.line.coverage> </properties> ... <execution> <id>report</id> <phase>verify</phase> <goals> <goal>report</goal> </goals> </execution> +<execution> + <id>check</id> + <phase>verify</phase> + <goals> + <goal>check</goal> + </goals> + <configuration> + <rules> + <rule> + <element>BUNDLE</element> + <limits> + <limit> + <counter>LINE</counter> + <value>COVEREDRATIO</value> + <minimum>${jacoco.minimum.line.coverage}</minimum> + </limit> + </limits> + </rule> + </rules> + </configuration> +</execution>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pom.xml` around lines 45 - 64, The JaCoCo plugin configuration currently only has executions for prepare-agent and report, so coverage regressions won't fail the build; add a new execution with <id>check</id> and <goals><goal>check</goal></goals> to the existing org.jacoco:jacoco-maven-plugin block, and configure <configuration><rules><rule>...</rule></rules></configuration> to enforce minimum thresholds (e.g., set limits for instruction/branch/line/complexity as needed) so the Maven build will fail in CI when coverage falls below the defined gates.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In @.github/workflows/unit-tests.yml:
- Around line 26-31: The "Upload coverage report" step (the step named "Upload
coverage report" that uses actions/upload-artifact@v4) currently runs only on
success; update that step to include the GitHub Actions conditional "if:
always()" so the artifact upload runs even when previous steps (like mvn verify)
fail, ensuring the jacoco-coverage-report at path target/site/jacoco is captured
for debugging.
In `@pom.xml`:
- Around line 45-64: The JaCoCo plugin configuration currently only has
executions for prepare-agent and report, so coverage regressions won't fail the
build; add a new execution with <id>check</id> and
<goals><goal>check</goal></goals> to the existing org.jacoco:jacoco-maven-plugin
block, and configure
<configuration><rules><rule>...</rule></rules></configuration> to enforce
minimum thresholds (e.g., set limits for instruction/branch/line/complexity as
needed) so the Maven build will fail in CI when coverage falls below the defined
gates.
In `@README.md`:
- Around line 12-13: The README contains a hard-coded line coverage badge using
the img tag with alt "Line coverage" and src set to a fixed 53.9% value; replace
that static badge by pointing the src to a dynamic coverage badge (e.g., your
CI/coverage provider or a shields.io dynamic endpoint) or remove the percentage
so the badge is generated from real run-time coverage data; update the img tag's
src attribute and keep the alt text ("Line coverage") so the README displays the
live coverage badge instead of the fixed 53.9% string.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 2a810437-bbbc-4b6c-9191-3b9d8b373f26
📒 Files selected for processing (3)
.github/workflows/unit-tests.ymlREADME.mdpom.xml
Summary by CodeRabbit
Documentation
Chores