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 2021/09/25 10:02:20 UTC
[GitHub] [maven-invoker-plugin] slawekjaranowski commented on a change in pull request #68: [MINVOKER-271] fix detection of setup projects
slawekjaranowski commented on a change in pull request #68:
URL: https://github.com/apache/maven-invoker-plugin/pull/68#discussion_r716018370
##########
File path: pom.xml
##########
@@ -275,6 +275,18 @@ under the License.
<version>${mavenVersion}</version>
<scope>test</scope>
</dependency>
+ <dependency>
+ <groupId>org.assertj</groupId>
+ <artifactId>assertj-core</artifactId>
+ <version>3.20.2</version>
+ <scope>test</scope>
+ </dependency>
+ <dependency>
+ <groupId>org.slf4j</groupId>
+ <artifactId>slf4j-simple</artifactId>
+ <version>1.7.32</version>
+ <scope>test</scope>
+ </dependency>
Review comment:
we have slf4j on classpath, coming from maven-artifact-transfer and maven-script-interpreter so in order to avoid worning about missing binding during unit test I added it.
##########
File path: src/main/java/org/apache/maven/plugins/invoker/AbstractInvokerMojo.java
##########
@@ -2399,11 +2396,46 @@ else if ( !invokerProperties.isExpectedResult( result.getExitCode(), invocationI
}
}
+ private List<String> calculateIncludes()
+ {
+ if ( invokerTest != null )
+ {
+ String[] testRegexes = StringUtils.split( invokerTest, "," );
+ return Arrays.stream( testRegexes )
+ .map( String::trim )
+ .filter( s -> !s.isEmpty() )
+ .filter( s -> !s.startsWith( "!" ) )
+ .collect( Collectors.toList() );
+ }
+ else
+ {
+ Set<String> uniqueIncludes = new HashSet<>();
+ uniqueIncludes.addAll( pomIncludes );
+ uniqueIncludes.addAll( setupIncludes );
+ return new ArrayList<>( uniqueIncludes );
+ }
+ }
+
private List<String> calculateExcludes()
throws IOException
{
- List<String> excludes =
- ( pomExcludes != null ) ? new ArrayList<>( pomExcludes ) : new ArrayList<>();
+ List<String> excludes;
+
+ if ( invokerTest != null )
+ {
+ String[] testRegexes = StringUtils.split( invokerTest, "," );
+ excludes = Arrays.stream( testRegexes )
+ .map( String::trim )
+ .filter( s -> !s.isEmpty() )
+ .filter( s -> s.startsWith( "!" ) )
+ .map( s -> s.substring( 1 ) )
+ .collect( Collectors.toList() );
+ }
+ else
+ {
+ excludes = pomExcludes != null ? new ArrayList<>( pomExcludes ) : new ArrayList<>();
+ }
+
Review comment:
the sama as `calculateIncludes` do in one place
##########
File path: src/main/mdo/invocation.mdo
##########
@@ -197,11 +196,6 @@ under the License.
*/
public static final String NORMAL = "normal";
- /**
- * A build job that was directly selected via the <code>-Dinvoker.test=xxx,yyy</code> parameter.
- */
- public static final String DIRECT = "direct";
-
Review comment:
simplify code - job type is set during scan jobs in the same way - it not depends on invoker.test so this type is no more used
##########
File path: src/main/java/org/apache/maven/plugins/invoker/AbstractInvokerMojo.java
##########
@@ -796,23 +797,13 @@ else if ( lastModifiedRecursive( projectsDirectory ) <= lastModifiedRecursive( c
}
// First run setup jobs.
- List<BuildJob> setupBuildJobs = null;
- try
- {
- setupBuildJobs = getSetupBuildJobsFromFolders();
- }
- catch ( IOException e )
- {
- getLog().error( "Failure during scanning of folders.", e );
- // TODO: Check shouldn't we fail in case of problems?
- }
+ List<BuildJob> setupBuildJobs = getSetupJobs( buildJobs );
Review comment:
method `getBuildJobs` returns now jobs with proper type - no separate method and way for searching setup jobs
##########
File path: src/main/java/org/apache/maven/plugins/invoker/AbstractInvokerMojo.java
##########
@@ -2417,25 +2449,6 @@ else if ( !invokerProperties.isExpectedResult( result.getExitCode(), invocationI
}
- /**
- * @return The list of setupUp jobs.
- * @throws IOException
- * @see {@link #setupIncludes}
- */
- private List<BuildJob> getSetupBuildJobsFromFolders()
- throws IOException, MojoExecutionException
- {
- List<String> excludes = calculateExcludes();
-
- List<BuildJob> setupPoms = scanProjectsDirectory( setupIncludes, excludes, BuildJob.Type.SETUP );
- if ( getLog().isDebugEnabled() )
- {
- getLog().debug( "Setup projects: " + setupPoms );
- }
-
- return setupPoms;
- }
-
Review comment:
`getBuildJobs` return all jobs with type - we needn't separate method
##########
File path: src/main/java/org/apache/maven/plugins/invoker/AbstractInvokerMojo.java
##########
@@ -2399,11 +2396,46 @@ else if ( !invokerProperties.isExpectedResult( result.getExitCode(), invocationI
}
}
+ private List<String> calculateIncludes()
+ {
+ if ( invokerTest != null )
+ {
+ String[] testRegexes = StringUtils.split( invokerTest, "," );
+ return Arrays.stream( testRegexes )
+ .map( String::trim )
+ .filter( s -> !s.isEmpty() )
+ .filter( s -> !s.startsWith( "!" ) )
+ .collect( Collectors.toList() );
+ }
+ else
+ {
+ Set<String> uniqueIncludes = new HashSet<>();
+ uniqueIncludes.addAll( pomIncludes );
+ uniqueIncludes.addAll( setupIncludes );
+ return new ArrayList<>( uniqueIncludes );
+ }
+ }
+
Review comment:
according to documentation and assumptions invokerTest should override rest params, so calculate it in one place
##########
File path: src/main/java/org/apache/maven/plugins/invoker/AbstractInvokerMojo.java
##########
@@ -741,7 +742,7 @@ public void execute()
+ "pom File parameter. Reason: " + e.getMessage(), e );
}
- buildJobs = Collections.singletonList( new BuildJob( pom.getName(), BuildJob.Type.NORMAL ) );
+ buildJobs = Collections.singletonList( new BuildJob( pom.getName() ) );
Review comment:
now default value is NORMAL
##########
File path: src/main/java/org/apache/maven/plugins/invoker/AbstractInvokerMojo.java
##########
@@ -2456,62 +2469,35 @@ public int compare( BuildJob o1, BuildJob o2 )
List<BuildJob> getBuildJobs()
throws IOException, MojoExecutionException
{
- List<BuildJob> buildJobs;
- if ( invokerTest == null )
- {
- List<String> excludes = calculateExcludes();
+ List<String> includes = calculateIncludes();
+ List<String> excludes = calculateExcludes();
+ List<BuildJob> buildJobsAll = scanProjectsDirectory( includes, excludes );
+ List<BuildJob> buildJobsSetup = scanProjectsDirectory( setupIncludes, excludes );
- List<BuildJob> setupPoms = scanProjectsDirectory( setupIncludes, excludes, BuildJob.Type.SETUP );
- if ( getLog().isDebugEnabled() )
- {
- getLog().debug( "Setup projects: " + Collections.singletonList( setupPoms ) );
- }
+ List<String> setupProjects = buildJobsSetup.stream()
+ .map( BuildJob::getProject )
+ .collect( Collectors.toList() );
- List<BuildJob> normalPoms = scanProjectsDirectory( pomIncludes, excludes, BuildJob.Type.NORMAL );
- Map<String, BuildJob> uniquePoms = new LinkedHashMap<>();
- for ( BuildJob setupPom : setupPoms )
- {
- uniquePoms.put( setupPom.getProject(), setupPom );
- }
- for ( BuildJob normalPom : normalPoms )
- {
- if ( !uniquePoms.containsKey( normalPom.getProject() ) )
- {
- uniquePoms.put( normalPom.getProject(), normalPom );
- }
- }
-
- buildJobs = new ArrayList<>( uniquePoms.values() );
- }
- else
+ for ( BuildJob job : buildJobsAll )
{
- String[] testRegexes = StringUtils.split( invokerTest, "," );
- List<String> includes = new ArrayList<>( testRegexes.length );
- List<String> excludes = new ArrayList<>();
-
- for ( String regex : testRegexes )
+ if ( setupProjects.contains( job.getProject() ) )
{
- // user just use -Dinvoker.test=MWAR191,MNG111 to use a directory thats the end is not pom.xml
- if ( regex.startsWith( "!" ) )
- {
- excludes.add( regex.substring( 1 ) );
- }
- else
- {
- includes.add( regex );
- }
+ job.setType( BuildJob.Type.SETUP );
}
-
Review comment:
simplify joss searching - calculate includes and excludes in the same way
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org