fix(coverage): make JaCoCo production gate non-vacuous - #164
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughJaCoCo 설정이 실제 production class 파일을 선택하고 빈 BUNDLE을 실패시키도록 변경되었습니다. 관련 문서와 정책 테스트가 추가되었습니다. ETL 서비스 테스트는 해시 제공자, 입력 경계, Jackson 방어적 분기를 검증합니다. ChangesJaCoCo 비공허 커버리지
ETL 서비스 경계 검증
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Fresh exact-head RCA for The exact current source mismatch is deterministic: Smallest safe next remedy on this already-active branch: update the older policy test test-first relative to its new contract so it asserts the separated report/check class-file filters and non-vacuity invariant rather than the removed dotted plugin-level filter, then rerun CI. Do not weaken/remove zero-missed thresholds or delete the older policy test. Acceptance still requires the subsequent hosted log to reach JaCoCo report/check, show a nonzero analyzed production bundle, and enforce exact zero missed instruction/line/method/branch. Current workflows use PR merge revisions unless literal-source checkout controls prove otherwise, so any future green remains synthetic-only evidence unless source identity is explicitly proven. |
|
@coderabbitai review |
|
|
Fresh follow-through on unchanged exact head This materially changes the RCA: configuration non-vacuity is now proven and the current first failing boundary is genuine production-path coverage, not filter syntax or a stale test. Do not weaken the bundle selection or thresholds. The next branch-local remedy should use the generated JaCoCo report (or equivalent exact class/line evidence) to add the smallest realistic tests for those residual branches/lines, then rerun the same gate. Current CI still checks the PR synthetic merge ( |
|
Fresh exact-head follow-through for The same job then fails the strict BUNDLE gate on real uncovered production code: 7 missed instructions, 2 missed lines, and 2 missed branches (the method counter does not fail). Therefore this is a valid product-test deficit, not infrastructure, selector, or threshold noise. Dependency Review RCA classification for the next source step: (1) This loop is not mutating the source ref because |
|
@coderabbitai review |
|
|
@coderabbitai review |
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
etl-service/src/test/java/com/xtrmetl/etl/documentation/JaCoCoCoverageConfigurationTest.java (1)
52-74: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win동일한 BUNDLE 규칙에서 zero-missed 제한을 구조적으로 검증하십시오.
현재 테스트는
CLASS TOTALCOUNT >= 1만BUNDLE규칙의<limits>아래에서 확인합니다.INSTRUCTION,LINE,METHOD,BRANCH의MISSEDCOUNT <= 0제한은 이 규칙과 연결하지 않습니다. 제한이 다른 실행 또는 다른 규칙으로 이동하면 현재 정책 테스트는 통과할 수 있습니다.수정 예시
assertTrue( foundNonEmptyGuard, "Coverage must fail closed when the selected production bundle contains zero classes" ); + for (String counter : Set.of("INSTRUCTION", "LINE", "METHOD", "BRANCH")) { + assertTrue( + directChildren(limits, "limit").stream().anyMatch(limit -> + counter.equals(directChildText(limit, "counter")) + && "MISSEDCOUNT".equals(directChildText(limit, "value")) + && "0".equals(directChildText(limit, "maximum")) + ), + "Coverage must require zero missed " + counter + " entries in the BUNDLE rule" + ); + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@etl-service/src/test/java/com/xtrmetl/etl/documentation/JaCoCoCoverageConfigurationTest.java` around lines 52 - 74, Update coverageCheckRequiresAtLeastOneAnalyzedProductionClass so it structurally verifies the zero-missed limits for INSTRUCTION, LINE, METHOD, and BRANCH within the same BUNDLE rule and its limits collection, alongside the existing CLASS TOTALCOUNT minimum check. Assert each expected counter has MISSEDCOUNT with a maximum of 0, ensuring all required limits are attached to this rule.docs/doctoring/jacoco-nonvacuous-coverage.md (1)
25-34: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value문서에 참조 검증 결과를 추가하십시오.
etl-service/pom.xml이 존재하며, 나열된 JaCoCo URL 7개가 모두 접근 가능합니다.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/doctoring/jacoco-nonvacuous-coverage.md` around lines 25 - 34, Update the documentation section covering etl-service/pom.xml and the JaCoCo references to add verification results: confirm that etl-service/pom.xml exists and that all seven listed JaCoCo URLs are accessible.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@docs/doctoring/jacoco-nonvacuous-coverage.md`:
- Around line 25-34: Update the documentation section covering
etl-service/pom.xml and the JaCoCo references to add verification results:
confirm that etl-service/pom.xml exists and that all seven listed JaCoCo URLs
are accessible.
In
`@etl-service/src/test/java/com/xtrmetl/etl/documentation/JaCoCoCoverageConfigurationTest.java`:
- Around line 52-74: Update
coverageCheckRequiresAtLeastOneAnalyzedProductionClass so it structurally
verifies the zero-missed limits for INSTRUCTION, LINE, METHOD, and BRANCH within
the same BUNDLE rule and its limits collection, alongside the existing CLASS
TOTALCOUNT minimum check. Assert each expected counter has MISSEDCOUNT with a
maximum of 0, ensuring all required limits are attached to this rule.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 8496c5f7-8197-41da-9819-e7fe3e364992
📒 Files selected for processing (7)
CHANGELOG.mddocs/doctoring/jacoco-nonvacuous-coverage.mdetl-service/pom.xmletl-service/src/test/java/com/xtrmetl/etl/documentation/JaCoCoCoverageConfigurationTest.javaetl-service/src/test/java/com/xtrmetl/etl/job/EtlJobCoveragePolicyTest.javaetl-service/src/test/java/com/xtrmetl/etl/job/EtlJobServiceBoundaryTest.javaetl-service/src/test/java/com/xtrmetl/etl/job/EtlJobServiceDefensiveBranchTest.java
opencode-agent
Bot
added
area: ci-cd
seonghobae
added
maintenance
type: test
ChatGPT Codex Connector
1 participant
Purpose
Fix issue #162: protected mightyETL's durable-job JaCoCo gate can analyze zero production classes and still report every zero-missed threshold as satisfied. This direct-
developslice makes production class selection explicit, adds a non-empty bundle invariant, preserves the zero-missed thresholds, and adds the tests needed to close the real deficits exposed once coverage stopped being vacuous.Exact current identity
develop@622e5e6c3d534f230c390f10e3832efadfc01825;fix/nonvacuous-jacoco-coverage;ba174ac98128da254358a5f8dccdfcf8eee93496;2838fbc0735cc5d5d70b254f2eaa793dc995dcd2;2436723db8fdf898be939fed7b406e8d6133f7e9;etl-service/pom.xml, four coverage/behavior tests, coverage doctoring, andCHANGELOG.md.Every check/review from a predecessor head or base is historical and does not transfer after movement.
RCA
Protected
etl-service/pom.xmlreused dotted class-name patterns at plugin level:JaCoCo agent instrumentation accepts class-name patterns, while Maven report/check apply file filters over compiled class files. Reusing one dotted selector across incompatible goal semantics left report/check with an empty production bundle. A second control failure made that dangerous: no rule required the analyzed bundle to contain even one class, so
MISSEDCOUNT <= 0was vacuously satisfied.Protected/direct-
developintegration evidence repeatedly reproduced the defect as:That is a quality-control false green, not 100% coverage.
TDD / remediation history
RED — configuration boundary
Fail-first head
ba174ac98128da254358a5f8dccdfcf8eee93496addedJaCoCoCoverageConfigurationTestbefore changing the POM. The test requires:com/xtrmetl/etl/job/*.classandcom/xtrmetl/etl/controller/EtlJobController*.class;CLASS / TOTALCOUNT / minimum 1non-vacuity rule;The protected POM violated the first three while the target classes were real, establishing fail-first evidence at the actual gate configuration rather than an absent fixture.
GREEN configuration exposes real coverage work
The selected production change:
CLASS / TOTALCOUNT >= 1so an empty selection fails closed.Once the eight real production classes became visible to JaCoCo, missing branches were treated as genuine test deficits rather than weakening the selector or thresholds. Subsequent bounded test commits cover the defensive and policy branches required by the existing production contract. The branch also contains
docs/doctoring/jacoco-nonvacuous-coverage.mdand aCHANGELOG.mdentry documenting evidence semantics and rollback.Current hosted proof
All exposed workflows associated with exact source head
2838fbc0735cc5d5d70b254f2eaa793dc995dcd2are terminal-success:31342365406: success;31342365405: success;31342365408: success;31342365420: success;31342365407: success.CI macOS checked out GitHub synthetic merge
2436723db8fdf898be939fed7b406e8d6133f7e9(Merge 2838fbc... into 622e5e6...). On that integration tree:The full reactor completed successfully; the ETL module ran 281 tests, CDC ran 106 tests, and gateway tests also passed. The selected eight production classes therefore satisfy the unchanged zero-missed instruction/line/method/branch rules while the explicit non-empty bundle rule proves the gate is no longer vacuous.
This is strong causal A/B evidence at the coverage boundary: the protected selector produces zero analyzed classes, while this candidate analyzes eight real classes under the same product codebase and zero-missed acceptance rules. It is still synthetic-merge compatibility/coverage evidence, not accepted literal-source evidence, because the current protected PR workflow checked out
refs/pull/164/mergerather than literal source head.Evidence / review boundary
Current formal review submissions are empty and current unresolved inline review threads are zero. No status, check, comment, model output, or synthetic merge is treated as qualifying independent approval.
The remaining merge blockers are therefore control/evidence boundaries rather than an unaddressed coverage defect:
Do not weaken the class selection, remove the non-empty rule, lower thresholds, add exclusions, or claim the current protected zero-class gate is acceptable merely because other PRs are green.
Acceptance
Keep Draft until the unchanged exact source head has accepted source-identity evidence, every applicable current quality/security/dependency/SBOM gate remains green, zero valid unresolved review findings, and qualifying independent approval where required. After protected integration, rerun protected-develop acceptance and close #162 only when the integrated quality gate itself proves a non-empty production class set before reporting 100% coverage.
Summary by CodeRabbit
버그 수정
품질 개선