You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@maven.apache.org by GitBox <gi...@apache.org> on 2020/10/03 20:00:08 UTC

[GitHub] [maven-ear-plugin] mabrarov opened a new pull request #19: [MEAR-267] - Fixed detection if EAR JAR module is included into classpath of particular EAR module manifest

mabrarov opened a new pull request #19:
URL: https://github.com/apache/maven-ear-plugin/pull/19


   Fixed detection if JAR module is included into classpath of particular EAR module manifest (like WAR or EJB module manifest). This is required for correct modification of Class-Path entry of manifest of EAR modules when skinnyWars option is turned on.
   Note that this PR has no tests (yet) but I tested it manually with feature/MEAR-267_test branch of https://github.com/mabrarov/dockerfile-test:
   
   ```bash
   $ git clone --branch feature/MEAR-267_test https://github.com/mabrarov/dockerfile-test.git && \
   mvn -f dockerfile-test/pom.xml clean package -Ddocker.skip -Dmaven-ear-plugin.version=3.1.1-SNAPSHOT
   ```
   
   where expected manifest of WAR inside built EAR, i.e. dockerfile-test/ear/target/ear-1.1.7-SNAPSHOT.ear/org.mabrarov.dockerfile-test-war-1.1.7-SNAPSHOT.war/META-INF/MANIFEST.MF looks like:
   
   ```text
   Manifest-Version: 1.0
   Class-Path: lib/org.apache.commons-commons-text-1.7.jar lib/org.apache
    .commons-commons-lang3-3.9.jar
   Build-Jdk-Spec: 1.8
   Created-By: Maven WAR Plugin 3.3.1
   git-commit: e0458e2c48333f2847abfe58b7d1d0b3e23e8941
   ```
   
   while original manifest of WAR, i.e. dockerfile-test/war/target/war-1.1.7-SNAPSHOT.war/META-INF/MANIFEST.MF looks like:
   
   ```text
   Manifest-Version: 1.0
   Class-Path: commons-text-1.7.jar commons-lang3-3.9.jar
   Build-Jdk-Spec: 1.8
   Created-By: Maven WAR Plugin 3.3.1
   git-commit: e0458e2c48333f2847abfe58b7d1d0b3e23e8941
   ```


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [maven-ear-plugin] mabrarov commented on a change in pull request #19: [MEAR-267] - Fixed detection if JAR module is included into classpath of particular EAR module manifest

Posted by GitBox <gi...@apache.org>.
mabrarov commented on a change in pull request #19:
URL: https://github.com/apache/maven-ear-plugin/pull/19#discussion_r501128109



