search

junit-team / junit5

āœ… The 5th major version of the programmer-friendly testing framework for Java and the JVM
https://junit.org
Eclipse Public License 2.0
6.44k stars 1.5k forks source link

Parameterized tests: misleading error if exception was thrown in static initializer #3054

Open sebastianhaberey opened 2 years ago

sebastianhaberey commented 2 years ago

When a parameterized test is executed, but an exception is thrown in the test class' static initializer, the error message is:

"Configuration error: You must configure at least one set of arguments for this @ParameterizedTest"

This distracts from the actual source of the problem. I know static initializers are generally frowned upon, but they are recommended for some edge cases, e.g. by the Testcontainers project.

Steps to reproduce

class FooTest {

    static {
        if (true) throw new RuntimeException("some exception");
    }

    private static Stream<Arguments> getArguments() {
        return Stream.of("foo", "bar").map(Arguments::of);
    }

    @ParameterizedTest
    @MethodSource("getArguments")
    void shouldProcessSmartFile(String testString) {
        assertThat(testString).hasSize(3);
    }

}

Context

Full error message

click here ```java java.lang.ExceptionInInitializerError at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:77) at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) at java.base/java.lang.reflect.Method.invoke(Method.java:568) at org.junit.platform.commons.util.ReflectionUtils.invokeMethod(ReflectionUtils.java:725) at org.junit.jupiter.params.provider.MethodArgumentsProvider.lambda$provideArguments$1(MethodArgumentsProvider.java:46) at java.base/java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:197) at java.base/java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:197) at java.base/java.util.Spliterators$ArraySpliterator.forEachRemaining(Spliterators.java:992) at java.base/java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:509) at java.base/java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:499) at java.base/java.util.stream.ForEachOps$ForEachOp.evaluateSequential(ForEachOps.java:150) at java.base/java.util.stream.ForEachOps$ForEachOp$OfRef.evaluateSequential(ForEachOps.java:173) at java.base/java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234) at java.base/java.util.stream.ReferencePipeline.forEach(ReferencePipeline.java:596) at java.base/java.util.stream.ReferencePipeline$7$1.accept(ReferencePipeline.java:276) at java.base/java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:197) at java.base/java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:197) at java.base/java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:197) at java.base/java.util.ArrayList$ArrayListSpliterator.forEachRemaining(ArrayList.java:1625) at java.base/java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:509) at java.base/java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:499) at java.base/java.util.stream.ForEachOps$ForEachOp.evaluateSequential(ForEachOps.java:150) at java.base/java.util.stream.ForEachOps$ForEachOp$OfRef.evaluateSequential(ForEachOps.java:173) at java.base/java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234) at java.base/java.util.stream.ReferencePipeline.forEach(ReferencePipeline.java:596) at java.base/java.util.stream.ReferencePipeline$7$1.accept(ReferencePipeline.java:276) at java.base/java.util.ArrayList$ArrayListSpliterator.forEachRemaining(ArrayList.java:1625) at java.base/java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:509) at java.base/java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:499) at java.base/java.util.stream.ForEachOps$ForEachOp.evaluateSequential(ForEachOps.java:150) at java.base/java.util.stream.ForEachOps$ForEachOp$OfRef.evaluateSequential(ForEachOps.java:173) at java.base/java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234) at java.base/java.util.stream.ReferencePipeline.forEach(ReferencePipeline.java:596) at org.junit.jupiter.engine.descriptor.TestTemplateTestDescriptor.execute(TestTemplateTestDescriptor.java:107) at org.junit.jupiter.engine.descriptor.TestTemplateTestDescriptor.execute(TestTemplateTestDescriptor.java:42) at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$6(NodeTestTask.java:151) at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73) at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$8(NodeTestTask.java:141) at org.junit.platform.engine.support.hierarchical.Node.around(Node.java:137) at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$9(NodeTestTask.java:139) at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73) at org.junit.platform.engine.support.hierarchical.NodeTestTask.executeRecursively(NodeTestTask.java:138) at org.junit.platform.engine.support.hierarchical.NodeTestTask.execute(NodeTestTask.java:95) at java.base/java.util.ArrayList.forEach(ArrayList.java:1511) at org.junit.platform.engine.support.hierarchical.SameThreadHierarchicalTestExecutorService.invokeAll(SameThreadHierarchicalTestExecutorService.java:41) at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$6(NodeTestTask.java:155) at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73) at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$8(NodeTestTask.java:141) at org.junit.platform.engine.support.hierarchical.Node.around(Node.java:137) at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$9(NodeTestTask.java:139) at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73) at org.junit.platform.engine.support.hierarchical.NodeTestTask.executeRecursively(NodeTestTask.java:138) at org.junit.platform.engine.support.hierarchical.NodeTestTask.execute(NodeTestTask.java:95) at java.base/java.util.ArrayList.forEach(ArrayList.java:1511) at org.junit.platform.engine.support.hierarchical.SameThreadHierarchicalTestExecutorService.invokeAll(SameThreadHierarchicalTestExecutorService.java:41) at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$6(NodeTestTask.java:155) at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73) at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$8(NodeTestTask.java:141) at org.junit.platform.engine.support.hierarchical.Node.around(Node.java:137) at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$9(NodeTestTask.java:139) at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73) at org.junit.platform.engine.support.hierarchical.NodeTestTask.executeRecursively(NodeTestTask.java:138) at org.junit.platform.engine.support.hierarchical.NodeTestTask.execute(NodeTestTask.java:95) at org.junit.platform.engine.support.hierarchical.SameThreadHierarchicalTestExecutorService.submit(SameThreadHierarchicalTestExecutorService.java:35) at org.junit.platform.engine.support.hierarchical.HierarchicalTestExecutor.execute(HierarchicalTestExecutor.java:57) at org.junit.platform.engine.support.hierarchical.HierarchicalTestEngine.execute(HierarchicalTestEngine.java:54) at org.junit.platform.launcher.core.EngineExecutionOrchestrator.execute(EngineExecutionOrchestrator.java:108) at org.junit.platform.launcher.core.EngineExecutionOrchestrator.execute(EngineExecutionOrchestrator.java:88) at org.junit.platform.launcher.core.EngineExecutionOrchestrator.lambda$execute$0(EngineExecutionOrchestrator.java:54) at org.junit.platform.launcher.core.EngineExecutionOrchestrator.withInterceptedStreams(EngineExecutionOrchestrator.java:67) at org.junit.platform.launcher.core.EngineExecutionOrchestrator.execute(EngineExecutionOrchestrator.java:52) at org.junit.platform.launcher.core.DefaultLauncher.execute(DefaultLauncher.java:96) at org.junit.platform.launcher.core.DefaultLauncher.execute(DefaultLauncher.java:75) at org.gradle.api.internal.tasks.testing.junitplatform.JUnitPlatformTestClassProcessor$CollectAllTestClassesExecutor.processAllTestClasses(JUnitPlatformTestClassProcessor.java:99) at org.gradle.api.internal.tasks.testing.junitplatform.JUnitPlatformTestClassProcessor$CollectAllTestClassesExecutor.access$000(JUnitPlatformTestClassProcessor.java:79) at org.gradle.api.internal.tasks.testing.junitplatform.JUnitPlatformTestClassProcessor.stop(JUnitPlatformTestClassProcessor.java:75) at org.gradle.api.internal.tasks.testing.SuiteTestClassProcessor.stop(SuiteTestClassProcessor.java:61) at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:77) at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) at java.base/java.lang.reflect.Method.invoke(Method.java:568) at org.gradle.internal.dispatch.ReflectionDispatch.dispatch(ReflectionDispatch.java:36) at org.gradle.internal.dispatch.ReflectionDispatch.dispatch(ReflectionDispatch.java:24) at org.gradle.internal.dispatch.ContextClassLoaderDispatch.dispatch(ContextClassLoaderDispatch.java:33) at org.gradle.internal.dispatch.ProxyDispatchAdapter$DispatchingInvocationHandler.invoke(ProxyDispatchAdapter.java:94) at jdk.proxy1/jdk.proxy1.$Proxy2.stop(Unknown Source) at org.gradle.api.internal.tasks.testing.worker.TestWorker$3.run(TestWorker.java:193) at org.gradle.api.internal.tasks.testing.worker.TestWorker.executeAndMaintainThreadName(TestWorker.java:129) at org.gradle.api.internal.tasks.testing.worker.TestWorker.execute(TestWorker.java:100) at org.gradle.api.internal.tasks.testing.worker.TestWorker.execute(TestWorker.java:60) at org.gradle.process.internal.worker.child.ActionExecutionWorker.execute(ActionExecutionWorker.java:56) at org.gradle.process.internal.worker.child.SystemApplicationClassLoaderWorker.call(SystemApplicationClassLoaderWorker.java:133) at org.gradle.process.internal.worker.child.SystemApplicationClassLoaderWorker.call(SystemApplicationClassLoaderWorker.java:71) at worker.org.gradle.process.internal.worker.GradleWorkerMain.run(GradleWorkerMain.java:69) at worker.org.gradle.process.internal.worker.GradleWorkerMain.main(GradleWorkerMain.java:74) Suppressed: org.junit.platform.commons.PreconditionViolationException: Configuration error: You must configure at least one set of arguments for this @ParameterizedTest at org.junit.platform.commons.util.Preconditions.condition(Preconditions.java:281) at org.junit.jupiter.params.ParameterizedTestExtension.lambda$provideTestTemplateInvocationContexts$5(ParameterizedTestExtension.java:98) at java.base/java.util.stream.AbstractPipeline.close(AbstractPipeline.java:323) at java.base/java.util.stream.ReferencePipeline$7$1.accept(ReferencePipeline.java:273) ... 69 more Caused by: java.lang.RuntimeException: some exception at de.xxx.mds.e2etest.tests.FooTest.(FooTest.java:14) ... 96 more ```
sbrannen commented 2 years ago

