gnodet-bot commented on code in PR #13723:
URL: https://github.com/apache/maven/pull/13723#discussion_r4381565956
##########
impl/maven-impl/src/test/java/org/apache/maven/impl/model/DefaultDependencyManagementImporterTest.java:
##########
@@ -179,9 +177,18 @@ void
testImportManagementInheritsVersionFromImportedBomWhenLocalEntryHasNoVersio
assertEquals(1, resultDeps.size());
Dependency merged = resultDeps.get(0);
+
+ // Verify that the BOM supplies the missing version.
assertEquals("org.junit.jupiter", merged.getGroupId());
assertEquals("junit-jupiter-api", merged.getArtifactId());
assertEquals("5.10.0", merged.getVersion());
+
+ // Verify that the locally declared scope is preserved.
assertEquals("provided", merged.getScope());
+
+ // Verify that the locally declared exclusion is preserved.
+ assertEquals(1, merged.getExclusions().size());
+ assertEquals("org.junit.vintage",
merged.getExclusions().get(0).getGroupId());
+ assertEquals("junit-vintage-engine",
merged.getExclusions().get(0).getArtifactId());
Review Comment:
⚠️ **Pre-fix validation required**: These exclusion assertions likely pass
even without any production code fix.
The `impl` `DefaultDependencyManagementImporter.importManagement()` already
handles this case correctly — its merge path for version-from-BOM uses:
```java
Dependency.newBuilder(present)
.version(dependency.getVersion())
.location("version", dependency.getLocation("version"))
.build()
```
`newBuilder(present)` copies **all fields** from the local entry, including
`exclusions`. So exclusions are already preserved — the code was never broken
in the `impl` layer.
The actual bug exists in the **compat layer**
(`compat/maven-model-builder/composition/DefaultDependencyManagementImporter`),
which uses a mutable clone-and-set approach that does not suffer from this
issue in the same way. This PR does not touch that code.
Please confirm the test was **failing** before this PR by running it on the
base branch:
```bash
mvn test -pl impl/maven-impl
-Dtest=DefaultDependencyManagementImporterTest#testImportManagementInheritsVersionFromImportedBomWhenLocalEntryHasNoVersion
-B 2>&1 | tail -10
```
If the test passes on the base branch without any production code change,
these assertions do not guard against a real regression and must be reworked to
target the actual broken behaviour.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