##########
File path: src/test/java/org/apache/maven/plugins/ear/it/AbstractEarPluginIT.java
##########
@@ -119,61 +128,72 @@ protected File executeMojo( final String projectName, final Properties propertie
     protected File executeMojo( final String projectName, final Properties properties )
         throws VerificationException, IOException
     {
-        return executeMojo( projectName, properties, true );
+        return executeMojo( projectName, properties, true, true );
     }
 
     /**
-     * Executes the specified projects and asserts the given artifacts. Assert the deployment descriptors are valid
-     * 
+     * Executes the specified projects and asserts the given artifacts. Asserts the deployment descriptors are valid.
+     * Asserts Class-Path entry of manifest of EAR modules.
+     *
      * @param projectName the project to test
+     * @param earModuleName the name of 1st level EAR module in multi-module project or null if project is single-module
      * @param expectedArtifacts the list of artifacts to be found in the EAR archive
      * @param artifactsDirectory whether the artifact is an exploded artifactsDirectory or not
+     * @param moduleArtifacts the list of artifacts representing EAR modules which manifest needs to be asserted or
+     *                        {@code null} if there is no need to validate Class-Path entry of EAR modules manifests
+     * @param moduleArtifactsDirectory whether the artifact from {@code moduleArtifacts} list is an exploded or not.
+     *                                 Can be {@code null} if {@code moduleArtifacts} is {@code null}
+     * @param expectedClassPathElements the list of elements of Class-Path entry of manifest. Rows should match
+     *                                  modules passed in {@code moduleArtifacts} parameter. Can be {@code null} if
+     *                                  {@code moduleArtifacts} is {@code null}
+     * @param cleanBeforeExecute call clean plugin before execution
      * @return the base directory of the project
      */
-    protected File doTestProject( final String projectName, final String[] expectedArtifacts,
-                                  final boolean[] artifactsDirectory )
+    protected File doTestProject( final String projectName, final String earModuleName,
+                                  final String[] expectedArtifacts, boolean[] artifactsDirectory,
+                                  final String[] moduleArtifacts, boolean[] moduleArtifactsDirectory,
+                                  final String[][] expectedClassPathElements,
+                                  final boolean cleanBeforeExecute )
         throws VerificationException, IOException
     {
-        final File baseDir = executeMojo( projectName, new Properties() );
-        assertEarArchive( baseDir, projectName );
-        assertEarDirectory( baseDir, projectName );
-        
-        assertArchiveContent( baseDir, projectName, expectedArtifacts, artifactsDirectory );
-        
-        assertDeploymentDescriptors( baseDir, projectName );
-        
-        return baseDir;
+        final File baseDir = executeMojo( projectName, new Properties(), true, cleanBeforeExecute );
+
+        final File earModuleDir = getEarModuleDirectory( baseDir, earModuleName );
+        assertEarArchive( earModuleDir, projectName );
+        assertEarDirectory( earModuleDir, projectName );
+        assertArchiveContent( earModuleDir, projectName, expectedArtifacts, artifactsDirectory );
+        assertDeploymentDescriptors( earModuleDir, projectName );
+        assertClassPathElements( earModuleDir, projectName, moduleArtifacts, moduleArtifactsDirectory, expectedClassPathElements );
 
+        return baseDir;
     }
 
     /**
-     * Executes the specified projects and asserts the given artifacts as artifacts (non directory)
-     * 
+     * Executes the specified projects and asserts the given artifacts. Assert the deployment descriptors are valid

Review comment:
       Added in 97bd177




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [maven-ear-plugin] mabrarov commented on a change in pull request #19: [MEAR-267] - Fixed detection if JAR module is included into classpath of particular EAR module manifest

Posted by GitBox <gi...@apache.org>.
mabrarov commented on a change in pull request #19:
URL: https://github.com/apache/maven-ear-plugin/pull/19#discussion_r499204091



##########
File path: src/test/java/org/apache/maven/plugins/ear/it/EarMojoIT.java
##########
@@ -1,996 +1,963 @@
-package org.apache.maven.plugins.ear.it;

Review comment:
       Reverted accidental changes in line endings




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [maven-ear-plugin] elharo commented on pull request #19: [MEAR-267] - Fixed detection if JAR module is included into classpath of particular EAR module manifest

Posted by GitBox <gi...@apache.org>.
elharo commented on pull request #19:
URL: https://github.com/apache/maven-ear-plugin/pull/19#issuecomment-707104096


   https://ci-builds.apache.org/job/Maven/job/maven-box/job/maven-ear-plugin/job/19/


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [maven-ear-plugin] mabrarov commented on pull request #19: [MEAR-267] - Fixed detection if JAR module is included into classpath of particular EAR module manifest

Posted by GitBox <gi...@apache.org>.
mabrarov commented on pull request #19:
URL: https://github.com/apache/maven-ear-plugin/pull/19#issuecomment-707097781


   > jenkins failed: https://ci-builds.apache.org/job/Maven/job/maven-box/job/maven-ear-plugin/job/mear-267/
   
   Looks like issue of Jenkins with checkout of pull request (https://ci-builds.apache.org/job/Maven/job/maven-box/job/maven-ear-plugin/job/mear-267/1/console):
   
   ```text
   Fetching without tags
   Fetching upstream changes from https://gitbox.apache.org/repos/asf/maven-jenkins-lib.git
    > git fetch --no-tags --force --progress -- https://gitbox.apache.org/repos/asf/maven-jenkins-lib.git +refs/heads/*:refs/remotes/origin/* # timeout=10
   Checking out Revision c10869c6952fe48d6d66f394802101bdc6d1a537 (master)
    > git config core.sparsecheckout # timeout=10
    > git checkout -f c10869c6952fe48d6d66f394802101bdc6d1a537 # timeout=10
   Commit message: "Restored the full maven version matrix default"
   First time build. Skipping changelog.
    > git --version # timeout=10
   fatal: bad object 4a8c35fa6acb6a409ac027ebf16ae7f3073bbbba
   ```


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [maven-ear-plugin] mabrarov commented on a change in pull request #19: [MEAR-267] - Fixed detection if JAR module is included into classpath of particular EAR module manifest

Posted by GitBox <gi...@apache.org>.
mabrarov commented on a change in pull request #19:
URL: https://github.com/apache/maven-ear-plugin/pull/19#discussion_r501128701



##########
File path: src/main/java/org/apache/maven/plugins/ear/EarMojo.java
##########
@@ -946,4 +946,31 @@ private void deleteOutdatedResources( final Collection<String> outdatedResources
             }
         }
     }
+
+    /**
+     * Searches JAR module in the list of classpath elements.
+     *
+     * @param classPathElements classpath elements to search among.
+     * @param module module to find among classpath elements defined by {@code classPathElements}
+     * @return -1 if {@code module} was not found in {@code classPathElements} or index of item of
+     * {@code classPathElements} which matches {@code module}
+     */
+    private int findModuleInClassPathElements( final List<String> classPathElements, final JarModule module )
+    {
+        if ( classPathElements.isEmpty() )
+        {
+            return -1;
+        }
+        int moduleClassPathIndex = classPathElements.indexOf( module.getBundleFileName() );
+        if ( moduleClassPathIndex != -1 )
+        {
+            return moduleClassPathIndex;
+        }
+        return classPathElements.indexOf( getArtifactFileName( module ) );
+    }
+
+    private String getArtifactFileName( final EarModule module )

Review comment:
       Inlined in 97bd177

##########
File path: src/main/java/org/apache/maven/plugins/ear/EarMojo.java
##########
@@ -946,4 +946,31 @@ private void deleteOutdatedResources( final Collection<String> outdatedResources
             }
         }
     }
+
+    /**
+     * Searches JAR module in the list of classpath elements.
+     *
+     * @param classPathElements classpath elements to search among.

Review comment:
       Removed period in 97bd177




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [maven-ear-plugin] mabrarov commented on a change in pull request #19: [MEAR-267] - Fixed detection if JAR module is included into classpath of particular EAR module manifest

Posted by GitBox <gi...@apache.org>.
mabrarov commented on a change in pull request #19:
URL: https://github.com/apache/maven-ear-plugin/pull/19#discussion_r501163372



##########
File path: src/main/java/org/apache/maven/plugins/ear/EarMojo.java
##########
@@ -946,4 +946,27 @@ private void deleteOutdatedResources( final Collection<String> outdatedResources
             }
         }
     }
+
+    /**
+     * Searches for the given JAR module in the list of classpath elements and if found matching returns index of
+     * class path element matching JAR module or -1 otherwise.
+     *
+     * @param classPathElements classpath elements to search among
+     * @param module module to find among classpath elements defined by {@code classPathElements}
+     * @return -1 if {@code module} was not found in {@code classPathElements} or index of item of
+     * {@code classPathElements} which matches {@code module}
+     */
+    private int findModuleInClassPathElements( final List<String> classPathElements, final JarModule module )