When a parameterized test is executed, but an exception is thrown in the test class' static initializer, the error message is:

"Configuration error: You must configure at least one set of arguments for this @ParameterizedTest"

That's the "suppressed" exception.

The actual exception that is thrown is the java.lang.ExceptionInInitializerError with the following cause:

Caused by: java.lang.RuntimeException: some exception

In Eclipse IDE, one sees the ExceptionInInitializerError by default and has to scroll to find the suppressed exception.

Does IntelliJ display the suppressed exception instead of the top-level exception?

sebastianhaberey commented 2 years ago

I am not familiar with the concept of "suppressed" exceptions. Like all Java developers I am used to scrolling down lengthy stacktraces hoping for a clue about what's going on. In my experience it's the plain text phrases that provide the most help. Such as "Configuration error: You must configure at least one set of arguments for this @ParameterizedTest".

Only in this case, that's not what's going on at all. My suggestion would be to simply not have that in the stack trace.

marcphilipp commented 2 years ago

Only in this case, that's not what's going on at all. My suggestion would be to simply not have that in the stack trace.

Well, I can see that it's a bit confusing but it did indeed fail to provide at least one set of arguments because it tried to load the test class in order to invoke the static method which failed which is indicated by the root cause.

sebastianhaberey commented 2 years ago

