Skip to content

Commit

Permalink
[SUREFIRE-1385] Add new parameter "promoteUserPropertiesToSystemPrope…
Browse files Browse the repository at this point in the history 
…rties" (#762)

This allows to disable merging of user properties into effective system properties
Log overwritten properties
Clarify effective properties merging order
  • Loading branch information
@kwin
kwin authored Aug 7, 2024
1 parent 1d19ec8 commit 4bd36a1
Show file tree
Hide file tree
Showing 31 changed files with 274 additions and 72 deletions.
2 changes: 1 addition & 1 deletion maven-failsafe-plugin/pom.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
<parent>
<groupId>org.apache.maven.surefire</groupId>
<artifactId>surefire</artifactId>
<version>3.3.2-SNAPSHOT</version>
<version>3.4.0-SNAPSHOT</version>
</parent>

<groupId>org.apache.maven.plugins</groupId>
Expand Down
2 changes: 1 addition & 1 deletion maven-surefire-common/pom.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
<parent>
<groupId>org.apache.maven.surefire</groupId>
<artifactId>surefire</artifactId>
<version>3.3.2-SNAPSHOT</version>
<version>3.4.0-SNAPSHOT</version>
</parent>

<artifactId>maven-surefire-common</artifactId>
Expand Down
95 changes: 83 additions & 12 deletions ...-surefire-common/src/main/java/org/apache/maven/plugin/surefire/AbstractSurefireMojo.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import java.io.IOException;
import java.math.BigDecimal;
import java.nio.file.Files;
import java.text.ChoiceFormat;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
Expand Down Expand Up @@ -319,25 +320,45 @@ 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 (only used as source if {@link #promoteUserPropertiesToSystemProperties} is {@code true})</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}.
* <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;

/**
* If set to {@code true} will also pass all user properties exposed via {@link MavenSession#getUserProperties()} as system properties to a provider.
* Those always take precedence over same named system properties set via any other means.
*
* @since 3.4.0
* @see #systemPropertyVariables
*/
@Parameter(defaultValue = "true")
boolean promoteUserPropertiesToSystemProperties;

/**
* List of properties for configuring the testing provider. This is the preferred method of
Expand Down Expand Up @@ -425,7 +446,7 @@ public abstract class AbstractSurefireMojo extends AbstractMojo implements Suref
private String jvm;

/**
* Arbitrary JVM options to set on the command line.
* Arbitrary JVM options to set on the command line. Only effective for forked executions.
* <br>
* <br>
* Since the Version 2.17 using an alternate syntax for {@code argLine}, <b>@{...}</b> allows late replacement
Expand All @@ -438,6 +459,7 @@ public abstract class AbstractSurefireMojo extends AbstractMojo implements Suref
* <a href="http://maven.apache.org/surefire/maven-failsafe-plugin/faq.html">
* http://maven.apache.org/surefire/maven-failsafe-plugin/faq.html</a>
*
* @see #forkCount
* @since 2.1
*/
@Parameter(property = "argLine")
Expand Down Expand Up @@ -543,7 +565,7 @@ public abstract class AbstractSurefireMojo extends AbstractMojo implements Suref
* @since 2.14
*/
@Parameter(property = "forkCount", defaultValue = "1")
private String forkCount;
String forkCount;

/**
* Indicates if forked VMs can be reused. If set to "false", a new VM is forked for each test class to be executed.
Expand Down Expand Up @@ -1139,10 +1161,10 @@ protected List<ProviderInfo> createProviders(TestClassPath testClasspath) throws
new JUnit3ProviderInfo());
}

private SurefireProperties setupProperties() {
SurefireProperties sysProps = null;
SurefireProperties setupProperties() {
SurefireProperties sysPropsFromFile = null;
try {
sysProps = SurefireProperties.loadProperties(getSystemPropertiesFile());
sysPropsFromFile = SurefireProperties.loadProperties(getSystemPropertiesFile());
} catch (IOException e) {
String msg = "The file '" + getSystemPropertiesFile().getAbsolutePath() + "' can't be read.";
if (getConsoleLogger().isDebugEnabled()) {
Expand All @@ -1152,8 +1174,11 @@ private SurefireProperties setupProperties() {
}
}

SurefireProperties result = SurefireProperties.calculateEffectiveProperties(
getSystemProperties(), getSystemPropertyVariables(), getUserProperties(), sysProps);
SurefireProperties result = calculateEffectiveProperties(
getSystemProperties(),
getSystemPropertyVariables(),
promoteUserPropertiesToSystemProperties ? getUserProperties() : new Properties(),
sysPropsFromFile);

result.setProperty("basedir", getBasedir().getAbsolutePath());
result.setProperty("localRepository", getLocalRepositoryPath());
Expand Down Expand Up @@ -1184,6 +1209,38 @@ 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, "systemPropertiesFile"));
}
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
if (!userProperties.isEmpty()) {
overwrittenProperties = result.copyPropertiesFrom(userProperties);
if (!overwrittenProperties.isEmpty()) {
getConsoleLogger()
.warning(getOverwrittenPropertiesLogMessage(
overwrittenProperties, "user properties from Maven session"));
}
}
return result;
}

private Set<Object> systemPropertiesMatchingArgLine(SurefireProperties result) {
Set<Object> intersection = new HashSet<>();
if (isNotBlank(getArgLine())) {
Expand All @@ -1199,6 +1256,20 @@ private Set<Object> systemPropertiesMatchingArgLine(SurefireProperties result) {
return intersection;
}

private String getOverwrittenPropertiesLogMessage(
Collection<String> overwrittenProperties, String overwrittenBySource) {
if (overwrittenProperties.isEmpty()) {
throw new IllegalArgumentException("overwrittenProperties must not be empty");
}
// one or multiple?
ChoiceFormat propertyChoice = new ChoiceFormat("1#property|1>properties");
StringBuilder message = new StringBuilder("System ");
message.append(propertyChoice.format(overwrittenProperties.size())).append(" ");
message.append(overwrittenProperties.stream().collect(Collectors.joining("], [", "[", "]")));
message.append(" overwritten by ").append(overwrittenBySource);
return message.toString();
}

private void showToLog(SurefireProperties props, ConsoleLogger log) {
for (Object key : props.getStringKeySet()) {
String value = props.getProperty((String) key);
Expand Down Expand Up @@ -2718,7 +2789,7 @@ private Classpath getArtifactClasspath(Artifact surefireArtifact) throws MojoExe
return existing;
}

private Properties getUserProperties() {
Properties getUserProperties() {
return getSession().getUserProperties();
}

Expand Down
64 changes: 36 additions & 28 deletions maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/SurefireProperties.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
import java.util.Enumeration;
import java.util.HashSet;
import java.util.LinkedHashSet;
import java.util.LinkedList;
import java.util.List;
import java.util.Map;
import java.util.Properties;
Expand All @@ -40,7 +41,7 @@
import static java.util.Map.Entry;

/**
* A properties implementation that preserves insertion order.
* A {@link Properties} implementation that preserves insertion order.
*/
public class SurefireProperties extends Properties implements KeyValueSource {
private static final Collection<String> KEYS_THAT_CANNOT_BE_USED_AS_SYSTEM_PROPERTIES =
Expand All @@ -64,9 +65,17 @@ public SurefireProperties(KeyValueSource source) {

@Override
public synchronized void putAll(Map<?, ?> t) {
for (Entry<?, ?> entry : t.entrySet()) {
put(entry.getKey(), entry.getValue());
putAllInternal(t);
}

private Collection<String> putAllInternal(Map<?, ?> source) {
Collection<String> overwrittenProperties = new LinkedList<>();
for (Entry<?, ?> entry : source.entrySet()) {
if (put(entry.getKey(), entry.getValue()) != null) {
overwrittenProperties.add(entry.getKey().toString());
}
}
return overwrittenProperties;
}

@Override
Expand All @@ -92,12 +101,28 @@ public synchronized Enumeration<Object> keys() {
return Collections.enumeration(items);
}

public void copyPropertiesFrom(Properties source) {
/**
* Copies all keys and values from source to these properties, overwriting existing properties with same name
* @param source
* @return all overwritten property names (may be empty if there was no property name clash)
*/
public Collection<String> copyPropertiesFrom(Properties source) {
if (source != null) {
putAll(source);
return putAllInternal(source);
} else {
return Collections.emptyList();
}
}

/**
* Copies all keys and values from source to these properties, overwriting existing properties with same name
* @param source
* @return all overwritten property names (may be empty if there was no property name clash)
*/
public Collection<String> copyPropertiesFrom(Map<String, String> source) {
return copyProperties(this, source);
}

public Iterable<Object> getStringKeySet() {
return keySet();
}
Expand All @@ -121,34 +146,17 @@ public void copyToSystemProperties() {
}
}

static SurefireProperties calculateEffectiveProperties(
Properties systemProperties,
Map<String, String> systemPropertyVariables,
Properties userProperties,
SurefireProperties props) {
SurefireProperties result = new SurefireProperties();
result.copyPropertiesFrom(systemProperties);

result.copyPropertiesFrom(props);

copyProperties(result, 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

result.copyPropertiesFrom(userProperties);
return result;
}

private static void copyProperties(Properties target, Map<String, String> source) {
private static Collection<String> copyProperties(Properties target, Map<String, String> source) {
Collection<String> overwrittenProperties = new LinkedList<>();
if (source != null) {
for (String key : source.keySet()) {
String value = source.get(key);
target.setProperty(key, value == null ? "" : value);
if (target.setProperty(key, value == null ? "" : value) != null) {
overwrittenProperties.add(key);
}
}
}
return overwrittenProperties;
}

@Override
Expand Down
Loading

0 comments on commit 4bd36a1

Please sign in to comment.