Skip to content

Introduce feature flag for auto-closing AutoCloseable in Jupiter's ExtensionContext.Store #4442

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 5 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,10 @@ repository on GitHub.
<<../user-guide/index.adoc#launcher-api-managing-state-across-test-engines, request-scoped>>
resources.

* CloseableResource is now deprecated and extends `AutoCloseable`. This change is
intended to improve the usability of the API and to align it with the standard Java
`AutoCloseable` interface.


[[release-notes-5.13.0-M3-junit-jupiter]]
=== JUnit Jupiter
Expand Down
6 changes: 5 additions & 1 deletion documentation/src/docs/asciidoc/user-guide/extensions.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -864,11 +864,15 @@ inner contexts may also be limited. Consult the corresponding Javadoc for detail
methods available for storing and retrieving values via the `{ExtensionContext_Store}`.

.`ExtensionContext.Store.CloseableResource`
NOTE: An extension context store is bound to its extension context lifecycle. When an
NOTE: `CloseableResource` is now deprecated and extends `AutoCloseable`.
An extension context store is bound to its extension context lifecycle. When an
extension context lifecycle ends it closes its associated store. All stored values
that are instances of `CloseableResource` are notified by an invocation of their `close()`
method in the inverse order they were added in.

When the `junit.jupiter.store.autocloseable.as.closeableresource` property is set to `true`,
any `AutoCloseable` instances stored via JUnit Jupiter\'s `Store` will be treated as `CloseableResource`.

An example implementation of `CloseableResource` is shown below, using an `HttpServer`
resource.

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,14 @@

import com.sun.net.httpserver.HttpServer;

import org.junit.jupiter.api.extension.ExtensionContext.Store.CloseableResource;
import org.junit.jupiter.api.extension.ExtensionContext.Store;

