Skip to content

Introduce PreInterruptCallback extension point#3431

Merged
marcphilipp merged 3 commits intojunit-team:mainfrom
AndreasTu:fix_2938
Oct 29, 2024
Merged

Introduce PreInterruptCallback extension point#3431
marcphilipp merged 3 commits intojunit-team:mainfrom
AndreasTu:fix_2938

Conversation

@AndreasTu
Copy link
Copy Markdown
Contributor

@AndreasTu AndreasTu commented Aug 19, 2023

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
Copy Markdown
Contributor 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?

Copy link
Copy Markdown
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?

Comment thread documentation/src/docs/asciidoc/user-guide/extensions.adoc Outdated
@AndreasTu
Copy link
Copy Markdown
Contributor Author

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

Comment thread documentation/src/docs/asciidoc/user-guide/extensions.adoc Outdated
Comment thread documentation/src/docs/asciidoc/user-guide/writing-tests.adoc Outdated
Comment thread documentation/src/docs/asciidoc/user-guide/writing-tests.adoc Outdated
Comment thread documentation/src/docs/asciidoc/user-guide/extensions.adoc Outdated
Comment thread documentation/src/docs/asciidoc/user-guide/writing-tests.adoc Outdated
Comment thread junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/Constants.java Outdated
Copy link
Copy Markdown
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.

Copy link
Copy Markdown
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.

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 <mail@marcphilipp.de>
@AndreasTu AndreasTu requested a review from marcphilipp October 25, 2024 19:03
@marcphilipp marcphilipp changed the title PreInterruptCallback extension Introduce PreInterruptCallback extension point Oct 29, 2024
@marcphilipp marcphilipp merged commit f32aa0a into junit-team:main Oct 29, 2024
@marcphilipp
Copy link
Copy Markdown
Member

@AndreasTu Thank you for your contribution! 👍

@AndreasTu AndreasTu deleted the fix_2938 branch October 29, 2024 11:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Hook before timeout exception is started by calling interrupt()

2 participants