Review comment:
       The only place where this method is used *position* is needed to replace existing class path element with its location within EAR.




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [maven-ear-plugin] mabrarov edited a comment on pull request #19: [MEAR-267] - Fixed detection if JAR module is included into classpath of particular EAR module manifest

Posted by GitBox <gi...@apache.org>.
mabrarov edited a comment on pull request #19:
URL: https://github.com/apache/maven-ear-plugin/pull/19#issuecomment-703195593


   I added some integration tests but they do not cover all permutations (i.e. all changed branches) of options like:
   
   1. [skinnyWars](https://maven.apache.org/plugins/maven-ear-plugin/ear-mojo.html#unpackTypes)
   1. [skipClassPathModification](https://maven.apache.org/plugins/maven-ear-plugin/ear-mojo.html#unpackTypes)
   1. "dirty" build without `clean` goal execution


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [maven-ear-plugin] mabrarov commented on a change in pull request #19: [MEAR-267] - Fixed detection if JAR module is included into classpath of particular EAR module manifest

Posted by GitBox <gi...@apache.org>.
mabrarov commented on a change in pull request #19:
URL: https://github.com/apache/maven-ear-plugin/pull/19#discussion_r501128109



##########
File path: src/test/java/org/apache/maven/plugins/ear/it/AbstractEarPluginIT.java
##########
@@ -119,61 +128,72 @@ protected File executeMojo( final String projectName, final Properties propertie
     protected File executeMojo( final String projectName, final Properties properties )
         throws VerificationException, IOException
     {
-        return executeMojo( projectName, properties, true );
+        return executeMojo( projectName, properties, true, true );
     }
 
     /**
-     * Executes the specified projects and asserts the given artifacts. Assert the deployment descriptors are valid
-     * 
+     * Executes the specified projects and asserts the given artifacts. Asserts the deployment descriptors are valid.
+     * Asserts Class-Path entry of manifest of EAR modules.
+     *
      * @param projectName the project to test
+     * @param earModuleName the name of 1st level EAR module in multi-module project or null if project is single-module
      * @param expectedArtifacts the list of artifacts to be found in the EAR archive
      * @param artifactsDirectory whether the artifact is an exploded artifactsDirectory or not
+     * @param moduleArtifacts the list of artifacts representing EAR modules which manifest needs to be asserted or
+     *                        {@code null} if there is no need to validate Class-Path entry of EAR modules manifests
+     * @param moduleArtifactsDirectory whether the artifact from {@code moduleArtifacts} list is an exploded or not.
+     *                                 Can be {@code null} if {@code moduleArtifacts} is {@code null}
+     * @param expectedClassPathElements the list of elements of Class-Path entry of manifest. Rows should match
+     *                                  modules passed in {@code moduleArtifacts} parameter. Can be {@code null} if
+     *                                  {@code moduleArtifacts} is {@code null}
+     * @param cleanBeforeExecute call clean plugin before execution
      * @return the base directory of the project
      */
-    protected File doTestProject( final String projectName, final String[] expectedArtifacts,
-                                  final boolean[] artifactsDirectory )
+    protected File doTestProject( final String projectName, final String earModuleName,
+                                  final String[] expectedArtifacts, boolean[] artifactsDirectory,
+                                  final String[] moduleArtifacts, boolean[] moduleArtifactsDirectory,
+                                  final String[][] expectedClassPathElements,
+                                  final boolean cleanBeforeExecute )
         throws VerificationException, IOException
     {
-        final File baseDir = executeMojo( projectName, new Properties() );
-        assertEarArchive( baseDir, projectName );
-        assertEarDirectory( baseDir, projectName );
-        
-        assertArchiveContent( baseDir, projectName, expectedArtifacts, artifactsDirectory );
-        
-        assertDeploymentDescriptors( baseDir, projectName );
-        
-        return baseDir;
+        final File baseDir = executeMojo( projectName, new Properties(), true, cleanBeforeExecute );
+
+        final File earModuleDir = getEarModuleDirectory( baseDir, earModuleName );
+        assertEarArchive( earModuleDir, projectName );
+        assertEarDirectory( earModuleDir, projectName );
+        assertArchiveContent( earModuleDir, projectName, expectedArtifacts, artifactsDirectory );
+        assertDeploymentDescriptors( earModuleDir, projectName );
+        assertClassPathElements( earModuleDir, projectName, moduleArtifacts, moduleArtifactsDirectory, expectedClassPathElements );
 
+        return baseDir;
     }
 
     /**
-     * Executes the specified projects and asserts the given artifacts as artifacts (non directory)
-     * 
+     * Executes the specified projects and asserts the given artifacts. Assert the deployment descriptors are valid

Review comment:
       Removed in 97bd177




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [maven-ear-plugin] mabrarov commented on a change in pull request #19: [MEAR-267] - Fixed detection if JAR module is included into classpath of particular EAR module manifest

Posted by GitBox <gi...@apache.org>.
mabrarov commented on a change in pull request #19:
URL: https://github.com/apache/maven-ear-plugin/pull/19#discussion_r501163372



##########
File path: src/main/java/org/apache/maven/plugins/ear/EarMojo.java
##########
@@ -946,4 +946,27 @@ private void deleteOutdatedResources( final Collection<String> outdatedResources
             }
         }
     }
+
+    /**
+     * Searches for the given JAR module in the list of classpath elements and if found matching returns index of
+     * class path element matching JAR module or -1 otherwise.
+     *
+     * @param classPathElements classpath elements to search among
+     * @param module module to find among classpath elements defined by {@code classPathElements}
+     * @return -1 if {@code module} was not found in {@code classPathElements} or index of item of
+     * {@code classPathElements} which matches {@code module}
+     */
+    private int findModuleInClassPathElements( final List<String> classPathElements, final JarModule module )

Review comment:
       The only place where this method is used *position* is needed too to replace existing class path element with its location within EAR.




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [maven-ear-plugin] mabrarov commented on pull request #19: [MEAR-267] - Fixed detection if JAR module is included into classpath of particular EAR module manifest

Posted by GitBox <gi...@apache.org>.
mabrarov commented on pull request #19:
URL: https://github.com/apache/maven-ear-plugin/pull/19#issuecomment-703171162


   I fixed [2nd root cause](https://github.com/apache/maven-ear-plugin/pull/19#issuecomment-703167969) of MEAR-267 in 89816ae


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [maven-ear-plugin] mabrarov commented on pull request #19: [MEAR-267] - Fixed detection if JAR module is included into classpath of particular EAR module manifest

Posted by GitBox <gi...@apache.org>.
mabrarov commented on pull request #19:
URL: https://github.com/apache/maven-ear-plugin/pull/19#issuecomment-707152278


   I just found an error during "dirty" build. Let me try to fix it before proceeding with this PR.


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [maven-ear-plugin] mabrarov commented on pull request #19: [MEAR-267] - Fixed detection if JAR module is included into classpath of particular EAR module manifest

Posted by GitBox <gi...@apache.org>.
mabrarov commented on pull request #19:
URL: https://github.com/apache/maven-ear-plugin/pull/19#issuecomment-707139419


   Merge conflicts were fixed


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [maven-ear-plugin] mabrarov edited a comment on pull request #19: [MEAR-267] - Fixed detection if JAR module is included into classpath of particular EAR module manifest

Posted by GitBox <gi...@apache.org>.
mabrarov edited a comment on pull request #19:
URL: https://github.com/apache/maven-ear-plugin/pull/19#issuecomment-703167969


   Hmm... I made one more test:
   
   ```bash
   $ git clone --branch MEAR-267_Manifest_classpath_modification https://github.com/mabrarov/MEAR-267-issue-demo.git && \
   mvn -q -B -f MEAR-267-issue-demo/pom.xml verify -Dmaven-ear-plugin.version=3.1.1-SNAPSHOT 2>&1 | grep WFLYSRV0059
   ```
   
   and it demonstrates:
   
   ```text
   00:35:52,808 WARN  [org.jboss.as.server.deployment] (MSC service thread 1-2) WFLYSRV0059: Class Path entry guava-19.0.jar in /content/MEAR-267.ear/com.leokom-MEAR-267-ejb-1.0-SNAPSHOT.jar  does not point to a valid jar for a Class-Path reference.
   ```
   
   that MEAR-267 has multiple root causes and I fixed in this pull request just one of them.
   
   In MEAR-267-issue-demo we have skinnyWars option turned off and it looks like this makes Maven EAR Plugin skipping modification of manifest of EJB JAR.


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [maven-ear-plugin] mabrarov commented on pull request #19: [MEAR-267] - Fixed detection if JAR module is included into classpath of particular EAR module manifest

Posted by GitBox <gi...@apache.org>.
mabrarov commented on pull request #19:
URL: https://github.com/apache/maven-ear-plugin/pull/19#issuecomment-707292374


   Test failure when building with JDK 11 - refer to https://ci-builds.apache.org/job/Maven/job/maven-box/job/maven-ear-plugin/job/19/2/testReport/ - was fixed in e5ea347


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [maven-ear-plugin] elharo commented on a change in pull request #19: [MEAR-267] - Fixed detection if JAR module is included into classpath of particular EAR module manifest

Posted by GitBox <gi...@apache.org>.
elharo commented on a change in pull request #19:
URL: https://github.com/apache/maven-ear-plugin/pull/19#discussion_r503248443



##########
File path: src/main/java/org/apache/maven/plugins/ear/EarMojo.java
##########
@@ -946,4 +946,27 @@ private void deleteOutdatedResources( final Collection<String> outdatedResources
             }
         }
     }
+
+    /**
+     * Searches for the given JAR module in the list of classpath elements and if found matching returns index of

Review comment:
       This comment is a bit hard for me to track. I get it, but I had to read it twice. It would probably be clearer if it were broken up into two or more complete sentences. 




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [maven-ear-plugin] elharo commented on a change in pull request #19: [MEAR-267] - Fixed detection if JAR module is included into classpath of particular EAR module manifest

Posted by GitBox <gi...@apache.org>.
elharo commented on a change in pull request #19:
URL: https://github.com/apache/maven-ear-plugin/pull/19#discussion_r501159098



##########
File path: src/main/java/org/apache/maven/plugins/ear/EarMojo.java
##########
@@ -946,4 +946,27 @@ private void deleteOutdatedResources( final Collection<String> outdatedResources
             }
         }
     }
+
+    /**
+     * Searches for the given JAR module in the list of classpath elements and if found matching returns index of
+     * class path element matching JAR module or -1 otherwise.
+     *
+     * @param classPathElements classpath elements to search among
+     * @param module module to find among classpath elements defined by {@code classPathElements}
+     * @return -1 if {@code module} was not found in {@code classPathElements} or index of item of
+     * {@code classPathElements} which matches {@code module}
+     */
+    private int findModuleInClassPathElements( final List<String> classPathElements, final JarModule module )

Review comment:
       The only place this is used, what's needed is a boolean. That is, you care whether it's present or not. The position doesn't matter. Does it make sense for this ti return a boolean? 




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [maven-ear-plugin] mabrarov commented on a change in pull request #19: [MEAR-267] - Fixed detection if JAR module is included into classpath of particular EAR module manifest

Posted by GitBox <gi...@apache.org>.
mabrarov commented on a change in pull request #19:
URL: https://github.com/apache/maven-ear-plugin/pull/19#discussion_r503268545



##########
File path: src/main/java/org/apache/maven/plugins/ear/EarMojo.java
##########
@@ -946,4 +946,27 @@ private void deleteOutdatedResources( final Collection<String> outdatedResources
             }
         }
     }
