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

Make config disabled tests use AsyncTaskManager #633

Merged
merged 2 commits into from
Jun 27, 2024

Conversation

Azquelt
Copy link
Member

@Azquelt Azquelt commented Jun 26, 2024

Previously these tests were using a fixed thread pool executor which will create unmanaged threads on a Jakarta EE server.

We already have AsyncCaller which is designed to avoid this, and AsyncTaskManager which provides additional framework for exactly these type of tests so we should use them here.

Unfortunately these test classes are also testing the disablement of the @Asynchronous annotation which AsyncCaller and AsyncTaskManager use. We could split these tests up, but it's easier to add additional config to enable the annotation for that class specifically.

Fixes #634

@Azquelt Azquelt force-pushed the config-tests-executor branch from 3c288d8 to a05c396 Compare June 26, 2024 13:18
@Azquelt Azquelt changed the title Make config disabled tests to use AsyncTaskManager Make config disabled tests use AsyncTaskManager Jun 26, 2024
@Azquelt Azquelt force-pushed the config-tests-executor branch from a05c396 to 3f0c9b7 Compare June 26, 2024 13:58
@Azquelt Azquelt changed the title Make config disabled tests use AsyncTaskManager Make config disabled tests use AsyncTaskManager Jun 26, 2024
@Azquelt Azquelt force-pushed the config-tests-executor branch from 3f0c9b7 to e25f8c2 Compare June 26, 2024 15:15
Previously these tests were using a fixed thread pool executor which
will create unmanaged threads on a Jakarta EE server.
@Azquelt Azquelt force-pushed the config-tests-executor branch from e25f8c2 to 46d2da8 Compare June 26, 2024 15:22
@Emily-Jiang
Copy link
Member

@Azquelt can you also add this to the release note in the spec doc?

@Azquelt
Copy link
Member Author

Azquelt commented Jun 27, 2024

I've updated the release notes.

@Emily-Jiang Emily-Jiang merged commit 48c90e7 into eclipse:main Jun 27, 2024
4 checks passed
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.

Config Disablement TCK tests use unmanaged threads
2 participants