Skip to content

Commit

Permalink
Fix task name in error messages and add hints about rename and case-s…
Browse files Browse the repository at this point in the history 
…ensitivity

Resolves: Kotlin#76
  • Loading branch information
@joffrey-bion
joffrey-bion committed Apr 4, 2022
1 parent e51712e commit 2dba7c7
Show file tree
Hide file tree
Showing 6 changed files with 251 additions and 49 deletions.
5 changes: 5 additions & 0 deletions src/functionalTest/kotlin/kotlinx/validation/api/Assert.kt
View file
Expand Up @@ -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
Expand Down Expand Up @@ -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)
}
3 changes: 3 additions & 0 deletions src/functionalTest/kotlin/kotlinx/validation/api/BaseKotlinGradleTest.kt
View file
Expand Up @@ -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)
}
100 changes: 99 additions & 1 deletion src/functionalTest/kotlin/kotlinx/validation/test/DefaultConfigTests.kt
View file
Expand Up @@ -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
Expand All @@ -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")
}
}
Expand All @@ -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 {
Expand All @@ -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 {
Expand Down
97 changes: 97 additions & 0 deletions src/functionalTest/kotlin/kotlinx/validation/test/SubprojectsWithPluginOnRootTests.kt
View file
Expand Up @@ -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
Expand All @@ -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() {
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down
69 changes: 33 additions & 36 deletions src/main/kotlin/ApiCompareCompareTask.kt
View file
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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<RelativePath>()
val expectedApiFiles = mutableSetOf<RelativePath>()
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<String>()
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<File>): 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? {
Expand Down

0 comments on commit 2dba7c7

Please sign in to comment.