Copilot commented on code in PR #563:
URL: https://github.com/apache/maven-ear-plugin/pull/563#discussion_r4360162669
##########
src/test/java/org/apache/maven/plugins/ear/EarModuleTest.java:
##########
@@ -40,4 +45,22 @@ void testCleanArchivePath() {
assertEquals("", AbstractEarModule.cleanArchivePath(""));
assertNull(AbstractEarModule.cleanArchivePath(null));
}
+
+ @Test
+ void testResolveArtifactRequiresExecutionContext() throws Exception {
+ JarModule module = new JarModule();
+ setField(module, "groupId", "groupId");
+ setField(module, "artifactId", "artifactId");
+
+ MojoFailureException exception = assertThrows(
+ MojoFailureException.class, () ->
module.resolveArtifact(Collections.emptySet()));
+
+ assertEquals("Ear execution context not initialized for module
jar:groupId:artifactId", exception.getMessage());
+ }
+
+ private static void setField(Object target, String name, Object value)
throws Exception {
+ Field field = AbstractEarModule.class.getDeclaredField(name);
+ field.setAccessible(true);
+ field.set(target, value);
+ }
Review Comment:
The test uses reflection to mutate private fields, which is brittle and can
break on refactors (e.g., renames) unrelated to behavior. Prefer constructing a
module with the needed identity via public/protected APIs (setters/constructor)
if available, or create a minimal test subclass in the test that exposes a
supported way to set `groupId`/`artifactId` for testing.
##########
src/main/java/org/apache/maven/plugins/ear/AbstractEarModule.java:
##########
@@ -155,6 +155,9 @@ public void resolveArtifact(Set<Artifact> artifacts) throws
EarPluginException,
throw new MojoFailureException(
"Could not resolve artifact[" + groupId + ":" +
artifactId + ":" + getType() + "]");
}
+ if (earExecutionContext == null) {
+ throw new MojoFailureException("Ear execution context not
initialized for module " + this);
+ }
Review Comment:
This null check occurs after `getType()` is used earlier in the method (see
the error message construction preceding this hunk). If `getType()` depends on
`earExecutionContext` (directly or indirectly), the method can still throw a
`NullPointerException` before reaching this guard. Move the
`earExecutionContext == null` check earlier (before any calls that might
consult the context), or ensure `getType()` is context-independent.
##########
src/test/java/org/apache/maven/plugins/ear/EarModuleTest.java:
##########
@@ -40,4 +45,22 @@ void testCleanArchivePath() {
assertEquals("", AbstractEarModule.cleanArchivePath(""));
assertNull(AbstractEarModule.cleanArchivePath(null));
}
+
+ @Test
+ void testResolveArtifactRequiresExecutionContext() throws Exception {
+ JarModule module = new JarModule();
+ setField(module, "groupId", "groupId");
+ setField(module, "artifactId", "artifactId");
+
+ MojoFailureException exception = assertThrows(
+ MojoFailureException.class, () ->
module.resolveArtifact(Collections.emptySet()));
+
+ assertEquals("Ear execution context not initialized for module
jar:groupId:artifactId", exception.getMessage());
Review Comment:
The assertion hard-codes the full exception message, which couples the test
to `toString()` formatting and exact wording. Consider asserting on stable
parts (e.g., `assertTrue(message.contains(\"Ear execution context not
initialized\"))` and `contains(\"jar:groupId:artifactId\")`) or switching to
`assertThrows` + message substring checks, to keep the test focused on behavior
rather than exact formatting.
--
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]
