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.

Sign up for GitHub

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

Jump to bottom

PreInterruptCallback extension #3431

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

PreInterruptCallback extension #3431

wants to merge 2 commits into from

Conversation

AndreasTu
Copy link

@AndreasTu AndreasTu commented Aug 19, 2023 •
edited
Loading

Overview

Added PreInterruptCallback extension to allow to hook into the @Timeout extension before the executing Thread is interrupted.

The default implementation of PreInterruptCallback will simply print the stacks of all Thread to System.out.
It is disabled by default and must be enabled with: junit.jupiter.execution.timeout.threaddump.enabled = true

Issue: #2938

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

Definition of Done

@AndreasTu
Copy link
Author

I am currently not happy with the change in MutableExtensionRegistry. Is there another way to get the current active ExtensionRegistry from a current ExtensionContext?

@AndreasTu AndreasTu mentioned this pull request Aug 19, 2023
Open
@AndreasTu AndreasTu force-pushed the fix_2938 branch 2 times, most recently from da26892 to 0c2a0ed Compare August 19, 2023 16:39
@marcphilipp marcphilipp linked an issue Jun 21, 2024 that may be closed by this pull request
Hook before timeout exception is started by calling interrupt() #2938
Open
marcphilipp
marcphilipp requested changes Oct 15, 2024
View reviewed changes
Copy link
Member

@marcphilipp marcphilipp left a comment

Choose a reason for hiding this comment

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

I am currently not happy with the change in MutableExtensionRegistry. Is there another way to get the current active ExtensionRegistry from a current ExtensionContext?

I think we should consider adding a <E extends Extension> List<E> getExtensions(Class<E> extensionType) method to ExtensionContext and storing an ExtensionRegistry as an instance field in the implementations of ExtensionContext. Could you please give that a try?

documentation/src/docs/asciidoc/user-guide/extensions.adoc Outdated Show resolved Hide resolved
junit-jupiter-api/src/main/java/org/junit/jupiter/api/extension/PreInterruptCallback.java Outdated Show resolved Hide resolved
junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/Constants.java Show resolved Hide resolved
...ter-engine/src/main/java/org/junit/jupiter/engine/extension/DefaultPreInterruptCallback.java Outdated Show resolved Hide resolved
@AndreasTu
Copy link
Author

@marcphilipp Thanks for the review. And your proposal regarding the ExtensionContext works, but I had to touch more files for that.

marcphilipp
marcphilipp requested changes Oct 16, 2024
View reviewed changes
junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/config/JupiterConfiguration.java Outdated Show resolved Hide resolved
...piter-engine/src/main/java/org/junit/jupiter/engine/descriptor/AbstractExtensionContext.java Outdated Show resolved Hide resolved
...ter-engine/src/main/java/org/junit/jupiter/engine/descriptor/TestTemplateTestDescriptor.java Outdated Show resolved Hide resolved
...piter-engine/src/main/java/org/junit/jupiter/engine/descriptor/ClassBasedTestDescriptor.java Outdated Show resolved Hide resolved
...upiter-engine/src/main/java/org/junit/jupiter/engine/descriptor/JupiterEngineDescriptor.java Outdated Show resolved Hide resolved
...piter-engine/src/main/java/org/junit/jupiter/engine/descriptor/TestMethodTestDescriptor.java Outdated Show resolved Hide resolved
...piter-engine/src/main/java/org/junit/jupiter/engine/descriptor/AbstractExtensionContext.java Outdated Show resolved Hide resolved
marcphilipp
marcphilipp requested changes Oct 17, 2024
View reviewed changes
documentation/src/docs/asciidoc/user-guide/extensions.adoc Outdated Show resolved Hide resolved
documentation/src/docs/asciidoc/user-guide/writing-tests.adoc Outdated Show resolved Hide resolved
documentation/src/docs/asciidoc/user-guide/writing-tests.adoc Outdated Show resolved Hide resolved
documentation/src/docs/asciidoc/user-guide/extensions.adoc Outdated Show resolved Hide resolved
documentation/src/docs/asciidoc/user-guide/writing-tests.adoc Outdated Show resolved Hide resolved
junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/Constants.java Outdated Show resolved Hide resolved
junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/config/JupiterConfiguration.java Outdated Show resolved Hide resolved
...upiter-engine/src/main/java/org/junit/jupiter/engine/extension/MutableExtensionRegistry.java Outdated Show resolved Hide resolved
...upiter-engine/src/main/java/org/junit/jupiter/engine/extension/TimeoutInvocationFactory.java Show resolved Hide resolved
junit-jupiter-api/src/main/java/org/junit/jupiter/api/extension/PreInterruptCallback.java Outdated Show resolved Hide resolved
marcphilipp
marcphilipp requested changes Oct 23, 2024
View reviewed changes
Copy link
Member

@marcphilipp marcphilipp 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 the requested changes! I've requested one additional change and I'd like to discuss the introduction of ExtensionContext.getExtensions (that I suggested) with the team before we merge this.

junit-jupiter-api/src/main/java/org/junit/jupiter/api/extension/PreInterruptCallback.java Outdated Show resolved Hide resolved
junit-jupiter-api/src/main/java/org/junit/jupiter/api/extension/PreInterruptContext.java Outdated Show resolved Hide resolved
junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/Constants.java Show resolved Hide resolved
marcphilipp
marcphilipp requested changes Oct 25, 2024
View reviewed changes
Copy link
Member

@marcphilipp marcphilipp left a comment

Choose a reason for hiding this comment

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

One more thing came out of our team discussion.

junit-jupiter-api/src/main/java/org/junit/jupiter/api/extension/ExtensionContext.java Outdated Show resolved Hide resolved
@AndreasTu @marcphilipp
PreInterruptCallback extension
fddd6b9 
Added PreInterruptCallback extension to allow to hook into the
@timeout extension before the executing Thread is interrupted.

The default implementation of PreInterruptCallback will simply print
the stacks of all Thread to System.out.
It is disabled by default and must be enabled with:
junit.jupiter.execution.timeout.threaddump.enabled = true

Issue: junit-team#2938

Co-authored-by: Marc Philipp <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@marcphilipp marcphilipp Awaiting requested review from marcphilipp

Requested changes must be addressed to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Hook before timeout exception is started by calling interrupt()
2 participants
@AndreasTu @marcphilipp