From 6e8596f7e9465fb9be51e8b3e5e63f0289849292 Mon Sep 17 00:00:00 2001 From: Vladimir Krivosheev Date: Sat, 20 Dec 2025 15:30:57 +0100 Subject: [PATCH] IJPL-224042 IJ-MR-181153 fix PluginDependencyGeneratorTest GitOrigin-RevId: 9bf46caa09a523952c3c1a604e664ac38753d9f1 --- .../api/src/ModuleOutputProvider.kt | 5 +++ .../src/discovery/ProductGeneration.kt | 6 +++- .../src/validation/ValidationModels.kt | 10 ------ .../rules/LibraryModuleValidation.kt | 6 +--- .../testSrc/ProductModulesContentSpecTest.kt | 4 +++ .../PluginDependencyGeneratorTest.kt | 36 +++++++++++++++---- .../build/impl/BazelModuleOutputProvider.kt | 8 +++++ .../build/impl/JpsModuleOutputProvider.kt | 8 +++++ .../PackagingContentChecker.kt | 23 ++++++------ 9 files changed, 73 insertions(+), 33 deletions(-) diff --git a/platform/build-scripts/api/src/ModuleOutputProvider.kt b/platform/build-scripts/api/src/ModuleOutputProvider.kt index 0df715908ba0..eacdfb716a9e 100644 --- a/platform/build-scripts/api/src/ModuleOutputProvider.kt +++ b/platform/build-scripts/api/src/ModuleOutputProvider.kt @@ -10,6 +10,11 @@ interface ModuleOutputProvider { fun findModule(name: String): JpsModule? + /** + * Returns the path to the module's .iml file. + */ + fun getModuleImlFile(module: JpsModule): Path + fun findRequiredModule(name: String): JpsModule fun findLibraryRoots(libraryName: String, moduleLibraryModuleName: String? = null): List diff --git a/platform/build-scripts/product-dsl/src/discovery/ProductGeneration.kt b/platform/build-scripts/product-dsl/src/discovery/ProductGeneration.kt index b1642406c811..719563cf62c9 100644 --- a/platform/build-scripts/product-dsl/src/discovery/ProductGeneration.kt +++ b/platform/build-scripts/product-dsl/src/discovery/ProductGeneration.kt @@ -228,7 +228,11 @@ data class GenerationResult( @JvmField val errors: List, @JvmField val diffs: List, @JvmField val stats: GenerationStats, -) +) { + /** Combined list of all validation issues (errors + diffs) */ + val allIssues: List + get() = errors + diffs +} /** * Generates all module sets and products with validation. diff --git a/platform/build-scripts/product-dsl/src/validation/ValidationModels.kt b/platform/build-scripts/product-dsl/src/validation/ValidationModels.kt index 6cbb7bd59f99..144c945104b5 100644 --- a/platform/build-scripts/product-dsl/src/validation/ValidationModels.kt +++ b/platform/build-scripts/product-dsl/src/validation/ValidationModels.kt @@ -132,14 +132,4 @@ internal data class ProductModuleIndex( // endregion -// region Structured Output for Tests -/** - * Result of model generator validation. - * Contains all validation issues (diffs, missing dependencies, etc.) in a unified list. - */ -data class ModelValidationResult( - @JvmField val issues: List, -) - -// endregion diff --git a/platform/build-scripts/product-dsl/src/validation/rules/LibraryModuleValidation.kt b/platform/build-scripts/product-dsl/src/validation/rules/LibraryModuleValidation.kt index 4c6d61c9f0d5..7c91d32f78cb 100644 --- a/platform/build-scripts/product-dsl/src/validation/rules/LibraryModuleValidation.kt +++ b/platform/build-scripts/product-dsl/src/validation/rules/LibraryModuleValidation.kt @@ -13,7 +13,6 @@ import org.jetbrains.jps.model.java.JpsJavaExtensionService import org.jetbrains.jps.model.module.JpsLibraryDependency import org.jetbrains.jps.model.module.JpsModuleDependency import org.jetbrains.jps.model.module.JpsModuleReference -import org.jetbrains.jps.model.serialization.JpsModelSerializationDataService import java.nio.file.Files /** @@ -130,10 +129,7 @@ private fun applyLibraryModuleFix( strategy: FileUpdateStrategy, ) { val module = outputProvider.findModule(moduleName) ?: return - val imlDir = requireNotNull(JpsModelSerializationDataService.getBaseDirectoryPath(module)) { - "Cannot find base directory for module $moduleName" - } - val imlFile = imlDir.resolve("${module.name}.iml") + val imlFile = outputProvider.getModuleImlFile(module) val currentContent = Files.readString(imlFile) val fixedContent = applyLibraryModuleFixes(currentContent, violations) diff --git a/platform/build-scripts/product-dsl/testSrc/ProductModulesContentSpecTest.kt b/platform/build-scripts/product-dsl/testSrc/ProductModulesContentSpecTest.kt index 60e0ddda1e22..c675f7c59199 100644 --- a/platform/build-scripts/product-dsl/testSrc/ProductModulesContentSpecTest.kt +++ b/platform/build-scripts/product-dsl/testSrc/ProductModulesContentSpecTest.kt @@ -424,4 +424,8 @@ private class MockModuleOutputProvider : ModuleOutputProvider { override fun getModuleOutputRoots(module: JpsModule, forTests: Boolean): List { throw UnsupportedOperationException("Not available in mock") } + + override fun getModuleImlFile(module: JpsModule): Path { + throw UnsupportedOperationException("Not available in mock") + } } \ No newline at end of file diff --git a/platform/build-scripts/product-dsl/testSrc/dependency/PluginDependencyGeneratorTest.kt b/platform/build-scripts/product-dsl/testSrc/dependency/PluginDependencyGeneratorTest.kt index 46efb52a6999..360d7413e89f 100644 --- a/platform/build-scripts/product-dsl/testSrc/dependency/PluginDependencyGeneratorTest.kt +++ b/platform/build-scripts/product-dsl/testSrc/dependency/PluginDependencyGeneratorTest.kt @@ -12,6 +12,7 @@ import org.jetbrains.jps.model.java.JpsJavaExtensionService import org.jetbrains.jps.model.java.JpsJavaLibraryType import org.jetbrains.jps.model.java.JpsJavaModuleType import org.jetbrains.jps.model.module.JpsModule +import org.jetbrains.jps.model.serialization.JpsModelSerializationDataService import org.jetbrains.jps.model.serialization.impl.JpsModuleSerializationDataExtensionImpl import org.junit.jupiter.api.Test import org.junit.jupiter.api.io.TempDir @@ -239,7 +240,18 @@ class PluginDependencyGeneratorTest { @Test fun `validateLibraryModuleDependencies detects direct library dependencies`(@TempDir tempDir: Path) { - // 1. Create test JPS project with modules + // 1. Create test .iml file with direct library dependency + val imlContent = """ + | + | + | + | + | + | + """.trimMargin() + Files.writeString(tempDir.resolve("test.plugin.iml"), imlContent) + + // 2. Create test JPS project with modules val model = JpsElementFactory.getInstance().createModel() val project = model.project @@ -256,15 +268,19 @@ class PluginDependencyGeneratorTest { // Create plugin module with DIRECT library dependency (the violation!) val pluginModule = project.addModule("test.plugin", JpsJavaModuleType.INSTANCE) + pluginModule.container.setChild( + JpsModuleSerializationDataExtensionImpl.ROLE, + JpsModuleSerializationDataExtensionImpl(tempDir), + ) val pluginLibDep = pluginModule.dependenciesList.addLibraryDependency(junit4Library) JpsJavaExtensionService.getInstance().getOrCreateDependencyExtension(pluginLibDep).apply { scope = JpsJavaDependencyScope.TEST } - // 2. Create test ModuleOutputProvider + // 3. Create test ModuleOutputProvider val outputProvider = createTestModuleOutputProvider(project) - // 3. Call validateLibraryModuleDependencies with strategy + // 4. Call validateLibraryModuleDependencies with strategy val strategy = DeferredFileUpdater(tempDir) validateLibraryModuleDependencies( modulesToCheck = setOf("intellij.libraries.junit4", "test.plugin"), @@ -272,11 +288,10 @@ class PluginDependencyGeneratorTest { strategy = strategy, ) - // 4. Verify no diff is returned because mock modules don't have real .iml files on disk - // In real usage, diffs contain the .iml changes needed for auto-fix + // 5. Verify diff is returned for the library dependency violation assertThat(strategy.getDiffs()) - .describedAs("No diffs returned for mock modules without .iml files") - .isEmpty() + .describedAs("Diff should be generated for library dependency violation") + .hasSize(1) } @Test @@ -388,5 +403,12 @@ private fun createTestModuleOutputProvider(project: JpsProject): ModuleOutputPro override suspend fun readFileContentFromModuleOutputAsync(module: JpsModule, relativePath: String, forTests: Boolean): ByteArray? { throw UnsupportedOperationException("Not needed for this test") } + + override fun getModuleImlFile(module: JpsModule): Path { + val baseDir = requireNotNull(JpsModelSerializationDataService.getBaseDirectoryPath(module)) { + "Cannot find base directory for module ${module.name}" + } + return baseDir.resolve("${module.name}.iml") + } } } diff --git a/platform/build-scripts/src/org/jetbrains/intellij/build/impl/BazelModuleOutputProvider.kt b/platform/build-scripts/src/org/jetbrains/intellij/build/impl/BazelModuleOutputProvider.kt index bfd9953fb82e..32fd982b1613 100644 --- a/platform/build-scripts/src/org/jetbrains/intellij/build/impl/BazelModuleOutputProvider.kt +++ b/platform/build-scripts/src/org/jetbrains/intellij/build/impl/BazelModuleOutputProvider.kt @@ -9,6 +9,7 @@ import org.jetbrains.intellij.build.ModuleOutputProvider import org.jetbrains.intellij.build.io.ZipEntryProcessorResult import org.jetbrains.intellij.build.io.readZipFile import org.jetbrains.jps.model.module.JpsModule +import org.jetbrains.jps.model.serialization.JpsModelSerializationDataService import java.nio.file.Files import java.nio.file.Path import kotlin.io.path.isRegularFile @@ -125,6 +126,13 @@ internal class BazelModuleOutputProvider( ) } + override fun getModuleImlFile(module: JpsModule): Path { + val baseDir = requireNotNull(JpsModelSerializationDataService.getBaseDirectoryPath(module)) { + "Cannot find base directory for module ${module.name}" + } + return baseDir.resolve("${module.name}.iml") + } + override fun toString(): String = "BazelModuleOutputProvider(projectHome=$projectHome, bazelOutputRoot=$bazelOutputRoot)" } diff --git a/platform/build-scripts/src/org/jetbrains/intellij/build/impl/JpsModuleOutputProvider.kt b/platform/build-scripts/src/org/jetbrains/intellij/build/impl/JpsModuleOutputProvider.kt index fc27e3287e80..40394a2c1f76 100644 --- a/platform/build-scripts/src/org/jetbrains/intellij/build/impl/JpsModuleOutputProvider.kt +++ b/platform/build-scripts/src/org/jetbrains/intellij/build/impl/JpsModuleOutputProvider.kt @@ -8,6 +8,7 @@ import org.jetbrains.jps.model.JpsProject import org.jetbrains.jps.model.java.JpsJavaExtensionService import org.jetbrains.jps.model.library.JpsOrderRootType import org.jetbrains.jps.model.module.JpsModule +import org.jetbrains.jps.model.serialization.JpsModelSerializationDataService import java.nio.file.Files import java.nio.file.NoSuchFileException import java.nio.file.Path @@ -79,4 +80,11 @@ internal class JpsModuleOutputProvider(private val project: JpsProject) : Module processedModules = processedModules, ) } + + override fun getModuleImlFile(module: JpsModule): Path { + val baseDir = requireNotNull(JpsModelSerializationDataService.getBaseDirectoryPath(module)) { + "Cannot find base directory for module ${module.name}" + } + return baseDir.resolve("${module.name}.iml") + } } diff --git a/platform/build-scripts/testFramework/src/com/intellij/platform/buildScripts/testFramework/distributionContent/PackagingContentChecker.kt b/platform/build-scripts/testFramework/src/com/intellij/platform/buildScripts/testFramework/distributionContent/PackagingContentChecker.kt index 4f9bf21b8200..9a911b498daf 100644 --- a/platform/build-scripts/testFramework/src/com/intellij/platform/buildScripts/testFramework/distributionContent/PackagingContentChecker.kt +++ b/platform/build-scripts/testFramework/src/com/intellij/platform/buildScripts/testFramework/distributionContent/PackagingContentChecker.kt @@ -29,8 +29,8 @@ import org.jetbrains.intellij.build.SoftwareBillOfMaterials import org.jetbrains.intellij.build.impl.buildDistributions import org.jetbrains.intellij.build.impl.createBuildContext import org.jetbrains.intellij.build.impl.createCompilationContext +import org.jetbrains.intellij.build.productLayout.discovery.GenerationResult import org.jetbrains.intellij.build.productLayout.validation.FileDiff -import org.jetbrains.intellij.build.productLayout.validation.ModelValidationResult import org.jetbrains.intellij.build.productLayout.validation.XIncludeResolutionError import org.jetbrains.intellij.build.productLayout.validation.formatValidationError import org.jetbrains.intellij.build.productLayout.validation.getErrorId @@ -60,7 +60,7 @@ private data class ContentReportList( * The function takes the project home path and returns validation result * containing file diffs and validation errors. */ -typealias GeneratorValidator = suspend (projectHome: Path, outputProvider: ModuleOutputProvider) -> ModelValidationResult +typealias GeneratorValidator = suspend (projectHome: Path, outputProvider: ModuleOutputProvider) -> GenerationResult @ApiStatus.Internal fun createContentCheckTests( @@ -91,9 +91,9 @@ fun createContentCheckTests( } // Start both tasks immediately in caller's scope (parallel after context is ready) - val validationDeferred: Deferred = scope.async { + val validationDeferred: Deferred = scope.async { val context = compilationContextDeferred.await() - modelValidator?.invoke(homePath, context.outputProvider) ?: ModelValidationResult(issues = emptyList()) + modelValidator?.invoke(homePath, context.outputProvider) } val packagingDeferred: Deferred = scope.async { @@ -114,11 +114,12 @@ fun createContentCheckTests( @Suppress("RunBlockingInSuspendFunction") val validationResult = runBlocking { validationDeferred.await() } + val validationIssues = validationResult?.allIssues ?: emptyList() - if (validationResult.issues.isNotEmpty()) { + if (validationIssues.isNotEmpty()) { // Check for xi-include errors first - they may cause cascading failures - val xiIncludeErrors = validationResult.issues.filterIsInstance() - for (issue in xiIncludeErrors.ifEmpty { validationResult.issues }) { + val xiIncludeErrors = validationIssues.filterIsInstance() + for (issue in xiIncludeErrors.ifEmpty { validationIssues }) { val testId = if (issue is FileDiff) "file-out-of-sync:${homePath.relativize(issue.path)}" else "model-validation:${getErrorId(issue)}" yield(DynamicTest.dynamicTest(testId) { if (issue is FileDiff) { @@ -148,22 +149,24 @@ fun createContentCheckTests( } private suspend fun SequenceScope.producePackagingTests( - validationResult: ModelValidationResult, + validationResult: GenerationResult?, packagingDeferred: Deferred, contentYamlPath: String, suggestedReviewer: String?, checkPlugins: Boolean, ) { + val issues = validationResult?.allIssues ?: emptyList() + // Packaging - awaits inside test to capture timing yield(DynamicTest.dynamicTest("packaging") { - if (validationResult.issues.isNotEmpty()) { + if (issues.isNotEmpty()) { throw TestAbortedException("Skipped: model validation failed") } runBlocking { packagingDeferred.await() } }) // Content check tests - use packaging result - if (validationResult.issues.isNotEmpty()) { + if (issues.isNotEmpty()) { return }