Improve directory comparison assertion

Significantly improve comparison between directories. Currently the failure messages are too obscure, and hard to diagnose. Example: https://ge.jetbrains.com/s/vl2jgh3ivevyi/tests/task/:dokka-integration-tests:gradle:test/details/org.jetbrains.dokka.it.gradle.AndroidProjectIT/generate%20dokka%20HTML(DokkaGradleProjectRunner)%5B1%5D?top-execution=1
Kotlin · Nov 5, 2024 · f8ffcb8 · f8ffcb8
1 parent 8dba7bb
commit f8ffcb8
Show file tree

Hide file tree

Showing 6 changed files with 193 additions and 88 deletions.
diff --git a/dokka-integration-tests/gradle/src/testExampleProjects/kotlin/ExampleProjectsTest.kt b/dokka-integration-tests/gradle/src/testExampleProjects/kotlin/ExampleProjectsTest.kt
@@ -210,8 +210,7 @@ class ExampleProjectsTest {
                     }
 
                     withClue("expect directories are the same") {
-                        dokkaOutputDir.shouldHaveSameStructureAs(expectedDataDir, skipEmptyDirs = true)
-                        dokkaOutputDir.shouldHaveSameStructureAndContentAs(expectedDataDir, skipEmptyDirs = true)
+                        dokkaOutputDir shouldBeADirectoryWithSameContentAs expectedDataDir
                     }
                 }
             }

