Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[MSHARED-1125] Require Maven args to be provided one by one #41

Merged
merged 1 commit into from
Sep 7, 2022
Merged

[MSHARED-1125] Require Maven args to be provided one by one #41

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
72 changes: 33 additions & 39 deletions src/main/java/org/apache/maven/shared/verifier/Verifier.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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}.
*/
Expand Down Expand Up @@ -164,13 +164,13 @@ public Verifier( String basedir, String settingsFile, boolean debug, boolean for
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 );
Expand Down Expand Up @@ -354,7 +354,7 @@ public Properties loadProperties( String filename )
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 );
}
Expand Down Expand Up @@ -876,7 +876,7 @@ public File filterFile( String srcPath, String dstPath, String fileEncoding, Map
/**
* 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
Expand Down Expand Up @@ -943,7 +943,7 @@ public Map<String, String> newDefaultFilterMap()

/**
* 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
*/
Expand All @@ -953,7 +953,7 @@ public void verifyFilePresent( String file ) throws VerificationException
}

/**
* 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
Expand All @@ -980,7 +980,7 @@ public void verifyFileContentMatches( String file, String regex ) throws Verific

/**
* Verifies that the given file does not exist.
*
*
* @param file the path of the file to check
* @throws VerificationException if the given file exists
*/
Expand All @@ -1001,7 +1001,7 @@ private void verifyArtifactPresence( boolean wanted, String groupId, String arti

/**
* 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)
Expand All @@ -1016,7 +1016,7 @@ public void verifyArtifactPresent( String groupId, String artifactId, String ver

/**
* 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)
Expand Down Expand Up @@ -1237,20 +1237,9 @@ public void executeGoals( List<String> goals, Map<String, String> envVars )

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 );
Expand Down Expand Up @@ -1303,7 +1292,7 @@ public void executeGoals( List<String> goals, Map<String, String> envVars )
}
}

private MavenLauncher getMavenLauncher( Map<String, String> envVars )
protected MavenLauncher getMavenLauncher( Map<String, String> envVars )
throws LauncherException
{
boolean fork;
Expand Down Expand Up @@ -1364,7 +1353,7 @@ private void initEmbeddedLauncher()
{
String defaultClasspath = System.getProperty( "maven.bootclasspath" );
String defaultClassworldConf = System.getProperty( "classworlds.conf" );
embeddedLauncher = Embedded3xLauncher.createFromMavenHome( mavenHome, defaultClassworldConf,
embeddedLauncher = Embedded3xLauncher.createFromMavenHome( mavenHome, defaultClassworldConf,
parseClasspath( defaultClasspath ) );
}
}
Expand Down Expand Up @@ -1423,18 +1412,6 @@ private static String getLogContents( File logFile )
}
}

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
{
Expand Down Expand Up @@ -1469,13 +1446,13 @@ private void findLocalRepo( String settingsFile )

/**
* 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 )
Expand Down Expand Up @@ -1599,11 +1576,28 @@ public void setCliOptions( List<String> cliOptions )
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;
Expand Down
97 changes: 95 additions & 2 deletions src/test/java/org/apache/maven/shared/verifier/VerifierTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,21 +19,31 @@
* under the License.
*/

import java.io.File;
import java.io.FileNotFoundException;
import java.io.IOException;
import java.nio.file.Path;
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
{
Expand Down Expand Up @@ -101,7 +111,7 @@ public void testStripAnsi()
}

@Test
public void testLoadPropertiesFNFE() throws VerificationException
public void testLoadPropertiesFNFE()
{
VerificationException exception = assertThrows( VerificationException.class, () -> {
Verifier verifier = new Verifier( "src/test/resources" );
Expand Down Expand Up @@ -130,4 +140,87 @@ void testDefaultFilterMap() throws VerificationException
assertTrue( filterMap.containsKey( "@basedir@" ) );
assertTrue( filterMap.containsKey( "@baseurl@" ) );
}

@Test
void testDefaultMavenArgument() throws VerificationException
{
TestVerifier verifier = new TestVerifier( "src/test/resources" );

verifier.executeGoal( "test" );
slawekjaranowski marked this conversation as resolved.
Show resolved Hide resolved

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;
}
}
}