Add fresh code quality gates for coverage and mutation testing#656
Add fresh code quality gates for coverage and mutation testing#656andrii0lomakin wants to merge 5 commits intodevelopfrom
Conversation
Summary of ChangesHello @andrii0lomakin, 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 significantly enhances the project's code quality assurance by implementing automated and enforced quality gates within the CI pipeline. It shifts from passive coverage monitoring to active enforcement of both code coverage and mutation testing standards for new and modified code, including specific thresholds for AI-assisted contributions. The changes are complemented by clear documentation to guide developers on these new requirements and processes. Highlights
Changelog
Ignored Files
Activity
Using Gemini Code AssistThe 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 by creating a comment using either
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 Limitations & Feedback Gemini Code Assist 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. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. 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
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces significant improvements to the project's quality gates by adding automated checks for code coverage and mutation testing. The changes are well-structured, including new CI jobs, updates to the build configuration, and comprehensive documentation. My review focuses on the new Python script for branch coverage checking. I've provided one suggestion to optimize its performance by improving the file matching logic. Overall, this is a valuable addition to ensure code quality.
Coverage Report
|
Qodana for JVM37 new problems were found
💡 Qodana analysis was run in the pull request mode: only the changed files were checked Contact Qodana teamContact us at qodana-support@jetbrains.com
|
.github/workflows/maven-pipeline.yml
Outdated
| - name: Run Ekstazi Selection for Integration Tests | ||
| if: steps.changes.outputs.skip != 'true' | ||
| id: ekstazi | ||
| run: | | ||
| # Run Ekstazi select on already-compiled classes to generate exclude files. | ||
| # The Ekstazi profile skips compilation (classes already exist from previous step). | ||
| # The restored cache is from the base branch (develop), so Ekstazi sees the | ||
| # PR's changes as "new" and selects only the affected integration tests. | ||
| ./mvnw -s .github/workflows/settings.xml \ | ||
| -pl ${{ steps.changes.outputs.modules }} \ | ||
| process-test-classes -P ekstazi -B || true |
There was a problem hiding this comment.
I took a closer look on Ekstazi and here are a few thoughts:
- nit: not sure if https://www.ekstazi.org/ even exists, maybe it's better to link gh repo directly https://github.com/gliga/ekstazi
- the project seems pretty niche and not actively maintained. The latest version (5.3.0) was published 7 years ago
Also (based on the docs), it looks like we need to pass a couple extra flags to make it work:
-Djava.security.manager=allowis missing everywhere (required for JDK 21+, which we use)-Djdk.attach.allowAttachSelf=trueis missing in the mutation-testing job
There are unreleased fixes on master (ASM 9.6, SecurityManager removal), but nothing published since 5.3.0. Given we have
|| true
I suspect it might be failing silently. Have you checked this?
Given all that, IMO, PIT alone (scoped to changed classes) would likely be enough, especially since we already limit mutation testing to pr-changed code. WDYT?
PS: just want to make sure this dependency is justified
There was a problem hiding this comment.
Hi, @lesley29
Could you elaborate more on how Ekstazi and PIT are connected? They there for a different purposes in this change. Without Ekstazi, we run a full set of IT tests, which will take hours, while with Ekstazi, only single tests from this suite are running.
There was a problem hiding this comment.
|| true
@lesley29 I have already used it on several tests, if integration tests are affected they triggered, and then next task
EnricoMi/publish-unit-test-result-action@v2
Reports failure.
It can fall silently, that is for sure,for example, when the Ekstazi cache is absent, but that is the reason for this safeguard.
There was a problem hiding this comment.
There was a problem hiding this comment.
-Djdk.attach.allowAttachSelf=true is missing in the mutation-testing job
It is not needed because PIT does not attach itself at runtime. It is already attached as part of the forked process. Could you elaborate more where you found this requirement in PIT plugin ?
There was a problem hiding this comment.
Hi, @lesley29 Could you elaborate more on how Ekstazi and PIT are connected? They there for a different purposes in this change. Without Ekstazi, we run a full set of IT tests, which will take hours, while with Ekstazi, only single tests from this suite are running.
I probably understood, you, PIT will mutate, only changed classes, but it will not run only tests that are related to those classes, it will run the full test suite.
There was a problem hiding this comment.
nit: not sure if https://www.ekstazi.org/ even exists,
It took time for me to find this URL, I will change it in the description of the buildflow.
As for || true let me add annotated warning there to be sure that we did not miss anything.
There was a problem hiding this comment.
Done, changed, both
There was a problem hiding this comment.
Hi, @lesley29 Could you elaborate more on how Ekstazi and PIT are connected? They there for a different purposes in this change. Without Ekstazi, we run a full set of IT tests, which will take hours, while with Ekstazi, only single tests from this suite are running.
I probably understood, you, PIT will mutate, only changed classes, but it will not run only tests that are related to those classes, it will run the full test suite.
Got it, thanks for clarifying. I was confused a bit with test selection and class scoping for mutation indeed, thanks!
- Remove Qodana code coverage tracking, keep JaCoCo data collection - Rename coverage output directory from .qodana/code-coverage to .coverage/reports - Add coverage-gate CI job with diff-cover (line) and custom script (branch) - 85% threshold for Claude Code co-authored commits, 70% otherwise - Add PIT mutation testing quality gate (85% kill rate) for changed code only - Ekstazi integration for IT test selection using base branch cache - Add test coverage requirements guideline to CLAUDE.md - Create docs/ with TOC and test quality requirements documentation Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Move ci-cd-diagram.md and testflows-runner-setup.md from .github/workflows/ to docs/ - Update CI/CD diagram with new quality gate jobs (coverage-gate, mutation-testing) - Update Qodana description (static analysis only, no coverage tracking) - Update test matrix description (temurin, oracle on self-hosted Hetzner runners) - Add PR Quality Gates summary table - Fix cross-document links for new directory structure - Update docs/README.md TOC with all three documents Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Pre-process changed files into a basename-keyed lookup map before matching against JaCoCo XML source file entries. Previously, for every <sourcefile> element in every JaCoCo report, the code iterated over all changed files to find a path suffix match, resulting in O(xml_files * packages * sourcefiles * changed_files) complexity. The new approach builds a dictionary mapping each file's basename (e.g. "Foo.java") to its full path(s) upfront. When processing a JaCoCo <sourcefile>, we perform an O(1) dict lookup by basename, then only check the (typically very short) list of same-named files for a full path suffix match. This reduces the inner loop from scanning all changed files to scanning only files sharing the same basename, which is a significant improvement for larger projects with many modules or many changed files. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Replace silent `|| true` with an explicit GitHub Actions warning annotation when Ekstazi selection fails. This makes it visible in the Actions UI that PIT is falling back to running against all integration tests. Also fix Ekstazi URL in documentation. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
andrii0lomakin
force-pushed
the
fresh-code-gate
branch
from
5837bc1 to
d89a4f6
Compare
2 participants
Motivation:
The project relied on Qodana for code coverage tracking, which only provided visibility but no enforcement. This PR introduces automated quality gates that ensure new code meets coverage and mutation testing standards before merging, with higher thresholds for AI-assisted code. It also consolidates all CI/CD documentation into a dedicated
docs/directory.Changes:
1. Qodana coverage tracking removed
CoverageReportexclusion from Qodana job.qodana/code-coverage/to.coverage/reports/)2. Code coverage gate (new CI job:
coverage-gate)diff-coveron new/changed lines only.github/scripts/check-branch-coverage.py) that parses JaCoCo XML line-level branch data3. Mutation testing gate (new CI job:
mutation-testing)mutation-testingMaven profilegit diff)4. CLAUDE.md guideline
5. Documentation consolidated in
docs/ci-cd-diagram.mdandtestflows-runner-setup.mdfrom.github/workflows/todocs/docs/README.mdas TOC linking all documentationdocs/test-quality-requirements.mdcovering coverage thresholds, mutation testing, test writing guidelines, and CI enforcement summary🤖 Generated with Claude Code