[
https://issues.apache.org/jira/browse/SUREFIRE-1385?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17869645#comment-17869645
]
ASF GitHub Bot commented on SUREFIRE-1385:
------------------------------------------
kwin commented on code in PR #762:
URL: https://github.com/apache/maven-surefire/pull/762#discussion_r1696964338
##########
maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/AbstractSurefireMojo.java:
##########
@@ -1184,6 +1208,39 @@ private SurefireProperties setupProperties() {
return result;
}
+ private SurefireProperties calculateEffectiveProperties(
+ Properties systemProperties,
+ Map<String, String> systemPropertyVariables,
+ Properties userProperties,
+ SurefireProperties sysPropsFromFile) {
+ SurefireProperties result = new SurefireProperties();
+ result.copyPropertiesFrom(systemProperties);
+
+ Collection<String> overwrittenProperties =
result.copyPropertiesFrom(sysPropsFromFile);
+ if (!overwrittenProperties.isEmpty() &&
getConsoleLogger().isDebugEnabled()) {
+
getConsoleLogger().debug(getOverwrittenPropertiesLogMessage(overwrittenProperties,
"sysPropsFile"));
+ }
+ overwrittenProperties =
result.copyPropertiesFrom(systemPropertyVariables);
+ if (!overwrittenProperties.isEmpty() &&
getConsoleLogger().isDebugEnabled()) {
+ getConsoleLogger()
+
.debug(getOverwrittenPropertiesLogMessage(overwrittenProperties,
"systemPropertyVariables"));
+ }
+ // We used to take all of our system properties and dump them in with
the
+ // user specified properties for SUREFIRE-121, causing SUREFIRE-491.
+ // Not gonna do THAT any more... instead, we only propagate those
system properties
+ // that have been explicitly specified by the user via -Dkey=value on
the CLI
+ // and only if useUserPropertiesAsSystemProperty is set to true
Review Comment:
Removed that line in
https://github.com/apache/maven-surefire/pull/762/commits/f21ac963663d27d8b316d789648cb6da44088e4d.
##########
maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/AbstractSurefireMojo.java:
##########
@@ -1184,6 +1208,39 @@ private SurefireProperties setupProperties() {
return result;
}
+ private SurefireProperties calculateEffectiveProperties(
+ Properties systemProperties,
+ Map<String, String> systemPropertyVariables,
+ Properties userProperties,
+ SurefireProperties sysPropsFromFile) {
+ SurefireProperties result = new SurefireProperties();
+ result.copyPropertiesFrom(systemProperties);
+
+ Collection<String> overwrittenProperties =
result.copyPropertiesFrom(sysPropsFromFile);
+ if (!overwrittenProperties.isEmpty() &&
getConsoleLogger().isDebugEnabled()) {
+
getConsoleLogger().debug(getOverwrittenPropertiesLogMessage(overwrittenProperties,
"sysPropsFile"));
Review Comment:
Fixed in
https://github.com/apache/maven-surefire/pull/762/commits/f21ac963663d27d8b316d789648cb6da44088e4d
> 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)
