Skip to content

Contribute SystemPropertyExtension#5258

Merged
mpkorstanje merged 58 commits into
mainfrom
4726_passsystempropext
Jan 12, 2026
Merged

Contribute SystemPropertyExtension#5258
mpkorstanje merged 58 commits into
mainfrom
4726_passsystempropext

Conversation

@mpkorstanje

@mpkorstanje mpkorstanje commented Jan 2, 2026

Copy link
Copy Markdown
Member

Adopts the SystemPropertyExtension 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:

	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:

	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: #4726


I hereby agree to the terms of the JUnit Contributor License Agreement.


Definition of Done

@testlens-app

testlens-app Bot commented Jan 2, 2026

Copy link
Copy Markdown

✅ All tests passed ✅

🏷️ Commit: 902ee30
▶️ Tests: 45118 executed
⚪️ Checks: 15/15 completed


Learn more about TestLens at testlens.app.

@mpkorstanje mpkorstanje force-pushed the 4726_passsystempropext branch from 5115788 to 8426118 Compare January 2, 2026 21:44
Comment thread documentation/modules/ROOT/pages/writing-tests/built-in-extensions.adoc Outdated
Comment thread documentation/antora.yml Outdated
@mpkorstanje mpkorstanje requested a review from jbduncan January 11, 2026 17:59
@mpkorstanje mpkorstanje merged commit b60d9f0 into main Jan 12, 2026
18 checks passed
@mpkorstanje mpkorstanje deleted the 4726_passsystempropext branch January 12, 2026 16:05
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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Support read / write of system properties during test execution

4 participants