Skip to content
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

Introduce @AutoClose mechanism for closing field resources after test execution #3367

Closed
bjmi opened this issue Jun 23, 2023 · 9 comments
Closed

Comments

@bjmi
Copy link
Contributor

bjmi commented Jun 23, 2023

Occasionally I create and use resources in unit tests that should be released after test execution. You usually do this in tear-down methods. But in a lot of cases a simple @AutoClose annotated field would be sufficient for that, something similar to @TempDir for temporary folders.

This would spare boilerplate code like:

Service service = new ClosableService();

@AfterEach
void cleanup() {
    service.close();
}

Allowing it to be replaced by:

@AutoClose
Service service = new ClosableService();

Static fields would be closed after all tests have run and instance fields after each test.

Other annotation names could be @Destroy or @AutoDestroy.

If there is no close() method, it should be possible to specify a custom method name -- for example, destroy, dispose, shutdown, etc.

Deliverables

  • A new @AutoClose annotation
  • Update User Guide Writing Tests -> Annotations
@sbrannen sbrannen changed the title Provide @AutoClose that closes annotated resources after text execution Introduce @AutoClose mechanism for closing resources after test execution Jun 23, 2023
@marcphilipp marcphilipp linked a pull request Jun 25, 2023 that will close this issue
6 tasks
@feech
Copy link

feech commented Nov 26, 2023

what's about the order several resources close?

@bjmi
Copy link
Contributor Author

bjmi commented Nov 27, 2023

I have no strong opinion about it. Two thoughts:

  • To simplify, one could define the order unspecific and refer to Class.getDeclaredFields() behaviour. If a sophisticated order is required than the existing tear-down method approach should be taken.

Javadoc: The elements in the returned array are not sorted and are not in any particular order.

  • Add @AutoClose.priority property where the order can be specified by e.g. a number.

@marcphilipp
Copy link
Member

Team decision: Introduce attribute-less @AutoClose annotation that only works on fields of types that implement java.lang.AutoCloseable without any ordering guarantees for now.

@bjmi
Copy link
Contributor Author

bjmi commented Dec 5, 2023

Limiting to java.lang.AutoCloseable will lose a lot of usages. Some examples are:

  • java.util.concurrent.ExecutorService.shutdown()
  • org.eclipse.swt.widgets.Shell.dispose()
  • com.icegreen.greenmail.util.GreenMail.stop()

A similar solution are Spring Beans destroy methods (@Bean.destroyMethod). Any public, no-arg method can be specified for a container-managed bean and the container automatically invokes them on application context shutdown to properly release these instances. The method names close() and shutdown() are automatically detected if not explicitely specified.

Therefore I propose following annotation and just search for a public and no-arg method and disregard the type of the field.

@interface AutoClose {
    /** @return method name to execute in order to release resource. */
    String value() default "close";
}

I could provide a solution that uses ExtensionContext.Store where @AutoClose annotated fields are registered as wrapped Store.CloseableResources and are automatically released on test shutdown via NamespacedHierarchicalStore.close().

@sormuras
Copy link
Member

sormuras commented Dec 5, 2023

Supporting java.lang.AutoCloseable will enable already a lot usages - and wrapping a resource in a Store.CloseableResource holder/helper is always possible (today).

java.util.concurrent.ExecutorService does implement java.lang.AutoCloseable since Java 19: https://docs.oracle.com/en/java/javase/21/docs/api/java.base/java/util/concurrent/ExecutorService.html#close()

@bjmi
Copy link
Contributor Author

bjmi commented Dec 5, 2023

java.util.concurrent.ExecutorService does implement java.lang.AutoCloseable since Java 19: https://docs.oracle.com/en/java/javase/21/docs/api/java.base/java/util/concurrent/ExecutorService.html#close()

Thanks for the tip. Unfortunately, this doesn't help users who are dealing with previous versions.

@sbrannen sbrannen mentioned this issue Dec 21, 2023
6 tasks
@sbrannen sbrannen changed the title Introduce @AutoClose mechanism for closing resources after test execution Introduce @AutoClose mechanism for closing field resources after test execution Dec 22, 2023
@sbrannen sbrannen self-assigned this Dec 24, 2023
@sbrannen sbrannen added this to the 5.11 M1 milestone Dec 24, 2023
sbrannen added a commit to sbrannen/junit5 that referenced this issue Dec 30, 2023
sbrannen added a commit to sbrannen/junit5 that referenced this issue Dec 30, 2023
sbrannen pushed a commit to sbrannen/junit5 that referenced this issue Dec 31, 2023
Signed-off-by: Björn Michael <b.michael@gmx.de>

See junit-team#3367
See junit-team#3592
sbrannen added a commit to sbrannen/junit5 that referenced this issue Dec 31, 2023
sbrannen added a commit to sbrannen/junit5 that referenced this issue Dec 31, 2023
@sbrannen
Copy link
Member

