diff --git a/src/functionalTest/kotlin/kotlinx/validation/api/Assert.kt b/src/functionalTest/kotlin/kotlinx/validation/api/Assert.kt index 64826676..f8edc0e9 100644 --- a/src/functionalTest/kotlin/kotlinx/validation/api/Assert.kt +++ b/src/functionalTest/kotlin/kotlinx/validation/api/Assert.kt @@ -5,6 +5,7 @@ package kotlinx.validation.api +import org.assertj.core.api.Assertions import org.gradle.testkit.runner.BuildResult import org.gradle.testkit.runner.TaskOutcome import kotlin.test.assertEquals @@ -35,3 +36,7 @@ private fun BuildResult.assertTaskOutcome(taskOutcome: TaskOutcome, taskName: St internal fun BuildResult.assertTaskNotRun(taskName: String) { assertNull(task(taskName), "task $taskName was not expected to be run") } + +internal fun BuildResult.assertOutputContains(text: String) { + Assertions.assertThat(output).contains(text) +} diff --git a/src/functionalTest/kotlin/kotlinx/validation/api/BaseKotlinGradleTest.kt b/src/functionalTest/kotlin/kotlinx/validation/api/BaseKotlinGradleTest.kt index 3f56f416..73d52268 100644 --- a/src/functionalTest/kotlin/kotlinx/validation/api/BaseKotlinGradleTest.kt +++ b/src/functionalTest/kotlin/kotlinx/validation/api/BaseKotlinGradleTest.kt @@ -17,4 +17,7 @@ internal open class BaseKotlinGradleTest { internal val rootProjectDir: File get() = testProjectDir.root internal val rootProjectApiDump: File get() = rootProjectDir.resolve("api/${rootProjectDir.name}.api") + + // Not using a simple string here to ensure the correct file separator is used on every system + internal val rootProjectApiDumpRelative: File get() = rootProjectApiDump.relativeTo(rootProjectDir) } diff --git a/src/functionalTest/kotlin/kotlinx/validation/test/DefaultConfigTests.kt b/src/functionalTest/kotlin/kotlinx/validation/test/DefaultConfigTests.kt index c01d8d25..78551350 100644 --- a/src/functionalTest/kotlin/kotlinx/validation/test/DefaultConfigTests.kt +++ b/src/functionalTest/kotlin/kotlinx/validation/test/DefaultConfigTests.kt @@ -5,6 +5,7 @@ package kotlinx.validation.test +import kotlinx.validation.API_DIR import kotlinx.validation.api.* import org.assertj.core.api.* import org.junit.Test @@ -24,7 +25,7 @@ internal class DefaultConfigTests : BaseKotlinGradleTest() { } runner.buildAndFail().apply { - assertTrue { output.contains("Please ensure that ':apiDump' was executed") } + assertOutputContains("Please ensure that ':apiDump' was executed") assertTaskFailure(":apiCheck") } } @@ -47,6 +48,26 @@ internal class DefaultConfigTests : BaseKotlinGradleTest() { } } + @Test + fun `apiCheck should fail when there is no api file, even if there is an api dir and no Kotlin sources`() { + val runner = test { + buildGradleKts { + resolve("examples/gradle/base/withPlugin.gradle.kts") + } + + emptyDir(API_DIR) + + runner { + arguments.add(":check") + } + } + + runner.buildAndFail().apply { + assertOutputContains("File $rootProjectApiDumpRelative is missing, please run task ':apiDump' to generate one") + assertTaskFailure(":apiCheck") + } + } + @Test fun `apiCheck should succeed, when api-File is empty, but no kotlin files are included in SourceSet`() { val runner = test { @@ -66,6 +87,83 @@ internal class DefaultConfigTests : BaseKotlinGradleTest() { } } + @Test + fun `apiCheck should fail with rename hint when the api file exists but with incorrect name`() { + val runner = test { + buildGradleKts { + resolve("examples/gradle/base/withPlugin.gradle.kts") + } + kotlin("AnotherBuildConfig.kt") { + resolve("examples/classes/AnotherBuildConfig.kt") + } + apiFile(projectName = "some-unrelated-name") { + resolve("examples/classes/AnotherBuildConfig.dump") + } + runner { + arguments.add(":apiCheck") + } + } + + runner.buildAndFail().apply { + assertOutputContains("File $rootProjectApiDumpRelative is missing, but another file was found instead: some-unrelated-name.api.") + assertOutputContains("If you renamed the project, please run task ':apiDump' again to re-generate the API file.") + assertTaskFailure(":apiCheck") + } + } + @OptIn(ExperimentalStdlibApi::class) + @Test + fun `apiCheck should fail with case-sensitivity hint when an api file exists but a name that differs only by case`() { + val projectNameUpperCase = rootProjectDir.name.uppercase() + val runner = test { + buildGradleKts { + resolve("examples/gradle/base/withPlugin.gradle.kts") + } + kotlin("AnotherBuildConfig.kt") { + resolve("examples/classes/AnotherBuildConfig.kt") + } + apiFile(projectName = projectNameUpperCase) { + resolve("examples/classes/AnotherBuildConfig.dump") + } + runner { + arguments.add(":apiCheck") + } + } + + runner.buildAndFail().apply { + assertOutputContains("File $rootProjectApiDumpRelative is missing, but a similar file was found instead: $projectNameUpperCase.api.") + assertOutputContains("If you renamed the project, please run task ':apiDump' again to re-generate the API file.") + assertOutputContains("Since the rename only involved a case change, you may need to delete the file manually " + + "before running the task (if your file system is case-insensitive.") + assertTaskFailure(":apiCheck") + } + } + + @Test + fun `apiCheck should fail when several api files exist but none with the correct name`() { + val runner = test { + buildGradleKts { + resolve("examples/gradle/base/withPlugin.gradle.kts") + } + kotlin("AnotherBuildConfig.kt") { + resolve("examples/classes/AnotherBuildConfig.kt") + } + apiFile(projectName = "some-unrelated-name") { + resolve("examples/classes/AnotherBuildConfig.dump") + } + apiFile(projectName = "some-unrelated-name2") { + resolve("examples/classes/AnotherBuildConfig.dump") + } + runner { + arguments.add(":apiCheck") + } + } + + runner.buildAndFail().apply { + assertOutputContains("File $rootProjectApiDumpRelative is missing, please run task ':apiDump' to generate it") + assertTaskFailure(":apiCheck") + } + } + @Test fun `apiCheck should succeed when public classes match api file`() { val runner = test { diff --git a/src/functionalTest/kotlin/kotlinx/validation/test/SubprojectsWithPluginOnRootTests.kt b/src/functionalTest/kotlin/kotlinx/validation/test/SubprojectsWithPluginOnRootTests.kt index 742c464b..bf23cd12 100644 --- a/src/functionalTest/kotlin/kotlinx/validation/test/SubprojectsWithPluginOnRootTests.kt +++ b/src/functionalTest/kotlin/kotlinx/validation/test/SubprojectsWithPluginOnRootTests.kt @@ -5,6 +5,7 @@ package kotlinx.validation.test +import kotlinx.validation.API_DIR import kotlinx.validation.api.* import kotlinx.validation.api.BaseKotlinGradleTest import kotlinx.validation.api.assertTaskSuccess @@ -14,6 +15,7 @@ import kotlinx.validation.api.runner import kotlinx.validation.api.test import org.assertj.core.api.Assertions import org.junit.Test +import java.nio.file.Paths import kotlin.test.assertTrue internal class SubprojectsWithPluginOnRootTests : BaseKotlinGradleTest() { @@ -96,6 +98,43 @@ internal class SubprojectsWithPluginOnRootTests : BaseKotlinGradleTest() { } } + @Test + fun `check should fail with rename hint when a subproject doesn't have a correct api file`() { + val runner = test { + createProjectHierarchyWithPluginOnRoot() + + emptyApiFile(projectName = rootProjectDir.name) + + dir("sub1") { + emptyApiFile(projectName = "sub1") + + dir("subsub1") { + emptyApiFile(projectName = "not-subsub1") + } + + dir("subsub2") { + emptyApiFile(projectName = "subsub2") + } + } + + dir("sub2") { + emptyApiFile(projectName = "sub2") + } + + runner { + arguments.add("check") + } + } + + runner.buildAndFail().apply { + assertTaskFailure(":sub1:subsub1:apiCheck") + // using Paths.get() to ensure correct separators on Windows + val apiFilePath = Paths.get("sub1/subsub1/api/subsub1.api") + assertOutputContains("File $apiFilePath is missing, but another file was found instead: not-subsub1.api") + assertOutputContains("If you renamed the project, please run task ':sub1:subsub1:apiDump' again to re-generate the API file.") + } + } + @Test fun `apiCheck should succeed on all subprojects when api files are empty but there are no Kotlin sources`() { val runner = test { @@ -199,6 +238,64 @@ internal class SubprojectsWithPluginOnRootTests : BaseKotlinGradleTest() { } } + @Test + fun `apiCheck should fail on sub-subproject when the api file is missing`() { + val runner = test { + createProjectHierarchyWithPluginOnRoot() + + dir("sub1") { + dir("subsub2") { + kotlin("Subsub2Class.kt") { + resolve("examples/classes/Subsub2Class.kt") + } + emptyDir(API_DIR) + } + } + + runner { + arguments.add(":sub1:subsub2:apiCheck") + } + } + + runner.buildAndFail().apply { + assertTaskFailure(":sub1:subsub2:apiCheck") + // using Paths.get() to ensure correct separators on Windows + val apiFilePath = Paths.get("sub1/subsub2/api/subsub2.api") + assertOutputContains("File $apiFilePath is missing, please run task ':sub1:subsub2:apiDump' to generate it") + } + } + + @Test + fun `apiCheck should fail on sub-subproject with rename hint when the api file doesn't have the correct name`() { + val runner = test { + createProjectHierarchyWithPluginOnRoot() + + dir("sub1") { + dir("subsub2") { + kotlin("Subsub2Class.kt") { + resolve("examples/classes/Subsub2Class.kt") + } + apiFile(projectName = "not-subsub2") { + resolve("examples/classes/Subsub2Class.dump") + } + } + } + + runner { + arguments.add(":sub1:subsub2:apiCheck") + } + } + + runner.buildAndFail().apply { + assertTaskFailure(":sub1:subsub2:apiCheck") + // using Paths.get() to ensure correct separators on Windows + val apiFilePath = Paths.get("sub1/subsub2/api/subsub2.api") + assertOutputContains("File $apiFilePath is missing, but another file was found instead: not-subsub2.api") + assertOutputContains("If you renamed the project, please run task ':sub1:subsub2:apiDump' again to re-generate the API file.") + + } + } + @Test fun `apiCheck should succeed on subprojects, when public classes match api files`() { val runner = test { diff --git a/src/main/kotlin/ApiCompareCompareTask.kt b/src/main/kotlin/ApiCompareCompareTask.kt index 5810bdab..f71ea420 100644 --- a/src/main/kotlin/ApiCompareCompareTask.kt +++ b/src/main/kotlin/ApiCompareCompareTask.kt @@ -7,13 +7,10 @@ package kotlinx.validation import difflib.* import org.gradle.api.* -import org.gradle.api.file.* -import org.gradle.api.model.ObjectFactory import org.gradle.api.tasks.* import java.io.* -import javax.inject.Inject -open class ApiCompareCompareTask @Inject constructor(private val objects: ObjectFactory): DefaultTask() { +open class ApiCompareCompareTask : DefaultTask() { /* * Nullability and optionality is a workaround for @@ -35,6 +32,11 @@ open class ApiCompareCompareTask @Inject constructor(private val objects: Object @PathSensitive(PathSensitivity.RELATIVE) lateinit var apiBuildDir: File + // Used for diagnostic error messages when API file is missing/incorrect + @Input + @Optional + lateinit var dumpTaskFqn: String + @OutputFile @Optional val dummyOutputFile: File? = null @@ -46,44 +48,39 @@ open class ApiCompareCompareTask @Inject constructor(private val objects: Object @TaskAction fun verify() { val projectApiDir = projectApiDir - if (projectApiDir == null) { - error("Expected folder with API declarations '$nonExistingProjectApiDir' does not exist.\n" + - "Please ensure that ':apiDump' was executed in order to get API dump to compare the build against") - } + ?: error("Expected folder with API declarations '$nonExistingProjectApiDir' does not exist.\n" + + "Please ensure that '$dumpTaskFqn' was executed in order to get API dump to compare the build against") - val subject = projectName - val apiBuildDirFiles = mutableSetOf() - val expectedApiFiles = mutableSetOf() - objects.fileTree().from(apiBuildDir).visit { file -> - apiBuildDirFiles.add(file.relativePath) - } - objects.fileTree().from(projectApiDir).visit { file -> - expectedApiFiles.add(file.relativePath) - } + val actualApiFiles = apiBuildDir.listFiles { f: File -> f.isFile } + val actualApiFile = actualApiFiles.singleOrNull() + ?: error("Expected a single file $projectName.api, but found: ${actualApiFiles.map { it.relativeTo(rootDir) }}") - if (apiBuildDirFiles.size != 1) { - error("Expected a single file $subject.api, but found: $expectedApiFiles") - } - - val expectedApiDeclaration = apiBuildDirFiles.single() - if (expectedApiDeclaration !in expectedApiFiles) { - error("File ${expectedApiDeclaration.lastName} is missing from ${projectApiDir.relativePath()}, please run " + - ":$subject:apiDump task to generate one") - } + val expectedApiFiles = projectApiDir.listFiles { f: File -> f.isFile } + val expectedApiFile = expectedApiFiles.find { it.name == actualApiFile.name } + ?: errorWithExplanation(projectApiDir, actualApiFile, expectedApiFiles) - val diffSet = mutableSetOf() - val expectedFile = expectedApiDeclaration.getFile(projectApiDir) - val actualFile = expectedApiDeclaration.getFile(apiBuildDir) - val diff = compareFiles(expectedFile, actualFile) - if (diff != null) diffSet.add(diff) - if (diffSet.isNotEmpty()) { - val diffText = diffSet.joinToString("\n\n") - error("API check failed for project $subject.\n$diffText\n\n You can run :$subject:apiDump task to overwrite API declarations") + val diff = compareFiles(expectedApiFile, actualApiFile) + if (diff != null) { + error("API check failed for project $projectName.\n$diff\n\n You can run task '$dumpTaskFqn' to overwrite API declarations") } } - private fun File.relativePath(): String { - return relativeTo(rootDir).toString() + "/" + private fun errorWithExplanation(projectApiDir: File, actualApiFile: File, expectedApiFiles: Array): Nothing { + val nonExistingExpectedApiFile = projectApiDir.resolve(actualApiFile.name).relativeTo(rootDir) + if (expectedApiFiles.size != 1) { + error("File $nonExistingExpectedApiFile is missing, please run task '$dumpTaskFqn' to generate it") + } + val incorrectApiFileName = expectedApiFiles.single().name + val caseChangeOnly = incorrectApiFileName.equals(actualApiFile.name, ignoreCase = true) + if (caseChangeOnly) { + error("File $nonExistingExpectedApiFile is missing, but a similar file was found instead: $incorrectApiFileName.\n" + + "If you renamed the project, please run task '$dumpTaskFqn' again to re-generate the API file. " + + "Since the rename only involved a case change, you may need to delete the file manually before " + + "running the task (if your file system is case-insensitive.") + } else { + error("File $nonExistingExpectedApiFile is missing, but another file was found instead: $incorrectApiFileName.\n" + + "If you renamed the project, please run task '$dumpTaskFqn' again to re-generate the API file.") + } } private fun compareFiles(checkFile: File, builtFile: File): String? { diff --git a/src/main/kotlin/BinaryCompatibilityValidatorPlugin.kt b/src/main/kotlin/BinaryCompatibilityValidatorPlugin.kt index 05773401..29cd3d9c 100644 --- a/src/main/kotlin/BinaryCompatibilityValidatorPlugin.kt +++ b/src/main/kotlin/BinaryCompatibilityValidatorPlugin.kt @@ -243,6 +243,19 @@ private fun Project.configureCheckTasks( logger.debug("Configuring api for ${targetConfig.targetName ?: "jvm"} to $r") } } + + val apiDump = task(targetConfig.apiTaskName("Dump")) { + isEnabled = apiCheckEnabled(extension) && apiBuild.map { it.enabled }.getOrElse(true) + group = "other" + description = "Syncs API from build dir to ${targetConfig.apiDir} dir for $projectName" + from(apiBuildDir) + into(apiCheckDir) + dependsOn(apiBuild) + doFirst { + apiCheckDir.get().mkdirs() + } + } + val apiCheck = task(targetConfig.apiTaskName("Check")) { isEnabled = apiCheckEnabled(extension) && apiBuild.map { it.enabled }.getOrElse(true) group = "verification" @@ -256,22 +269,11 @@ private fun Project.configureCheckTasks( null } this.apiBuildDir = apiBuildDir.get() + dumpTaskFqn = apiDump.get().path } dependsOn(apiBuild) } - val apiDump = task(targetConfig.apiTaskName("Dump")) { - isEnabled = apiCheckEnabled(extension) && apiBuild.map { it.enabled }.getOrElse(true) - group = "other" - description = "Syncs API from build dir to ${targetConfig.apiDir} dir for $projectName" - from(apiBuildDir) - into(apiCheckDir) - dependsOn(apiBuild) - doFirst { - apiCheckDir.get().mkdirs() - } - } - commonApiDump?.configure { it.dependsOn(apiDump) } when (commonApiCheck) {