Always update the baseline if it exist (#4445)

* reduce visibility * Add missing test * Don't fail if the baseline doesn't exist * Fix flaky test * Improve tests * Always update the baseline file if it exists already * Only write the file if it is different than the current one
detekt · Jan 5, 2022 · 4f6793f · 4f6793f
1 parent c94a5d9
commit 4f6793f
Show file tree

Hide file tree

Showing 4 changed files with 86 additions and 46 deletions.
diff --git a/detekt-core/src/main/kotlin/io/gitlab/arturbosch/detekt/core/baseline/BaselineFacade.kt b/detekt-core/src/main/kotlin/io/gitlab/arturbosch/detekt/core/baseline/BaselineFacade.kt
@@ -9,19 +9,27 @@ import java.nio.file.Path
 
 class BaselineFacade {
 
-    fun transformResult(baselineFile: Path, result: Detektion): Detektion =
-        BaselineFilteredResult(result, Baseline.load(baselineFile))
+    fun transformResult(baselineFile: Path, result: Detektion): Detektion {
+        return if (baselineExists(baselineFile)) {
+            BaselineFilteredResult(result, Baseline.load(baselineFile))
+        } else {
+            result
+        }
+    }
 
     fun createOrUpdate(baselineFile: Path, findings: List<Finding>) {
         val ids = findings.map { it.baselineId }.toSortedSet()
-        val baseline = if (baselineExists(baselineFile)) {
-            Baseline.load(baselineFile).copy(currentIssues = ids)
+        val oldBaseline = if (baselineExists(baselineFile)) {
+            Baseline.load(baselineFile)
         } else {
-            Baseline(emptySet(), ids)
+            Baseline(emptySet(), emptySet())
+        }
+        val baseline = oldBaseline.copy(currentIssues = ids)
+        if (oldBaseline != baseline) {
+            baselineFile.parent?.let { Files.createDirectories(it) }
+            BaselineFormat().write(baseline, baselineFile)
         }
-        baselineFile.parent?.let { Files.createDirectories(it) }
-        BaselineFormat().write(baseline, baselineFile)
     }
 
-    internal fun baselineExists(baseline: Path) = baseline.exists() && baseline.isFile()
+    private fun baselineExists(baseline: Path) = baseline.exists() && baseline.isFile()
 }
diff --git a/...t-core/src/main/kotlin/io/gitlab/arturbosch/detekt/core/baseline/BaselineResultMapping.kt b/...t-core/src/main/kotlin/io/gitlab/arturbosch/detekt/core/baseline/BaselineResultMapping.kt
@@ -33,10 +33,6 @@ class BaselineResultMapping : ReportingExtension {
         val facade = BaselineFacade()
         val flatten = this.flatMap { it.value }
 
-        if (flatten.isEmpty()) {
-            return this
-        }
-
         if (createBaseline) {
             facade.createOrUpdate(baselinePath, flatten)
         }

diff --git a/detekt-core/src/test/kotlin/io/gitlab/arturbosch/detekt/core/baseline/BaselineFacadeSpec.kt b/detekt-core/src/test/kotlin/io/gitlab/arturbosch/detekt/core/baseline/BaselineFacadeSpec.kt
@@ -2,37 +2,81 @@ package io.gitlab.arturbosch.detekt.core.baseline
 
 import io.github.detekt.test.utils.createTempDirectoryForTest
 import io.github.detekt.test.utils.resourceAsPath
+import io.gitlab.arturbosch.detekt.test.TestDetektion
+import io.gitlab.arturbosch.detekt.test.createFinding
 import org.assertj.core.api.Assertions.assertThat
 import org.spekframework.spek2.Spek
 import org.spekframework.spek2.style.specification.describe
 import java.nio.file.Files
-import java.nio.file.Path
-import java.nio.file.StandardCopyOption
 
 class BaselineFacadeSpec : Spek({
 
     describe("a baseline facade") {
 
-        val dir by memoized { createTempDirectoryForTest("baseline_format") }
+        val baselineFile by memoized { createTempDirectoryForTest("baseline_format").resolve("baseline.xml") }
         val validBaseline = resourceAsPath("/baseline_feature/valid-baseline.xml")
 
-        it("creates a baseline file") {
-            val fullPath = dir.resolve("baseline.xml")
-            assertNonEmptyBaseline(fullPath)
+        afterEachTest {
+            Files.deleteIfExists(baselineFile)
         }
 
-        it("creates on top of an existing a baseline file") {
-            val fullPath = dir.resolve("baseline2.xml")
+        it("returns a BaselineFilteredResult when the baseline exists") {
+            val detektion = BaselineFacade().transformResult(validBaseline, TestDetektion())
 
-            Files.copy(validBaseline, fullPath, StandardCopyOption.REPLACE_EXISTING)
+            assertThat(detektion).isInstanceOf(BaselineFilteredResult::class.java)
+        }
+
+        it("returns the same detektion when the baseline doesn't exist") {
+            val initialDetektion = TestDetektion()
+            val detektion = BaselineFacade().transformResult(baselineFile, initialDetektion)
+
+            assertThat(detektion).isEqualTo(initialDetektion)
+        }
+
+        it("doesn't create a baseline file without findings") {
+            BaselineFacade().createOrUpdate(baselineFile, emptyList())
+
+            assertThat(baselineFile).doesNotExist()
+        }
 
-            assertNonEmptyBaseline(fullPath)
+        it("creates on top of an existing a baseline file without findings") {
+            Files.copy(validBaseline, baselineFile)
+
+            BaselineFacade().createOrUpdate(baselineFile, emptyList())
+
+            assertThat(baselineFile).hasContent(
+                """
+                <?xml version="1.0" ?>
+                <SmellBaseline>
+                  <ManuallySuppressedIssues>
+                    <ID>LongParameterList:Signature</ID>
+                    <ID>LongMethod:Signature</ID>
+                  </ManuallySuppressedIssues>
+                  <CurrentIssues></CurrentIssues>
+                </SmellBaseline>
+                """.trimIndent()
+            )
+        }
+
+        it("creates on top of an existing a baseline file with findings") {
+            Files.copy(validBaseline, baselineFile)
+
+            BaselineFacade().createOrUpdate(baselineFile, listOf(createFinding()))
+
+            assertThat(baselineFile).hasContent(
+                """
+                <?xml version="1.0" ?>
+                <SmellBaseline>
+                  <ManuallySuppressedIssues>
+                    <ID>LongParameterList:Signature</ID>
+                    <ID>LongMethod:Signature</ID>
+                  </ManuallySuppressedIssues>
+                  <CurrentIssues>
+                    <ID>TestSmell:TestEntitySignature</ID>
+                  </CurrentIssues>
+                </SmellBaseline>
+                """.trimIndent()
+            )
         }
     }
 })
-
-fun assertNonEmptyBaseline(fullPath: Path) {
-    BaselineFacade().createOrUpdate(fullPath, emptyList())
-    val lines = Files.readAllLines(fullPath)
-    assertThat(lines).isNotEmpty
-}
diff --git a/...re/src/test/kotlin/io/gitlab/arturbosch/detekt/core/baseline/BaselineResultMappingSpec.kt b/...re/src/test/kotlin/io/gitlab/arturbosch/detekt/core/baseline/BaselineResultMappingSpec.kt
@@ -15,6 +15,7 @@ import org.spekframework.spek2.Spek
 import org.spekframework.spek2.style.specification.describe
 import java.io.PrintStream
 import java.net.URI
+import java.nio.file.Files
 import java.nio.file.Path
 
 @OptIn(UnstableApi::class)
@@ -33,6 +34,10 @@ class BaselineResultMappingSpec : Spek({
         }
         val findings by memoized { mapOf("RuleSet" to listOf(finding)) }
 
+        afterEachTest {
+            Files.deleteIfExists(baselineFile)
+        }
+
         it("should not create a new baseline file when no findings occurred") {
             val mapping = resultMapping(
                 baselineFile = baselineFile,
@@ -44,19 +49,6 @@ class BaselineResultMappingSpec : Spek({
             assertThat(baselineFile.exists()).isFalse()
         }
 
-        it("should not update an existing baseline file when no findings occurred") {
-            val existing = Baseline.load(existingBaselineFile)
-            val mapping = resultMapping(
-                baselineFile = existingBaselineFile,
-                createBaseline = true,
-            )
-
-            mapping.transformFindings(emptyMap())
-
-            val changed = Baseline.load(existingBaselineFile)
-            assertThat(existing).isEqualTo(changed)
-        }
-
         it("should not update an existing baseline file if option configured as false") {
             val existing = Baseline.load(existingBaselineFile)
             val mapping = resultMapping(
@@ -106,23 +98,23 @@ class BaselineResultMappingSpec : Spek({
         }
 
         it("should update an existing baseline file if a file is configured") {
-            val existing = Baseline.load(existingBaselineFile)
+            Files.copy(existingBaselineFile, baselineFile)
+            val existing = Baseline.load(baselineFile)
             val mapping = resultMapping(
-
-                baselineFile = existingBaselineFile,
+                baselineFile = baselineFile,
                 createBaseline = true,
             )
 
             mapping.transformFindings(findings)
 
-            val changed = Baseline.load(existingBaselineFile)
+            val changed = Baseline.load(baselineFile)
             assertThat(existing).isNotEqualTo(changed)
         }
     }
 })
 
 @OptIn(UnstableApi::class)
-internal fun resultMapping(baselineFile: Path?, createBaseline: Boolean?) =
+private fun resultMapping(baselineFile: Path?, createBaseline: Boolean?) =
     BaselineResultMapping().apply {
         init(object : SetupContext {
             override val configUris: Collection<URI> = mockk()