/**
* Demonstrates an implementation of {@link CloseableResource} using an {@link HttpServer}.
* Demonstrates an implementation of {@link Store.CloseableResource} using an {@link HttpServer}.
*/
// tag::user_guide[]
class HttpServerResource implements CloseableResource {
@SuppressWarnings("deprecation")
class HttpServerResource implements Store.CloseableResource {

private final HttpServer httpServer;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
package org.junit.jupiter.api.extension;

import static java.util.Collections.unmodifiableList;
import static org.apiguardian.api.API.Status.DEPRECATED;
import static org.apiguardian.api.API.Status.EXPERIMENTAL;
import static org.apiguardian.api.API.Status.INTERNAL;
import static org.apiguardian.api.API.Status.STABLE;
Expand Down Expand Up @@ -503,16 +504,19 @@ interface Store {
* inverse order they were added in.
*
* @since 5.1
* @deprecated Please extend {@code AutoCloseable} directly.
*/
@API(status = STABLE, since = "5.1")
interface CloseableResource {
@SuppressWarnings("try")
@Deprecated
@API(status = DEPRECATED, since = "5.13")
interface CloseableResource extends AutoCloseable {

/**
* Close underlying resources.
*
* @throws Throwable any throwable will be caught and rethrown
* @throws Exception any exception will be caught and rethrown
*/
void close() throws Throwable;
void close() throws Exception;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changing this would be binary-incompatible. Thus, I don't think we can have CloseableResource extend AutoCloseable after all. I guess we could support both CloseableResource and AutoCloseable instead as separate interfaces. I'll discuss this with the team.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with you.
Please discuss this with the team and share the outcome! 👍🏻

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@YongGoose We've updated the issue description in #4434.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it!
I will close this PR, start a new task, and then create a new PR.

Also, if you have some time, could you briefly explain how the discussion went on the issue below? 🙂

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, if you have some time, could you briefly explain how the discussion went on the issue below? 🙂

I just posted an update: #1767 (comment)


}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
import org.apiguardian.api.API;
import org.junit.jupiter.api.TestInstance;
import org.junit.jupiter.api.extension.ExtensionContext.Store;
import org.junit.jupiter.api.extension.ExtensionContext.Store.CloseableResource;

/**
* Interface for {@link Extension Extensions} that are aware and can influence
Expand Down Expand Up @@ -65,7 +64,7 @@ public interface TestInstantiationAwareExtension extends Extension {
* <li>{@link ExtensionContext#getTestMethod() getTestMethod()} is no longer
* empty, unless the {@link TestInstance.Lifecycle#PER_CLASS PER_CLASS}
* lifecycle is used.</li>
* <li>If the callback adds a new {@link CloseableResource} to the
* <li>If the callback adds a new {@link Store.CloseableResource} to the
* {@link Store Store}, the resource is closed just after the instance is
* destroyed.</li>
* <li>The callbacks can now access data previously stored by
Expand All @@ -88,6 +87,7 @@ public interface TestInstantiationAwareExtension extends Extension {
* configuration parameters; never {@code null}
* @since 5.12
*/
@SuppressWarnings("deprecation")
@API(status = EXPERIMENTAL, since = "5.12")
default ExtensionContextScope getTestInstantiationExtensionContextScope(ExtensionContext rootContext) {
return ExtensionContextScope.DEFAULT;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -210,6 +210,18 @@ public final class Constants {
@API(status = STABLE, since = "5.10")
public static final String PARALLEL_EXECUTION_ENABLED_PROPERTY_NAME = JupiterConfiguration.PARALLEL_EXECUTION_ENABLED_PROPERTY_NAME;

/**
* Property name used to enable the use of {@link AutoCloseable} instances as
* {@link org.junit.jupiter.api.extension.ExtensionContext.Store.CloseableResource} resources: {@value}
*
* <p>By default, this feature is disabled.
*
* @since 5.13
*/
@SuppressWarnings("deprecation")
@API(status = EXPERIMENTAL, since = "5.13")
public static final String AUTOCLOSEABLE_AS_CLOSEABLERESOURCE_PROPERTY_NAME = JupiterConfiguration.AUTOCLOSEABLE_AS_CLOSEABLERESOURCE_PROPERTY_NAME;

/**
* Property name used to set the default test execution mode: {@value}
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,12 @@ public boolean isExtensionAutoDetectionEnabled() {
__ -> delegate.isExtensionAutoDetectionEnabled());
}

@Override
public boolean isAutocloseableAsCloseableResourceEnabled() {
return (boolean) cache.computeIfAbsent(AUTOCLOSEABLE_AS_CLOSEABLERESOURCE_PROPERTY_NAME,
__ -> delegate.isAutocloseableAsCloseableResourceEnabled());
}

@Override
public boolean isThreadDumpOnTimeoutEnabled() {
return (boolean) cache.computeIfAbsent(EXTENSIONS_TIMEOUT_THREAD_DUMP_ENABLED_PROPERTY_NAME,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,12 @@ public boolean isParallelExecutionEnabled() {
return configurationParameters.getBoolean(PARALLEL_EXECUTION_ENABLED_PROPERTY_NAME).orElse(false);
}

// TODO [6.0]: Switch the default to 'true' for this feature in JUnit5 6.0.
@Override
public boolean isAutocloseableAsCloseableResourceEnabled() {
return configurationParameters.getBoolean(AUTOCLOSEABLE_AS_CLOSEABLERESOURCE_PROPERTY_NAME).orElse(false);
}

@Override
public boolean isExtensionAutoDetectionEnabled() {
return configurationParameters.getBoolean(EXTENSIONS_AUTODETECTION_ENABLED_PROPERTY_NAME).orElse(false);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ public interface JupiterConfiguration {
String EXTENSIONS_AUTODETECTION_EXCLUDE_PROPERTY_NAME = "junit.jupiter.extensions.autodetection.exclude";
String DEACTIVATE_CONDITIONS_PATTERN_PROPERTY_NAME = "junit.jupiter.conditions.deactivate";
String PARALLEL_EXECUTION_ENABLED_PROPERTY_NAME = "junit.jupiter.execution.parallel.enabled";
String AUTOCLOSEABLE_AS_CLOSEABLERESOURCE_PROPERTY_NAME = "junit.jupiter.store.autocloseable.as.closeableresource";
String DEFAULT_EXECUTION_MODE_PROPERTY_NAME = Execution.DEFAULT_EXECUTION_MODE_PROPERTY_NAME;
String DEFAULT_CLASSES_EXECUTION_MODE_PROPERTY_NAME = Execution.DEFAULT_CLASSES_EXECUTION_MODE_PROPERTY_NAME;
String EXTENSIONS_AUTODETECTION_ENABLED_PROPERTY_NAME = "junit.jupiter.extensions.autodetection.enabled";
Expand All @@ -60,6 +61,8 @@ public interface JupiterConfiguration {

boolean isParallelExecutionEnabled();

boolean isAutocloseableAsCloseableResourceEnabled();

boolean isExtensionAutoDetectionEnabled();

boolean isThreadDumpOnTimeoutEnabled();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@
import org.junit.jupiter.api.extension.ExecutableInvoker;
import org.junit.jupiter.api.extension.Extension;
import org.junit.jupiter.api.extension.ExtensionContext;
import org.junit.jupiter.api.extension.ExtensionContext.Store.CloseableResource;
import org.junit.jupiter.api.extension.MediaType;
import org.junit.jupiter.api.function.ThrowingConsumer;
import org.junit.jupiter.api.parallel.ExecutionMode;
Expand All @@ -51,10 +50,11 @@
*/
abstract class AbstractExtensionContext<T extends TestDescriptor> implements ExtensionContextInternal, AutoCloseable {

@SuppressWarnings("deprecation")
private static final NamespacedHierarchicalStore.CloseAction<org.junit.platform.engine.support.store.Namespace> CLOSE_RESOURCES = (
__, ___, value) -> {
if (value instanceof CloseableResource) {
((CloseableResource) value).close();
if (value instanceof Store.CloseableResource) {
((Store.CloseableResource) value).close();
}
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@
import org.junit.jupiter.api.extension.ExtensionConfigurationException;
import org.junit.jupiter.api.extension.ExtensionContext;
import org.junit.jupiter.api.extension.ExtensionContext.Namespace;
import org.junit.jupiter.api.extension.ExtensionContext.Store.CloseableResource;
import org.junit.jupiter.api.extension.ExtensionContext.Store;
import org.junit.jupiter.api.extension.ParameterContext;
import org.junit.jupiter.api.extension.ParameterResolver;
import org.junit.jupiter.api.io.CleanupMode;
Expand Down Expand Up @@ -128,8 +128,9 @@ public void beforeEach(ExtensionContext context) {
.forEach(instance -> injectInstanceFields(context, instance));
}

@SuppressWarnings("deprecation")
private static void installFailureTracker(ExtensionContext context) {
context.getStore(NAMESPACE).put(FAILURE_TRACKER, (CloseableResource) () -> context.getParent() //
context.getStore(NAMESPACE).put(FAILURE_TRACKER, (Store.CloseableResource) () -> context.getParent() //
.ifPresent(parentContext -> {
if (selfOrChildFailed(context)) {
parentContext.getStore(NAMESPACE).put(CHILD_FAILED, true);
Expand Down Expand Up @@ -289,7 +290,8 @@ private static boolean selfOrChildFailed(ExtensionContext context) {
|| context.getStore(NAMESPACE).getOrDefault(CHILD_FAILED, Boolean.class, false);
}

static class CloseablePath implements CloseableResource {
@SuppressWarnings("deprecation")
static class CloseablePath implements Store.CloseableResource {

private static final Logger LOGGER = LoggerFactory.getLogger(CloseablePath.class);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@

import org.junit.jupiter.api.Timeout.ThreadMode;
import org.junit.jupiter.api.extension.ExtensionContext.Store;
import org.junit.jupiter.api.extension.ExtensionContext.Store.CloseableResource;
import org.junit.jupiter.api.extension.InvocationInterceptor.Invocation;
import org.junit.platform.commons.JUnitException;
import org.junit.platform.commons.util.Preconditions;
Expand Down Expand Up @@ -52,7 +51,8 @@ private ScheduledExecutorService getThreadExecutorForSameThreadInvocation() {
return store.getOrComputeIfAbsent(SingleThreadExecutorResource.class).get();
}

private static abstract class ExecutorResource implements CloseableResource {
@SuppressWarnings({ "deprecation", "try" })
private static abstract class ExecutorResource implements Store.CloseableResource {

protected final ScheduledExecutorService executor;

Expand All @@ -65,16 +65,24 @@ ScheduledExecutorService get() {
}

@Override
public void close() throws Throwable {
public void close() throws Exception {
executor.shutdown();
boolean terminated = executor.awaitTermination(5, TimeUnit.SECONDS);
if (!terminated) {
try {
boolean terminated = executor.awaitTermination(5, TimeUnit.SECONDS);
if (!terminated) {
executor.shutdownNow();
throw new JUnitException("Scheduled executor could not be stopped in an orderly manner");
}
}
catch (InterruptedException exception) {
Thread.currentThread().interrupt();
executor.shutdownNow();
throw new JUnitException("Scheduled executor could not be stopped in an orderly manner");
throw new JUnitException("Scheduled executor could not be stopped in an orderly manner", exception);
}
}
}

@SuppressWarnings("try")
static class SingleThreadExecutorResource extends ExecutorResource {

@SuppressWarnings("unused")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ private void storeParameterInfo(ExtensionContext context) {
new DefaultParameterInfo(declarations, accessor).store(context);
}

@SuppressWarnings({ "deprecation", "try" })
private static class CloseableArgument implements ExtensionContext.Store.CloseableResource {

private final AutoCloseable autoCloseable;
Expand All @@ -85,7 +86,7 @@ private static class CloseableArgument implements ExtensionContext.Store.Closeab
}

@Override
public void close() throws Throwable {
public void close() throws Exception {
this.autoCloseable.close();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ void closesCloseableResourcesInExtensionContext(ExtensionContext extensionContex
store.put("baz", reportEntryOnClose(extensionContext, "3"));
}

@SuppressWarnings("deprecation")
private ExtensionContext.Store.CloseableResource reportEntryOnClose(ExtensionContext extensionContext,
String key) {
return () -> extensionContext.publishReportEntry(Map.of(key, "closed"));
Expand Down Expand Up @@ -78,6 +79,7 @@ void test() {

static class ThrowingOnCloseExtension implements BeforeEachCallback {

@SuppressWarnings("deprecation")
@Override
public void beforeEach(ExtensionContext context) {
context.getStore(GLOBAL).put("throwingResource", (ExtensionContext.Store.CloseableResource) () -> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@
import org.junit.jupiter.api.extension.Extension;
import org.junit.jupiter.api.extension.ExtensionContext;
import org.junit.jupiter.api.extension.ExtensionContext.Namespace;
import org.junit.jupiter.api.extension.ExtensionContext.Store.CloseableResource;
import org.junit.jupiter.api.extension.ExtensionContext.Store;
import org.junit.jupiter.api.extension.ParameterContext;
import org.junit.jupiter.api.extension.ParameterResolutionException;
import org.junit.jupiter.api.extension.ParameterResolver;
Expand Down Expand Up @@ -1175,7 +1175,8 @@ public Object resolveParameter(ParameterContext parameterContext, ExtensionConte
}
}

static class SomeResource implements CloseableResource {
@SuppressWarnings("deprecation")
static class SomeResource implements Store.CloseableResource {
private boolean closed;

@Override
Expand Down Expand Up @@ -1439,7 +1440,8 @@ public Object resolveParameter(ParameterContext parameterContext, ExtensionConte

}

private static class CustomCloseableResource implements CloseableResource {
@SuppressWarnings("deprecation")
private static class CustomCloseableResource implements Store.CloseableResource {

static boolean closed;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -924,6 +924,7 @@ public Object resolveParameter(ParameterContext parameterContext, ExtensionConte

}

@SuppressWarnings("deprecation")
private static class CustomCloseableResource implements CloseableResource {

static boolean closed;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -737,6 +737,7 @@ void test2() {

private static abstract class AbstractTestInstanceFactory implements TestInstanceFactory {

@SuppressWarnings("deprecation")
@Override
public Object createTestInstance(TestInstanceFactoryContext factoryContext, ExtensionContext extensionContext) {
Class<?> testClass = factoryContext.getTestClass();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,7 @@ static abstract class AbstractInstancePostProcessor implements TestInstancePostP
this.name = name;
}

@SuppressWarnings("deprecation")
@Override
public void postProcessTestInstance(Object testInstance, ExtensionContext context) {
if (testInstance instanceof Named) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -536,6 +536,7 @@ static abstract class AbstractTestInstancePreConstructCallback implements TestIn
this.name = name;
}

@SuppressWarnings("deprecation")
@Override
public void preConstructTestInstance(TestInstanceFactoryContext factoryContext, ExtensionContext context) {
assertThat(context.getTestInstance()).isNotPresent();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ interface Resource {
* {@link ResourceValue#close()} method implementation is needed to comply
* with both interfaces.
*/
@SuppressWarnings("deprecation")
static class ResourceValue implements Resource, CloseableResource, AutoCloseable {

static final AtomicInteger creations = new AtomicInteger();
Expand All @@ -80,6 +81,7 @@ public int usages() {
}
}

@SuppressWarnings("deprecation")
private static class CloseableOnlyOnceResource implements CloseableResource {

private final AtomicBoolean closed = new AtomicBoolean();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@ private <T> Resource<T> getOrCreateResource(ExtensionContext extensionContext, C
}
}

@SuppressWarnings({ "deprecation", "try" })
class Resource<T> implements CloseableResource {

private final T value;
Expand All @@ -115,7 +116,7 @@ private T get() {
}

@Override
public void close() throws Throwable {
public void close() throws Exception {
((AutoCloseable) value).close();
}
}
Expand Down
Loading
Loading