Skip to content

Commit 95656d2

Browse files
marcphilippmarcphilipp
authored
Fail for invalid enum constants supplied as configuration parameters (#4781)
* Fail for invalid enum constants supplied as configuration parameters * Document case insensitivity of config params Resolves #4617.
1 parent 76f97e9 commit 95656d2

File tree

14 files changed

+215
-174
lines changed

14 files changed

+215
-174
lines changed

‎documentation/src/docs/asciidoc/release-notes/release-notes-6.0.0-M2.adoc

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,10 @@ repository on GitHub.
3131
format used by the deprecated `Locale(String)` constructor.
3232
* Deprecate `getOrComputeIfAbsent(...)` methods in `NamespacedHierarchicalStore` in favor
3333
of the new `computeIfAbsent(...)` methods.
34+
* Setting an invalid value for one of the following enum-based configuration parameters
35+
now causes test discovery to fail:
36+
- `junit.platform.discovery.issue.failure.phase`
37+
- `junit.platform.discovery.issue.severity.critical`
3438

3539
[[release-notes-6.0.0-M2-junit-platform-new-features-and-improvements]]
3640
==== New Features and Improvements
@@ -78,6 +82,15 @@ repository on GitHub.
7882
method.
7983
* Deprecate `getOrComputeIfAbsent(...)` methods in `ExtensionContext.Store` in favor of
8084
the new `computeIfAbsent(...)` methods.
85+
* Setting an invalid value for one of the following enum-based configuration parameters
86+
now causes test discovery or execution to fail:
87+
- `junit.jupiter.execution.parallel.mode.default`
88+
- `junit.jupiter.execution.parallel.mode.classes.default`
89+
- `junit.jupiter.execution.timeout.mode`
90+
- `junit.jupiter.execution.timeout.thread.mode.default`
91+
- `junit.jupiter.extensions.testinstantiation.extensioncontextscope.default`
92+
- `junit.jupiter.tempdir.cleanup.mode.default`
93+
- `junit.jupiter.testinstance.lifecycle.default`
8194

8295
[[release-notes-6.0.0-M2-junit-jupiter-new-features-and-improvements]]
8396
==== New Features and Improvements

‎junit-jupiter-api/src/main/java/org/junit/jupiter/api/Timeout.java

Lines changed: 16 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -298,36 +298,40 @@
298298
String DEFAULT_AFTER_ALL_METHOD_TIMEOUT_PROPERTY_NAME = "junit.jupiter.execution.timeout.afterall.method.default";
299299

300300
/**
301-
* Property name used to configure whether timeouts are applied to tests: {@value}.
301+
* Property name used to configure whether timeouts are applied to tests:
302+
* {@value}.
302303
*
303304
* <p>The value of this property will be used to toggle whether
304305
* {@link Timeout @Timeout} is applied to tests.</p>
305306
*
306-
* <h4>Supported timeout mode values:</h4>
307+
* <h4>Supported timeout mode values (case insensitive):</h4>
307308
* <ul>
308-
* <li>{@code enabled}: enables timeouts
309-
* <li>{@code disabled}: disables timeouts
310-
* <li>{@code disabled_on_debug}: disables timeouts while debugging
309+
* <li>{@code ENABLED}: enables timeouts
310+
* <li>{@code DISABLED}: disables timeouts
311+
* <li>{@code DISABLED_ON_DEBUG}: disables timeouts while debugging
311312
* </ul>
312313
*
313-
* <p>If not specified, the default is {@code enabled}.
314+
* <p>If not specified, the default is {@code ENABLED}.
314315
*
315316
* @since 5.6
316317
*/
317318
@API(status = STABLE, since = "5.9")
318319
String TIMEOUT_MODE_PROPERTY_NAME = "junit.jupiter.execution.timeout.mode";
319320

320321
/**
321-
* Property name used to set the default thread mode for all testable and lifecycle
322-
* methods: "junit.jupiter.execution.timeout.thread.mode.default".
322+
* Property name used to set the default thread mode for all testable and
323+
* lifecycle methods: {@value}.
323324
*
324-
* <p>The value of this property will be used unless overridden by a {@link Timeout @Timeout}
325-
* annotation present on the method or on an enclosing test class (for testable methods).
325+
* <p>The value of this property will be used unless overridden by a
326+
* {@link Timeout @Timeout} annotation present on the method or on an
327+
* enclosing test class (for testable methods).
326328
*
327-
* <p>The supported values are {@code SAME_THREAD} or {@code SEPARATE_THREAD}, if none is provided
329+
* <p>The supported values are {@code SAME_THREAD} or
330+
* {@code SEPARATE_THREAD}, ignoring case. If none is provided,
328331
* {@code SAME_THREAD} is used as default.
329332
*
330333
* @since 5.9
334+
* @see #threadMode()
331335
*/
332336
@API(status = MAINTAINED, since = "5.13.3")
333337
String DEFAULT_TIMEOUT_THREAD_MODE_PROPERTY_NAME = "junit.jupiter.execution.timeout.thread.mode.default";
@@ -353,6 +357,7 @@
353357
* @return thread mode
354358
* @since 5.9
355359
* @see ThreadMode
360+
* @see #DEFAULT_TIMEOUT_THREAD_MODE_PROPERTY_NAME
356361
*/
357362
@API(status = STABLE, since = "5.11")
358363
ThreadMode threadMode() default ThreadMode.INFERRED;

‎junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/config/DefaultJupiterConfiguration.java

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -149,19 +149,19 @@ public boolean isThreadDumpOnTimeoutEnabled() {
149149

150150
@Override
151151
public ExecutionMode getDefaultExecutionMode() {
152-
return executionModeConverter.get(configurationParameters, DEFAULT_EXECUTION_MODE_PROPERTY_NAME,
152+
return executionModeConverter.getOrDefault(configurationParameters, DEFAULT_EXECUTION_MODE_PROPERTY_NAME,
153153
ExecutionMode.SAME_THREAD);
154154
}
155155

156156
@Override
157157
public ExecutionMode getDefaultClassesExecutionMode() {
158-
return executionModeConverter.get(configurationParameters, DEFAULT_CLASSES_EXECUTION_MODE_PROPERTY_NAME,
159-
getDefaultExecutionMode());
158+
return executionModeConverter.getOrDefault(configurationParameters,
159+
DEFAULT_CLASSES_EXECUTION_MODE_PROPERTY_NAME, getDefaultExecutionMode());
160160
}
161161

162162
@Override
163163
public Lifecycle getDefaultTestInstanceLifecycle() {
164-
return lifecycleConverter.get(configurationParameters, DEFAULT_TEST_INSTANCE_LIFECYCLE_PROPERTY_NAME,
164+
return lifecycleConverter.getOrDefault(configurationParameters, DEFAULT_TEST_INSTANCE_LIFECYCLE_PROPERTY_NAME,
165165
Lifecycle.PER_METHOD);
166166
}
167167

@@ -189,7 +189,7 @@ public Optional<ClassOrderer> getDefaultTestClassOrderer() {
189189

190190
@Override
191191
public CleanupMode getDefaultTempDirCleanupMode() {
192-
return cleanupModeConverter.get(configurationParameters, DEFAULT_CLEANUP_MODE_PROPERTY_NAME, ALWAYS);
192+
return cleanupModeConverter.getOrDefault(configurationParameters, DEFAULT_CLEANUP_MODE_PROPERTY_NAME, ALWAYS);
193193
}
194194

195195
@Override
@@ -202,7 +202,7 @@ public Supplier<TempDirFactory> getDefaultTempDirFactorySupplier() {
202202
@SuppressWarnings("deprecation")
203203
@Override
204204
public ExtensionContextScope getDefaultTestInstantiationExtensionContextScope() {
205-
return extensionContextScopeConverter.get(configurationParameters,
205+
return extensionContextScopeConverter.getOrDefault(configurationParameters,
206206
DEFAULT_TEST_INSTANTIATION_EXTENSION_CONTEXT_SCOPE_PROPERTY_NAME, ExtensionContextScope.DEFAULT);
207207
}
208208

‎junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/config/EnumConfigurationParameterConverter.java

Lines changed: 21 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -14,12 +14,12 @@
1414

1515
import java.util.Locale;
1616
import java.util.Optional;
17-
import java.util.function.Function;
1817

1918
import org.apiguardian.api.API;
19+
import org.junit.jupiter.api.extension.ExtensionContext;
20+
import org.junit.platform.commons.JUnitException;
2021
import org.junit.platform.commons.logging.Logger;
2122
import org.junit.platform.commons.logging.LoggerFactory;
22-
import org.junit.platform.commons.util.Preconditions;
2323
import org.junit.platform.engine.ConfigurationParameters;
2424

2525
/**
@@ -38,36 +38,29 @@ public EnumConfigurationParameterConverter(Class<E> enumType, String enumDisplay
3838
this.enumDisplayName = enumDisplayName;
3939
}
4040

41-
E get(ConfigurationParameters configParams, String key, E defaultValue) {
42-
Preconditions.notNull(configParams, "ConfigurationParameters must not be null");
43-
44-
return get(key, configParams::get, defaultValue);
41+
public Optional<E> get(ExtensionContext extensionContext, String key) {
42+
return extensionContext.getConfigurationParameter(key, value -> convert(key, value));
4543
}
4644

47-
public E get(String key, Function<String, Optional<String>> lookup, E defaultValue) {
48-
49-
Optional<String> value = lookup.apply(key);
45+
E getOrDefault(ConfigurationParameters configParams, String key, E defaultValue) {
46+
return configParams.get(key) //
47+
.map(value -> convert(key, value)) //
48+
.orElse(defaultValue);
49+
}
5050

51-
if (value.isPresent()) {
52-
String constantName = null;
53-
try {
54-
constantName = value.get().strip().toUpperCase(Locale.ROOT);
55-
E result = Enum.valueOf(enumType, constantName);
56-
logger.config(() -> "Using %s '%s' set via the '%s' configuration parameter.".formatted(enumDisplayName,
57-
result, key));
58-
return result;
59-
}
60-
catch (Exception ex) {
61-
// local copy necessary for use in lambda expression
62-
String constant = constantName;
63-
logger.warn(() -> """
64-
Invalid %s '%s' set via the '%s' configuration parameter. \
65-
Falling back to the %s default value.""".formatted(enumDisplayName, constant, key,
66-
defaultValue.name()));
67-
}
51+
private E convert(String key, String value) {
52+
String constantName = null;
53+
try {
54+
constantName = value.strip().toUpperCase(Locale.ROOT);
55+
E result = Enum.valueOf(enumType, constantName);
56+
logger.config(() -> "Using %s '%s' set via the '%s' configuration parameter.".formatted(enumDisplayName,
57+
result, key));
58+
return result;
59+
}
60+
catch (Exception ex) {
61+
throw new JUnitException("Invalid %s '%s' set via the '%s' configuration parameter.".formatted(
62+
enumDisplayName, constantName, key));
6863
}
69-
70-
return defaultValue;
7164
}
7265

7366
}

‎junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/extension/TimeoutConfiguration.java

Lines changed: 39 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -21,10 +21,8 @@
2121
import static org.junit.jupiter.api.Timeout.DEFAULT_TEST_TEMPLATE_METHOD_TIMEOUT_PROPERTY_NAME;
2222
import static org.junit.jupiter.api.Timeout.DEFAULT_TIMEOUT_PROPERTY_NAME;
2323
import static org.junit.jupiter.api.Timeout.DEFAULT_TIMEOUT_THREAD_MODE_PROPERTY_NAME;
24-
import static org.junit.jupiter.api.Timeout.ThreadMode.SAME_THREAD;
25-
import static org.junit.jupiter.api.Timeout.ThreadMode.SEPARATE_THREAD;
24+
import static org.junit.jupiter.api.Timeout.TIMEOUT_MODE_PROPERTY_NAME;
2625

27-
import java.util.Locale;
2826
import java.util.Map;
2927
import java.util.Optional;
3028
import java.util.concurrent.ConcurrentHashMap;
@@ -33,8 +31,10 @@
3331

3432
import org.junit.jupiter.api.Timeout.ThreadMode;
3533
import org.junit.jupiter.api.extension.ExtensionContext;
34+
import org.junit.jupiter.engine.config.EnumConfigurationParameterConverter;
3635
import org.junit.platform.commons.logging.Logger;
3736
import org.junit.platform.commons.logging.LoggerFactory;
37+
import org.junit.platform.commons.util.RuntimeUtils;
3838

3939
/**
4040
* @since 5.5
@@ -47,9 +47,18 @@ class TimeoutConfiguration {
4747
private final Map<String, Optional<TimeoutDuration>> cache = new ConcurrentHashMap<>();
4848
private final AtomicReference<Optional<ThreadMode>> threadMode = new AtomicReference<>();
4949
private final ExtensionContext extensionContext;
50+
private final boolean timeoutDisabled;
5051

5152
TimeoutConfiguration(ExtensionContext extensionContext) {
5253
this.extensionContext = extensionContext;
54+
this.timeoutDisabled = new EnumConfigurationParameterConverter<>(TimeoutMode.class, "timeout mode") //
55+
.get(extensionContext, TIMEOUT_MODE_PROPERTY_NAME) //
56+
.map(TimeoutMode::isTimeoutDisabled) //
57+
.orElse(false);
58+
}
59+
60+
boolean isTimeoutDisabled() {
61+
return timeoutDisabled;
5362
}
5463

5564
Optional<TimeoutDuration> getDefaultTestMethodTimeout() {
@@ -125,23 +134,34 @@ Optional<ThreadMode> getDefaultTimeoutThreadMode() {
125134
}
126135

127136
private Optional<ThreadMode> parseTimeoutThreadModeConfiguration() {
128-
return extensionContext.getConfigurationParameter(DEFAULT_TIMEOUT_THREAD_MODE_PROPERTY_NAME).map(value -> {
129-
try {
130-
ThreadMode threadMode = ThreadMode.valueOf(value.toUpperCase(Locale.ROOT));
131-
if (threadMode == ThreadMode.INFERRED) {
132-
logger.warn(
133-
() -> "Invalid timeout thread mode '%s', only %s and %s can be used as configuration parameter for %s.".formatted(
134-
value, SAME_THREAD, SEPARATE_THREAD, DEFAULT_TIMEOUT_THREAD_MODE_PROPERTY_NAME));
135-
return null;
136-
}
137-
return threadMode;
137+
return new EnumConfigurationParameterConverter<>(ThreadMode.class, "timeout thread mode") //
138+
.get(extensionContext, DEFAULT_TIMEOUT_THREAD_MODE_PROPERTY_NAME);
139+
}
140+
141+
private enum TimeoutMode {
142+
143+
ENABLED {
144+
@Override
145+
boolean isTimeoutDisabled() {
146+
return false;
138147
}
139-
catch (Exception e) {
140-
logger.warn(e,
141-
() -> "Invalid timeout thread mode '%s' set via the '%s' configuration parameter.".formatted(value,
142-
DEFAULT_TIMEOUT_THREAD_MODE_PROPERTY_NAME));
143-
return null;
148+
},
149+
150+
DISABLED {
151+
@Override
152+
boolean isTimeoutDisabled() {
153+
return true;
144154
}
145-
});
155+
},
156+
157+
DISABLED_ON_DEBUG {
158+
@Override
159+
boolean isTimeoutDisabled() {
160+
return RuntimeUtils.isDebugMode();
161+
}
162+
};
163+
164+
abstract boolean isTimeoutDisabled();
146165
}
166+
147167
}

0 commit comments

Comments
 (0)