Copilot commented on code in PR #12632:
URL: https://github.com/apache/maven/pull/12632#discussion_r3683449793
##########
impl/maven-impl/src/test/java/org/apache/maven/impl/model/DefaultInheritanceAssemblerTest.java:
##########
@@ -18,81 +18,81 @@
*/
package org.apache.maven.impl.model;
-import java.nio.file.Files;
-import java.nio.file.Path;
import java.nio.file.Paths;
+import java.util.List;
import org.apache.maven.api.model.Model;
-import org.apache.maven.api.services.xml.XmlReaderRequest;
-import org.apache.maven.api.services.xml.XmlWriterRequest;
-import org.apache.maven.impl.DefaultModelXmlFactory;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
-import org.xmlunit.builder.DiffBuilder;
-import org.xmlunit.diff.Diff;
-import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
class DefaultInheritanceAssemblerTest {
- private DefaultModelXmlFactory xmlFactory;
-
private DefaultInheritanceAssembler assembler;
@BeforeEach
void setUp() {
- xmlFactory = new DefaultModelXmlFactory();
assembler = new DefaultInheritanceAssembler();
}
- private Path getPom(String name) {
- return
Paths.get("../../compat/maven-model-builder/src/test/resources/poms/inheritance/"
+ name + ".xml");
- }
-
- private Model getModel(String name) throws Exception {
- return
xmlFactory.read(XmlReaderRequest.builder().path(getPom(name)).build());
- }
-
@Test
- void testPluginConfiguration() throws Exception {
- testInheritance("plugin-configuration");
- }
+ void testAssembleWithNullArtifactIdDoesNotThrowNpe() {
+ Model parent = Model.newBuilder()
+ .modelVersion("4.0.0")
+ .groupId("test")
+ .artifactId("parent")
+ .version("1.0")
+ .build();
- public void testInheritance(String baseName) throws Exception {
- testInheritance(baseName, false);
- testInheritance(baseName, true);
- }
+ Model child = Model.newBuilder()
+ .modelVersion("4.0.0")
+ .groupId("test")
+ .version("1.0")
+ .build();
- public void testInheritance(String baseName, boolean fromRepo) throws
Exception {
- Model parent = getModel(baseName + "-parent");
- Model child = getModel(baseName + "-child");
+ assertDoesNotThrow(() -> assembler.assembleModelInheritance(child,
parent, null, null));
+ }
Review Comment:
The new tests only assert the absence of an exception, which makes them weak
regression tests (they can pass even if the adjustment logic becomes
incorrect). Consider additionally asserting on a concrete outcome of
`assembleModelInheritance` that is expected to differ or remain stable (e.g., a
specific field or URL/path-related value in the assembled model) for these edge
cases.
##########
impl/maven-impl/src/test/java/org/apache/maven/impl/model/DefaultInheritanceAssemblerTest.java:
##########
@@ -18,81 +18,81 @@
*/
package org.apache.maven.impl.model;
-import java.nio.file.Files;
-import java.nio.file.Path;
import java.nio.file.Paths;
+import java.util.List;
import org.apache.maven.api.model.Model;
-import org.apache.maven.api.services.xml.XmlReaderRequest;
-import org.apache.maven.api.services.xml.XmlWriterRequest;
-import org.apache.maven.impl.DefaultModelXmlFactory;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
-import org.xmlunit.builder.DiffBuilder;
-import org.xmlunit.diff.Diff;
-import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
class DefaultInheritanceAssemblerTest {
- private DefaultModelXmlFactory xmlFactory;
-
private DefaultInheritanceAssembler assembler;
@BeforeEach
void setUp() {
- xmlFactory = new DefaultModelXmlFactory();
assembler = new DefaultInheritanceAssembler();
}
- private Path getPom(String name) {
- return
Paths.get("../../compat/maven-model-builder/src/test/resources/poms/inheritance/"
+ name + ".xml");
- }
-
- private Model getModel(String name) throws Exception {
- return
xmlFactory.read(XmlReaderRequest.builder().path(getPom(name)).build());
- }
-
@Test
- void testPluginConfiguration() throws Exception {
- testInheritance("plugin-configuration");
- }
+ void testAssembleWithNullArtifactIdDoesNotThrowNpe() {
+ Model parent = Model.newBuilder()
+ .modelVersion("4.0.0")
+ .groupId("test")
+ .artifactId("parent")
+ .version("1.0")
+ .build();
- public void testInheritance(String baseName) throws Exception {
- testInheritance(baseName, false);
- testInheritance(baseName, true);
- }
+ Model child = Model.newBuilder()
+ .modelVersion("4.0.0")
+ .groupId("test")
+ .version("1.0")
+ .build();
- public void testInheritance(String baseName, boolean fromRepo) throws
Exception {
- Model parent = getModel(baseName + "-parent");
- Model child = getModel(baseName + "-child");
+ assertDoesNotThrow(() -> assembler.assembleModelInheritance(child,
parent, null, null));
+ }
- if (!fromRepo) {
- // when model is built from disk, pomFile is set
- // (has consequences in inheritance algorithm since
getProjectDirectory() returns non-null)
- parent = parent.withPomFile(getPom(baseName +
"-parent").toAbsolutePath());
- child = child.withPomFile(getPom(baseName +
"-child").toAbsolutePath());
- }
+ @Test
+ void testAssembleWithRootProjectDirectoryDoesNotThrowNpe() {
+ Model parent = Model.newBuilder()
+ .modelVersion("4.0.0")
+ .groupId("test")
+ .artifactId("parent")
+ .version("1.0")
+ .build();
- Model assembled = assembler.assembleModelInheritance(child, parent,
null, null);
+ // child has pomFile at root, so getProjectDirectory() returns root
path
+ // and getFileName() on root path returns null
+ Model child = Model.newBuilder()
+ .modelVersion("4.0.0")
+ .groupId("test")
+ .artifactId("child")
+ .version("1.0")
+ .pomFile(Paths.get("/pom.xml"))
+ .build();
Review Comment:
Using an absolute root path (`/pom.xml`) makes the test platform-dependent
(it can behave differently on Windows and other non-POSIX filesystems). Prefer
building a root-path scenario in a platform-neutral way (e.g., using
`Paths.get(FileSystems.getDefault().getSeparator())` equivalent patterns, or
deriving the filesystem root from an absolute path and then resolving
`pom.xml`).
##########
impl/maven-impl/src/test/java/org/apache/maven/impl/model/DefaultInheritanceAssemblerTest.java:
##########
@@ -18,81 +18,81 @@
*/
package org.apache.maven.impl.model;
-import java.nio.file.Files;
-import java.nio.file.Path;
import java.nio.file.Paths;
+import java.util.List;
import org.apache.maven.api.model.Model;
-import org.apache.maven.api.services.xml.XmlReaderRequest;
-import org.apache.maven.api.services.xml.XmlWriterRequest;
-import org.apache.maven.impl.DefaultModelXmlFactory;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
-import org.xmlunit.builder.DiffBuilder;
-import org.xmlunit.diff.Diff;
-import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
class DefaultInheritanceAssemblerTest {
- private DefaultModelXmlFactory xmlFactory;
-
private DefaultInheritanceAssembler assembler;
@BeforeEach
void setUp() {
- xmlFactory = new DefaultModelXmlFactory();
assembler = new DefaultInheritanceAssembler();
}
- private Path getPom(String name) {
- return
Paths.get("../../compat/maven-model-builder/src/test/resources/poms/inheritance/"
+ name + ".xml");
- }
-
- private Model getModel(String name) throws Exception {
- return
xmlFactory.read(XmlReaderRequest.builder().path(getPom(name)).build());
- }
-
@Test
- void testPluginConfiguration() throws Exception {
- testInheritance("plugin-configuration");
- }
+ void testAssembleWithNullArtifactIdDoesNotThrowNpe() {
+ Model parent = Model.newBuilder()
+ .modelVersion("4.0.0")
+ .groupId("test")
+ .artifactId("parent")
+ .version("1.0")
+ .build();
- public void testInheritance(String baseName) throws Exception {
- testInheritance(baseName, false);
- testInheritance(baseName, true);
- }
+ Model child = Model.newBuilder()
+ .modelVersion("4.0.0")
+ .groupId("test")
+ .version("1.0")
+ .build();
- public void testInheritance(String baseName, boolean fromRepo) throws
Exception {
- Model parent = getModel(baseName + "-parent");
- Model child = getModel(baseName + "-child");
+ assertDoesNotThrow(() -> assembler.assembleModelInheritance(child,
parent, null, null));
+ }
- if (!fromRepo) {
- // when model is built from disk, pomFile is set
- // (has consequences in inheritance algorithm since
getProjectDirectory() returns non-null)
- parent = parent.withPomFile(getPom(baseName +
"-parent").toAbsolutePath());
- child = child.withPomFile(getPom(baseName +
"-child").toAbsolutePath());
- }
+ @Test
+ void testAssembleWithRootProjectDirectoryDoesNotThrowNpe() {
+ Model parent = Model.newBuilder()
+ .modelVersion("4.0.0")
+ .groupId("test")
+ .artifactId("parent")
+ .version("1.0")
+ .build();
- Model assembled = assembler.assembleModelInheritance(child, parent,
null, null);
+ // child has pomFile at root, so getProjectDirectory() returns root
path
+ // and getFileName() on root path returns null
+ Model child = Model.newBuilder()
+ .modelVersion("4.0.0")
+ .groupId("test")
+ .artifactId("child")
+ .version("1.0")
+ .pomFile(Paths.get("/pom.xml"))
+ .build();
- // write baseName + "-actual"
- Path actual = Paths.get(
- "target/test-classes/poms/inheritance/" + baseName + (fromRepo
? "-build" : "-repo") + "-actual.xml");
- Files.createDirectories(actual.getParent());
- xmlFactory.write(XmlWriterRequest.<Model>builder()
- .content(assembled)
- .path(actual)
- .build());
+ assertDoesNotThrow(() -> assembler.assembleModelInheritance(child,
parent, null, null));
+ }
- // check with getPom( baseName + "-expected" )
- Path expected = getPom(baseName + "-expected");
+ @Test
+ void
testAssembleWithNullArtifactIdAndRootProjectDirectoryDoesNotThrowNpe() {
+ Model parent = Model.newBuilder()
+ .modelVersion("4.0.0")
+ .groupId("test")
+ .artifactId("parent")
+ .version("1.0")
+ .modules(List.of("../child/pom.xml"))
+ .build();
Review Comment:
This test introduces a relative module path string (`../child/pom.xml`) but
does not assert any behavior tied to module resolution; it currently only
asserts 'does not throw'. If modules are required to reproduce the original
stack path/adjustment behavior, add a brief note explaining why this particular
module entry matters; otherwise, consider removing it to keep the test minimal
and focused.
--
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]
