Skip to content

Commit

Permalink
Code review amendments
Browse files Browse the repository at this point in the history
Signed-off-by: Björn Michael <[email protected]>
  • Loading branch information
bjmi committed Dec 22, 2023
1 parent fb27107 commit 3c1472f
Show file tree
Hide file tree
Showing 4 changed files with 63 additions and 61 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ in the `junit-jupiter-api` module.
| `@ExtendWith` | Used to <<extensions-registration-declarative,register extensions declaratively>>. Such annotations are _inherited_.
| `@RegisterExtension` | Used to <<extensions-registration-programmatic,register extensions programmatically>> via fields. Such fields are _inherited_ unless they are _shadowed_.
| `@TempDir` | Used to supply a <<writing-tests-built-in-extensions-TempDirectory,temporary directory>> via field injection or parameter injection in a lifecycle method or test method; located in the `org.junit.jupiter.api.io` package.
| @AutoClose | Indicates that a field in a JUnit 5 test class represents a resource that should be automatically closed after test execution.
| `@AutoClose` | Denotes that the annotated field represents a resource that should be automatically closed after test execution.
|===

WARNING: Some annotations may currently be _experimental_. Consult the table in
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,29 +18,24 @@
import org.apiguardian.api.API;

/**
* The {@code AutoClose} annotation is used to automatically close resources used in JUnit 5 tests.
* The {@code AutoClose} annotation is used to automatically close resources
* used in tests.
*
* <p>
* This annotation should be applied to fields within JUnit 5 test classes. It indicates that the annotated
* resource should be automatically closed after the test execution. The annotation targets
* {@link java.lang.annotation.ElementType#FIELD} elements, allowing it to be applied to instance variables.
* </p>
* <p>This annotation should be applied to fields within test classes. It
* indicates that the annotated resource should be automatically closed after
* the test execution.
*
* <p>
* By default, the {@code AutoClose} annotation expects the annotated resource to provide a {@code close()} method
* that will be invoked for closing the resource. However, developers can customize the closing behavior by providing
* a different method name through the {@code value} attribute. For example, setting {@code value = "shutdown"} will
* look for a method named {@code shutdown()} to close the resource.
* When multiple annotated resources exist the order of closing them is unspecified.
* </p>
*
* <p>
* The {@code AutoClose} annotation is retained at runtime, allowing it to be accessed and processed during test execution.
* </p>
* <p>By default, the {@code AutoClose} annotation expects the annotated
* resource to provide a {@code close()} method that will be invoked for closing
* the resource. However, developers can customize the closing behavior by
* providing a different method name through the {@link #value} attribute. For
* example, setting {@code value = "shutdown"} will look for a method named
* {@code shutdown()} to close the resource. When multiple annotated resources
* exist the order of closing them is unspecified.
*
* @since 5.11
* @see java.lang.annotation.Retention
* @see java.lang.annotation.Target
* @since 5.11
*/
@Retention(RetentionPolicy.RUNTIME)
@Target(ElementType.FIELD)
Expand All @@ -49,7 +44,8 @@

/**
* Specifies the name of the method to invoke for closing the resource.
* The default value is "close".
*
* <p>The default value is {@code close}.
*
* @return the method name for closing the resource
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
import java.lang.reflect.Method;
import java.util.function.Predicate;

import org.apiguardian.api.API;
import org.junit.jupiter.api.AutoClose;
import org.junit.jupiter.api.extension.AfterAllCallback;
import org.junit.jupiter.api.extension.AfterEachCallback;
Expand All @@ -30,20 +29,20 @@
import org.junit.platform.commons.util.ReflectionUtils;

/**
* {@code AutoCloseExtension} is a JUnit Jupiter extension that closes resources if a field in a test class is annotated
* with {@link AutoClose @AutoClose}.
* {@code AutoCloseExtension} is a JUnit Jupiter extension that closes resources
* if a field in a test class is annotated with {@link AutoClose @AutoClose}.
*
* <p>Consult the Javadoc for {@link AutoClose} for details on the contract.
* <p>Consult the Javadoc for {@link AutoClose @AutoClose} for details on the
* contract.
*
* @since 5.11
* @see AutoClose
* @see AutoCloseable
*/
@API(status = API.Status.EXPERIMENTAL, since = "5.11")
public class AutoCloseExtension implements AfterAllCallback, AfterEachCallback {
class AutoCloseExtension implements AfterAllCallback, AfterEachCallback {

private static final Logger logger = LoggerFactory.getLogger(AutoCloseExtension.class);
static final Namespace NAMESPACE = Namespace.create(AutoClose.class);
private static final Namespace NAMESPACE = Namespace.create(AutoClose.class);

@Override
public void afterAll(ExtensionContext context) {
Expand All @@ -62,9 +61,9 @@ public void afterEach(ExtensionContext context) {
}
}

private void registerCloseables(Store contextStore, Class<?> testClass, /* @Nullable */ Object testInstance) {
Predicate<Field> isStatic = testInstance == null ? ReflectionUtils::isStatic : ReflectionUtils::isNotStatic;
findAnnotatedFields(testClass, AutoClose.class, isStatic).forEach(field -> {
private void registerCloseables(Store contextStore, Class<?> testClass, Object testInstance) {
Predicate<Field> predicate = testInstance == null ? ReflectionUtils::isStatic : ReflectionUtils::isNotStatic;
findAnnotatedFields(testClass, AutoClose.class, predicate).forEach(field -> {
try {
contextStore.put(field, asCloseableResource(testInstance, field));
}
Expand All @@ -74,28 +73,29 @@ private void registerCloseables(Store contextStore, Class<?> testClass, /* @Null
});
}

private static Store.CloseableResource asCloseableResource(/* @Nullable */ Object testInstance, Field field) {
private static Store.CloseableResource asCloseableResource(Object testInstance, Field field) {
return () -> {
Object toBeClosed = ReflectionUtils.tryToReadFieldValue(field, testInstance).get();
if (toBeClosed == null) {
logger.warn(() -> "@AutoClose: Field " + getQualifiedFieldName(field)
+ " couldn't be closed because it was null.");
logger.warn(() -> "@AutoClose couldn't close object for field " + getQualifiedFieldName(field)
+ " because it was null.");
return;
}
getAndDestroy(field, toBeClosed);
invokeCloseMethod(field, toBeClosed);
};
}

private static void getAndDestroy(Field field, Object toBeClosed) {
private static void invokeCloseMethod(Field field, Object toBeClosed) {
String methodName = field.getAnnotation(AutoClose.class).value();
Method destroyMethod = ReflectionUtils.findMethod(toBeClosed.getClass(), methodName).orElseThrow(
() -> new ExtensionConfigurationException("@AutoClose: Cannot resolve the destroy method " + methodName
+ "() at " + getQualifiedFieldName(field) + ": " + field.getType().getSimpleName()));
() -> new ExtensionConfigurationException(
"@AutoClose failed to close object for field " + getQualifiedFieldName(field) + " because the "
+ methodName + "() method could not be " + "resolved."));
ReflectionUtils.invokeMethod(destroyMethod, toBeClosed);
}

private static String getQualifiedFieldName(Field field) {
return field.getDeclaringClass().getSimpleName() + "." + field.getName();
return field.getDeclaringClass().getName() + "." + field.getName();
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,11 @@
package org.junit.jupiter.engine.extension;

import static java.util.Arrays.asList;
import static org.assertj.core.api.Assertions.assertThat;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertNull;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.extension.ExtensionContext.Namespace;
import static org.junit.platform.testkit.engine.EventConditions.finishedWithFailure;
import static org.junit.platform.testkit.engine.TestExecutionResultConditions.cause;
import static org.junit.platform.testkit.engine.TestExecutionResultConditions.message;
Expand All @@ -32,7 +32,8 @@
import org.junit.platform.testkit.engine.Events;

/**
* Integration tests for the behavior of the {@link AutoCloseExtension} to release resources after test execution.
* Integration tests for the behavior of the {@link AutoCloseExtension} to
* release resources after test execution.
*
* @since 5.11
*/
Expand All @@ -52,34 +53,33 @@ void fieldsAreProperlyClosed() {
Events tests = engineExecutionResults.testEvents();
tests.assertStatistics(stats -> stats.succeeded(2));
// @formatter:off
assertEquals(asList(
assertThat(recorder).containsExactly(
"afterEach-close()", "afterEach-run()",
"afterEach-close()", "afterEach-run()",
"afterAll-close()"), recorder);
// @formatter:onf
"afterAll-close()");
// @formatter:on
}

@Test
void noCloseMethod() {
String msg = "@AutoClose: Cannot resolve the destroy method close() at AutoCloseNoCloseMethodFailingTestCase.resource: String";
String msg = "@AutoClose failed to close object for field "
+ "org.junit.jupiter.engine.extension.AutoCloseTests$AutoCloseNoCloseMethodFailingTestCase.resource "
+ "because the close() method could not be resolved.";

Events tests = executeTestsForClass(AutoCloseNoCloseMethodFailingTestCase.class).testEvents();
assertFailingWithMessage(tests, msg);
}

@Test
void noShutdownMethod() {
String msg = "@AutoClose: Cannot resolve the destroy method shutdown() at AutoCloseNoShutdownMethodFailingTestCase.resource: String";
String msg = "@AutoClose failed to close object for field "
+ "org.junit.jupiter.engine.extension.AutoCloseTests$AutoCloseNoShutdownMethodFailingTestCase.resource "
+ "because the shutdown() method could not be resolved.";

Events tests = executeTestsForClass(AutoCloseNoShutdownMethodFailingTestCase.class).testEvents();
assertFailingWithMessage(tests, msg);
}

@Test
void namespace() {
assertEquals(Namespace.create(AutoClose.class), AutoCloseExtension.NAMESPACE);
}

@Test
void spyPermitsOnlyASingleAction() {
AutoCloseSpy spy = new AutoCloseSpy("");
Expand All @@ -98,12 +98,22 @@ private static void assertFailingWithMessage(Events testEvent, String msg) {

static class AutoCloseTestCase {

private static @AutoClose AutoCloseable staticClosable;
private static @AutoClose AutoCloseable nullStatic;
@AutoClose
private static AutoCloseable staticClosable;
@AutoClose
private static AutoCloseable nullStatic;

private final @AutoClose AutoCloseable closable = new AutoCloseSpy("afterEach-");
private final @AutoClose("run") Runnable runnable = new AutoCloseSpy("afterEach-");
private @AutoClose AutoCloseable nullField;
@AutoClose
private final AutoCloseable closable = new AutoCloseSpy("afterEach-");
@AutoClose("run")
private final Runnable runnable = new AutoCloseSpy("afterEach-");
@AutoClose
private AutoCloseable nullField;

@BeforeAll
static void setup() {
staticClosable = new AutoCloseSpy("afterAll-");
}

@Test
void justPass() {
Expand All @@ -124,11 +134,6 @@ private void assertFields() {
assertNull(nullField);
}

@BeforeAll
static void setup() {
staticClosable = new AutoCloseSpy("afterAll-");
}

}

static class AutoCloseNoCloseMethodFailingTestCase {
Expand Down Expand Up @@ -177,8 +182,9 @@ public void close() {
}

private void checkIfAlreadyInvoked() {
if (!invokedMethod.isEmpty())
if (!invokedMethod.isEmpty()) {
throw new IllegalStateException();
}
}

private void recordInvocation(String methodName) {
Expand Down

0 comments on commit 3c1472f

Please sign in to comment.