You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@maven.apache.org by sj...@apache.org on 2022/09/07 05:54:51 UTC

[maven-verifier] branch master updated: [MSHARED-1125] Require Maven args to be provided one by one

This is an automated email from the ASF dual-hosted git repository.

sjaranowski pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/maven-verifier.git


The following commit(s) were added to refs/heads/master by this push:
     new f2ff914  [MSHARED-1125] Require Maven args to be provided one by one
f2ff914 is described below

commit f2ff914753f71515d1ea2390b4713669ce70e4be
Author: Slawomir Jaranowski <s....@gmail.com>
AuthorDate: Tue Sep 6 08:31:28 2022 +0200

    [MSHARED-1125] Require Maven args to be provided one by one
---
 .../org/apache/maven/shared/verifier/Verifier.java | 72 ++++++++--------
 .../apache/maven/shared/verifier/VerifierTest.java | 97 +++++++++++++++++++++-
 2 files changed, 128 insertions(+), 41 deletions(-)

diff --git a/src/main/java/org/apache/maven/shared/verifier/Verifier.java b/src/main/java/org/apache/maven/shared/verifier/Verifier.java
index adc7b80..3c29d0d 100644
--- a/src/main/java/org/apache/maven/shared/verifier/Verifier.java
+++ b/src/main/java/org/apache/maven/shared/verifier/Verifier.java
@@ -94,7 +94,7 @@ public class Verifier
 
     private boolean debug;
 
-    /** 
+    /**
      * If {@code true} uses {@link ForkedLauncher}, if {@code false} uses {@link Embedded3xLauncher},
      * otherwise considers the value {@link #forkMode}.
      */
@@ -164,13 +164,13 @@ public class Verifier
         this( basedir, settingsFile, debug, forkJvm, defaultCliOptions, null );
     }
 
-    public Verifier( String basedir, String settingsFile, boolean debug, String mavenHome ) 
+    public Verifier( String basedir, String settingsFile, boolean debug, String mavenHome )
             throws VerificationException
     {
         this( basedir, settingsFile, debug, null, DEFAULT_CLI_OPTIONS, mavenHome );
     }
 
-    public Verifier( String basedir, String settingsFile, boolean debug, String mavenHome, String[] defaultCliOptions ) 
+    public Verifier( String basedir, String settingsFile, boolean debug, String mavenHome, String[] defaultCliOptions )
         throws VerificationException
     {
         this( basedir, settingsFile, debug, null, defaultCliOptions, mavenHome );
@@ -354,7 +354,7 @@ public class Verifier
         Properties properties = new Properties();
 
         File propertiesFile = new File( getBasedir(), filename );
-        try ( FileInputStream fis = new FileInputStream( propertiesFile ) ) 
+        try ( FileInputStream fis = new FileInputStream( propertiesFile ) )
         {
             properties.load( fis );
         }
@@ -876,7 +876,7 @@ public class Verifier
     /**
      * There are 226 references to this method in Maven core ITs. In most (all?) cases it is used together with
      * {@link #newDefaultFilterProperties()}. Need to remove both methods and update all clients eventually/
-     * 
+     *
      * @param srcPath          The path to the input file, relative to the base directory, must not be
      *                         <code>null</code>.
      * @param dstPath          The path to the output file, relative to the base directory and possibly equal to the
@@ -943,7 +943,7 @@ public class Verifier
 
     /**
      * Verifies that the given file exists.
-     * 
+     *
      * @param file the path of the file to check
      * @throws VerificationException in case the given file does not exist
      */
@@ -953,7 +953,7 @@ public class Verifier
     }
 
     /**
-     * Verifies the given file's content matches an regular expression. 
+     * Verifies the given file's content matches an regular expression.
      * Note this method also checks that the file exists and is readable.
      *
      * @param file the path of the file to check
@@ -980,7 +980,7 @@ public class Verifier
 
     /**
      * Verifies that the given file does not exist.
-     * 
+     *
      * @param file the path of the file to check
      * @throws VerificationException if the given file exists
      */
