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/06 06:31:50 UTC

[maven-verifier] 01/01: [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 MSHARED-1125
in repository https://gitbox.apache.org/repos/asf/maven-verifier.git

commit 96c30e3cea783fcbb06963f98754a32148957f1b
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 | 49 ++++++-------
 .../apache/maven/shared/verifier/VerifierTest.java | 81 +++++++++++++++++++++-
 2 files changed, 102 insertions(+), 28 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..83c7db1 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( resolveCommandLineArg( cliOption ) );
         }
 
         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 ) );
             }
         }
@@ -1469,13 +1458,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,6 +1588,12 @@ 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()}
+     * @param option an argument to add
+     */
     public void addCliOption( String option )
     {
         cliOptions.add( option );
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..98d0d85 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,22 @@ 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.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 +110,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 +139,74 @@ 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( "test/${basedir}/test", "test/src/test/resources/test" ),
+            arguments( "argument with space", "argument with space" )
+        );
+    }
+
+    @ParameterizedTest
+    @MethodSource( "argumentsForTest" )
+    void argumentShouldBePassAsIs(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 ) );
+    }
+
+    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;
+        }
+    }
 }