-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
#3592
Introduce @AutoClose
#3592
Conversation
This comment was marked as outdated.
This comment was marked as outdated.
965af1b
to
c137a93
Compare
junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/extension/AutoCloseExtension.java
Outdated
Show resolved
Hide resolved
junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/extension/AutoCloseExtension.java
Outdated
Show resolved
Hide resolved
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR! 👍
I've requested several minor changes.
In addition, please introduce at least one example in the User Guide in the "Writing Tests" chapter.
Thanks
junit-jupiter-api/src/main/java/org/junit/jupiter/api/AutoClose.java
Outdated
Show resolved
Hide resolved
junit-jupiter-api/src/main/java/org/junit/jupiter/api/AutoClose.java
Outdated
Show resolved
Hide resolved
junit-jupiter-api/src/main/java/org/junit/jupiter/api/AutoClose.java
Outdated
Show resolved
Hide resolved
junit-jupiter-engine/src/test/java/org/junit/jupiter/engine/extension/AutoCloseTests.java
Outdated
Show resolved
Hide resolved
junit-jupiter-engine/src/test/java/org/junit/jupiter/engine/extension/AutoCloseTests.java
Outdated
Show resolved
Hide resolved
junit-jupiter-engine/src/test/java/org/junit/jupiter/engine/extension/AutoCloseTests.java
Outdated
Show resolved
Hide resolved
junit-jupiter-engine/src/test/java/org/junit/jupiter/engine/extension/AutoCloseTests.java
Outdated
Show resolved
Hide resolved
junit-jupiter-engine/src/test/java/org/junit/jupiter/engine/extension/AutoCloseTests.java
Outdated
Show resolved
Hide resolved
junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/extension/AutoCloseExtension.java
Outdated
Show resolved
Hide resolved
junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/extension/AutoCloseExtension.java
Outdated
Show resolved
Hide resolved
junit-jupiter-engine/src/test/java/org/junit/jupiter/engine/extension/AutoCloseTests.java
Outdated
Show resolved
Hide resolved
8f439b3
to
d26b6fb
Compare
junit-jupiter-api/src/main/java/org/junit/jupiter/api/AutoCloseExtension.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for making all of the requested changes! 👍
I think this PR looks good now.
All that's left are the changes to the link attributes and the demo.
Though we can take care of those for you if you don't have time.
Note, however, that there is no rush. I don't plan to merge this until 2024. 😉
Enjoy your end-of-year celebrations!
This comment was marked as outdated.
This comment was marked as outdated.
Signed-off-by: Björn Michael <[email protected]>
* Test AutoCloseExtension * Register extension as stateless Signed-off-by: Björn Michael <[email protected]>
Signed-off-by: Björn Michael <[email protected]>
Signed-off-by: Björn Michael <[email protected]>
Implement TestInstancePreDestroyCallback instead Signed-off-by: Björn Michael <[email protected]>
Signed-off-by: Björn Michael <[email protected]>
junit-jupiter-engine/src/test/java/org/junit/jupiter/engine/extension/AutoCloseTests.java
Show resolved
Hide resolved
Signed-off-by: Björn Michael <[email protected]> Closes 3367 Closes junit-team#3592
Hey @bjmi, Since I started investigating the support for Consequently, I have decided to mark this PR as a "draft", and I will continue the rest of the work on my branch, which you can see here: main...sbrannen:junit5:issues/3592-AutoClose. In light of that, there's no need to make further modifications to this PR. All of your hard work is contained in the first commit on the above branch! 👍 |
This comment was marked as outdated.
This comment was marked as outdated.
junit-jupiter-api/src/main/java/org/junit/jupiter/api/AutoCloseExtension.java
Show resolved
Hide resolved
junit-jupiter-api/src/main/java/org/junit/jupiter/api/AutoCloseExtension.java
Show resolved
Hide resolved
Signed-off-by: Björn Michael <[email protected]> See junit-team#3367 See junit-team#3592
This comment was marked as outdated.
This comment was marked as outdated.
Signed-off-by: Björn Michael <[email protected]> See junit-team#3367 See junit-team#3592
Signed-off-by: Björn Michael <[email protected]> See junit-team#3367 See junit-team#3592
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 <[email protected]> See junit-team#3367 See junit-team#3592
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
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 <[email protected]> See #3367 See #3592
Overview
Closes #3367
I hereby agree to the terms of the JUnit Contributor License Agreement.
Definition of Done
@API
annotations