Team decision: Introduce attribute-less @AutoClose annotation that only works on fields of types that implement java.lang.AutoCloseable without any ordering guarantees for now.

We have decided to support a configurable close-method name which defaults to close.

In addition, due to the nature of our reflection utilities, we do in fact provide some guarantees regarding ordering, as I've outlined in the Javadoc for @AutoClose:

Inheritance

@AutoClose fields are inherited from superclasses as long as they are not hidden. Furthermore, @AutoClose fields from subclasses will be closed before @AutoClose fields in superclasses.

Registration Order

When multiple @AutoClose fields exist within a given test class, the order in which the resources are closed depends on an algorithm that is deterministic but intentionally nonobvious. This ensures that subsequent runs of a test suite close resources in the same order, thereby allowing for repeatable builds.

sbrannen added a commit to sbrannen/junit5 that referenced this issue Dec 31, 2023
sbrannen added a commit to sbrannen/junit5 that referenced this issue Dec 31, 2023
sbrannen added a commit to sbrannen/junit5 that referenced this issue Dec 31, 2023
sbrannen pushed a commit to sbrannen/junit5 that referenced this issue Jan 1, 2024
Signed-off-by: Björn Michael <b.michael@gmx.de>

See junit-team#3367
See junit-team#3592
sbrannen added a commit to sbrannen/junit5 that referenced this issue Jan 1, 2024
sbrannen added a commit to sbrannen/junit5 that referenced this issue Jan 1, 2024
sbrannen added a commit to sbrannen/junit5 that referenced this issue Jan 2, 2024
sbrannen pushed a commit to sbrannen/junit5 that referenced this issue Jan 2, 2024
Signed-off-by: Björn Michael <b.michael@gmx.de>

See junit-team#3367
See junit-team#3592
sbrannen added a commit to sbrannen/junit5 that referenced this issue Jan 2, 2024
sbrannen pushed a commit to sbrannen/junit5 that referenced this issue Jan 4, 2024
This commit introduces an @⁠AutoClose annotation that can be applied to
fields within JUnit Jupiter tests to automatically close the annotated
resource after test execution.

Signed-off-by: Björn Michael <b.michael@gmx.de>

See junit-team#3367
See junit-team#3592
sbrannen added a commit to sbrannen/junit5 that referenced this issue Jan 4, 2024
This commit revises the @⁠AutoClose support that was introduced in the
previous commit as follows.

- Javadoc and User Guide documentation for @⁠AutoClose, now explicitly
  explain ordering, inheritance, evaluation order, scope, and lifecycle
  for @⁠AutoClose fields.

- Overhauled and simplified the implementation of the
  AutoCloseExtension by closing fields immediately instead of wrapping
  each field in a CloseableResource and storing it in the
  ExtensionContext.Store, and revised the implementation of
  preDestroyTestInstance() so that it makes use of
  TestInstancePreDestroyCallback.preDestroyTestInstances() to ensure
  proper semantics within @⁠Nested test class hierarchies. Combined,
  these changes ensure that fields are closed in the proper order and
  that any resulting exceptions are thrown directly instead of nested
  as a cause of an exception that occurs when the
  ExtensionContext.Store is closed.

- Improved diagnostics by failing eagerly if an @⁠AutoClose annotation
  is declared with a blank close-method name or if the annotated field
  is a primitive type or array.

- Since having the AutoCloseExtension in the org.junit.jupiter.api
  package introduced a cycle between the api and api.extension
  packages, the AutoCloseExtension now resides in the
  org.junit.jupiter.engine.extension package in junit-jupiter-engine
  and is loaded as a built-in extension.

- The AutoCloseDemo now uses a WebClient that needs to be closed
  instead of a JDBC Connection, thereby avoiding the need to throw an
  SQLException and simplifying the example.

- Overhauled AutoCloseTests to test all known use cases (including test
  class hierarchies and @⁠Nested test class hierarchies) and error
  cases.

Closes junit-team#3367
Closes junit-team#3592
sbrannen pushed a commit that referenced this issue Jan 4, 2024
This commit introduces an @⁠AutoClose annotation that can be applied to
fields within JUnit Jupiter tests to automatically close the annotated
resource after test execution.

Signed-off-by: Björn Michael <b.michael@gmx.de>

See #3367
See #3592
@sbrannen
Copy link
Member

sbrannen commented Jan 4, 2024

Reopening to make use of ThrowableCollector when closing resources to ensure that all fields are given a chance to be closed.

@sbrannen
Copy link
Member

sbrannen commented Jan 4, 2024

RE-Reopening to make sure that the use of ThrowableCollector is properly applied in @Nested test class hierarchies. 👀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment