[
https://issues.apache.org/jira/browse/SUREFIRE-1385?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17868181#comment-17868181
]
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_r1688677896
##########
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:
@slawekjaranowski I agree with you that passing user properties by default
is a bad idea.
> 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)
