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 #3592

Closed
wants to merge 11 commits into from
Closed

Introduce @AutoClose #3592

wants to merge 11 commits into from

Conversation

bjmi
Copy link
Contributor

@bjmi bjmi commented Dec 5, 2023

Overview

Closes #3367


I hereby agree to the terms of the JUnit Contributor License Agreement.


Definition of Done

@bjmi

This comment was marked as outdated.

@ibrahimesseddyq

This comment was marked as outdated.

@bjmi

This comment was marked as outdated.

Copy link
Member

@sbrannen sbrannen left a 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

@sbrannen sbrannen changed the title Introduce @AutoClose Introduce @AutoClose Dec 21, 2023
Copy link
Member

@sbrannen sbrannen left a 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!

@sbrannen

This comment was marked as outdated.

Signed-off-by: Björn Michael <b.michael@gmx.de>
* Test AutoCloseExtension
* Register extension as stateless

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

Signed-off-by: Björn Michael <b.michael@gmx.de>
Signed-off-by: Björn Michael <b.michael@gmx.de>
sbrannen pushed a commit to sbrannen/junit5 that referenced this pull request Dec 30, 2023
Signed-off-by: Björn Michael <b.michael@gmx.de>

Closes 3367
Closes junit-team#3592
sbrannen added a commit to sbrannen/junit5 that referenced this pull request Dec 30, 2023
@sbrannen
Copy link
Member

Hey @bjmi,

Since I started investigating the support for @Nested test classes, I ended up putting in a fair amount of work locally on my machine.

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! 👍

@sbrannen sbrannen marked this pull request as draft December 30, 2023 14:37
sbrannen added a commit to sbrannen/junit5 that referenced this pull request Dec 30, 2023
@sbrannen

This comment was marked as outdated.

sbrannen pushed a commit to sbrannen/junit5 that referenced this pull request 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 pull request Dec 31, 2023
sbrannen added a commit to sbrannen/junit5 that referenced this pull request Dec 31, 2023
sbrannen added a commit to sbrannen/junit5 that referenced this pull request Dec 31, 2023
sbrannen added a commit to sbrannen/junit5 that referenced this pull request Dec 31, 2023
sbrannen added a commit to sbrannen/junit5 that referenced this pull request Dec 31, 2023
@sbrannen

This comment was marked as outdated.

sbrannen pushed a commit to sbrannen/junit5 that referenced this pull request 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 pull request Jan 1, 2024
sbrannen added a commit to sbrannen/junit5 that referenced this pull request Jan 1, 2024
sbrannen added a commit to sbrannen/junit5 that referenced this pull request Jan 2, 2024
sbrannen pushed a commit to sbrannen/junit5 that referenced this pull request 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 pull request Jan 2, 2024
sbrannen pushed a commit to sbrannen/junit5 that referenced this pull request 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 pull request 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 pull request 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 sbrannen closed this in 89a0201 Jan 4, 2024
@sbrannen
Copy link
Member

sbrannen commented Jan 4, 2024

This has been merged into main in e32c2e3 and revised in 89a0201.

Thanks again for the contribution! 👍

@bjmi bjmi deleted the Introduce_@AutoClose branch February 25, 2024 12:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Introduce @AutoClose mechanism for closing field resources after test execution
5 participants