[ 
https://issues.apache.org/jira/browse/SUREFIRE-1385?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17868177#comment-17868177
 ] 

ASF GitHub Bot commented on SUREFIRE-1385:
------------------------------------------

michael-o commented on code in PR #762:
URL: https://github.com/apache/maven-surefire/pull/762#discussion_r1678243230


##########
maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/AbstractSurefireMojo.java:
##########
@@ -319,25 +320,49 @@ public abstract class AbstractSurefireMojo extends 
AbstractMojo implements Suref
      */
     @Deprecated
     @Parameter
-    private Properties systemProperties;
+    Properties systemProperties;
 
     /**
      * List of System properties to pass to a provider.
      * The effective system properties given to a provider are contributed 
from several sources:
      * <ol>
+     * <li>properties set via {@link #argLine} with {@code -D} (only for 
forked executions)</li>
      * <li>{@link #systemProperties}</li>
      * <li>{@link AbstractSurefireMojo#getSystemPropertiesFile()} (set via 
parameter {@code systemPropertiesFile} on some goals)</li>
      * <li>{@link #systemPropertyVariables}</li>
-     * <li>User properties from {@link MavenSession#getUserProperties()}, 
usually set via CLI options given with {@code -D}</li>
+     * <li>User properties from {@link MavenSession#getUserProperties()}, 
usually set via CLI options given with {@code -D} on the current Maven 
process</li>
+     * <li>{@link #userPropertyVariables}</li>
      * </ol>
      * Later sources may overwrite same named properties from earlier sources, 
that means for example that one cannot overwrite user properties with either
-     * {@link #systemProperties}, {@link 
AbstractSurefireMojo#getSystemPropertiesFile()} or {@link 
#systemPropertyVariables}.
+     * {@link #systemProperties}, {@link #getSystemPropertiesFile()} or {@link 
#systemPropertyVariables} but only with
+     * {@link #userPropertyVariables}.
+     * <p>
+     * Certain properties may only be overwritten via {@link #argLine} 
(applicable only for forked executions) because their values are cached and 
only evaluated at the start of the JRE.
+     * Those include:
+     * <ul>
+     * <li>{@code java.library.path}</li>
+     * <li>{@code file.encoding}</li>
+     * <li>{@code jdk.map.althashing.threshold}</li>
+     * <li>{@code line.separator}</li>
+     * </ul>
      *
      * @since 2.5
      * @see #systemProperties
      */
     @Parameter
-    private Map<String, String> systemPropertyVariables;
+    Map<String, String> systemPropertyVariables;
+
+    /**
+     * List of user properties to pass to a provider.
+     * Similar to {@link #systemPropertyVariables} but having a higher 
precedence, therefore allows to overwrite user properties from the current 
Maven session.
+     * This should only be used in case a user property from the parent 
process needs to be explicitly overwritten.
+     * Regular properties should be set via {@link #systemPropertyVariables} 
instead in order to allow them to be overwritten
+     * via CLI arguments ({@code -Dmyproperty=myvalue})
+     * @since 3.4
+     * @see #systemPropertyVariables
+     */
+    @Parameter
+    Map<String, String> userPropertyVariables;

Review Comment:
   My abstract understanding:
   * system properties (include MAVEN_OPTS logically)
   * system properties file
   * system property variables in POM
   * user properties file
   * user property variables in POM
   * user properties from CLI
   
   WDYT?





> System properties defined in the Surefire and Failsafe plugin configuration 
> should override user properties
> -----------------------------------------------------------------------------------------------------------
>
>                 Key: SUREFIRE-1385
>                 URL: https://issues.apache.org/jira/browse/SUREFIRE-1385
>             Project: Maven Surefire
>          Issue Type: Improvement
>          Components: Maven Failsafe Plugin, Maven Surefire Plugin
>    Affects Versions: 2.20
>            Reporter: Guillaume Boué
>            Assignee: Konrad Windszus
>            Priority: Major
>
> Consider a build with the following POM configuration for the Maven Failsafe 
> Plugin:
> {code:xml}
> <configuration>
>   <systemPropertyVariables>
>     <prop>foo</prop>
>   </systemPropertyVariables>
> </configuration>
> {code}
> When running the build with the command line {{mvn -Dprop=bar ...}}, the 
> tests would be passed a system property with a value of {{bar}} instead of 
> {{foo}}.
> This is counter-intuitive since direct configuration of the plugin is 
> overriden by the more general properties passed on the command line. I would 
> have expected the closer definition in the POM to override the one passed 
> with the CLI. Furthermore, in the case of the above sample, it would not be 
> possible for the tests run by the Failsafe Plugin to have a system property 
> {{prop}} with a value of {{foo}} if the build happens to already define a 
> system property with the same name. While using a different name to avoid a 
> clash is possible, it still doesn't make the test self-contained and 
> consistent since anyone could run Maven with that other name and compromise 
> the test that really relies on the system property having a value of {{foo}}.
> The proposal is thus to make the {{systemPropertyVariables}} and 
> {{systemPropertiesFile}} configuration elements of the Surefire and Failsafe 
> Plugin take precedence over user properties passed on the command line.
> Proposed commit 
> [4de017b38b101b0b28f9fbed135eae3921b99d0d|http://git-wip-us.apache.org/repos/asf/maven-surefire/commit/4de017b3]
>  on SUREFIRE-1385 branch.



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

Reply via email to