+
+    /**
+     * Searches for the given JAR module in the list of classpath elements and if found matching returns index of

Review comment:
       Agree and broke into 3 sentences in c6d6d0a




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [maven-ear-plugin] elharo merged pull request #19: [MEAR-267] - Fixed detection if JAR module is included into classpath of particular EAR module manifest

Posted by GitBox <gi...@apache.org>.
elharo merged pull request #19:
URL: https://github.com/apache/maven-ear-plugin/pull/19


   


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [maven-ear-plugin] mabrarov commented on a change in pull request #19: [MEAR-267] - Fixed detection if JAR module is included into classpath of particular EAR module manifest

Posted by GitBox <gi...@apache.org>.
mabrarov commented on a change in pull request #19:
URL: https://github.com/apache/maven-ear-plugin/pull/19#discussion_r499201646



##########
File path: src/test/java/org/apache/maven/plugins/ear/it/EarMojoIT.java
##########
@@ -1,996 +1,963 @@
-package org.apache.maven.plugins.ear.it;

Review comment:
       I have no idea why GitHub shows this file as fully rewritten.




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [maven-ear-plugin] mabrarov commented on a change in pull request #19: [MEAR-267] - Fixed detection if JAR module is included into classpath of particular EAR module manifest

Posted by GitBox <gi...@apache.org>.
mabrarov commented on a change in pull request #19:
URL: https://github.com/apache/maven-ear-plugin/pull/19#discussion_r501129230



##########
File path: src/main/java/org/apache/maven/plugins/ear/EarMojo.java
##########
@@ -946,4 +946,31 @@ private void deleteOutdatedResources( final Collection<String> outdatedResources
             }
         }
     }
+
+    /**
+     * Searches JAR module in the list of classpath elements.

Review comment:
       Replaced with "searches for a JAR module" in 97bd177




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [maven-ear-plugin] mabrarov commented on a change in pull request #19: [MEAR-267] - Fixed detection if JAR module is included into classpath of particular EAR module manifest

Posted by GitBox <gi...@apache.org>.
mabrarov commented on a change in pull request #19:
URL: https://github.com/apache/maven-ear-plugin/pull/19#discussion_r499202940



##########
File path: src/test/java/org/apache/maven/plugins/ear/it/EarMojoIT.java
##########
@@ -1,996 +1,963 @@
-package org.apache.maven.plugins.ear.it;

Review comment:
       Looks like accidental changes in line endings (which I failed to revert) causes wrong diff shown for this file. Use "Hide whitespace changes" option for review.




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [maven-ear-plugin] mabrarov commented on pull request #19: [MEAR-267] - Fixed detection if JAR module is included into classpath of particular EAR module manifest

Posted by GitBox <gi...@apache.org>.
mabrarov commented on pull request #19:
URL: https://github.com/apache/maven-ear-plugin/pull/19#issuecomment-707168003


   > I just found an error during "dirty" build. Let me try to fix it before proceeding with this PR.
   
   That was fixed in f070e47. This PR is ready for testing and review now.


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [maven-ear-plugin] mabrarov commented on a change in pull request #19: [MEAR-267] - Fixed detection if JAR module is included into classpath of particular EAR module manifest

Posted by GitBox <gi...@apache.org>.
mabrarov commented on a change in pull request #19:
URL: https://github.com/apache/maven-ear-plugin/pull/19#discussion_r501163372



##########
File path: src/main/java/org/apache/maven/plugins/ear/EarMojo.java
##########
@@ -946,4 +946,27 @@ private void deleteOutdatedResources( final Collection<String> outdatedResources
             }
         }
     }
+
+    /**
+     * Searches for the given JAR module in the list of classpath elements and if found matching returns index of
+     * class path element matching JAR module or -1 otherwise.
+     *
+     * @param classPathElements classpath elements to search among
+     * @param module module to find among classpath elements defined by {@code classPathElements}
+     * @return -1 if {@code module} was not found in {@code classPathElements} or index of item of
+     * {@code classPathElements} which matches {@code module}
+     */
+    private int findModuleInClassPathElements( final List<String> classPathElements, final JarModule module )

