rmannibucau commented on a change in pull request #340:
URL: https://github.com/apache/maven-surefire/pull/340#discussion_r590002068
##########
File path:
maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/AbstractSurefireMojo.java
##########
@@ -3623,9 +3623,20 @@ public void setSystemProperties( Properties
systemProperties )
}
@SuppressWarnings( "UnusedDeclaration" )
- public void setSystemPropertyVariables( Map<String, String>
systemPropertyVariables )
+ public void setSystemPropertyVariables( Map<String, ?>
systemPropertyVariables )
{
- this.systemPropertyVariables = systemPropertyVariables;
+ if (systemPropertyVariables != null)
+ {
+ this.systemPropertyVariables = new HashMap<>();
+ for ( final Map.Entry<String, ?> entry :
systemPropertyVariables.entrySet() )
+ {
+ this.systemPropertyVariables.put( entry.getKey(),
String.valueOf( entry.getValue() ) );
Review comment:
Yep got it but have to admit I'm mixed about it, I like your setter
proposal because it makes the abstract data consistent for any consumer whereas
copyProperties can let subclasses/consumers see wrong data which is worse IMHO.
About the utility I'm tempted to say the gain is very light and mainly
envisionned because we can't use lambda yet. Since maven master is on java 8
and plugins already started to move I suspect this will drop as soon as we move
to java 8 so let's keep it simple maybe?
Wdyt?
----------------------------------------------------------------
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:
us...@infra.apache.org
