From 74d43d8c9e48966e00e0acc82377f7cb0756109d Mon Sep 17 00:00:00 2001 From: Filipp Zhinkin Date: Wed, 5 Jun 2024 16:47:27 +0200 Subject: [PATCH 1/3] Added regression test for #234 --- .../validation/test/MultipleJvmTargetsTest.kt | 28 +++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/src/functionalTest/kotlin/kotlinx/validation/test/MultipleJvmTargetsTest.kt b/src/functionalTest/kotlin/kotlinx/validation/test/MultipleJvmTargetsTest.kt index 9ecfc3a8..a142bd58 100644 --- a/src/functionalTest/kotlin/kotlinx/validation/test/MultipleJvmTargetsTest.kt +++ b/src/functionalTest/kotlin/kotlinx/validation/test/MultipleJvmTargetsTest.kt @@ -134,6 +134,34 @@ internal class MultipleJvmTargetsTest : BaseKotlinGradleTest() { } } + // Scenario from #233: if there were two targets (and two dumps, correspondingly), + // removal of one of the targets should trigger validation failure. + @Test + fun testValidationAfterTargetRemoval() { + val runner = test { + buildGradleKts { + resolve("/examples/gradle/base/withPlugin.gradle.kts") + } + kotlin("AnotherBuildConfig.kt") { + resolve("/examples/classes/AnotherBuildConfig.kt") + } + // let's pretend, there were multiple targets before + for (tgtName in listOf("jvm", "anotherJvm")) { + dir("$API_DIR/$tgtName/") { + file("${rootProjectDir.name.lowercase()}.api") { + resolve("/examples/classes/AnotherBuildConfig.dump") + } + } + } + runner { + arguments.add(":apiCheck") + } + } + runner.buildAndFail().apply { + assertTaskFailure(":apiCheck") + } + } + private val jvmApiDump: File get() = rootProjectDir.resolve("$API_DIR/jvm/testproject.api") private val anotherApiDump: File get() = rootProjectDir.resolve("$API_DIR/anotherJvm/testproject.api") From 656833c3d0369e3eac43e382fff9dc393d69357f Mon Sep 17 00:00:00 2001 From: Filipp Zhinkin Date: Wed, 5 Jun 2024 16:48:43 +0200 Subject: [PATCH 2/3] Use exact file names when comparing dumps Filename case mismatch is no longer tolerated on case-sensitive filesystems. Fixes #231 --- api/binary-compatibility-validator.api | 2 +- src/main/kotlin/KotlinApiCompareTask.kt | 37 +++---------------------- 2 files changed, 5 insertions(+), 34 deletions(-) diff --git a/api/binary-compatibility-validator.api b/api/binary-compatibility-validator.api index 8e683d09..ea5804ca 100644 --- a/api/binary-compatibility-validator.api +++ b/api/binary-compatibility-validator.api @@ -78,7 +78,7 @@ public class kotlinx/validation/KotlinApiBuildTask : kotlinx/validation/BuildTas public class kotlinx/validation/KotlinApiCompareTask : org/gradle/api/DefaultTask { public field generatedApiFile Ljava/io/File; public field projectApiFile Ljava/io/File; - public fun (Lorg/gradle/api/model/ObjectFactory;)V + public fun ()V public final fun getGeneratedApiFile ()Ljava/io/File; public final fun getProjectApiFile ()Ljava/io/File; public final fun setGeneratedApiFile (Ljava/io/File;)V diff --git a/src/main/kotlin/KotlinApiCompareTask.kt b/src/main/kotlin/KotlinApiCompareTask.kt index 88e693ef..72399b6a 100644 --- a/src/main/kotlin/KotlinApiCompareTask.kt +++ b/src/main/kotlin/KotlinApiCompareTask.kt @@ -8,14 +8,11 @@ package kotlinx.validation import com.github.difflib.DiffUtils import com.github.difflib.UnifiedDiffUtils import java.io.* -import java.util.TreeMap import javax.inject.Inject import org.gradle.api.* -import org.gradle.api.file.* -import org.gradle.api.model.ObjectFactory import org.gradle.api.tasks.* -public open class KotlinApiCompareTask @Inject constructor(private val objects: ObjectFactory): DefaultTask() { +public open class KotlinApiCompareTask @Inject constructor(): DefaultTask() { @InputFiles @PathSensitive(PathSensitivity.RELATIVE) @@ -41,43 +38,17 @@ public open class KotlinApiCompareTask @Inject constructor(private val objects: } val subject = projectName - /* - * We use case-insensitive comparison to workaround issues with case-insensitive OSes - * and Gradle behaving slightly different on different platforms. - * We neither know original sensitivity of existing .api files, not - * build ones, because projectName that is part of the path can have any sensitvity. - * To workaround that, we replace paths we are looking for the same paths that - * actually exist on FS. - */ - fun caseInsensitiveMap() = TreeMap { rp, rp2 -> - rp.compareTo(rp2, true) - } - - val apiBuildDirFiles = caseInsensitiveMap() - val expectedApiFiles = caseInsensitiveMap() - - objects.fileTree().from(buildApiDir).visit { file -> - apiBuildDirFiles[file.name] = file.relativePath - } - objects.fileTree().from(projectApiDir).visit { file -> - expectedApiFiles[file.name] = file.relativePath - } - - if (!expectedApiFiles.containsKey(projectApiFile.name)) { + if (!projectApiFile.exists()) { error("File ${projectApiFile.name} is missing from ${projectApiDir.relativeDirPath()}, please run " + ":$subject:apiDump task to generate one") } - if (!apiBuildDirFiles.containsKey(generatedApiFile.name)) { + if (!generatedApiFile.exists()) { error("File ${generatedApiFile.name} is missing from dump results.") } // Normalize case-sensitivity - val expectedApiDeclaration = expectedApiFiles.getValue(projectApiFile.name) - val actualApiDeclaration = apiBuildDirFiles.getValue(generatedApiFile.name) val diffSet = mutableSetOf() - val expectedFile = expectedApiDeclaration.getFile(projectApiDir) - val actualFile = actualApiDeclaration.getFile(buildApiDir) - val diff = compareFiles(expectedFile, actualFile) + val diff = compareFiles(projectApiFile, generatedApiFile) if (diff != null) diffSet.add(diff) if (diffSet.isNotEmpty()) { val diffText = diffSet.joinToString("\n\n") From c75d694dfef91f55cfb92ef62ca917c45ffc727d Mon Sep 17 00:00:00 2001 From: Filipp Zhinkin Date: Wed, 5 Jun 2024 17:04:50 +0200 Subject: [PATCH 3/3] Update old test to fail on filename case mismatch --- .../validation/test/DefaultConfigTests.kt | 23 ++++++++++++++++--- 1 file changed, 20 insertions(+), 3 deletions(-) diff --git a/src/functionalTest/kotlin/kotlinx/validation/test/DefaultConfigTests.kt b/src/functionalTest/kotlin/kotlinx/validation/test/DefaultConfigTests.kt index adfca8c2..3879d9f4 100644 --- a/src/functionalTest/kotlin/kotlinx/validation/test/DefaultConfigTests.kt +++ b/src/functionalTest/kotlin/kotlinx/validation/test/DefaultConfigTests.kt @@ -7,7 +7,10 @@ package kotlinx.validation.test import kotlinx.validation.api.* import org.assertj.core.api.* +import org.junit.Assume import org.junit.Test +import java.io.File +import java.nio.file.Files import kotlin.test.* internal class DefaultConfigTests : BaseKotlinGradleTest() { @@ -90,7 +93,9 @@ internal class DefaultConfigTests : BaseKotlinGradleTest() { } @Test - fun `apiCheck should succeed when public classes match api file ignoring case`() { + fun `apiCheck should fail when public classes match api file ignoring case`() { + Assume.assumeTrue(underlyingFsIsCaseSensitive()) + val runner = test { buildGradleKts { resolve("/examples/gradle/base/withPlugin.gradle.kts") @@ -107,8 +112,8 @@ internal class DefaultConfigTests : BaseKotlinGradleTest() { } } - runner.build().apply { - assertTaskSuccess(":apiCheck") + runner.buildAndFail().apply { + assertTaskFailure(":apiCheck") } } @@ -237,4 +242,16 @@ internal class DefaultConfigTests : BaseKotlinGradleTest() { assertTaskSuccess(":apiCheck") } } + + + private fun underlyingFsIsCaseSensitive(): Boolean { + val f = Files.createTempFile("UPPER", "UPPER").toFile() + f.deleteOnExit() + try { + val lower = File(f.absolutePath.lowercase()) + return !lower.exists() + } finally { + f.delete() + } + } }