I am convinced the message makes perfect sense for anyone familiar with the internals of the API. As a user I found it confusing. I'd be interested in how other users see it, but since I'm the first one to report it, it may well be a "me-issue" šŸ™‚ So feel free to close this and re-visit it if it turns out others feel the same!

juliette-derancourt commented 2 years ago

As a user I found it confusing. I'd be interested in how other users see it, but since I'm the first one to report it, it may well be a "me-issue" šŸ™‚

I agree that the message could be confusing in this case

Well, I can see that it's a bit confusing but it did indeed fail to provide at least one set of arguments because it tried to load the test class in order to invoke the static method which failed which is indicated by the root cause.

I think the confusion lies in the You must configure part of the error message. It suggests that the user forgot to configure the arguments (which is not the case here), when it's actually the method providing said arguments that couldn't be invoked. Maybe a simple change of wording could make it more adequate?

marcphilipp commented 2 years ago

Team decision: Investigate options for avoiding inclusion of the suppressed PreconditionViolationException in the stacktrace.

BassemElMasry commented 1 year ago

@marcphilipp @sbrannen if no one is working on this I would like to check it out, is it available to take a stab at?

sbrannen commented 1 year ago

@BassemElMasry, this issue is labeled as up-for-grabs.

So that means anyone can attempt to solve it and submit a PR.