Review comment:
       Position is needed too because it's used to replace existing class path element with its location within EAR.




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [maven-ear-plugin] mabrarov edited a comment on pull request #19: [MEAR-267] - Fixed detection if JAR module is included into classpath of particular EAR module manifest

Posted by GitBox <gi...@apache.org>.
mabrarov edited a comment on pull request #19:
URL: https://github.com/apache/maven-ear-plugin/pull/19#issuecomment-703167969


   Hmm... I made one more test:
   
   ```bash
   $ git clone --branch MEAR-267_Manifest_classpath_modification https://github.com/mabrarov/MEAR-267-issue-demo.git && \
   mvn -q -B -f MEAR-267-issue-demo/pom.xml verify -Dmaven-ear-plugin.version=3.1.1-SNAPSHOT 2>&1 | grep WFLYSRV0059
   ```
   
   and it demonstrates:
   
   ```text
   00:35:52,808 WARN  [org.jboss.as.server.deployment] (MSC service thread 1-2) WFLYSRV0059: Class Path entry guava-19.0.jar in /content/MEAR-267.ear/com.leokom-MEAR-267-ejb-1.0-SNAPSHOT.jar  does not point to a valid jar for a Class-Path reference.
   ```
   
   that MEAR-267 has multiple root causes and I fixed in this pull request just one of them.


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [maven-ear-plugin] mabrarov commented on pull request #19: [MEAR-267] - Fixed detection if JAR module is included into classpath of particular EAR module manifest

