Contribute SystemPropertyExtension#5258
Merged
Merged
Conversation
6 tasks
✅ All tests passed ✅🏷️ Commit: 902ee30 Learn more about TestLens at testlens.app. |
mpkorstanje
commented
Jan 2, 2026
5115788 to
8426118
Compare
mpkorstanje
commented
Jan 2, 2026
mpkorstanje
commented
Jan 2, 2026
mpkorstanje
commented
Jan 2, 2026
The implementation differs from the original JUnit Pioneer implementation.
mpkorstanje
commented
Jan 11, 2026
mpkorstanje
commented
Jan 11, 2026
mpkorstanje
commented
Jan 11, 2026
marcphilipp
approved these changes
Jan 12, 2026
mehulimukherjee
pushed a commit
to mehulimukherjee/junit-framework
that referenced
this pull request
Jan 14, 2026
Adopts the `SystemPropertyExtension`[1] from JUnit Pioneer into JUnit
Jupiter. While this is a faithful port some notable changes have been
made. With that in mind the API status has been set to experimental.
### `@RestoreSystemProperties` uses `Properties::clone`
When using `@RestoreSystemProperties` JUnit Pioneer would create a copy
of the effective properties in the object. This would include any
defaults as if they were regular values. I.e:
```java
static Properties createEffectiveClone(Properties original) {
Properties clone = new Properties();
// This implementation is used because:
// System.getProperties() returns the actual Properties object, not a copy.
// Clone doesn't include nested defaults, but propertyNames() does.
original.propertyNames().asIterator().forEachRemaining(k -> {
String v = original.getProperty(k.toString());
if (v != null) {
// v will be null if the actual value was an object
clone.put(k, original.getProperty(k.toString()));
}
});
return clone;
}
```
JUnit instead uses `Properties::clone` and performs a best effort
attempt to detect default properties and fail if any were detected. For
classes that extend `Properties` it is assumed that `clone` is
implemented with sufficient fidelity. I.e:
```java
static Properties cloneWithoutDefaults(ExtensionContext context, Properties properties) {
// Custom implementations have to implement clone correctly.
if (properties.getClass() == Properties.class) {
throwIfHasObservableDefaults(context, properties);
}
return (Properties) properties.clone();
}
```
We do this because:
1. System properties are to the best of our knowledge created without
defaults. Nor are we aware of a good usecase for defaults. If you do
have a use case for this, please do create a new issue to explain it.
2. Using `Properties::clone` provides a contract that is easier to
document and understand. It also allows subclasses of `Properties` to be
provided to tests. For example, a variant that does include defaults
when cloned.
### Restore "compromised" entries
While strongly discouraged non-string values can be added to a
properties object. The Pioneer implementation would not restore these
"compromised" values. This has been fixed in the JUnit implementation.
Closes: junit-team#4726
1. https://github.com/junit-pioneer/junit-pioneer/blob/062ce51296dab909119fd6c9f090b9d6a2982530/src/main/java/org/junitpioneer/jupiter/SystemPropertyExtension.java
Signed-off-by: mehulimukherjee <mehulimukherjee2017@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adopts the
SystemPropertyExtensionfrom JUnit Pioneer into JUnit Jupiter. While this is a faithful port some notable changes have been made. With that in mind the API status has been set to experimental.@RestoreSystemPropertiesusesProperties::cloneWhen using
@RestoreSystemPropertiesJUnit Pioneer would create a copy of the effective properties in the object. This would include any defaults as if they were regular values. I.e:JUnit instead uses
Properties::cloneand performs a best effort attempt to detect default properties and fail if any were detected. For classes that extendPropertiesit is assumed thatcloneis implemented with sufficient fidelity. I.e:We do this because:
Properties::cloneprovides a contract that is easier to document and understand. It also allows subclasses ofPropertiesto be provided to tests. For example, a variant that does include defaults when cloned.Restore "compromised" entries
While strongly discouraged non-string values can be added to a properties object. The Pioneer implementation would not restore these "compromised" values. This has been fixed in the JUnit implementation.
Closes: #4726
I hereby agree to the terms of the JUnit Contributor License Agreement.
Definition of Done
@APIannotations