If you do start working on a PR, please post back here so that the community knows you're working on it.

FYI: I looked into this issue, and at a glance it appears it might be slightly challenging.

Specifically, the described behavior (a suppressed exception in a stream pipeline) is a direct result of the documented contract for Stream#onClose, which is what is used in ParameterizedTestExtension.provideTestTemplateInvocationContexts(ExtensionContext).

BassemElMasry commented 1 year ago

Thank you @sbrannen for letting me know. I will take a look and try to come up with the solution and submit a PR.

sbrannen commented 1 year ago

Thank you @sbrannen for letting me know.

You're welcome.

I will take a look and try to come up with the solution and submit a PR.

OK. This is now assigned to you.

Please keep us posted on your progress.

BassemElMasry commented 1 year ago

Hello @sbrannen @marcphilipp , what do you think if we let it to the consumer do decide if the suppressed exceptions should be included or only the top level exceptions when using the parametrized tests?

BassemElMasry commented 1 year ago

Hello @sbrannen @marcphilipp , I have been trying this for a while now, and the way I was able to achieve this was by introducing a new customized InitializationErrorException and a static method reThrowIfStaticIntializerError that checks if the exception is an ExceptionInInitializerError and reThrow it as unchecked exception of type InitializationErrorException. I invoke this method in ThrowableCollector#execute


`public void execute(Executable executable) {
        try {
            executable.execute();
        }
        catch (Throwable t) {
            UnrecoverableExceptions.rethrowIfUnrecoverable(t);
            UnrecoverableExceptions.reThrowIfStaticIntializerError(t);
            add(t);
        }
}`
`public static void reThrowIfStaticIntializerError(Throwable exception) {
        if (exception instanceof ExceptionInInitializerError && exception.getSuppressed().length >= 1) {
            ExceptionUtils.throwAsUncheckedException(
                new InitializationErrorException("Initialization Error", exception.getCause(), false, true));
        }
}`

`@SuppressWarnings("serial")
public class InitializationErrorException extends Exception {
    public InitializationErrorException(String message, Throwable cause, boolean enableSuppression,
            boolean writableStackTrace) {
        super(message, cause, enableSuppression, writableStackTrace);
    }

}`

any ideas of better ways to do this?

marcphilipp commented 3 weeks ago

@BassemElMasry I think ThrowableCollector is not the right place to change this as its used in different places in the codebase.

Since the suppressed exception is thrown when the Stream is closed, I think the right place to handle this is here:

https://github.com/junit-team/junit5/blob/f32aa0a840da0115c8dd2d46547493e356e04475/junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/descriptor/TestTemplateTestDescriptor.java#L112-L115

Changing that to the following should do the trick:

Stream<TestTemplateInvocationContext> stream = invocationContexts(provider, extensionContext);
try {
    stream.forEach(
            invocationContext -> toTestDescriptor(invocationContext, invocationIndex.incrementAndGet()) //
                    .ifPresent(testDescriptor -> execute(dynamicTestExecutor, testDescriptor)));
}
catch (Throwable t) {
    try {
        stream.close();
    } catch (Throwable t2) {
        // ignore exceptions from close() to avoid masking the original failure
        UnrecoverableExceptions.rethrowIfUnrecoverable(t2);
    }
    throw ExceptionUtils.throwAsUncheckedException(t);
} finally {
    stream.close();
}
BassemElMasry commented 3 weeks ago

@marcphilipp Thanks for replying Marc! I see, I will work on it