Posted by GitBox <gi...@apache.org>.
mabrarov commented on pull request #19:
URL: https://github.com/apache/maven-ear-plugin/pull/19#issuecomment-707226752


   @elharo, could you please trigger one more Jenkins build?


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [maven-ear-plugin] elharo commented on a change in pull request #19: [MEAR-267] - Fixed detection if JAR module is included into classpath of particular EAR module manifest

Posted by GitBox <gi...@apache.org>.
elharo commented on a change in pull request #19:
URL: https://github.com/apache/maven-ear-plugin/pull/19#discussion_r501064051



##########
File path: src/main/java/org/apache/maven/plugins/ear/EarMojo.java
##########
@@ -946,4 +946,31 @@ private void deleteOutdatedResources( final Collection<String> outdatedResources
             }
         }
     }
+
+    /**
+     * Searches JAR module in the list of classpath elements.

Review comment:
       searches the JAR module or searches for a JAR module? 

##########
File path: src/test/java/org/apache/maven/plugins/ear/it/AbstractEarPluginIT.java
##########
@@ -119,61 +128,72 @@ protected File executeMojo( final String projectName, final Properties propertie
     protected File executeMojo( final String projectName, final Properties properties )
         throws VerificationException, IOException
     {
-        return executeMojo( projectName, properties, true );
+        return executeMojo( projectName, properties, true, true );
     }
 
     /**
-     * Executes the specified projects and asserts the given artifacts. Assert the deployment descriptors are valid
-     * 
+     * Executes the specified projects and asserts the given artifacts. Asserts the deployment descriptors are valid.
+     * Asserts Class-Path entry of manifest of EAR modules.
+     *
      * @param projectName the project to test
+     * @param earModuleName the name of 1st level EAR module in multi-module project or null if project is single-module
      * @param expectedArtifacts the list of artifacts to be found in the EAR archive
      * @param artifactsDirectory whether the artifact is an exploded artifactsDirectory or not
+     * @param moduleArtifacts the list of artifacts representing EAR modules which manifest needs to be asserted or
+     *                        {@code null} if there is no need to validate Class-Path entry of EAR modules manifests
+     * @param moduleArtifactsDirectory whether the artifact from {@code moduleArtifacts} list is an exploded or not.
+     *                                 Can be {@code null} if {@code moduleArtifacts} is {@code null}
+     * @param expectedClassPathElements the list of elements of Class-Path entry of manifest. Rows should match
+     *                                  modules passed in {@code moduleArtifacts} parameter. Can be {@code null} if
+     *                                  {@code moduleArtifacts} is {@code null}
+     * @param cleanBeforeExecute call clean plugin before execution
      * @return the base directory of the project
      */
-    protected File doTestProject( final String projectName, final String[] expectedArtifacts,
-                                  final boolean[] artifactsDirectory )
+    protected File doTestProject( final String projectName, final String earModuleName,
+                                  final String[] expectedArtifacts, boolean[] artifactsDirectory,
+                                  final String[] moduleArtifacts, boolean[] moduleArtifactsDirectory,
+                                  final String[][] expectedClassPathElements,
+                                  final boolean cleanBeforeExecute )
         throws VerificationException, IOException
     {
-        final File baseDir = executeMojo( projectName, new Properties() );
-        assertEarArchive( baseDir, projectName );
-        assertEarDirectory( baseDir, projectName );
-        
-        assertArchiveContent( baseDir, projectName, expectedArtifacts, artifactsDirectory );
-        
-        assertDeploymentDescriptors( baseDir, projectName );
-        
-        return baseDir;
+        final File baseDir = executeMojo( projectName, new Properties(), true, cleanBeforeExecute );
+
+        final File earModuleDir = getEarModuleDirectory( baseDir, earModuleName );
+        assertEarArchive( earModuleDir, projectName );
+        assertEarDirectory( earModuleDir, projectName );
+        assertArchiveContent( earModuleDir, projectName, expectedArtifacts, artifactsDirectory );
+        assertDeploymentDescriptors( earModuleDir, projectName );
+        assertClassPathElements( earModuleDir, projectName, moduleArtifacts, moduleArtifactsDirectory, expectedClassPathElements );
 
+        return baseDir;
     }
 
     /**
-     * Executes the specified projects and asserts the given artifacts as artifacts (non directory)
-     * 
+     * Executes the specified projects and asserts the given artifacts. Assert the deployment descriptors are valid

Review comment:
       period at end

##########
File path: src/main/java/org/apache/maven/plugins/ear/EarMojo.java
##########
@@ -946,4 +946,31 @@ private void deleteOutdatedResources( final Collection<String> outdatedResources
             }
         }
     }
+
+    /**
+     * Searches JAR module in the list of classpath elements.
+     *
+     * @param classPathElements classpath elements to search among.
+     * @param module module to find among classpath elements defined by {@code classPathElements}
+     * @return -1 if {@code module} was not found in {@code classPathElements} or index of item of
+     * {@code classPathElements} which matches {@code module}
+     */
+    private int findModuleInClassPathElements( final List<String> classPathElements, final JarModule module )
+    {
+        if ( classPathElements.isEmpty() )
+        {
+            return -1;
+        }
+        int moduleClassPathIndex = classPathElements.indexOf( module.getBundleFileName() );
+        if ( moduleClassPathIndex != -1 )
+        {
+            return moduleClassPathIndex;
+        }
+        return classPathElements.indexOf( getArtifactFileName( module ) );
+    }
+
+    private String getArtifactFileName( final EarModule module )