@@ -1001,7 +1001,7 @@ public class Verifier
 
     /**
      * Verifies that the artifact given through its Maven coordinates exists.
-     * 
+     *
      * @param groupId the groupId of the artifact (must not be null)
      * @param artifactId the artifactId of the artifact (must not be null)
      * @param version the version of the artifact (must not be null)
@@ -1016,7 +1016,7 @@ public class Verifier
 
     /**
      * Verifies that the artifact given through its Maven coordinates does not exist.
-     * 
+     *
      * @param groupId the groupId of the artifact (must not be null)
      * @param artifactId the artifactId of the artifact (must not be null)
      * @param version the version of the artifact (must not be null)
@@ -1237,20 +1237,9 @@ public class Verifier
 
         File logFile = new File( getBasedir(), getLogFileName() );
 
-        for ( Object cliOption : cliOptions )
+        for ( String cliOption : cliOptions )
         {
-            String key = String.valueOf( cliOption );
-
-            String resolvedArg = resolveCommandLineArg( key );
-
-            try
-            {
-                args.addAll( Arrays.asList( CommandLineUtils.translateCommandline( resolvedArg ) ) );
-            }
-            catch ( Exception e )
-            {
-                e.printStackTrace();
-            }
+            args.add( cliOption.replace( "${basedir}", getBasedir() ) );
         }
 
         Collections.addAll( args, defaultCliOptions );
@@ -1303,7 +1292,7 @@ public class Verifier
         }
     }
 
-    private MavenLauncher getMavenLauncher( Map<String, String> envVars )
+    protected MavenLauncher getMavenLauncher( Map<String, String> envVars )
         throws LauncherException
     {
         boolean fork;
@@ -1364,7 +1353,7 @@ public class Verifier
             {
                 String defaultClasspath = System.getProperty( "maven.bootclasspath" );
                 String defaultClassworldConf = System.getProperty( "classworlds.conf" );
-                embeddedLauncher = Embedded3xLauncher.createFromMavenHome( mavenHome, defaultClassworldConf, 
+                embeddedLauncher = Embedded3xLauncher.createFromMavenHome( mavenHome, defaultClassworldConf,
                         parseClasspath( defaultClasspath ) );
             }
         }
@@ -1423,18 +1412,6 @@ public class Verifier
         }
     }
 
-    private String resolveCommandLineArg( String key )
-    {
-        String result = key.replaceAll( "\\$\\{basedir\\}", getBasedir() );
-        if ( result.contains( "\\\\" ) )
-        {
-            result = result.replaceAll( "\\\\", "\\" );
-        }
-        result = result.replaceAll( "\\/\\/", "\\/" );
-
-        return result;
-    }
-
     private void findLocalRepo( String settingsFile )
         throws VerificationException
     {
@@ -1469,13 +1446,13 @@ public class Verifier
 
     /**
      * Verifies that the artifact given by its Maven coordinates exists and contains the given content.
-     * 
+     *
      * @param groupId the groupId of the artifact (must not be null)
      * @param artifactId the artifactId of the artifact (must not be null)
      * @param version the version of the artifact (must not be null)
      * @param ext the extension of the artifact (must not be null)
      * @param content the expected content
-     * @throws IOException if reading from the artifact fails 
+     * @throws IOException if reading from the artifact fails
      * @throws VerificationException if the content of the artifact differs
      */
     public void verifyArtifactContent( String groupId, String artifactId, String version, String ext, String content )
@@ -1599,11 +1576,28 @@ public class Verifier
         this.cliOptions = cliOptions;
     }
 
+    /**
+     * Add a command line argument, each argument must be set separately one by one.
+     * <p>
+     * <code>${basedir}</code> in argument will be replaced by value of {@link #getBasedir()} during execution.
+     * @param option an argument to add
+     */
     public void addCliOption( String option )
     {
         cliOptions.add( option );
     }
 
