-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Add TestMethodReturnValueHandler SPI for non-void @Test methods #5999
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,71 @@ | ||
| = Test Method Return Value Handling | ||
|
|
||
| `{TestMethodReturnValueHandler}` defines an extension point that allows `@Test` methods | ||
| to return non-void types. By default, JUnit Jupiter requires `@Test` methods to return | ||
| `void`. A registered `TestMethodReturnValueHandler` relaxes this requirement for return | ||
| types it supports. | ||
|
|
||
| [[registration]] | ||
| == Registration | ||
|
|
||
| Implementations are discovered via Java's `{ServiceLoader}` mechanism and must be | ||
| registered in a file named | ||
| `META-INF/services/org.junit.jupiter.api.extension.TestMethodReturnValueHandler` on the | ||
| classpath. | ||
|
|
||
| [[how-it-works]] | ||
| == How It Works | ||
|
|
||
| A `TestMethodReturnValueHandler` has two responsibilities: | ||
|
|
||
| 1. **Declare supported return types** via `supportsReturnType(Class<?>)` — during | ||
| discovery, JUnit calls this method to decide whether a non-void `@Test` method should | ||
| be accepted. | ||
|
|
||
| 2. **Execute the test method** via `execute(Invocation<Object>, ExtensionContext)` — at | ||
| execution time, the handler receives an `{InvocationInterceptor}.Invocation` that wraps | ||
| the test method call. The handler must call `invocation.proceed()` to invoke the test | ||
| and obtain the return value. It is then responsible for processing the result — for | ||
| example, subscribing to a reactive type and awaiting completion. | ||
|
|
||
| Because the handler controls _when_ `proceed()` is called, it can set up a custom | ||
| execution context beforehand — for example, running the test on a specific thread or | ||
| executor. | ||
|
|
||
| [[example]] | ||
| == Example | ||
|
|
||
| The following handler adds support for `@Test` methods that return `CompletableFuture`: | ||
|
|
||
| [source,java,indent=0] | ||
| ---- | ||
| public class CompletableFutureHandler implements TestMethodReturnValueHandler { | ||
|
|
||
| @Override | ||
| public boolean supportsReturnType(Class<?> returnType) { | ||
| return CompletableFuture.class.isAssignableFrom(returnType); | ||
| } | ||
|
|
||
| @Override | ||
| public void execute(InvocationInterceptor.Invocation<Object> invocation, | ||
| ExtensionContext context) throws Throwable { | ||
| Object result = invocation.proceed(); | ||
| if (result != null) { | ||
| ((CompletableFuture<?>) result).get(30, TimeUnit.SECONDS); | ||
| } | ||
| } | ||
| } | ||
| ---- | ||
|
|
||
| With this handler registered, the following test is discovered and executed: | ||
|
|
||
| [source,java,indent=0] | ||
| ---- | ||
| @Test | ||
| CompletableFuture<String> asyncTest() { | ||
| return CompletableFuture.supplyAsync(() -> { | ||
| // test logic | ||
| return "result"; | ||
| }); | ||
| } | ||
| ---- |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,95 @@ | ||
| /* | ||
| * Copyright 2015-2026 the original author or authors. | ||
| * | ||
| * All rights reserved. This program and the accompanying materials are | ||
| * made available under the terms of the Eclipse Public License v2.0 which | ||
| * accompanies this distribution and is available at | ||
| * | ||
| * https://www.eclipse.org/legal/epl-v20.html | ||
| */ | ||
|
|
||
| package org.junit.jupiter.api.extension; | ||
|
|
||
| import static org.apiguardian.api.API.Status.EXPERIMENTAL; | ||
|
|
||
| import org.apiguardian.api.API; | ||
|
|
||
| /** | ||
| * {@code TestMethodReturnValueHandler} allows extensions to support | ||
| * {@link org.junit.jupiter.api.Test @Test} methods that return a non-void | ||
| * value. | ||
| * | ||
| * <p>By default, JUnit Jupiter requires {@code @Test} methods to return | ||
| * {@code void}. A registered {@code TestMethodReturnValueHandler} relaxes | ||
| * this requirement for return types it {@linkplain #supportsReturnType | ||
| * supports}. The handler is responsible for invoking the test method | ||
| * (via {@link InvocationInterceptor.Invocation#proceed() | ||
| * invocation.proceed()}) and processing the return value. | ||
| * | ||
| * <p>Because the handler controls the invocation, it can set up a | ||
| * custom execution context before calling {@code proceed()} — for | ||
| * example, running the test method on a specific thread or executor. | ||
| * | ||
| * <p>Implementations are discovered via Java's {@link java.util.ServiceLoader} | ||
| * mechanism and must be registered in | ||
| * {@code META-INF/services/org.junit.jupiter.api.extension.TestMethodReturnValueHandler}. | ||
| * | ||
| * <h2>Example</h2> | ||
| * <pre>{@code | ||
| * public class CompletableFutureHandler implements TestMethodReturnValueHandler { | ||
| * | ||
| * @Override | ||
| * public boolean supportsReturnType(Class<?> returnType) { | ||
| * return CompletableFuture.class.isAssignableFrom(returnType); | ||
| * } | ||
| * | ||
| * @Override | ||
| * public void execute(InvocationInterceptor.Invocation<Object> invocation, | ||
| * ExtensionContext context) throws Throwable { | ||
| * Object result = invocation.proceed(); | ||
| * if (result != null) { | ||
| * ((CompletableFuture<?>) result).get(30, TimeUnit.SECONDS); | ||
| * } | ||
| * } | ||
| * } | ||
| * }</pre> | ||
| * | ||
| * @since 6.2 | ||
| * @see org.junit.jupiter.api.Test | ||
| */ | ||
| @API(status = EXPERIMENTAL, since = "6.2") | ||
| public interface TestMethodReturnValueHandler extends Extension { | ||
|
|
||
| /** | ||
| * Determine if this handler supports the supplied return type. | ||
| * | ||
| * @param returnType the return type of the test method; never {@code null} | ||
| * @return {@code true} if this handler can handle the return type | ||
| */ | ||
| boolean supportsReturnType(Class<?> returnType); | ||
|
|
||
| /** | ||
| * Execute a {@code @Test} method that returns a supported type. | ||
| * | ||
| * <p>The handler must call {@link InvocationInterceptor.Invocation#proceed() | ||
| * invocation.proceed()} to invoke the test method and obtain the return | ||
| * value. The handler is then responsible for processing the result | ||
| * — for example, subscribing to a reactive type and awaiting | ||
| * completion. | ||
| * | ||
| * <p>Because the handler controls when {@code proceed()} is called, it | ||
| * can set up a custom execution context beforehand — for example, | ||
| * running the test on a specific thread or executor. | ||
| * | ||
| * <p>If this method throws, the test is marked as failed with the thrown | ||
| * exception. | ||
| * | ||
| * @param invocation the invocation that executes the test method; | ||
| * calling {@code proceed()} invokes the method and returns its result; | ||
| * never {@code null} | ||
| * @param context the current extension context; never {@code null} | ||
| * @throws Throwable if the return value indicates a test failure | ||
| */ | ||
| void execute(InvocationInterceptor.Invocation<Object> invocation, ExtensionContext context) throws Throwable; | ||
|
|
||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -23,14 +23,17 @@ | |
| import java.util.function.UnaryOperator; | ||
|
|
||
| import org.apiguardian.api.API; | ||
| import org.jspecify.annotations.Nullable; | ||
| import org.junit.jupiter.api.TestInstance.Lifecycle; | ||
| import org.junit.jupiter.api.extension.AfterEachCallback; | ||
| import org.junit.jupiter.api.extension.AfterTestExecutionCallback; | ||
| import org.junit.jupiter.api.extension.BeforeEachCallback; | ||
| import org.junit.jupiter.api.extension.BeforeTestExecutionCallback; | ||
| import org.junit.jupiter.api.extension.ExtensionContext; | ||
| import org.junit.jupiter.api.extension.InvocationInterceptor; | ||
| import org.junit.jupiter.api.extension.InvocationInterceptor.Invocation; | ||
| import org.junit.jupiter.api.extension.LifecycleMethodExecutionExceptionHandler; | ||
| import org.junit.jupiter.api.extension.TestMethodReturnValueHandler; | ||
| import org.junit.jupiter.api.extension.TestExecutionExceptionHandler; | ||
| import org.junit.jupiter.api.extension.TestInstancePreDestroyCallback; | ||
| import org.junit.jupiter.api.extension.TestInstances; | ||
|
|
@@ -39,10 +42,12 @@ | |
| import org.junit.jupiter.engine.execution.AfterEachMethodAdapter; | ||
| import org.junit.jupiter.engine.execution.BeforeEachMethodAdapter; | ||
| import org.junit.jupiter.engine.execution.InterceptingExecutableInvoker; | ||
| import org.junit.jupiter.engine.execution.InterceptingExecutableInvoker.ReflectiveInterceptorCall; | ||
| import org.junit.jupiter.engine.execution.InterceptingExecutableInvoker.ReflectiveInterceptorCall.VoidMethodInterceptorCall; | ||
| import org.junit.jupiter.engine.execution.JupiterEngineExecutionContext; | ||
| import org.junit.jupiter.engine.extension.ExtensionRegistry; | ||
| import org.junit.jupiter.engine.extension.MutableExtensionRegistry; | ||
| import org.junit.jupiter.engine.support.MethodReflectionUtils; | ||
| import org.junit.platform.commons.util.UnrecoverableExceptions; | ||
| import org.junit.platform.engine.TestDescriptor; | ||
| import org.junit.platform.engine.TestExecutionResult; | ||
|
|
@@ -216,8 +221,15 @@ protected void invokeTestMethod(JupiterEngineExecutionContext context, DynamicTe | |
| try { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. shouldn't it be reversed so be reactive by default to enable really concurrent method execution instead of reactive/async impl being made blocking? side note: virtual threads can help but would also hide issues when not used in the enclosing environment so are not an answer to that need alone
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure I understand. What should be reversed? |
||
| Method testMethod = getTestMethod(); | ||
| Object instance = extensionContext.getRequiredTestInstance(); | ||
| executableInvoker.invokeVoid(testMethod, instance, extensionContext, context.getExtensionRegistry(), | ||
| interceptorCall); | ||
| TestMethodReturnValueHandler handler = MethodReflectionUtils.findReturnValueHandler(testMethod); | ||
| if (handler != null) { | ||
| handler.execute(invokeTestMethodCapturingResult(testMethod, instance, | ||
| extensionContext, context.getExtensionRegistry()), extensionContext); | ||
| } | ||
| else { | ||
| executableInvoker.invokeVoid(testMethod, instance, extensionContext, | ||
| context.getExtensionRegistry(), interceptorCall); | ||
| } | ||
| } | ||
| catch (Throwable throwable) { | ||
| UnrecoverableExceptions.rethrowIfUnrecoverable(throwable); | ||
|
|
@@ -226,6 +238,33 @@ protected void invokeTestMethod(JupiterEngineExecutionContext context, DynamicTe | |
| }); | ||
| } | ||
|
|
||
| @SuppressWarnings("NullAway") | ||
| private InvocationInterceptor.Invocation<Object> invokeTestMethodCapturingResult(Method testMethod, Object instance, | ||
| ExtensionContext extensionContext, ExtensionRegistry extensionRegistry) { | ||
| return () -> executableInvoker.<@Nullable Object> invoke(testMethod, instance, | ||
| extensionContext, extensionRegistry, returnValueCapturingInterceptorCall()); | ||
| } | ||
|
|
||
| @SuppressWarnings("NullAway") | ||
| private static ReflectiveInterceptorCall<Method, @Nullable Object> returnValueCapturingInterceptorCall() { | ||
| return (interceptor, invocation, invocationContext, extensionContext) -> { | ||
| Object[] resultHolder = new Object[1]; | ||
| interceptor.interceptTestMethod(new InvocationInterceptor.Invocation<@Nullable Void>() { | ||
| @Override | ||
| public @Nullable Void proceed() throws Throwable { | ||
| resultHolder[0] = invocation.proceed(); | ||
| return null; | ||
| } | ||
|
|
||
| @Override | ||
| public void skip() { | ||
| invocation.skip(); | ||
| } | ||
| }, invocationContext, extensionContext); | ||
| return resultHolder[0]; | ||
| }; | ||
| } | ||
|
|
||
| private void invokeTestExecutionExceptionHandlers(ExtensionRegistry registry, ExtensionContext context, | ||
| Throwable throwable) { | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,46 @@ | ||
| /* | ||
| * Copyright 2015-2026 the original author or authors. | ||
| * | ||
| * All rights reserved. This program and the accompanying materials are | ||
| * made available under the terms of the Eclipse Public License v2.0 which | ||
| * accompanies this distribution and is available at | ||
| * | ||
| * https://www.eclipse.org/legal/epl-v20.html | ||
| */ | ||
|
|
||
| package org.junit.jupiter.engine; | ||
|
|
||
| import java.util.concurrent.CompletableFuture; | ||
| import java.util.concurrent.ExecutionException; | ||
| import java.util.concurrent.TimeUnit; | ||
| import java.util.concurrent.TimeoutException; | ||
|
|
||
| import org.junit.jupiter.api.extension.ExtensionContext; | ||
| import org.junit.jupiter.api.extension.InvocationInterceptor; | ||
| import org.junit.jupiter.api.extension.TestMethodReturnValueHandler; | ||
|
|
||
| public class CompletableFutureReturnValueHandler implements TestMethodReturnValueHandler { | ||
|
|
||
| @Override | ||
| public boolean supportsReturnType(Class<?> returnType) { | ||
| return CompletableFuture.class.isAssignableFrom(returnType); | ||
| } | ||
|
|
||
| @Override | ||
| public void execute(InvocationInterceptor.Invocation<Object> invocation, ExtensionContext context) throws Throwable { | ||
| Object result = invocation.proceed(); | ||
| if (result == null) { | ||
| return; | ||
| } | ||
| try { | ||
| ((CompletableFuture<?>) result).get(30, TimeUnit.SECONDS); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. should likely be infinite or respect @timeout on the method but not 30s by default IMHO as mentionned before it would be better to ensure the execution is reactive (not blocking there)
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. instead of making not void method void friendly, make void methods reactive friendly (CompletionStage can be a neutral contract), this way you can just run all reactive tests concurrently - respect lock semantic indeed. the PR proposal is to make reactive method blocking so just enables syntaxic sugar but not a paradigm change/enablement.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is a test, not an implementation, but otherwise I agree. Perhaps the API should make sure we can access any As for the execution having to block, I'm afraid that's pretty much forced, at least in the case of an event-loop reactive implementation such as Vert.x and Quarkus, because the tests are run on platform threads, and the test body has to be delegated to the event loop, so we cannot avoid delegation and blocking, but we only block the test thread, the test method is properly non-blocking. I definitely don't mean to add support for async/reactive in JUnit, that would be a much bigger endeavour. Not one I would recommend, and definitely not just for this feature.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
no, the idea is that junit retrieve a is replaced by this way the test execution becomes reactive, indeed the "void" (current) execute will be blocking as of today: TIP: it can use a dedicated thread pool ;) so literally you can bridge any reactive or not framework and have a fully reactive execution (just take care of thread pools to not lock but this can be a detail of the SPI impl)
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In most cases I would not want to run reactive tests any more concurrently than blocking tests. Most of my tests have cleanup/teardown that would break if run concurrently. That's not the point of reactive tests. IMO the only point is to test APIs that are reactive in a way that is natural for people writing reactive tests, rather than rely on syntax that is workaround-specific. But true, given a properly parallelisable test suite, it might require something different to be able to collect all reactive tests and run them in parallel. It would be different than how blocking tests run in parallel, because we could just compose all the tests and block on a single thread, rather than start a thread pool. I was hoping this kind of support could come later, as I don't need it and would not know how to achieve it, and this initial support could be a good first step, as it would open the door to tests that return a reactive type, and later if someone contributes support for more efficient parallelising of reactive tests, the syntax from the PoV of the users would not have to change: you can already return reactive types. But the SPI might have to change then, in ways I'm unsure about without experimentation/research. In general I prefer to have something valuable now and small than something perfect later, but that's up to you, I can understand both points of view 😅
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Most frameworks not written with async in mind won't support this easily. I am not familiar with the insides of JUnit, so probably you know better, but I would assume this would be hard to do.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
🤔 it is in the original ticket - and is exatly the reference to https://docs.nunit.org/articles/nunit/writing-tests/attributes/test.html or https://learn.microsoft.com/en-us/archive/msdn-magazine/2014/november/async-programming-unit-testing-asynchronous-code for ex. Else you just save a Indeed your current tests will be broken but it will also require some rewriting. Side note: it is already the case since junit supports concurrent test execution on half of the related aspects so nothing new there.
while it is wrapped it will not be worse than today and threading model is controlled by junit so think it will be ~the same no?
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
These are for programming languages that are not Java 🤔
I did mention we currently achieve this via a parameter and a style that is not reactive-friendly. This is not just throwing a And BTW, I don't think just because a test is reactive it should be run in parallel, I think that the same rule should apply as for blocking methods wrt. running them in parallel or not.
I would defer to the JUnit experts to answer that. Given that they support parallel tests, it should be doable. Not necessarily easy. It was definitely beyond the scope of my PR, though.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. .net is quite aligned on "modern" (java >= 8) java on that aspect, java ecosystem just never payed the bill (likely cause it is split compared to MS ecosystem - blind guess).
this is where this programming model shines, you do decide since the caller just let you hande the context (thread) you do use, so all good and it embraces your statement 100% IMHO |
||
| } | ||
| catch (ExecutionException ex) { | ||
| throw ex.getCause(); | ||
| } | ||
| catch (TimeoutException ex) { | ||
| throw new AssertionError("CompletableFuture did not complete within 30 seconds", ex); | ||
| } | ||
| } | ||
|
|
||
| } | ||
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.
maybe use
Typeinstead ofClass<?>?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.
It would be more versatile, for sure. I can't think of any reason why anybody would need more than the raw type to determine whether it's reactive or not, but other use-cases might benefit from this. It would probably be wise to also make that
Typeavailable somehow fromexecute()(unless it's already present via the parameters somehow).