Skip to content

Issue 3054 avoiding inclusion of the suppressed PreconditionViolation… #4439

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

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

Issue 3054 avoiding inclusion of the suppressed PreconditionViolation… #4439

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
bba5355
Issue 3054 avoiding inclusion of the suppressed PreconditionViolation…
BassemElMasry Mar 31, 2025
cb3d5d6
Add test
marcphilipp Apr 1, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 17 additions & 1 deletion junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/descriptor/TemplateExecutor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,9 @@
import org.junit.jupiter.api.extension.ExtensionContext;
import org.junit.jupiter.engine.execution.JupiterEngineExecutionContext;
import org.junit.jupiter.engine.extension.ExtensionRegistry;
import org.junit.platform.commons.util.ExceptionUtils;
import org.junit.platform.commons.util.Preconditions;
import org.junit.platform.commons.util.UnrecoverableExceptions;
import org.junit.platform.engine.TestDescriptor;
import org.junit.platform.engine.UniqueId;
import org.junit.platform.engine.support.hierarchical.Node;
Expand Down Expand Up @@ -52,11 +54,25 @@ private void executeForProvider(P provider, AtomicInteger invocationIndex,

int initialValue = invocationIndex.get();

try (Stream<? extends C> stream = provideContexts(provider, extensionContext)) {
Stream<? extends C> stream = provideContexts(provider, extensionContext);
try {
stream.forEach(invocationContext -> createInvocationTestDescriptor(invocationContext,
invocationIndex.incrementAndGet()) //
.ifPresent(testDescriptor -> execute(dynamicTestExecutor, testDescriptor)));
}
catch (Throwable t) {
try {
stream.close();
}
catch (Throwable t2) {
Copy link
Member

Choose a reason for hiding this comment

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

I don't think we should generally discard all Throwables thrown from providers' onClose callbacks. The exception could contain an important hint so adding it as a suppressed exception could be useful. Therefore, I think we should introduce a custom TemplateInvocationValidationException (in org.junit.jupiter.api.extension) that extends JUnitException and throw it from here:

junit5/junit-jupiter-params/src/main/java/org/junit/jupiter/params/ParameterizedInvocationContextProvider.java

Lines 51 to 52 in c4faa6c

Preconditions.condition(invocationCount.get() > 0 || declarationContext.isAllowingZeroInvocations(),
() -> String.format("Configuration error: You must configure at least one set of arguments for this @%s", declarationContext.getAnnotationName())));

Copy link
Member

Choose a reason for hiding this comment

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

@BassemElMasry Do you have time to make the change or should I take it from here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hello @marcphilipp , I can finalize it this week. Sorry I did not see the comments earlier. If this is not feasible for you, feel free to finalize it of course.

Copy link
Member

Choose a reason for hiding this comment

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

No worries! Finalizing it this week is fine, please go ahead! 🙂

// ignore exceptions from close() to avoid masking the original failure
UnrecoverableExceptions.rethrowIfUnrecoverable(t2);
}
throw ExceptionUtils.throwAsUncheckedException(t);
}
finally {
stream.close();
}

Preconditions.condition(
invocationIndex.get() != initialValue || mayReturnZeroContexts(provider, extensionContext),
Expand Down
54 changes: 43 additions & 11 deletions jupiter-tests/src/test/java/org/junit/jupiter/params/ParameterizedTestIntegrationTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@
import static org.junit.jupiter.engine.discovery.JupiterUniqueIdBuilder.appendTestTemplateInvocationSegment;
import static org.junit.jupiter.engine.discovery.JupiterUniqueIdBuilder.uniqueIdForTestTemplateMethod;
import static org.junit.jupiter.params.provider.Arguments.arguments;
import static org.junit.platform.engine.discovery.DiscoverySelectors.selectClass;
import static org.junit.platform.engine.discovery.DiscoverySelectors.selectIteration;
import static org.junit.platform.engine.discovery.DiscoverySelectors.selectMethod;
import static org.junit.platform.engine.discovery.DiscoverySelectors.selectUniqueId;
Expand Down Expand Up @@ -87,6 +86,7 @@
import org.junit.jupiter.api.extension.ParameterResolutionException;
import org.junit.jupiter.api.extension.ParameterResolver;
import org.junit.jupiter.api.extension.RegisterExtension;
import org.junit.jupiter.engine.AbstractJupiterTestEngineTests;
import org.junit.jupiter.engine.JupiterTestEngine;
import org.junit.jupiter.params.ParameterizedTestIntegrationTests.RepeatableSourcesTestCase.Action;
import org.junit.jupiter.params.aggregator.AggregateWith;
Expand All @@ -111,8 +111,8 @@
import org.junit.jupiter.params.support.ParameterDeclarations;
import org.junit.platform.commons.PreconditionViolationException;
import org.junit.platform.commons.util.ClassUtils;
import org.junit.platform.engine.DiscoverySelector;
import org.junit.platform.engine.TestDescriptor;
import org.junit.platform.engine.TestExecutionResult;
import org.junit.platform.testkit.engine.EngineExecutionResults;
import org.junit.platform.testkit.engine.EngineTestKit;
import org.junit.platform.testkit.engine.Event;
Expand All @@ -122,7 +122,7 @@
/**
* @since 5.0
*/
class ParameterizedTestIntegrationTests {
class ParameterizedTestIntegrationTests extends AbstractJupiterTestEngineTests {

private final Locale originalLocale = Locale.getDefault(Locale.Category.FORMAT);

Expand Down Expand Up @@ -394,7 +394,7 @@ void executesLifecycleMethods() {
LifecycleTestCase.lifecycleEvents.clear();
LifecycleTestCase.testMethods.clear();

var results = execute(selectClass(LifecycleTestCase.class));
var results = executeTestsForClass(LifecycleTestCase.class);
results.allEvents().assertThatEvents() //
.haveExactly(1,
event(test("test1"), displayName("[1] argument=foo"), finishedWithFailure(message("foo")))) //
Expand Down Expand Up @@ -476,18 +476,30 @@ void failsWhenNoArgumentsSourceIsDeclared() {
"Configuration error: You must configure at least one arguments source for this @ParameterizedTest"))));
}

private EngineExecutionResults execute(DiscoverySelector... selectors) {
return EngineTestKit.engine(new JupiterTestEngine()).selectors(selectors).execute();
}
@Test
void reportsExceptionInStaticInitializersWithoutInvocationCountValidation() {
var results = executeTestsForClass(ExceptionInStaticInitializerTestCase.class);

private EngineExecutionResults execute(Class<?> testClass, String methodName, Class<?>... methodParameterTypes) {
return execute(selectMethod(testClass, methodName, ClassUtils.nullSafeToString(methodParameterTypes)));
var failure = results.containerEvents().stream() //
.filter(finishedWithFailure()::matches) //
.findAny() //
.orElseThrow();

var throwable = failure.getRequiredPayload(TestExecutionResult.class).getThrowable().orElseThrow();

assertThat(throwable) //
.isInstanceOf(ExceptionInInitializerError.class) //
.hasNoSuppressedExceptions();
}

private EngineExecutionResults execute(String methodName, Class<?>... methodParameterTypes) {
return execute(TestCase.class, methodName, methodParameterTypes);
}

private EngineExecutionResults execute(Class<?> testClass, String methodName, Class<?>... methodParameterTypes) {
return executeTests(selectMethod(testClass, methodName, ClassUtils.nullSafeToString(methodParameterTypes)));
}

/**
* @since 5.4
*/
Expand Down Expand Up @@ -915,7 +927,7 @@ void duplicateMethodNames() {
// other words, we're not really testing the support for @RepeatedTest
// and @TestFactory, but their presence also contributes to the bug
// reported in #3001.
ParameterizedTestIntegrationTests.this.execute(selectClass(DuplicateMethodNamesMethodSourceTestCase.class))//
executeTestsForClass(DuplicateMethodNamesMethodSourceTestCase.class)//
.testEvents()//
.assertStatistics(stats -> stats.started(8).failed(0).finished(8));
}
Expand Down Expand Up @@ -1334,7 +1346,7 @@ void closeAutoCloseableArgumentsAfterTestDespiteEarlyFailure() {
@Test
void executesTwoIterationsBasedOnIterationAndUniqueIdSelector() {
var methodId = uniqueIdForTestTemplateMethod(TestCase.class, "testWithThreeIterations(int)");
var results = execute(selectUniqueId(appendTestTemplateInvocationSegment(methodId, 3)),
var results = executeTests(selectUniqueId(appendTestTemplateInvocationSegment(methodId, 3)),
selectIteration(selectMethod(TestCase.class, "testWithThreeIterations", "int"), 1));

results.allEvents().assertThatEvents() //
Expand Down Expand Up @@ -2598,4 +2610,24 @@ void test(AutoCloseableArgument autoCloseable) {
}
}

static class ExceptionInStaticInitializerTestCase {

static {
//noinspection ConstantValue
if (true)
throw new RuntimeException("boom");
}

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

@ParameterizedTest
@MethodSource("getArguments")
void test(String value) {
fail("should not be called: " + value);
}

}

}