Copilot commented on code in PR #566:
URL: https://github.com/apache/maven-ear-plugin/pull/566#discussion_r4360120132
##########
src/test/java/org/apache/maven/plugins/ear/EarMojoTest.java:
##########
@@ -37,6 +40,17 @@ private File invokeGetEarFile(String finalName, String
classifier) throws Except
return (File) method.invoke(null, tempDir.getAbsolutePath(),
finalName, classifier);
}
+ @SuppressWarnings("unchecked")
+ private List<String> invokeCreateUnpackList(String unpackTypes) throws
Exception {
+ EarMojo mojo = new EarMojo(null, null, null, null, null, null);
+ Field field = EarMojo.class.getDeclaredField("unpackTypes");
+ field.setAccessible(true);
+ field.set(mojo, unpackTypes);
+ Method method = EarMojo.class.getDeclaredMethod("createUnpackList");
+ method.setAccessible(true);
+ return (List<String>) method.invoke(mojo);
+ }
Review Comment:
This test depends on reflective access to private field/methods
(`setAccessible(true)`), which is brittle and can become problematic under
stronger encapsulation or future refactors. Prefer testing through a
public/protected/package-private seam (e.g., extracting parsing into a
package-private helper, or making `createUnpackList` package-private and
annotating appropriately) so behavior is verified without reflection.
##########
src/main/java/org/apache/maven/plugins/ear/EarMojo.java:
##########
@@ -466,6 +466,8 @@ private List<String> createUnpackList() throws
MojoExecutionException {
List<String> unpackTypesList = new ArrayList<>();
if (unpackTypes != null) {
unpackTypesList = Arrays.asList(unpackTypes.split(","));
+ unpackTypesList = new ArrayList<>(unpackTypesList);
+ unpackTypesList.removeIf(String::isEmpty);
Review Comment:
`removeIf(String::isEmpty)` only removes exactly-empty tokens; `unpackTypes`
values with whitespace around commas (e.g., `\"jar, war,\"` or `\"jar, ,war\"`)
will still produce tokens like `\" war\"` / `\" \"` and continue to fail
validation. Consider trimming tokens and filtering blanks (e.g., map
`String::trim` then remove entries that are empty after trimming) so the
intended fix works with commonly formatted comma-separated config.
##########
src/main/java/org/apache/maven/plugins/ear/EarMojo.java:
##########
@@ -466,6 +466,8 @@ private List<String> createUnpackList() throws
MojoExecutionException {
List<String> unpackTypesList = new ArrayList<>();
if (unpackTypes != null) {
unpackTypesList = Arrays.asList(unpackTypes.split(","));
+ unpackTypesList = new ArrayList<>(unpackTypesList);
Review Comment:
This can be simplified to avoid the intermediate fixed-size list by
constructing the mutable list directly (e.g., `new
ArrayList<>(Arrays.asList(...))`). It reduces a reassignment and makes the
intent clearer.
--
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]
