From 9b71bd5bb4ff322015c5cf95c491495a93b0eecc Mon Sep 17 00:00:00 2001 From: Hung Nguyen Date: Mon, 6 Sep 2021 10:07:00 +0100 Subject: [PATCH] KT-45777: Address review comments - update tests --- .../gradle/IncrementalCompilationBaseIT.kt | 11 ++ .../IncrementalCompilationMultiProjectIT.kt | 151 ++++++++++++------ .../kotlin/gradle/IncrementalJavaChangeIT.kt | 67 +++++--- 3 files changed, 155 insertions(+), 74 deletions(-) diff --git a/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/kotlin/org/jetbrains/kotlin/gradle/IncrementalCompilationBaseIT.kt b/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/kotlin/org/jetbrains/kotlin/gradle/IncrementalCompilationBaseIT.kt index 2fa3275047a..e28e159b4fa 100644 --- a/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/kotlin/org/jetbrains/kotlin/gradle/IncrementalCompilationBaseIT.kt +++ b/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/kotlin/org/jetbrains/kotlin/gradle/IncrementalCompilationBaseIT.kt @@ -26,6 +26,17 @@ abstract class IncrementalCompilationBaseIT : BaseGradleIT() { ) } + protected fun doTest( + fileToModify: String, + modifyFileContents: (originalContents: String) -> String, + assertResults: CompiledProject.() -> Unit, + ) { + doTest( + modifyProject = { projectDir.getFileByName(fileToModify).modify(modifyFileContents) }, + assertResults = { assertResults() } + ) + } + protected fun doTest( project: Project = defaultProject(), task: String = "build", diff --git a/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/kotlin/org/jetbrains/kotlin/gradle/IncrementalCompilationMultiProjectIT.kt b/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/kotlin/org/jetbrains/kotlin/gradle/IncrementalCompilationMultiProjectIT.kt index 25490e10c57..1a50b533b0e 100644 --- a/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/kotlin/org/jetbrains/kotlin/gradle/IncrementalCompilationMultiProjectIT.kt +++ b/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/kotlin/org/jetbrains/kotlin/gradle/IncrementalCompilationMultiProjectIT.kt @@ -55,11 +55,18 @@ open class IncrementalCompilationJvmMultiProjectIT : BaseIncrementalCompilationM } } - // checks that multi-project ic is disabled when there is a task that outputs to javaDestination dir // that is not JavaCompile or KotlinCompile @Test - fun testCompileLibWithGroovy() { + open fun testCompileLibWithGroovy() { + testCompileLibWithGroovy_doTest { + assertCompiledKotlinFiles( + File(project.projectDir, "app").allKotlinFiles() + File(project.projectDir, "lib").getFileByName("A.kt") + ) + } + } + + protected fun testCompileLibWithGroovy_doTest(assertResults: CompiledProject.() -> Unit) { val project = defaultProject() project.setupWorkingDir() val lib = File(project.projectDir, "lib") @@ -93,15 +100,10 @@ open class IncrementalCompilationJvmMultiProjectIT : BaseIncrementalCompilationM project.changeMethodBodyInLib() project.build("build") { assertSuccessful() - assertCompiledKotlinFiles(testCompileLibWithGroovy_expectedFiles(project)) + assertResults() } } - /** Expected files to be recompiled for [testCompileLibWithGroovy], which may be overridden in subclasses. */ - open fun testCompileLibWithGroovy_expectedFiles(project: Project): Iterable { - return File(project.projectDir, "app").allKotlinFiles() + File(project.projectDir, "lib").getFileByName("A.kt") - } - /** Regression test for KT-43489. Make sure build history mapping is not initialized too early. */ @Test fun testBuildHistoryMappingLazilyComputedWithWorkers() { @@ -151,20 +153,72 @@ class IncrementalCompilationClasspathSnapshotJvmMultiProjectIT : IncrementalComp override fun defaultBuildOptions() = super.defaultBuildOptions().copy(useClasspathSnapshot = true) - override fun testAddDependencyInLib_expectedFiles(project: Project): Iterable { - // With classpath snapshot, no files are recompiled - return emptyList() + @Test + override fun testNonAbiChangeInLib_changeMethodBody() { + doTest( + modifyProject = changeMethodBodyInLib, + assertResults = { + assertTasksExecuted(":lib:$compileKotlinTaskName") + assertTasksExecuted(":app:$compileKotlinTaskName") // TODO: App compilation should have 'compile avoidance' + assertCompiledKotlinFiles(File(project.projectDir, "lib").getFilesByNames("A.kt")) + } + ) } - override fun testAbiChangeInLib_afterLibClean_expectedFiles(project: Project): Iterable { - // With classpath snapshot, app compilation is incremental - return File(project.projectDir, "app").getFilesByNames("AA.kt", "AAA.kt", "BB.kt", "fooUseA.kt") + - File(project.projectDir, "lib").allKotlinFiles() + @Test + override fun testAddDependencyInLib() { + doTest( + modifyProject = { testAddDependencyInLib_modifyProject() }, + assertResults = { + assertTasksExecuted(":lib:$compileKotlinTaskName") + assertTasksUpToDate(":app:$compileKotlinTaskName") + assertCompiledKotlinFiles(emptyList()) // Lib compilation is incremental (no files are recompiled) + } + ) } - override fun testCompileLibWithGroovy_expectedFiles(project: Project): Iterable { - // With classpath snapshot, no files in app are recompiled - return listOf(File(project.projectDir, "lib").getFileByName("A.kt")) + @Test + override fun testAbiChangeInLib_afterLibClean() { + doTest( + modifyProject = { + build(":lib:clean") { assertSuccessful() } + changeMethodSignatureInLib() + }, + assertResults = { + assertCompiledKotlinFiles( + // App compilation is incremental + File(project.projectDir, "app").getFilesByNames("AA.kt", "AAA.kt", "BB.kt", "fooUseA.kt") + + File(project.projectDir, "lib").allKotlinFiles() + ) + } + ) + } + + @Test + override fun testCompileLibWithGroovy() { + testCompileLibWithGroovy_doTest { + assertTasksExecuted(":lib:$compileKotlinTaskName") + assertTasksExecuted(":app:$compileKotlinTaskName") // TODO: App compilation should have 'compile avoidance' + assertCompiledKotlinFiles(listOf(File(project.projectDir, "lib").getFileByName("A.kt"))) + } + } + + @Test + override fun testAbiChangeInLib_afterLibClean_withAbiSnapshot() { + doTest( + options = defaultBuildOptions().copy(abiSnapshot = true), + modifyProject = { + build(":lib:clean") { assertSuccessful() } + changeMethodSignatureInLib() + }, + assertResults = { + assertCompiledKotlinFiles( + // App compilation is incremental + File(project.projectDir, "app").getFilesByNames("AA.kt", "AAA.kt", "BB.kt", "fooUseA.kt") + + File(project.projectDir, "lib").allKotlinFiles() + ) + } + ) } } @@ -213,56 +267,53 @@ abstract class BaseIncrementalCompilationMultiProjectIT : IncrementalCompilation } @Test - fun testNonAbiChangeInLib_changeMethodBody() { + open fun testNonAbiChangeInLib_changeMethodBody() { doTest( modifyProject = changeMethodBodyInLib, - expectedCompiledFileNames = listOf("A.kt") // In lib - ) - } - - @Test - fun testAddDependencyInLib() { - doTest( - modifyProject = { - File(projectDir, "lib/build.gradle").modify { - """ - $it - - dependencies { - $additionalLibDependencies - } - """.trimIndent() - } - }, assertResults = { - assertCompiledKotlinFiles(testAddDependencyInLib_expectedFiles(project)) + assertCompiledKotlinFiles(File(project.projectDir, "lib").getFilesByNames("A.kt")) } ) } - /** Expected files to be recompiled for [testAddDependencyInLib], which may be overridden in subclasses. */ - open fun testAddDependencyInLib_expectedFiles(project: Project): Iterable { - return File(project.projectDir, "lib").allKotlinFiles() + @Test + open fun testAddDependencyInLib() { + doTest( + modifyProject = { testAddDependencyInLib_modifyProject() }, + assertResults = { + assertTasksExecuted(":lib:$compileKotlinTaskName") + assertTasksUpToDate(":app:$compileKotlinTaskName") + assertCompiledKotlinFiles(File(project.projectDir, "lib").allKotlinFiles()) + } + ) + } + + protected fun Project.testAddDependencyInLib_modifyProject() { + File(projectDir, "lib/build.gradle").modify { + """ + $it + + dependencies { + $additionalLibDependencies + } + """.trimIndent() + } } @Test - fun testAbiChangeInLib_afterLibClean() { // To see if app compilation can be incremental after non-incremental lib compilation + open fun testAbiChangeInLib_afterLibClean() { // To see if app compilation can be incremental after non-incremental lib compilation doTest( modifyProject = { build(":lib:clean") { assertSuccessful() } changeMethodSignatureInLib() }, assertResults = { - assertCompiledKotlinFiles(testAbiChangeInLib_afterLibClean_expectedFiles(project)) + // App compilation is non-incremental + assertCompiledKotlinFiles(project.projectDir.allKotlinFiles()) } ) } - /** Expected files for [testAbiChangeInLib_afterLibClean], which may be overridden in subclasses. */ - open fun testAbiChangeInLib_afterLibClean_expectedFiles(project: Project): Iterable { - return project.projectDir.allKotlinFiles() - } - @Test fun testMoveFunctionFromLibToApp() { doTest( @@ -410,7 +461,7 @@ abstract class BaseIncrementalCompilationMultiProjectIT : IncrementalCompilation } @Test - fun testAbiChangeInLib_afterLibClean_withAbiSnapshot() { + open fun testAbiChangeInLib_afterLibClean_withAbiSnapshot() { doTest( options = defaultBuildOptions().copy(abiSnapshot = true), modifyProject = { @@ -419,7 +470,7 @@ abstract class BaseIncrementalCompilationMultiProjectIT : IncrementalCompilation }, assertResults = { // TODO: With ABI snapshot, app compilation should be incremental, currently it is not. - assertCompiledKotlinFiles(testAbiChangeInLib_afterLibClean_expectedFiles(project)) + assertCompiledKotlinFiles(project.projectDir.allKotlinFiles()) } ) } diff --git a/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/kotlin/org/jetbrains/kotlin/gradle/IncrementalJavaChangeIT.kt b/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/kotlin/org/jetbrains/kotlin/gradle/IncrementalJavaChangeIT.kt index 758f48534a4..1357d3ce030 100644 --- a/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/kotlin/org/jetbrains/kotlin/gradle/IncrementalJavaChangeIT.kt +++ b/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/kotlin/org/jetbrains/kotlin/gradle/IncrementalJavaChangeIT.kt @@ -5,17 +5,25 @@ package org.jetbrains.kotlin.gradle +import org.jetbrains.kotlin.gradle.util.getFilesByNames import org.junit.Test +import java.io.File open class IncrementalJavaChangeDefaultIT : IncrementalCompilationJavaChangesBase(usePreciseJavaTracking = null) { @Test override fun testAbiChangeInLib_changeMethodSignature_tracked() { - doTest(trackedJavaClass, changeSignature, expectedCompiledFileNames = listOf("TrackedJavaClassChild.kt", "useTrackedJavaClass.kt")) + doTest( + trackedJavaClassInLib, changeMethodSignature, + expectedCompiledFileNames = listOf("TrackedJavaClassChild.kt", "useTrackedJavaClass.kt") // In app + ) } @Test override fun testNonAbiChangeInLib_changeMethodBody_tracked() { - doTest(trackedJavaClass, changeBody, expectedCompiledFileNames = listOf()) + doTest( + trackedJavaClassInLib, changeMethodBody, + expectedCompiledFileNames = emptyList() + ) } } @@ -25,19 +33,26 @@ class IncrementalJavaChangeClasspathSnapshotIT : IncrementalJavaChangeDefaultIT( @Test override fun testAbiChangeInLib_changeMethodSignature() { - // With classpath snapshot, fewer Kotlin files are recompiled doTest( - javaClass, changeSignature, - expectedCompiledFileNames = listOf("JavaClassChild.kt", "useJavaClass.kt") + javaClassInLib, changeMethodSignature, + assertResults = { + // Fewer Kotlin files are recompiled + assertCompiledKotlinFiles( + File(project.projectDir, "app").getFilesByNames("JavaClassChild.kt", "useJavaClass.kt") + ) + } ) } @Test override fun testNonAbiChangeInLib_changeMethodBody() { - // With classpath snapshot, no Kotlin files are recompiled doTest( - javaClass, changeBody, - expectedCompiledFileNames = emptyList() + javaClassInLib, changeMethodBody, + assertResults = { + assertTasksExecuted(":lib:compileKotlin") + assertTasksExecuted(":app:compileKotlin") // TODO: App compilation should have 'compile avoidance' + assertCompiledKotlinFiles(emptyList()) + } ) } } @@ -45,12 +60,16 @@ class IncrementalJavaChangeClasspathSnapshotIT : IncrementalJavaChangeDefaultIT( class IncrementalJavaChangePreciseIT : IncrementalCompilationJavaChangesBase(usePreciseJavaTracking = true) { @Test override fun testAbiChangeInLib_changeMethodSignature_tracked() { - doTest(trackedJavaClass, changeSignature, expectedCompiledFileNames = listOf("TrackedJavaClassChild.kt", "useTrackedJavaClass.kt")) + doTest( + trackedJavaClassInLib, + changeMethodSignature, + expectedCompiledFileNames = listOf("TrackedJavaClassChild.kt", "useTrackedJavaClass.kt") // In app + ) } @Test override fun testNonAbiChangeInLib_changeMethodBody_tracked() { - doTest(trackedJavaClass, changeBody, expectedCompiledFileNames = listOf()) + doTest(trackedJavaClassInLib, changeMethodBody, expectedCompiledFileNames = emptyList()) } } @@ -58,10 +77,10 @@ open class IncrementalJavaChangeDisablePreciseIT : IncrementalCompilationJavaCha @Test override fun testAbiChangeInLib_changeMethodSignature_tracked() { doTest( - trackedJavaClass, changeSignature, + trackedJavaClassInLib, changeMethodSignature, expectedCompiledFileNames = listOf( - "TrackedJavaClassChild.kt", "useTrackedJavaClass.kt", "useTrackedJavaClassFooMethodUsage.kt", - "useTrackedJavaClassSameModule.kt" + "TrackedJavaClassChild.kt", "useTrackedJavaClass.kt", "useTrackedJavaClassFooMethodUsage.kt", // In app + "useTrackedJavaClassSameModule.kt" // In lib ) ) } @@ -69,10 +88,10 @@ open class IncrementalJavaChangeDisablePreciseIT : IncrementalCompilationJavaCha @Test override fun testNonAbiChangeInLib_changeMethodBody_tracked() { doTest( - trackedJavaClass, changeBody, + trackedJavaClassInLib, changeMethodBody, expectedCompiledFileNames = listOf( - "TrackedJavaClassChild.kt", "useTrackedJavaClass.kt", "useTrackedJavaClassFooMethodUsage.kt", - "useTrackedJavaClassSameModule.kt" + "TrackedJavaClassChild.kt", "useTrackedJavaClass.kt", "useTrackedJavaClassFooMethodUsage.kt", // In app + "useTrackedJavaClassSameModule.kt" // In lib ) ) } @@ -88,24 +107,24 @@ abstract class IncrementalCompilationJavaChangesBase(val usePreciseJavaTracking: override fun defaultProject() = Project("incrementalMultiproject") override fun defaultBuildOptions() = super.defaultBuildOptions().copy(usePreciseJavaTracking = usePreciseJavaTracking) - protected val trackedJavaClass = "TrackedJavaClass.java" - protected val javaClass = "JavaClass.java" - protected val changeBody: (String) -> String = { it.replace("Hello, World!", "Hello, World!!!!") } - protected val changeSignature: (String) -> String = { it.replace("String getString", "Object getString") } + protected val javaClassInLib = "JavaClass.java" + protected val trackedJavaClassInLib = "TrackedJavaClass.java" + protected val changeMethodSignature: (String) -> String = { it.replace("String getString", "Object getString") } + protected val changeMethodBody: (String) -> String = { it.replace("Hello, World!", "Hello, World!!!!") } @Test open fun testAbiChangeInLib_changeMethodSignature() { doTest( - javaClass, changeSignature, - expectedCompiledFileNames = listOf("JavaClassChild.kt", "useJavaClass.kt", "useJavaClassFooMethodUsage.kt") + javaClassInLib, changeMethodSignature, + expectedCompiledFileNames = listOf("JavaClassChild.kt", "useJavaClass.kt", "useJavaClassFooMethodUsage.kt") // In app ) } @Test open fun testNonAbiChangeInLib_changeMethodBody() { doTest( - javaClass, changeBody, - expectedCompiledFileNames = listOf("JavaClassChild.kt", "useJavaClass.kt", "useJavaClassFooMethodUsage.kt") + javaClassInLib, changeMethodBody, + expectedCompiledFileNames = listOf("JavaClassChild.kt", "useJavaClass.kt", "useJavaClassFooMethodUsage.kt") // In app ) }