+    /**
+     * Add a command line arguments, each argument must be set separately one by one.
+     * <p>
+     * <code>${basedir}</code> in argument will be replaced by value of {@link #getBasedir()} during execution.
+     * @param options an arguments list to add
+     */
+    public void addCliOptions( String... options )
+    {
+        Collections.addAll( cliOptions, options );
+    }
+
     public Properties getSystemProperties()
     {
         return systemProperties;
diff --git a/src/test/java/org/apache/maven/shared/verifier/VerifierTest.java b/src/test/java/org/apache/maven/shared/verifier/VerifierTest.java
index 0531611..5d16ca5 100644
--- a/src/test/java/org/apache/maven/shared/verifier/VerifierTest.java
+++ b/src/test/java/org/apache/maven/shared/verifier/VerifierTest.java
@@ -19,6 +19,7 @@ package org.apache.maven.shared.verifier;
  * under the License.
  */
 
+import java.io.File;
 import java.io.FileNotFoundException;
 import java.io.IOException;
 import java.nio.file.Path;
@@ -26,14 +27,23 @@ import java.nio.file.Paths;
 import java.util.Arrays;
 import java.util.Map;
 import java.util.Properties;
+import java.util.stream.Stream;
 
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.io.TempDir;
-
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
+
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.allOf;
+import static org.hamcrest.Matchers.arrayContaining;
+import static org.hamcrest.Matchers.hasItemInArray;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertInstanceOf;
 import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.junit.jupiter.params.provider.Arguments.arguments;
 
 public class VerifierTest
 {
@@ -101,7 +111,7 @@ public class VerifierTest
     }
 
     @Test
-    public void testLoadPropertiesFNFE() throws VerificationException
+    public void testLoadPropertiesFNFE()
     {
         VerificationException exception = assertThrows( VerificationException.class, () -> {
             Verifier verifier = new Verifier( "src/test/resources" );
@@ -130,4 +140,87 @@ public class VerifierTest
         assertTrue( filterMap.containsKey( "@basedir@" ) );
         assertTrue( filterMap.containsKey( "@baseurl@" ) );
     }
+
+    @Test
+    void testDefaultMavenArgument() throws VerificationException
+    {
+        TestVerifier verifier = new TestVerifier( "src/test/resources" );
+
+        verifier.executeGoal( "test" );
+
+        assertThat( verifier.launcher.cliArgs, arrayContaining(
+            "-e", "--batch-mode", "-Dmaven.repo.local=test-local-repo",
+            "org.apache.maven.plugins:maven-clean-plugin:clean", "test" ) );
+    }
+
+    public static Stream<Arguments> argumentsForTest()
+    {
+        return Stream.of(
+            arguments( "test-argument", "test-argument" ),
+            arguments( "test1/${basedir}/test2/${basedir}", "test1/src/test/resources/test2/src/test/resources" ),
+            arguments( "argument with space", "argument with space" )
+        );
+    }
+
+    @ParameterizedTest
+    @MethodSource( "argumentsForTest" )
+    void argumentShouldBePassedAsIs( String inputArgument, String expectedArgument ) throws VerificationException
+    {
+        TestVerifier verifier = new TestVerifier( "src/test/resources" );
+
+        verifier.addCliOption( inputArgument );
+        verifier.executeGoal( "test" );
+
+        assertThat( verifier.launcher.cliArgs, hasItemInArray( expectedArgument ) );
+    }
+
+    @Test
+    void cliOptionsShouldAddSeparateArguments() throws VerificationException
+    {
+        TestVerifier verifier = new TestVerifier( "src/test/resources" );
+
+        verifier.addCliOptions( "cliArg1", "cliArg2" );
+        verifier.executeGoal( "test" );
+
+        assertThat( verifier.launcher.cliArgs, allOf(
+            hasItemInArray( "cliArg1" ), hasItemInArray( "cliArg2" ) ) );
+
+    }
+
+    private static class TestMavenLauncher implements MavenLauncher
+    {
+        String[] cliArgs;
+
+        @Override
+        public int run( String[] cliArgs, Properties systemProperties, String workingDirectory, File logFile )
+            throws IOException, LauncherException
+        {
+            this.cliArgs = cliArgs;
+            return 0;
+        }
+
+        @Override
+        public String getMavenVersion() throws IOException, LauncherException
+        {
+            return null;
+        }
+    }
+
+    private static class TestVerifier extends Verifier
+    {
+        TestMavenLauncher launcher;
+
+        public TestVerifier( String basedir ) throws VerificationException
+        {
+            super( basedir );
+            setLocalRepo( "test-local-repo" );
+            launcher = new TestMavenLauncher();
+        }
+
+        @Override
+        protected MavenLauncher getMavenLauncher( Map<String, String> envVars ) throws LauncherException
+        {
+            return launcher;
+        }
+    }
 }