rmannibucau commented on a change in pull request #340:
URL: https://github.com/apache/maven-surefire/pull/340#discussion_r593720594
##########
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:
About java 8: it is maven 4 time since it is already set to 8 but not
v3, agree.
About why setter is saner than any other "flow" method: because class is
public+abstract and getter as well so it is part of the contract of the class
to convert it in the setter - as you said on slack - and not later on in the
flow which can be missed by the contract. So we actually don't have much
choice. We can create an utility but as mentionned I think it is worse than
duplicating the inline test IMHO so think we are good.
Now if you want to fix it differently I'll not fight while it behaves the
same.
side note: i'll not get time this week end/start of next week to modify this
PR and can be slow to answer.
----------------------------------------------------------------
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:
[email protected]