diff --git a/dokka-runners/dokka-gradle-plugin/build.gradle.kts b/dokka-runners/dokka-gradle-plugin/build.gradle.kts
@@ -61,6 +61,8 @@ dependencies {
     testFixturesImplementation(gradleApi())
     testFixturesImplementation(gradleTestKit())
 
+    testFixturesImplementation(libs.javaDiffUtils)
+
     testFixturesCompileOnly("org.jetbrains.dokka:dokka-core:${project.version}")
     testFixturesImplementation(platform(libs.kotlinxSerialization.bom))
     testFixturesImplementation(libs.kotlinxSerialization.json)

diff --git a/...ners/dokka-gradle-plugin/src/test/kotlin/utils/ShouldBeADirectoryWithSameContentAsTest.kt b/...ners/dokka-gradle-plugin/src/test/kotlin/utils/ShouldBeADirectoryWithSameContentAsTest.kt
@@ -0,0 +1,87 @@
+/*
+ * Copyright 2014-2024 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license.
+ */
+package org.jetbrains.dokka.gradle.utils
+
+import io.kotest.assertions.shouldFail
+import io.kotest.core.spec.style.FunSpec
+import io.kotest.matchers.throwable.shouldHaveMessage
+import kotlin.io.path.deleteRecursively
+import kotlin.io.path.writeText
+
+class ShouldBeADirectoryWithSameContentAsTest : FunSpec({
+
+    test("when expected directory doesn't exist, expect failure") {
+        val expectedDir = tempDir()
+        val actualDir = tempDir()
+
+        expectedDir.deleteRecursively()
+
+        val failure = shouldFail { expectedDir.shouldBeADirectoryWithSameContentAs(actualDir) }
+
+        failure.shouldHaveMessage("expectedDir '$expectedDir' is not a directory (exists:false, file:false)")
+    }
+
+    test("when actual directory doesn't exist, expect failure") {
+        val expectedDir = tempDir()
+        val actualDir = tempDir()
+
+        actualDir.deleteRecursively()
+
+        val failure = shouldFail { expectedDir.shouldBeADirectoryWithSameContentAs(actualDir) }
+
+        failure.shouldHaveMessage("actualDir '$actualDir' is not a directory (exists:false, file:false)")
+    }
+
+    test("when directories have different files, expect failure") {
+        val expectedDir = tempDir().apply {
+            resolve("file0.txt").writeText("valid file")
+            resolve("file1.txt").writeText("not in actual")
+            resolve("file2.txt").writeText("not in actual")
+            resolve("file3.txt").writeText("not in actual")
+        }
+
+        val actualDir = tempDir().apply {
+            resolve("file0.txt").writeText("valid file")
+            resolve("file-a.txt").writeText("not in expected")
+            resolve("file-b.txt").writeText("not in expected")
+            resolve("file-c.txt").writeText("not in expected")
+        }
+
+        val failure = shouldFail { expectedDir.shouldBeADirectoryWithSameContentAs(actualDir) }
+
+        failure.shouldHaveMessage(
+            """
+            actualDir is missing 3 files:
+              - file1.txt
+              - file2.txt
+              - file3.txt
+            actualDir has 3 unexpected files:
+              - file-a.txt
+              - file-b.txt
+              - file-c.txt
+            """.trimIndent()
+        )
+    }
+
+    test("when file in directories has different content, expect failure with contents of file") {
+        val expectedDir = tempDir()
+        expectedDir.resolve("file.txt").writeText("content")
+
+        val actualDir = tempDir()
+        actualDir.resolve("file.txt").writeText("unexpected content")
+
+        val failure = shouldFail { expectedDir.shouldBeADirectoryWithSameContentAs(actualDir) }
+
+        failure.shouldHaveMessage(
+            """
+            file.txt has 1 differences in content:
+                --- file.txt
+                +++ file.txt
+                @@ -1,1 +1,1 @@
+                -content
+                +unexpected content
+            """.trimIndent()
+        )
+    }
+})
diff --git a/dokka-runners/dokka-gradle-plugin/src/testFixtures/kotlin/files.kt b/dokka-runners/dokka-gradle-plugin/src/testFixtures/kotlin/files.kt
@@ -3,8 +3,11 @@
  */
 package org.jetbrains.dokka.gradle.utils
 
+import io.kotest.core.TestConfiguration
 import java.io.File
 import java.nio.file.Path
+import kotlin.io.path.createTempDirectory
+import kotlin.io.path.deleteRecursively
 import kotlin.io.path.invariantSeparatorsPathString
 import kotlin.io.path.walk
 
@@ -21,3 +24,18 @@ fun Path.listRelativePathsMatching(predicate: (Path) -> Boolean): List<String> {
         .toList()
         .sorted()
 }
+
+/**
+ * Create a temporary directory.
+ *
+ * Kotest will attempt to delete the file after the current spec has completed.
+ *
+ * (@see [io.kotest.engine.spec.tempdir], but this returns a [Path], not a [java.io.File].)
+ */
+fun TestConfiguration.tempDir(prefix: String? = null): Path {
+    val dir = createTempDirectory(prefix ?: javaClass.name)
+    afterSpec {
+        dir.deleteRecursively()
+    }
+    return dir
+}
diff --git a/dokka-runners/dokka-gradle-plugin/src/testFixtures/kotlin/kotestFiles.kt b/dokka-runners/dokka-gradle-plugin/src/testFixtures/kotlin/kotestFiles.kt
@@ -3,106 +3,100 @@
  */
 package org.jetbrains.dokka.gradle.utils
 
-import io.kotest.matchers.collections.shouldBeSameSizeAs
-import io.kotest.matchers.file.shouldBeADirectory
-import io.kotest.matchers.file.shouldHaveSameContentAs
-import io.kotest.matchers.shouldBe
-import java.io.File
-import java.nio.file.Files
+import com.github.difflib.DiffUtils
+import com.github.difflib.UnifiedDiffUtils
+import io.kotest.assertions.fail
 import java.nio.file.Path
+import kotlin.io.path.*
 
-fun Path.shouldHaveSameStructureAs(path: Path, skipEmptyDirs: Boolean) {
-    if (skipEmptyDirs) {
-        toFile().shouldHaveSameStructureAs2(path.toFile(), ::isNotEmptyDir, ::isNotEmptyDir)
-    } else {
-        toFile().shouldHaveSameStructureAs2(path.toFile())
-    }
-}
 
-fun Path.shouldHaveSameStructureAndContentAs(path: Path, skipEmptyDirs: Boolean) {
-    if (skipEmptyDirs) {
-        toFile().shouldHaveSameStructureAndContentAs2(path.toFile(), ::isNotEmptyDir, ::isNotEmptyDir)
-    } else {
-        toFile().shouldHaveSameStructureAndContentAs2(path.toFile())
+/**
+ * Compare the contents of this directory with that of [path].
+ *
+ * Only files will be compared, directories are ignored.
+ */
+infix fun Path.shouldBeADirectoryWithSameContentAs(path: Path) {
+    val differences = describeFileDifferences(this, path)
+    if (differences.isNotEmpty()) {
+        fail(differences)
     }
 }
 
-private fun isNotEmptyDir(file: File): Boolean =
-    file.isFile || Files.newDirectoryStream(file.toPath()).use { it.count() } > 0
-
-
-private fun File.shouldHaveSameStructureAs2(
-    file: File,
-    filterLhs: (File) -> Boolean = { false },
-    filterRhs: (File) -> Boolean = { false },
-) {
-    shouldHaveSameStructureAndContentAs2(
-        file,
-        filterLhs = filterLhs,
-        filterRhs = filterRhs
-    ) { expect, actual ->
-        val expectPath = expect.invariantSeparatorsPath.removePrefix(expectParentPath)
-        val actualPath = actual.invariantSeparatorsPath.removePrefix(actualParentPath)
-        expectPath shouldBe actualPath
-    }
-}
 
-fun File.shouldHaveSameStructureAndContentAs2(
-    file: File,
-    filterLhs: (File) -> Boolean = { false },
-    filterRhs: (File) -> Boolean = { false },
-) {
-    shouldHaveSameStructureAndContentAs2(
-        file,
-        filterLhs = filterLhs,
-        filterRhs = filterRhs
-    ) { expect, actual ->
-        val expectPath = expect.invariantSeparatorsPath.removePrefix(expectParentPath)
-        val actualPath = actual.invariantSeparatorsPath.removePrefix(actualParentPath)
-        expectPath shouldBe actualPath
-
-        expect.shouldHaveSameContentAs(actual)
+/**
+ * Build a string that describes the differences between [expectedDir] and [actualDir].
+ *
+ * Both the location and content of files is compared.
+ * Only files are compared, directories are excluded.
+ *
+ * If the string is empty then no differences were detected.
+ */
+private fun describeFileDifferences(
+    expectedDir: Path,
+    actualDir: Path,
+): String = buildString {
+    if (!expectedDir.isDirectory()) {
+        appendLine("expectedDir '$expectedDir' is not a directory (exists:${expectedDir.exists()}, file:${expectedDir.isRegularFile()})")
+        return@buildString
+    }
+    if (!actualDir.isDirectory()) {
+        appendLine("actualDir '$actualDir' is not a directory (exists:${actualDir.exists()}, file:${actualDir.isRegularFile()})")
+        return@buildString
     }
-}
 
+    // Collect all files from directories recursively
+    fun Path.allFiles(): Set<Path> =
+        walk().filter { it.isRegularFile() }.map { it.relativeTo(this@allFiles) }.toSet()
 
-private fun File.shouldHaveSameStructureAndContentAs2(
-    file: File,
-    filterLhs: (File) -> Boolean = { false },
-    filterRhs: (File) -> Boolean = { false },
-    fileAssert: FileAsserter,
-) {
-    val expectFiles = this.walkTopDown().filter(filterLhs).toList()
-    val actualFiles = file.walkTopDown().filter(filterRhs).toList()
-
-    expectFiles shouldBeSameSizeAs actualFiles
-
-    val assertContext = FileAsserter.Context(
-        expectParentPath = this.invariantSeparatorsPath,
-        actualParentPath = file.invariantSeparatorsPath,
-    )
-
-    expectFiles.zip(actualFiles) { expect, actual ->
-        when {
-            expect.isDirectory -> actual.shouldBeADirectory()
-            expect.isFile -> {
-                with(fileAssert) {
-                    assertContext.assert(expect, actual)
-                }
-            }
+    val expectedFiles = expectedDir.allFiles()
+    val actualFiles = actualDir.allFiles()
 
-            else -> error("There is an unexpected error analyzing file trees. Failed to determine filetype of $expect")
-        }
+    // Check for files present in one directory but not the other
+    val onlyInExpected = expectedFiles - actualFiles
+    val onlyInActual = actualFiles - expectedFiles
+
+    if (onlyInExpected.isNotEmpty()) {
+        appendLine("actualDir is missing ${onlyInExpected.size} files:")
+        appendLine(onlyInExpected.sorted().joinToFormattedList())
     }
-}
+    if (onlyInActual.isNotEmpty()) {
+        appendLine("actualDir has ${onlyInActual.size} unexpected files:")
+        appendLine(onlyInActual.sorted().joinToFormattedList())
+    }
+
+    // Compare contents of files that are present in both directories
+    val commonFiles = actualFiles intersect expectedFiles
+
+    commonFiles
+        .sorted()
+        .forEach { relativePath ->
+            val expectedFile = expectedDir.resolve(relativePath)
+            val actualFile = actualDir.resolve(relativePath)
+
+            val expectedLines = expectedFile.readLines()
+            val actualLines = actualFile.readLines()
 
+            val patch = DiffUtils.diff(expectedLines, actualLines)
 
-private fun interface FileAsserter {
+            if (patch.deltas.isNotEmpty()) {
+                appendLine("${relativePath.invariantSeparatorsPathString} has ${patch.deltas.size} differences in content:")
 
-    data class Context(
-        val expectParentPath: String,
-        val actualParentPath: String,
-    )
+                val diff = UnifiedDiffUtils.generateUnifiedDiff(
+                    /* originalFileName = */ expectedFile.relativeTo(expectedDir).invariantSeparatorsPathString,
+                    /* revisedFileName = */ actualFile.relativeTo(actualDir).invariantSeparatorsPathString,
+                    /* originalLines = */ expectedLines,
+                    /* patch = */ patch,
+                    /* contextSize = */ 3,
+                )
 
-    fun Context.assert(expect: File, actual: File)
+                appendLine(diff.joinToString("\n").prependIndent())
+            }
+        }
 }
+
+
+/**
+ * Pretty print files as a list.
+ */
+private fun Collection<Path>.joinToFormattedList(limit: Int = 10): String =
+    joinToString("\n", limit = limit) { "  - ${it.invariantSeparatorsPathString}" }
diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml
@@ -12,6 +12,8 @@ kotlinx-bcv = "0.13.2"
 
 ktor = "2.3.11"
 
+javaDiffUtils = "4.12"
+
 ## Analysis
 kotlin-compiler = "2.0.20"
 kotlin-compiler-k2 = "2.1.0-dev-5441"
@@ -131,6 +133,9 @@ apacheMaven-pluginAnnotations = { module = "org.apache.maven.plugin-tools:maven-
 apacheMaven-pluginApi = { module = "org.apache.maven:maven-plugin-api", version.ref = "apacheMaven-core" }
 apacheMaven-artifact = { module = "org.apache.maven:maven-artifact", version.ref = "apacheMaven-artifact" }
 
+#### Diff Utils ####
+javaDiffUtils = { module = "io.github.java-diff-utils:java-diff-utils", version.ref = "javaDiffUtils" }
+
 #### CLI #####
 kotlinx-cli = { module = "org.jetbrains.kotlinx:kotlinx-cli-jvm", version.ref = "kotlinx-cli" }