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.

Sign up for GitHub

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

Jump to bottom

Add unit test coverage report and enforcement #529

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

Add unit test coverage report and enforcement #529

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
92 changes: 90 additions & 2 deletions build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ buildscript {

plugins {
id "com.diffplug.spotless" version "5.9.0"
id "jacoco"
}

apply from: "gradle/shipkit.gradle"
Expand All @@ -30,6 +31,7 @@ configurations {
allprojects {
group = "com.linkedin.coral"
apply plugin: "com.diffplug.spotless"
apply plugin: "jacoco"

repositories {
mavenCentral()
Expand All @@ -52,20 +54,82 @@ allprojects {
target '**/*.md'
targetExclude 'docs/release-notes.md'
endWithNewline()
// Disabling Prettier since it causes TravisCI to barf
// prettier()
}
}

jacoco {
toolVersion = "0.8.7"
}
}

ext.moduleCoverageThresholds = [
// Default coverage applied unless overridden
'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,
Comment on lines +67 to +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)

// Explicit exclusions
'coral-common': 0.00,
'coral-hive': 0.00,
]

subprojects {
plugins.withType(JavaPlugin) {
dependencies {
testCompile deps.'testing'
}
test {
useTestNG()
finalizedBy jacocoTestReport
jacoco {
excludes = [
'com.linkedin.coral.hive.hive2rel.parsetree.parser.*',
// Jacoco method too large
'org.apache.hadoop.hive.ql.parse.HiveParser_IdentifiersParser',
// Jacoco method too large
'org.apache.hadoop.hive.ql.parse.HiveParser_IdentifiersParser$*' // Jacoco method too large
]
}
}
jacocoTestReport {
reports {
xml.enabled true
csv.enabled false
html.destination file("${buildDir}/jacocoHtml")
}
executionData.from = files(fileTree(dir: project.buildDir, includes: ['jacoco/*.exec']).files.findAll { it.exists() })
}

def moduleName = project.name
def threshold = moduleCoverageThresholds.containsKey(moduleName)
? moduleCoverageThresholds[moduleName]
: 0.95 // Default to 100% if not specified

jacocoTestCoverageVerification {
violationRules {
rule {
element = 'BUNDLE'
limit {
counter = 'INSTRUCTION'
value = 'COVEREDRATIO'
minimum = threshold
}
excludes = [
'com.linkedin.coral.hive.hive2rel.parsetree.parser.*',
// Jacoco method too large
'org.apache.hadoop.hive.ql.parse.HiveParser_IdentifiersParser',
// Jacoco method too large
'org.apache.hadoop.hive.ql.parse.HiveParser_IdentifiersParser$*' // Jacoco method too large
]
}
}
}
check.dependsOn jacocoTestCoverageVerification
spotless {
java {
importOrder('java', 'javax', 'com', 'org', 'com.linkedin.coral', '\\#')
Expand All @@ -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

dependsOn = subprojects.test
additionalSourceDirs.from = subprojects.sourceSets.main.allSource.srcDirs
sourceDirectories.from = subprojects.sourceSets.main.allSource.srcDirs
classDirectories.from = subprojects.sourceSets.main.output

executionData.from = files(subprojects.findAll { p ->
p.plugins.hasPlugin(JavaPlugin)
}.collect { p ->
p.file("${p.buildDir}/jacoco/test.exec")
}.findAll { file ->
file.exists()
})

reports {
html.enabled = true
xml.enabled = true
csv.enabled = false
html.destination file("${buildDir}/reports/jacoco")
}
}

check.dependsOn jacocoRootReport
Loading