From 8147aa7dd66649f117a4c0051c3e499efa44a11a Mon Sep 17 00:00:00 2001 From: Dmitriy Dolovov Date: Tue, 28 Mar 2023 13:00:17 +0200 Subject: [PATCH] [PL][tests] Don't compile source->binary directly in Native KLIB ABI tests Avoid using source+KLIB+cache?->binary compilations in KLIB ABI tests. Such compilations are not supported by Native K2 and thus will block migrating ABI tests to K2. --- .../jetbrains/kotlin/klib/KlibABITestUtils.kt | 51 +++++- .../js/test/ir/AbstractJsKLibABITestCase.kt | 11 +- .../blackboxtest/AbstractNativeKlibABITest.kt | 159 +++++++++++------- 3 files changed, 143 insertions(+), 78 deletions(-) diff --git a/compiler/tests-common/tests/org/jetbrains/kotlin/klib/KlibABITestUtils.kt b/compiler/tests-common/tests/org/jetbrains/kotlin/klib/KlibABITestUtils.kt index 82707ec1b43..ecd21f41510 100644 --- a/compiler/tests-common/tests/org/jetbrains/kotlin/klib/KlibABITestUtils.kt +++ b/compiler/tests-common/tests/org/jetbrains/kotlin/klib/KlibABITestUtils.kt @@ -9,6 +9,7 @@ import org.jetbrains.kotlin.codegen.* import org.jetbrains.kotlin.klib.KlibABITestUtils.ModuleBuildDirs.Companion.OUTPUT_DIR_NAME import org.jetbrains.kotlin.klib.KlibABITestUtils.ModuleBuildDirs.Companion.SOURCE_DIR_NAME import org.jetbrains.kotlin.test.testFramework.KtUsefulTestCase +import org.junit.jupiter.api.Assertions.assertTrue import org.junit.jupiter.api.fail import java.io.File @@ -19,21 +20,53 @@ object KlibABITestUtils { val stdlibFile: File val testModeName: String + // Customize the source code of a module before compiling it to a KLIB. + fun customizeModuleSources(moduleName: String, moduleSourceDir: File) = Unit + + // Build a KLIB from a module. fun buildKlib(moduleName: String, moduleSourceDir: File, dependencies: Dependencies, klibFile: File) + + // Build a binary (executable) file given the main KLIB and dependencies. fun buildBinaryAndRun(mainModuleKlibFile: File, dependencies: Dependencies) + // Take measures if the build directory is non-empty before the compilation + // (ex: backup the previously generated artifacts stored in the build directory). fun onNonEmptyBuildDirectory(directory: File) + // A way to check if a test is ignored or not. Override this function if necessary. fun isIgnoredTest(projectInfo: ProjectInfo): Boolean = projectInfo.muted + + // How to handle the test that is known to be ignored. fun onIgnoredTest() } - class Dependencies(val regularDependencies: Set, val friendDependencies: Set) { + data class Dependency(val moduleName: String, val libraryFile: File) + + class Dependencies(val regularDependencies: Set, val friendDependencies: Set) { + init { + regularDependencies.checkNoDuplicates("regular") + regularDependencies.checkNoDuplicates("friend") + } + fun mergeWith(other: Dependencies): Dependencies = Dependencies(regularDependencies + other.regularDependencies, friendDependencies + other.friendDependencies) companion object { val EMPTY = Dependencies(emptySet(), emptySet()) + + private fun Set.checkNoDuplicates(kind: String) { + fun Map>.dump(): String = values.flatten().sortedBy { it.moduleName }.joinToString() + + val duplicatedModules = groupBy { it.moduleName }.filterValues { it.size > 1 } + assertTrue(duplicatedModules.isEmpty()) { + "There are duplicated $kind module dependencies: ${duplicatedModules.dump()}" + } + + val duplicatedFiles = groupBy { it.libraryFile.absolutePath }.filterValues { it.size > 1 } + assertTrue(duplicatedFiles.isEmpty()) { + "There are $kind module dependencies with conflicting paths: ${duplicatedFiles.dump()}" + } + } } } @@ -70,6 +103,8 @@ object KlibABITestUtils { } } + customizeModuleSources(moduleName, moduleBuildDirs.sourceDir) + val moduleOutputDir = moduleBuildDirs.outputDir.apply { mkdirs() } val klibFile = moduleOutputDir.resolve("$moduleName.klib") @@ -99,17 +134,18 @@ object KlibABITestUtils { if (!moduleBuildDirs.outputDir.list().isNullOrEmpty()) onNonEmptyBuildDirectory(moduleBuildDirs.outputDir) - val regularDependencies = hashSetOf() - val friendDependencies = hashSetOf() + val regularDependencies = hashSetOf() + val friendDependencies = hashSetOf() moduleStep.dependencies.forEach { dependency -> if (dependency.moduleName == "stdlib") - regularDependencies += stdlibFile + regularDependencies += Dependency("stdlib", stdlibFile) else { val moduleFile = modulesMap[dependency.moduleName]?.klibFile ?: fail { "No module ${dependency.moduleName} found on step ${projectStep.id}" } - regularDependencies += moduleFile - if (dependency.isFriend) friendDependencies += moduleFile + val moduleDependency = Dependency(dependency.moduleName, moduleFile) + regularDependencies += moduleDependency + if (dependency.isFriend) friendDependencies += moduleDependency } } @@ -121,7 +157,8 @@ object KlibABITestUtils { } val mainModuleKlibFile = modulesMap[MAIN_MODULE_NAME]?.klibFile ?: fail { "No main module $MAIN_MODULE_NAME found" } - binaryDependencies = binaryDependencies.mergeWith(Dependencies(setOf(mainModuleKlibFile), emptySet())) + val mainModuleDependency = Dependency(MAIN_MODULE_NAME, mainModuleKlibFile) + binaryDependencies = binaryDependencies.mergeWith(Dependencies(setOf(mainModuleDependency), emptySet())) buildBinaryAndRun(mainModuleKlibFile, binaryDependencies) } diff --git a/js/js.tests/test/org/jetbrains/kotlin/js/test/ir/AbstractJsKLibABITestCase.kt b/js/js.tests/test/org/jetbrains/kotlin/js/test/ir/AbstractJsKLibABITestCase.kt index d39f7911d30..fe5c7ad75e6 100644 --- a/js/js.tests/test/org/jetbrains/kotlin/js/test/ir/AbstractJsKLibABITestCase.kt +++ b/js/js.tests/test/org/jetbrains/kotlin/js/test/ir/AbstractJsKLibABITestCase.kt @@ -15,7 +15,6 @@ import org.jetbrains.kotlin.cli.common.messages.AnalyzerWithCompilerReport import org.jetbrains.kotlin.cli.js.klib.generateIrForKlibSerialization import org.jetbrains.kotlin.cli.jvm.compiler.EnvironmentConfigFiles import org.jetbrains.kotlin.cli.jvm.compiler.KotlinCoreEnvironment -import org.jetbrains.kotlin.codegen.ProjectInfo import org.jetbrains.kotlin.config.CommonConfigurationKeys import org.jetbrains.kotlin.config.CompilerConfiguration import org.jetbrains.kotlin.descriptors.DeclarationDescriptor @@ -120,8 +119,8 @@ abstract class AbstractJsKLibABITestCase : KtUsefulTestCase() { environment.project, ktFiles, config, - dependencies.regularDependencies.map { it.path }, - dependencies.friendDependencies.map { it.path }, + dependencies.regularDependencies.map { it.libraryFile.path }, + dependencies.friendDependencies.map { it.libraryFile.path }, AnalyzerWithCompilerReport(config) ) @@ -182,7 +181,7 @@ abstract class AbstractJsKLibABITestCase : KtUsefulTestCase() { // TODO: what about friend dependencies? val cacheUpdater = CacheUpdater( mainModule = mainModuleKlibFile.absolutePath, - allModules = allDependencies.regularDependencies.map { it.path }, + allModules = allDependencies.regularDependencies.map { it.libraryFile.path }, mainModuleFriends = emptyList(), cacheDir = buildDir.resolve("libs-cache").absolutePath, compilerConfiguration = configuration, @@ -216,8 +215,8 @@ abstract class AbstractJsKLibABITestCase : KtUsefulTestCase() { environment.project, klib, configuration, - allDependencies.regularDependencies.map { it.path }, - allDependencies.friendDependencies.map { it.path } + allDependencies.regularDependencies.map { it.libraryFile.path }, + allDependencies.friendDependencies.map { it.libraryFile.path } ) val ir = compile( diff --git a/native/native.tests/tests/org/jetbrains/kotlin/konan/blackboxtest/AbstractNativeKlibABITest.kt b/native/native.tests/tests/org/jetbrains/kotlin/konan/blackboxtest/AbstractNativeKlibABITest.kt index 30240274e38..2645f97a377 100644 --- a/native/native.tests/tests/org/jetbrains/kotlin/konan/blackboxtest/AbstractNativeKlibABITest.kt +++ b/native/native.tests/tests/org/jetbrains/kotlin/konan/blackboxtest/AbstractNativeKlibABITest.kt @@ -7,11 +7,13 @@ package org.jetbrains.kotlin.konan.blackboxtest import com.intellij.testFramework.TestDataFile import org.jetbrains.kotlin.klib.KlibABITestUtils +import org.jetbrains.kotlin.klib.KlibABITestUtils.Dependencies +import org.jetbrains.kotlin.klib.KlibABITestUtils.MAIN_MODULE_NAME import org.jetbrains.kotlin.konan.blackboxtest.support.* import org.jetbrains.kotlin.konan.blackboxtest.support.TestCase.WithTestRunnerExtras import org.jetbrains.kotlin.konan.blackboxtest.support.compilation.* import org.jetbrains.kotlin.konan.blackboxtest.support.compilation.TestCompilationArtifact.* -import org.jetbrains.kotlin.konan.blackboxtest.support.compilation.TestCompilationDependencyType.Library +import org.jetbrains.kotlin.konan.blackboxtest.support.compilation.TestCompilationDependencyType.* import org.jetbrains.kotlin.konan.blackboxtest.support.compilation.TestCompilationResult.Companion.assertSuccess import org.jetbrains.kotlin.konan.blackboxtest.support.runner.TestExecutable import org.jetbrains.kotlin.konan.blackboxtest.support.runner.TestRunChecks @@ -23,8 +25,6 @@ import java.io.File @Tag("klib-abi") abstract class AbstractNativeKlibABITest : AbstractNativeSimpleTest() { - private val producedKlibs = linkedMapOf() // IMPORTANT: The order makes sense! - private inner class NativeTestConfiguration(testPath: String) : KlibABITestUtils.TestConfiguration { override val testDir = getAbsoluteFile(testPath) override val buildDir get() = this@AbstractNativeKlibABITest.buildDir @@ -40,10 +40,15 @@ abstract class AbstractNativeKlibABITest : AbstractNativeSimpleTest() { "NATIVE_CACHE_${cacheModeAlias}" } - override fun buildKlib(moduleName: String, moduleSourceDir: File, dependencies: KlibABITestUtils.Dependencies, klibFile: File) = + override fun customizeModuleSources(moduleName: String, moduleSourceDir: File) { + if (moduleName == MAIN_MODULE_NAME) + customizeMainModuleSources(moduleSourceDir) + } + + override fun buildKlib(moduleName: String, moduleSourceDir: File, dependencies: Dependencies, klibFile: File) = this@AbstractNativeKlibABITest.buildKlib(moduleName, moduleSourceDir, dependencies, klibFile) - override fun buildBinaryAndRun(mainModuleKlibFile: File, dependencies: KlibABITestUtils.Dependencies) = + override fun buildBinaryAndRun(mainModuleKlibFile: File, dependencies: Dependencies) = this@AbstractNativeKlibABITest.buildBinaryAndRun(dependencies) override fun onNonEmptyBuildDirectory(directory: File) = backupDirectoryContents(directory) @@ -51,59 +56,67 @@ abstract class AbstractNativeKlibABITest : AbstractNativeSimpleTest() { override fun onIgnoredTest() = throw TestAbortedException() } + private class ProducedKlib(val moduleName: String, val klibArtifact: KLIB, val dependencies: Dependencies) { + override fun equals(other: Any?) = (other as? ProducedKlib)?.moduleName == moduleName + override fun hashCode() = moduleName.hashCode() + } + + private val producedKlibs = linkedSetOf() // IMPORTANT: The order makes sense! + + private val executableArtifact: Executable by lazy { + val (_, outputDir) = KlibABITestUtils.createModuleDirs(buildDir, LAUNCHER_MODULE_NAME) + val executableFile = outputDir.resolve("app." + testRunSettings.get().testTarget.family.exeSuffix) + Executable(executableFile) + } + // The entry point to generated test classes. protected fun runTest(@TestDataFile testPath: String) = KlibABITestUtils.runTest(NativeTestConfiguration(testPath)) - private fun buildKlib(moduleName: String, moduleSourceDir: File, dependencies: KlibABITestUtils.Dependencies, klibFile: File) { - val module = createModule(moduleName) - moduleSourceDir.walk() - .filter { file -> file.isFile && file.extension == "kt" } - .forEach { file -> module.files += TestFile.createCommitted(file, module) } - - val testCase = createTestCase(module, COMPILER_ARGS_FOR_KLIB) + private fun customizeMainModuleSources(moduleSourceDir: File) { + // Add a "box" function launcher to the main module. + moduleSourceDir.resolve(LAUNCHER_FILE_NAME).writeText(generateBoxFunctionLauncher("box")) + } + private fun buildKlib(moduleName: String, moduleSourceDir: File, dependencies: Dependencies, klibFile: File) { val klibArtifact = KLIB(klibFile) + + val testCase = createTestCase(moduleName, moduleSourceDir, COMPILER_ARGS_FOR_KLIB) + val compilation = LibraryCompilation( settings = testRunSettings, freeCompilerArgs = testCase.freeCompilerArgs, sourceModules = testCase.modules, - dependencies = createLibraryDependencies(dependencies), + dependencies = createLibraryDependencies(dependencies, forExecutable = false), expectedArtifact = klibArtifact ) compilation.result.assertSuccess() // <-- trigger compilation - producedKlibs[klibArtifact] = dependencies // Remember the artifact with its dependencies. + producedKlibs += ProducedKlib(moduleName, klibArtifact, dependencies) // Remember the artifact with its dependencies. } - private fun buildBinaryAndRun(allDependencies: KlibABITestUtils.Dependencies) { + private fun buildBinaryAndRun(allDependencies: Dependencies) { val cacheDependencies = if (staticCacheRequiredForEveryLibrary) { - producedKlibs.map { (klibArtifact, moduleDependencies) -> - buildCacheForKlib(moduleDependencies, klibArtifact) - klibArtifact.toStaticCacheArtifact().toDependency() + producedKlibs.map { producedKlib -> + buildCacheForKlib(producedKlib) + producedKlib.klibArtifact.toStaticCacheArtifact().toDependency() } } else emptyList() - val (sourceDir, outputDir) = KlibABITestUtils.createModuleDirs(buildDir, LAUNCHER_MODULE_NAME) - val executableFile = outputDir.resolve("app." + testRunSettings.get().testTarget.family.exeSuffix) - - val module = createModule(LAUNCHER_MODULE_NAME) - module.files += TestFile.createUncommitted( - location = sourceDir.resolve(LAUNCHER_FILE_NAME), - module = module, - text = generateBoxFunctionLauncher("box") + val testCase = createTestCase( + moduleName = LAUNCHER_MODULE_NAME, + moduleSourceDir = null, // No sources. + compilerArgs = COMPILER_ARGS_FOR_STATIC_CACHE_AND_EXECUTABLE ) - val testCase = createTestCase(module, COMPILER_ARGS_FOR_STATIC_CACHE_AND_EXECUTABLE) - val compilation = ExecutableCompilation( settings = testRunSettings, freeCompilerArgs = testCase.freeCompilerArgs, sourceModules = testCase.modules, extras = testCase.extras, - dependencies = createLibraryDependencies(allDependencies) + cacheDependencies, - expectedArtifact = Executable(executableFile) + dependencies = createLibraryDependencies(allDependencies, forExecutable = true) + cacheDependencies, + expectedArtifact = executableArtifact ) val compilationResult = compilation.result.assertSuccess() // <-- trigger compilation @@ -112,54 +125,70 @@ abstract class AbstractNativeKlibABITest : AbstractNativeSimpleTest() { runExecutableAndVerify(testCase, executable) // <-- run executable and verify } - private fun buildCacheForKlib(moduleDependencies: KlibABITestUtils.Dependencies, klibArtifact: KLIB) { + private fun buildCacheForKlib(producedKlib: ProducedKlib) { val compilation = StaticCacheCompilation( settings = testRunSettings, freeCompilerArgs = COMPILER_ARGS_FOR_STATIC_CACHE_AND_EXECUTABLE, - options = StaticCacheCompilation.Options.Regular, + options = if (producedKlib.moduleName == MAIN_MODULE_NAME) + StaticCacheCompilation.Options.ForIncludedLibraryWithTests(executableArtifact, DEFAULT_EXTRAS) + else + StaticCacheCompilation.Options.Regular, pipelineType = testRunSettings.get(), - dependencies = createLibraryCacheDependencies(moduleDependencies) + klibArtifact.toDependency(), - expectedArtifact = klibArtifact.toStaticCacheArtifact() + dependencies = createLibraryCacheDependencies(producedKlib.dependencies) + producedKlib.klibArtifact.toDependency(), + expectedArtifact = producedKlib.klibArtifact.toStaticCacheArtifact() ) compilation.result.assertSuccess() // <-- trigger compilation } - private fun createModule(moduleName: String) = TestModule.Exclusive( - name = moduleName, - directDependencySymbols = emptySet(), /* Don't need to pass any dependency symbols here. - Dependencies are already handled by the AbstractKlibABITestCase class. */ - directFriendSymbols = emptySet(), - directDependsOnSymbols = emptySet(), - ) - - private fun createTestCase(module: TestModule.Exclusive, compilerArgs: TestCompilerArgs) = TestCase( - id = TestCaseId.Named(module.name), - kind = TestKind.STANDALONE, - modules = setOf(module), - freeCompilerArgs = compilerArgs, - nominalPackageName = PackageName.EMPTY, - checks = TestRunChecks.Default(testRunSettings.get().executionTimeout), - extras = DEFAULT_EXTRAS - ).apply { - initialize(null, null) - } - - private fun createLibraryDependencies(dependencies: KlibABITestUtils.Dependencies): Iterable> = - with(dependencies) { - regularDependencies.map { KLIB(it).toDependency() } + friendDependencies.map { KLIB(it).toFriendDependency() } - } - - private fun createLibraryCacheDependencies(dependencies: KlibABITestUtils.Dependencies): Iterable> = - with(dependencies) { - regularDependencies.mapNotNull { klibFile -> - if (klibFile != stdlibFile) KLIB(klibFile).toStaticCacheArtifact().toDependency() else null + private fun createTestCase(moduleName: String, moduleSourceDir: File?, compilerArgs: TestCompilerArgs): TestCase { + // Note: Don't generate a module if there are no actual sources to compile. + val module: TestModule.Exclusive? = moduleSourceDir?.let { + TestModule.Exclusive( + name = moduleName, + directDependencySymbols = emptySet(), /* Don't need to pass any dependency symbols here. + Dependencies are already handled by the AbstractKlibABITestCase class. */ + directFriendSymbols = emptySet(), + directDependsOnSymbols = emptySet(), + ).also { module -> + moduleSourceDir.walk() + .filter { file -> file.isFile && file.extension == "kt" } + .forEach { file -> module.files += TestFile.createCommitted(file, module) } } } + return TestCase( + id = TestCaseId.Named(moduleName), + kind = TestKind.STANDALONE, + modules = setOfNotNull(module), + freeCompilerArgs = compilerArgs, + nominalPackageName = PackageName.EMPTY, + checks = TestRunChecks.Default(testRunSettings.get().executionTimeout), + extras = DEFAULT_EXTRAS + ).apply { + initialize(null, null) + } + } + + private fun createLibraryDependencies( + dependencies: Dependencies, + forExecutable: Boolean + ): Iterable> = + dependencies.regularDependencies.map { dependency -> + val klib = KLIB(dependency.libraryFile) + if (forExecutable && dependency.moduleName == MAIN_MODULE_NAME) klib.toIncludedDependency() else klib.toDependency() + } + dependencies.friendDependencies.map { KLIB(it.libraryFile).toFriendDependency() } + + private fun createLibraryCacheDependencies( + dependencies: Dependencies + ): Iterable> = dependencies.regularDependencies.mapNotNull { dependency -> + if (dependency.libraryFile != stdlibFile) KLIB(dependency.libraryFile).toStaticCacheArtifact().toDependency() else null + } + private fun KLIB.toDependency() = ExistingDependency(this, Library) - private fun KLIB.toFriendDependency() = ExistingDependency(this, TestCompilationDependencyType.FriendLibrary) - private fun KLIBStaticCache.toDependency() = ExistingDependency(this, TestCompilationDependencyType.LibraryStaticCache) + private fun KLIB.toIncludedDependency() = ExistingDependency(this, IncludedLibrary) + private fun KLIB.toFriendDependency() = ExistingDependency(this, FriendLibrary) + private fun KLIBStaticCache.toDependency() = ExistingDependency(this, LibraryStaticCache) private fun KLIB.toStaticCacheArtifact() = KLIBStaticCache( cacheDir = klibFile.parentFile.resolve(STATIC_CACHE_DIR_NAME).apply { mkdirs() },