Review comment:
       inline this

##########
File path: src/main/java/org/apache/maven/plugins/ear/EarMojo.java
##########
@@ -946,4 +946,31 @@ private void deleteOutdatedResources( final Collection<String> outdatedResources
             }
         }
     }
+
+    /**
+     * Searches JAR module in the list of classpath elements.
+     *
+     * @param classPathElements classpath elements to search among.

Review comment:
       no period since not a complete sentence

##########
File path: src/test/java/org/apache/maven/plugins/ear/it/AbstractEarPluginIT.java
##########
@@ -119,61 +128,72 @@ protected File executeMojo( final String projectName, final Properties propertie
     protected File executeMojo( final String projectName, final Properties properties )
         throws VerificationException, IOException
     {
-        return executeMojo( projectName, properties, true );
+        return executeMojo( projectName, properties, true, true );
     }
 
     /**
-     * Executes the specified projects and asserts the given artifacts. Assert the deployment descriptors are valid
-     * 
+     * Executes the specified projects and asserts the given artifacts. Asserts the deployment descriptors are valid.
+     * Asserts Class-Path entry of manifest of EAR modules.
+     *
      * @param projectName the project to test
+     * @param earModuleName the name of 1st level EAR module in multi-module project or null if project is single-module
      * @param expectedArtifacts the list of artifacts to be found in the EAR archive
      * @param artifactsDirectory whether the artifact is an exploded artifactsDirectory or not
+     * @param moduleArtifacts the list of artifacts representing EAR modules which manifest needs to be asserted or

Review comment:
       I don't follow. Can you rephrase?




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [maven-ear-plugin] mabrarov commented on a change in pull request #19: [MEAR-267] - Fixed detection if JAR module is included into classpath of particular EAR module manifest

Posted by GitBox <gi...@apache.org>.
mabrarov commented on a change in pull request #19:
URL: https://github.com/apache/maven-ear-plugin/pull/19#discussion_r499201646



##########
File path: src/test/java/org/apache/maven/plugins/ear/it/EarMojoIT.java
##########
@@ -1,996 +1,963 @@
-package org.apache.maven.plugins.ear.it;

Review comment:
       I have no idea why GitHub shows this file as fully rewritten. Maybe GitHub just cannot show large diff consisting of many changed lines (due to removal of useless `@throws` JavaDoc).




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [maven-ear-plugin] mabrarov commented on pull request #19: [MEAR-267] - Fixed detection if JAR module is included into classpath of particular EAR module manifest

Posted by GitBox <gi...@apache.org>.
mabrarov commented on pull request #19:
URL: https://github.com/apache/maven-ear-plugin/pull/19#issuecomment-706788620


   What's pending / required for merging this pull request?


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [maven-ear-plugin] elharo commented on pull request #19: [MEAR-267] - Fixed detection if JAR module is included into classpath of particular EAR module manifest

Posted by GitBox <gi...@apache.org>.
elharo commented on pull request #19:
URL: https://github.com/apache/maven-ear-plugin/pull/19#issuecomment-707742181


   https://ci-builds.apache.org/job/Maven/job/maven-box/job/maven-ear-plugin/job/19/


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [maven-ear-plugin] mabrarov commented on pull request #19: [MEAR-267] - Fixed detection if JAR module is included into classpath of particular EAR module manifest

Posted by GitBox <gi...@apache.org>.
mabrarov commented on pull request #19:
URL: https://github.com/apache/maven-ear-plugin/pull/19#issuecomment-703167969


   Hmm... I made one more test:
   
   ```bash
   $ git clone --branch MEAR-267_Manifest_classpath_modification https://github.com/mabrarov/MEAR-267-issue-demo.git && \
   mvn -q -B -f MEAR-267-issue-demo/pom.xml verify 2>&1 | grep WFLYSRV0059
   ```
   
   and it demonstrates:
   
   ```text
   00:35:52,808 WARN  [org.jboss.as.server.deployment] (MSC service thread 1-2) WFLYSRV0059: Class Path entry guava-19.0.jar in /content/MEAR-267.ear/com.leokom-MEAR-267-ejb-1.0-SNAPSHOT.jar  does not point to a valid jar for a Class-Path reference.
   ```
   
   that MEAR-267 has multiple root causes and I fixed in this pull request just one of them.


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [maven-ear-plugin] mabrarov commented on pull request #19: [MEAR-267] - Fixed detection if JAR module is included into classpath of particular EAR module manifest

Posted by GitBox <gi...@apache.org>.
mabrarov commented on pull request #19:
URL: https://github.com/apache/maven-ear-plugin/pull/19#issuecomment-703162560


   Taking into account enhancements in `EarMojoIT` made in pull request #17 I would create tests for this pull request after that pull request is merged, because it looks like we need those enhancements here too.


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [maven-ear-plugin] mabrarov commented on pull request #19: [MEAR-267] - Fixed detection if JAR module is included into classpath of particular EAR module manifest

Posted by GitBox <gi...@apache.org>.
mabrarov commented on pull request #19:
URL: https://github.com/apache/maven-ear-plugin/pull/19#issuecomment-703195593


   I added some integration tests but they do not cover all permutations (i.e. all changed branches) of options like:
   
   1. [skinnyWars](https://maven.apache.org/plugins/maven-ear-plugin/ear-mojo.html#unpackTypes)
   1. [skipClassPathModification](https://maven.apache.org/plugins/maven-ear-plugin/ear-mojo.html#unpackTypes)
   1. [unpackTypes](https://maven.apache.org/plugins/maven-ear-plugin/ear-mojo.html#unpackTypes)
   1. "dirty" build without `clean` goal execution


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [maven-ear-plugin] mabrarov commented on a change in pull request #19: [MEAR-267] - Fixed detection if JAR module is included into classpath of particular EAR module manifest

Posted by GitBox <gi...@apache.org>.
mabrarov commented on a change in pull request #19:
URL: https://github.com/apache/maven-ear-plugin/pull/19#discussion_r501129230



##########
File path: src/main/java/org/apache/maven/plugins/ear/EarMojo.java
##########
@@ -946,4 +946,31 @@ private void deleteOutdatedResources( final Collection<String> outdatedResources
             }
         }
     }
+
+    /**
+     * Searches JAR module in the list of classpath elements.

Review comment:
       Replaced with "Searches for the given JAR module" in 97bd177




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [maven-ear-plugin] mabrarov commented on a change in pull request #19: [MEAR-267] - Fixed detection if JAR module is included into classpath of particular EAR module manifest

Posted by GitBox <gi...@apache.org>.
mabrarov commented on a change in pull request #19:
URL: https://github.com/apache/maven-ear-plugin/pull/19#discussion_r505119592



##########
File path: src/main/java/org/apache/maven/plugins/ear/EarMojo.java
##########
@@ -461,7 +461,7 @@ private void copyModules( final JavaEEVersion javaEEVersion,
                         getLog().info( "Copying artifact [" + module + "] to [" + module.getUri() + "]" );
                         FileUtils.copyFile( sourceFile, destinationFile );
 
-                        if ( skinnyWars && module.changeManifestClasspath() )
+                        if ( module.changeManifestClasspath() && ( skinnyWars || module.getLibDir() == null ) )

Review comment:
       It looks that this change introduces non backward compatible behavior - modification of Class-Path entry of manifest - for SAR and HAR modules. Refer to pull request #24 which is going to fix that (with `false` default value of skinnyModules option). 




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [maven-ear-plugin] mabrarov edited a comment on pull request #19: [MEAR-267] - Fixed detection if JAR module is included into classpath of particular EAR module manifest

Posted by GitBox <gi...@apache.org>.
mabrarov edited a comment on pull request #19:
URL: https://github.com/apache/maven-ear-plugin/pull/19#issuecomment-703171162


   I fixed [2nd root cause](https://github.com/apache/maven-ear-plugin/pull/19#issuecomment-703167969) of MEAR-267 in 89816ae:
   
   ```bash
   $ git clone --branch MEAR-267_Manifest_classpath_modification https://github.com/mabrarov/MEAR-267-issue-demo.git && \
   mvn -q -B -f MEAR-267-issue-demo/pom.xml verify -Dmaven-ear-plugin.version=3.1.1-SNAPSHOT 2>&1 | grep -c WFLYSRV0059
   ...
   0
   ```


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [maven-ear-plugin] mabrarov commented on a change in pull request #19: [MEAR-267] - Fixed detection if JAR module is included into classpath of particular EAR module manifest

Posted by GitBox <gi...@apache.org>.
mabrarov commented on a change in pull request #19:
URL: https://github.com/apache/maven-ear-plugin/pull/19#discussion_r501128499



##########
File path: src/test/java/org/apache/maven/plugins/ear/it/AbstractEarPluginIT.java
##########
@@ -119,61 +128,72 @@ protected File executeMojo( final String projectName, final Properties propertie
     protected File executeMojo( final String projectName, final Properties properties )
         throws VerificationException, IOException
     {
-        return executeMojo( projectName, properties, true );
+        return executeMojo( projectName, properties, true, true );
     }
 
     /**
-     * Executes the specified projects and asserts the given artifacts. Assert the deployment descriptors are valid
-     * 
+     * Executes the specified projects and asserts the given artifacts. Asserts the deployment descriptors are valid.
+     * Asserts Class-Path entry of manifest of EAR modules.
+     *
      * @param projectName the project to test
+     * @param earModuleName the name of 1st level EAR module in multi-module project or null if project is single-module
      * @param expectedArtifacts the list of artifacts to be found in the EAR archive
      * @param artifactsDirectory whether the artifact is an exploded artifactsDirectory or not
+     * @param moduleArtifacts the list of artifacts representing EAR modules which manifest needs to be asserted or

Review comment:
       Rephrased in 97bd177 (includes renaming of parameters too)




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org