From 22a2f03219cd09140ba41f579903328830008d27 Mon Sep 17 00:00:00 2001 From: Zalim Bashorov Date: Wed, 28 Oct 2015 21:49:10 +0300 Subject: [PATCH] Added "touch" command to incremental tests and implemented; Implemented more accurate checking lookups after modifications Original commit: 24037c8bf4b26400d4a0a9315607032c92576a4f --- .../jps/build/AbstractIncrementalJpsTest.kt | 94 +++++++++++++------ .../jps/build/AbstractLookupTrackerTest.kt | 24 +---- 2 files changed, 69 insertions(+), 49 deletions(-) diff --git a/jps/jps-plugin/test/org/jetbrains/kotlin/jps/build/AbstractIncrementalJpsTest.kt b/jps/jps-plugin/test/org/jetbrains/kotlin/jps/build/AbstractIncrementalJpsTest.kt index 36d4b075bd9..2599ef866f5 100644 --- a/jps/jps-plugin/test/org/jetbrains/kotlin/jps/build/AbstractIncrementalJpsTest.kt +++ b/jps/jps-plugin/test/org/jetbrains/kotlin/jps/build/AbstractIncrementalJpsTest.kt @@ -69,6 +69,10 @@ public abstract class AbstractIncrementalJpsTest( val TEMP_DIRECTORY_TO_USE = File(FileUtilRt.getTempDirectory()) val DEBUG_LOGGING_ENABLED = System.getProperty("debug.logging.enabled") == "true" + + private val COMMANDS = listOf("new", "touch", "delete") + private val COMMANDS_AS_REGEX_PART = COMMANDS.joinToString("|") + private val COMMANDS_AS_MESSAGE_PART = COMMANDS.joinToString("/") { "\".$it\"" } } protected var testDataDir: File by Delegates.notNull() @@ -77,6 +81,8 @@ public abstract class AbstractIncrementalJpsTest( protected var projectDescriptor: ProjectDescriptor by Delegates.notNull() + protected val mapWorkingToOriginalFile: MutableMap = hashMapOf() + private fun enableDebugLogging() { com.intellij.openapi.diagnostic.Logger.setFactory(javaClass()) TestLoggerFactory.dumpLogToStdout("") @@ -108,10 +114,10 @@ public abstract class AbstractIncrementalJpsTest( protected open fun createLookupTracker(): LookupTracker = LookupTracker.DO_NOTHING - protected open fun checkLookups(modifications: List, @Suppress("UNUSED_PARAMETER") lookupTracker: LookupTracker) { + protected open fun checkLookups(@Suppress("UNUSED_PARAMETER") lookupTracker: LookupTracker, compiledFiles: Set) { } - private fun build(scope: CompileScopeTestBuilder = CompileScopeTestBuilder.make().all(), modifications: List, checkLookups: Boolean = true): MakeResult { + private fun build(scope: CompileScopeTestBuilder = CompileScopeTestBuilder.make().all(), checkLookups: Boolean = true): MakeResult { val workDirPath = FileUtil.toSystemIndependentName(workDir.absolutePath) val logger = MyLogger(workDirPath) projectDescriptor = createProjectDescriptor(BuildLoggingManager(logger)) @@ -127,7 +133,7 @@ public abstract class AbstractIncrementalJpsTest( builder.build(scope.build(), false) if (checkLookups) { - checkLookups(modifications, lookupTracker) + checkLookups(lookupTracker, logger.compiledFiles) } if (!buildResult.isSuccessful) { @@ -150,7 +156,7 @@ public abstract class AbstractIncrementalJpsTest( } private fun initialMake(): MakeResult { - val makeResult = build(modifications = emptyList()) + val makeResult = build() val initBuildLogFile = File(testDataDir, "init-build.log") if (initBuildLogFile.exists()) { @@ -163,17 +169,17 @@ public abstract class AbstractIncrementalJpsTest( return makeResult } - private fun make(modifications: List): MakeResult { - return build(modifications = modifications) + private fun make(): MakeResult { + return build() } private fun rebuild(): MakeResult { - return build(CompileScopeTestBuilder.rebuild().allModules(), emptyList(), checkLookups = false) + return build(CompileScopeTestBuilder.rebuild().allModules(), checkLookups = false) } private fun getModificationsToPerform(moduleNames: Collection?): List> { - fun getModificationsForIteration(newSuffix: String, deleteSuffix: String): List { + fun getModificationsForIteration(newSuffix: String, touchSuffix: String, deleteSuffix: String): List { fun getDirPrefix(fileName: String): String { val underscore = fileName.indexOf("_") @@ -198,6 +204,9 @@ public abstract class AbstractIncrementalJpsTest( if (fileName.endsWith(newSuffix)) { modifications.add(ModifyContent(getDirPrefix(fileName) + "/" + fileName.removeSuffix(newSuffix), file)) } + if (fileName.endsWith(touchSuffix)) { + modifications.add(TouchFile(getDirPrefix(fileName) + "/" + fileName.removeSuffix(touchSuffix))) + } if (fileName.endsWith(deleteSuffix)) { modifications.add(DeleteFile(getDirPrefix(fileName) + "/" + fileName.removeSuffix(deleteSuffix))) } @@ -205,27 +214,27 @@ public abstract class AbstractIncrementalJpsTest( return modifications } - val haveFilesWithoutNumbers = testDataDir.listFiles { it.getName().matches(".+\\.(new|delete)$".toRegex()) }?.isNotEmpty() ?: false - val haveFilesWithNumbers = testDataDir.listFiles { it.getName().matches(".+\\.(new|delete)\\.\\d+$".toRegex()) }?.isNotEmpty() ?: false + val haveFilesWithoutNumbers = testDataDir.listFiles { it.getName().matches(".+\\.($COMMANDS_AS_REGEX_PART)$".toRegex()) }?.isNotEmpty() ?: false + val haveFilesWithNumbers = testDataDir.listFiles { it.getName().matches(".+\\.($COMMANDS_AS_REGEX_PART)\\.\\d+$".toRegex()) }?.isNotEmpty() ?: false if (haveFilesWithoutNumbers && haveFilesWithNumbers) { - fail("Bad test data format: files ending with both unnumbered and numbered \".new\"/\".delete\" were found") + fail("Bad test data format: files ending with both unnumbered and numbered $COMMANDS_AS_MESSAGE_PART were found") } if (!haveFilesWithoutNumbers && !haveFilesWithNumbers) { if (allowNoFilesWithSuffixInTestData) { return listOf(listOf()) } else { - fail("Bad test data format: no files ending with \".new\" or \".delete\" found") + fail("Bad test data format: no files ending with $COMMANDS_AS_MESSAGE_PART found") } } if (haveFilesWithoutNumbers) { - return listOf(getModificationsForIteration(".new", ".delete")) + return listOf(getModificationsForIteration(".new", ".touch", ".delete")) } else { return (1..10) - .map { getModificationsForIteration(".new.$it", ".delete.$it") } + .map { getModificationsForIteration(".new.$it", ".touch.$it", ".delete.$it") } .filter { it.isNotEmpty() } } } @@ -361,7 +370,7 @@ public abstract class AbstractIncrementalJpsTest( val modifications = getModificationsToPerform(moduleNames) for (step in modifications) { - step.forEach { it.perform(workDir) } + step.forEach { it.perform(workDir, mapWorkingToOriginalFile) } performAdditionalModifications(step) if (moduleNames == null) { preProcessSources(File(workDir, "src")) @@ -370,7 +379,7 @@ public abstract class AbstractIncrementalJpsTest( moduleNames.forEach { preProcessSources(File(workDir, "$it/src")) } } - results.add(make(step)) + results.add(make()) } return results } @@ -380,6 +389,16 @@ public abstract class AbstractIncrementalJpsTest( // null means one module private fun configureModules(): Set? { + + fun prepareSources(relativePathToSrc: String, filePrefix: String) { + val srcDir = File(workDir, relativePathToSrc) + FileUtil.copyDir(testDataDir, srcDir) { it.name.startsWith(filePrefix) && (it.name.endsWith(".kt") || it.name.endsWith(".java")) } + + srcDir.walk().forEach { mapWorkingToOriginalFile[it] = File(testDataDir, filePrefix + it.name) } + + preProcessSources(srcDir) + } + var moduleNames: Set? JpsJavaExtensionService.getInstance().getOrCreateProjectExtension(myProject).outputUrl = JpsPathUtil.pathToUrl(getAbsolutePath("out")) @@ -388,10 +407,7 @@ public abstract class AbstractIncrementalJpsTest( if (moduleDependencies == null) { addModule("module", arrayOf(getAbsolutePath("src")), null, null, jdk) - val srcDir = File(workDir, "src") - FileUtil.copyDir(testDataDir, srcDir, { it.getName().endsWith(".kt") || it.getName().endsWith(".java") }) - - preProcessSources(srcDir) + prepareSources(relativePathToSrc = "src", filePrefix = "") moduleNames = null } @@ -410,12 +426,7 @@ public abstract class AbstractIncrementalJpsTest( for (module in nameToModule.values()) { val moduleName = module.name - - val srcDir = File(workDir, "$moduleName/src") - FileUtil.copyDir(testDataDir, srcDir, - { it.getName().startsWith(moduleName + "_") && (it.getName().endsWith(".kt") || it.getName().endsWith(".java")) }) - - preProcessSources(srcDir) + prepareSources(relativePathToSrc = "$moduleName/src", filePrefix = moduleName + "_") } moduleNames = nameToModule.keySet() @@ -429,26 +440,37 @@ public abstract class AbstractIncrementalJpsTest( override fun doGetProjectDir(): File? = workDir + // TODO replace with org.jetbrains.jps.builders.TestProjectBuilderLogger private class MyLogger(val rootPath: String) : ProjectBuilderLoggerBase() { private val logBuf = StringBuilder() public val log: String get() = logBuf.toString() + val compiledFiles = hashSetOf() + override fun isEnabled(): Boolean = true + override fun logCompiledFiles(files: MutableCollection?, builderName: String?, description: String?) { + super.logCompiledFiles(files, builderName, description) + + if (builderName == KotlinBuilder.KOTLIN_BUILDER_NAME) { + compiledFiles.addAll(files!!) + } + } + override fun logLine(message: String?) { logBuf.append(JetTestUtils.replaceHashWithStar(message!!.removePrefix("$rootPath/"))).append('\n') } } protected abstract class Modification(val path: String) { - abstract fun perform(workDir: File) + abstract fun perform(workDir: File, mapping: MutableMap) override fun toString(): String = "${javaClass.simpleName} $path" } protected class ModifyContent(path: String, val dataFile: File) : Modification(path) { - override fun perform(workDir: File) { + override fun perform(workDir: File, mapping: MutableMap) { val file = File(workDir, path) val oldLastModified = file.lastModified() @@ -460,15 +482,29 @@ public abstract class AbstractIncrementalJpsTest( //Mac OS and some versions of Linux truncate timestamp to nearest second file.setLastModified(oldLastModified + 1000) } + + mapping[file] = dataFile + } + } + + protected class TouchFile(path: String) : Modification(path) { + override fun perform(workDir: File, mapping: MutableMap) { + val file = File(workDir, path) + + val oldLastModified = file.lastModified() + //Mac OS and some versions of Linux truncate timestamp to nearest second + file.setLastModified(Math.max(System.currentTimeMillis(), oldLastModified + 1000)) } } protected class DeleteFile(path: String) : Modification(path) { - override fun perform(workDir: File) { + override fun perform(workDir: File, mapping: MutableMap) { val fileToDelete = File(workDir, path) if (!fileToDelete.delete()) { throw AssertionError("Couldn't delete $fileToDelete") } + + mapping.remove(fileToDelete) } } } diff --git a/jps/jps-plugin/test/org/jetbrains/kotlin/jps/build/AbstractLookupTrackerTest.kt b/jps/jps-plugin/test/org/jetbrains/kotlin/jps/build/AbstractLookupTrackerTest.kt index 44646a79e42..9e70025c019 100644 --- a/jps/jps-plugin/test/org/jetbrains/kotlin/jps/build/AbstractLookupTrackerTest.kt +++ b/jps/jps-plugin/test/org/jetbrains/kotlin/jps/build/AbstractLookupTrackerTest.kt @@ -36,11 +36,10 @@ abstract class AbstractLookupTrackerTest : AbstractIncrementalJpsTest( override fun createLookupTracker(): LookupTracker = TestLookupTracker() - override fun checkLookups(modifications: List, lookupTracker: LookupTracker) { + override fun checkLookups(lookupTracker: LookupTracker, compiledFiles: Set) { if (lookupTracker !is TestLookupTracker) throw AssertionError("Expected TestLookupTracker, but: ${lookupTracker.javaClass}") val fileToLookups = lookupTracker.lookups.groupBy { it.lookupContainingFile } - val workSrcDir = File(workDir, "src") fun checkLookupsInFile(expectedFile: File, actualFile: File) { @@ -96,25 +95,10 @@ abstract class AbstractLookupTrackerTest : AbstractIncrementalJpsTest( JetTestUtils.assertEqualsToFile(expectedFile, actual) } - if (modifications.isNotEmpty()) { - for (modification in modifications) { - if (modification !is ModifyContent) continue + for (actualFile in compiledFiles) { + val expectedFile = mapWorkingToOriginalFile[actualFile]!! - val expectedFile = modification.dataFile - val actualFile = File(workDir, modification.path) - - checkLookupsInFile(expectedFile, actualFile) - } - } - else { - for (actualFile in workSrcDir.walkTopDown()) { - if (!actualFile.isFile) continue - - val independentFilePath = FileUtil.toSystemIndependentName(actualFile.path) - val expectedFile = File(testDataDir, independentFilePath.replace(".*/src/".toRegex(), "")) - - checkLookupsInFile(expectedFile, actualFile) - } + checkLookupsInFile(expectedFile, actualFile) } }