Skip to content

Commit

Permalink
Fixed adding data to an already existing binary report file
Browse files Browse the repository at this point in the history
If the build cache is not used and the build directory is not cleared before the test run, then the coverage data from previous test runs are merged with the data from the recent run.
In this case, the report may include coverage from those tests that were removed in the latest version of the code.

To solve this, it is necessary to delete the binary report file before each run of the test task.

Fixes #489
PR #490
  • Loading branch information
shanshin authored Oct 13, 2023
1 parent 2547f52 commit ec94ad9
Show file tree
Hide file tree
Showing 8 changed files with 130 additions and 24 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
*/
package kotlinx.kover.gradle.plugin.test.functional.cases

import kotlinx.kover.gradle.plugin.commons.CoverageToolVendor
import kotlinx.kover.gradle.plugin.test.functional.framework.configurator.*
import kotlinx.kover.gradle.plugin.test.functional.framework.starter.*

Expand All @@ -29,36 +30,39 @@ internal class ReportsCachingTests {
addProjectWithKover {
sourcesFrom("simple")
}
reportAndCheck("SUCCESS", true)
reportAndCheck("SUCCESS", cached = true)

run("clean", "--build-cache") {
checkDefaultBinReport(false)
checkDefaultReports(false)
}
reportAndCheck("FROM-CACHE", true)
reportAndCheck("FROM-CACHE", cached = true)
}

