Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add unit test coverage report and enforcement #529

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

1fanwang
Copy link
Collaborator

@1fanwang 1fanwang commented Sep 14, 2024

Closes #528

What changes are proposed in this pull request, and why are they necessary?

  • Jacoco Test Coverage Report for Java modules
  • Unit Test Coverage percentage enforcement for Java modules

How was this patch tested?

Build and Test locally

BUILD SUCCESSFUL in 3m 19s
660 actionable tasks: 186 executed, 474 up-to-date
Screenshot 2024-09-17 at 1 41 19 PM [reports.zip](https://github.com/user-attachments/files/17034785/reports.zip)

@1fanwang 1fanwang added the enhancement New feature or request label Sep 14, 2024
@1fanwang 1fanwang force-pushed the 1fanwang/testCoverageReportAndEnforcement branch 3 times, most recently from 5896141 to 4f63ee4 Compare September 16, 2024 23:35
@1fanwang 1fanwang force-pushed the 1fanwang/testCoverageReportAndEnforcement branch from 4f63ee4 to d064108 Compare September 17, 2024 20:41
@1fanwang 1fanwang marked this pull request as ready for review September 17, 2024 20:44
@1fanwang 1fanwang self-assigned this Sep 17, 2024
Comment on lines +67 to +75
'coral-dbt': 0.95,
'coral-incremental': 0.95,
'coral-pig': 0.85,
'coral-schema': 0.80,
'coral-service': 0.95,
'coral-spark': 0.90,
'coral-spark-plan': 0.74,
'coral-trino': 0.80,
'coral-visualization': 0.75,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm assuming these thresholds are more or less just arbitrary?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I came down to these values based on their existing test coverage in the codebase, and set the threshold a bit below the current coverage to make sure it won't cause flaky immediate build failures (this coverage not passing does fail build now, from past experience the coverage might have very slight difference depending on different execution environment)

@@ -79,3 +143,27 @@ subprojects {
apply from: "${rootDir}/gradle/dependencies.gradle"
apply from: "${rootDir}/gradle/java-publication.gradle"
}

task jacocoRootReport(type: JacocoReport) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this task run every time we build or must we explicitly call jacocoRootReport?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes this way it currently is in the PR is it's run as part of build so it will fail build if code coverage drop below the threshold, (and devs can look at the test coverage report from each build)

and jacocoRootReport can also be executed *independently

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add Test Coverage Report and Enforcement
2 participants