@SlicedGeneratedTest(allTools = true)
fun BuildConfigurator.testOuOfDateOnSources() {
fun BuildConfigurator.testOutOfDateOnSources() {
useLocalCache()

addProjectWithKover {
sourcesFrom("simple")
}
reportAndCheck("SUCCESS", true)
reportAndCheck("SUCCESS", cached = true)

edit("src/main/kotlin/Sources.kt") {
"$it\n class Additional"
}
// tasks must be restarted after the source code is edited
reportAndCheck("SUCCESS", true)
reportAndCheck("SUCCESS", cached = true)

edit("src/test/kotlin/TestClass.kt") {
"$it\n class AdditionalTest"
}

// tasks must be restarted after tests are edited
reportAndCheck("SUCCESS", true)
// test task must be restarted after test class is edited
// , but reports can be up-to-date because no sources changed.
// For JaCoCo .exec binary report contains instrumentation time, so it's unstable between builds and can cause the report generation.
// For Kover it is also not guaranteed to have a stable binary .ic file, so sometimes reports can be regenerated.
reportAndCheck("SUCCESS", "UP-TO-DATE",true)
}

@SlicedGeneratedTest(allTools = true)
Expand All @@ -68,27 +72,31 @@ internal class ReportsCachingTests {
addProjectWithKover {
sourcesFrom("simple")
}
reportAndCheck("SUCCESS", true)
reportAndCheck("SUCCESS", cached = true)
run("clean", "--build-cache") {
checkDefaultBinReport(false)
checkDefaultReports(false)
}
reportAndCheck("FROM-CACHE", true)
reportAndCheck("FROM-CACHE", cached = true)
}


private fun BuildConfigurator.reportAndCheck(outcome: String, cached: Boolean = false) {
private fun BuildConfigurator.reportAndCheck(
outcome: String,
reportsAlternativeOutcome: String = outcome,
cached: Boolean = false
) {
val args = if (cached) {
arrayOf("koverXmlReport", "koverHtmlReport", "--build-cache")
arrayOf("koverXmlReport", "koverHtmlReport", "--build-cache", "--info")
} else {
arrayOf("koverXmlReport", "koverHtmlReport")
arrayOf("koverXmlReport", "koverHtmlReport", "--info")
}
run(*args) {
checkDefaultBinReport()
checkDefaultReports()
checkOutcome("test", outcome)
checkOutcome("koverXmlReport", outcome)
checkOutcome("koverHtmlReport", outcome)
checkOutcome("koverXmlReport", outcome, reportsAlternativeOutcome)
checkOutcome("koverHtmlReport", outcome, reportsAlternativeOutcome)
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
/*
* Copyright 2017-2023 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license.
*/
package kotlinx.kover.gradle.plugin.test.functional.cases

import kotlinx.kover.gradle.plugin.test.functional.framework.configurator.BuildConfigurator
import kotlinx.kover.gradle.plugin.test.functional.framework.starter.SlicedGeneratedTest

internal class ReportsUpToDateTests {

@SlicedGeneratedTest(allTools = true)
fun BuildConfigurator.testDeleteTest() {
addProjectWithKover {
sourcesFrom("simple")
}

add("src/test/kotlin/ExtraTestClass.kt") {
"""
package org.jetbrains.serialuser
import org.jetbrains.Unused
import kotlin.test.Test
class AdditionalTest {
@Test
fun extra() {
Unused().functionInUsedClass()
}
}
""".trimMargin()
}
run("koverXmlReport") {
xmlReport {
classCounter("org.jetbrains.Unused").assertCovered()
}
}

// report should be regenerated if test are deleted
delete("src/test/kotlin/ExtraTestClass.kt")
run("koverXmlReport") {
xmlReport {
classCounter("org.jetbrains.Unused").assertFullyMissed()
}
}
}

}

Original file line number Diff line number Diff line change
Expand Up @@ -135,11 +135,11 @@ private class CheckerContextImpl(
checkHtmlReport(mustExist = mustExist)
}

override fun checkOutcome(taskNameOrPath: String, expectedOutcome: String) {
override fun checkOutcome(taskNameOrPath: String, vararg expectedOutcome: String) {
val taskPath = taskNameOrPath.asPath()
val outcome = result.taskOutcome(taskPath) ?: noTaskFound(taskNameOrPath, taskPath)

assertEquals(expectedOutcome, outcome, "Unexpected outcome for task '$taskPath'")
assertContains(expectedOutcome.toSet(), outcome, "Unexpected outcome for task '$taskPath'")
}

override fun taskNotCalled(taskNameOrPath: String) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ internal interface CheckerContext {

fun checkXmlReport(variantName: String = "", mustExist: Boolean = true)
fun checkHtmlReport(variantName: String = "", mustExist: Boolean = true)
fun checkOutcome(taskNameOrPath: String, expectedOutcome: String)
fun checkOutcome(taskNameOrPath: String, vararg expectedOutcome: String)
fun taskNotCalled(taskNameOrPath: String)
fun checkDefaultReports(mustExist: Boolean = true)
fun checkDefaultBinReport(mustExist: Boolean = true)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,18 @@ internal data class TestFileEditStep(
val filePath: String,
val editor: (String) -> String
): TestExecutionStep() {
override val name: String = "Edit: $filePath"
override val name: String = "Edit file: $filePath"
}

internal data class TestFileAddStep(
val filePath: String,
val editor: () -> String
): TestExecutionStep() {
override val name: String = "Add file: $filePath"
}

internal data class TestFileDeleteStep(val filePath: String): TestExecutionStep() {
override val name: String = "Delete file: $filePath"
}

private open class TestBuildConfigurator : BuildConfigurator {
Expand Down Expand Up @@ -80,6 +91,14 @@ private open class TestBuildConfigurator : BuildConfigurator {
steps += TestFileEditStep(filePath, editor)
}

override fun add(filePath: String, editor: () -> String) {
steps += TestFileAddStep(filePath, editor)
}

override fun delete(filePath: String) {
steps += TestFileDeleteStep(filePath)
}

override fun useLocalCache(use: Boolean) {
useCache = use
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,10 @@ internal interface BuildConfigurator {

fun edit(filePath: String, editor: (String) -> String)

fun add(filePath: String, editor: () -> String)

fun delete(filePath: String)

fun useLocalCache(use: Boolean = true)

fun prepare(): TestBuildConfig
Expand Down Expand Up @@ -72,6 +76,14 @@ internal abstract class BuilderConfiguratorWrapper(private val origin: BuildConf
origin.edit(filePath, editor)
}

override fun add(filePath: String, editor: () -> String) {
origin.add(filePath, editor)
}

override fun delete(filePath: String) {
origin.delete(filePath)
}

override fun useLocalCache(use: Boolean) {
origin.useLocalCache(use)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,11 +30,7 @@ ${this.targetDir.buildScript()}
}

is TestFileEditStep -> {
if (File(step.filePath).isAbsolute) {
throw Exception("It is not allowed to edit a file by an absolute path. For $description")
}

val file = this.targetDir.resolve(step.filePath)
val file = projectFile(step.filePath, description)
if (!file.exists()) {
throw Exception("Project file not found for editing. For $description")
}
Expand All @@ -43,12 +39,29 @@ ${this.targetDir.buildScript()}
val newContent = step.editor(content)
file.writeText(newContent)
}
}

is TestFileAddStep -> {
val file = projectFile(step.filePath, description)

file.writeText(step.editor())
}

is TestFileDeleteStep -> {
val file = projectFile(step.filePath, description)
file.delete()
}
}
}
}

private fun GradleBuild.projectFile(path: String, description: String): File {
if (File(path).isAbsolute) {
throw Exception("It is not allowed to edit a file by an absolute path. For $description")
}

return targetDir.resolve(path)
}

private fun File.buildScript(): String {
var file = this.resolve("build.gradle")
if (file.exists() && file.isFile) return file.readText()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,12 @@ internal class JvmTestTaskConfigurator(
}
testTask.dependsOn(data.findAgentJarTask)

testTask.doFirst {
// delete report so that when the data is re-measured, it is not appended to an already existing file
// see https://github.com/Kotlin/kotlinx-kover/issues/489
binReportProvider.get().asFile.delete()
}

// Always excludes android classes, see https://github.com/Kotlin/kotlinx-kover/issues/89
val excluded = data.excludedClasses + listOf("android.*", "com.android.*")

Expand Down

0 comments on commit ec94ad9

Please sign in to comment.