diff --git a/junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/descriptor/ClassBasedTestDescriptor.java b/junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/descriptor/ClassBasedTestDescriptor.java index 97d310085dbf..9e08f5c2d307 100644 --- a/junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/descriptor/ClassBasedTestDescriptor.java +++ b/junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/descriptor/ClassBasedTestDescriptor.java @@ -214,7 +214,7 @@ public void after(JupiterEngineExecutionContext context) { } if (isPerClassLifecycle(context)) { - invokeTestInstancePreDestroyCallback(context); + invokeTestInstancePreDestroyCallbacks(context); } // If the previous Throwable was not null when this method was called, @@ -425,7 +425,7 @@ private void invokeAfterAllCallbacks(JupiterEngineExecutionContext context) { .forEach(extension -> throwableCollector.execute(() -> extension.afterAll(extensionContext))); } - private void invokeTestInstancePreDestroyCallback(JupiterEngineExecutionContext context) { + private void invokeTestInstancePreDestroyCallbacks(JupiterEngineExecutionContext context) { ExtensionContext extensionContext = context.getExtensionContext(); ThrowableCollector throwableCollector = context.getThrowableCollector(); diff --git a/junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/descriptor/TestMethodTestDescriptor.java b/junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/descriptor/TestMethodTestDescriptor.java index 49dd3598b16c..b514a54a9796 100644 --- a/junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/descriptor/TestMethodTestDescriptor.java +++ b/junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/descriptor/TestMethodTestDescriptor.java @@ -20,7 +20,7 @@ import java.util.function.Consumer; import org.apiguardian.api.API; -import org.junit.jupiter.api.TestInstance; +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; @@ -141,7 +141,9 @@ public JupiterEngineExecutionContext execute(JupiterEngineExecutionContext conte invokeAfterEachMethods(context); } invokeAfterEachCallbacks(context); - invokeTestInstancePreDestroyCallback(context); + if (isPerMethodLifecycle(context)) { + invokeTestInstancePreDestroyCallbacks(context); + } // @formatter:on throwableCollector.assertEmpty(); @@ -149,6 +151,11 @@ public JupiterEngineExecutionContext execute(JupiterEngineExecutionContext conte return context; } + private boolean isPerMethodLifecycle(JupiterEngineExecutionContext context) { + return context.getExtensionContext().getTestInstanceLifecycle().orElse( + Lifecycle.PER_CLASS) == Lifecycle.PER_METHOD; + } + private void invokeBeforeEachCallbacks(JupiterEngineExecutionContext context) { invokeBeforeMethodsOrCallbacksUntilExceptionOccurs(BeforeEachCallback.class, context, (callback, extensionContext) -> callback.beforeEach(extensionContext)); @@ -248,13 +255,9 @@ private void invokeAfterEachCallbacks(JupiterEngineExecutionContext context) { (callback, extensionContext) -> callback.afterEach(extensionContext)); } - private void invokeTestInstancePreDestroyCallback(JupiterEngineExecutionContext context) { - context.getExtensionContext().getTestInstanceLifecycle().ifPresent(lifecycle -> { - if (TestInstance.Lifecycle.PER_METHOD == lifecycle) { - invokeAllAfterMethodsOrCallbacks(TestInstancePreDestroyCallback.class, context, - TestInstancePreDestroyCallback::preDestroyTestInstance); - } - }); + private void invokeTestInstancePreDestroyCallbacks(JupiterEngineExecutionContext context) { + invokeAllAfterMethodsOrCallbacks(TestInstancePreDestroyCallback.class, context, + TestInstancePreDestroyCallback::preDestroyTestInstance); } private void invokeAllAfterMethodsOrCallbacks(Class type, diff --git a/junit-jupiter-engine/src/test/java/org/junit/jupiter/engine/extension/TestInstancePreDestroyCallbackTests.java b/junit-jupiter-engine/src/test/java/org/junit/jupiter/engine/extension/TestInstancePreDestroyCallbackTests.java index bed1b866fd47..5981a8576b69 100644 --- a/junit-jupiter-engine/src/test/java/org/junit/jupiter/engine/extension/TestInstancePreDestroyCallbackTests.java +++ b/junit-jupiter-engine/src/test/java/org/junit/jupiter/engine/extension/TestInstancePreDestroyCallbackTests.java @@ -12,7 +12,7 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.junit.jupiter.api.Assertions.assertFalse; -import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.TestInstance.Lifecycle.PER_CLASS; import java.util.ArrayList; import java.util.List; @@ -46,7 +46,6 @@ void instancePreDestroyCallbacksInNestedClasses() { // @formatter:off assertThat(callSequence).containsExactly( - // OuterTestCase "beforeOuterMethod", "testOuter", @@ -57,7 +56,6 @@ void instancePreDestroyCallbacksInNestedClasses() { "beforeInnerMethod", "testInner", "barPreDestroyCallbackTestInstance:InnerTestCase", - "fooPreDestroyCallbackTestInstance:InnerTestCase" ); // @formatter:on @@ -65,9 +63,8 @@ void instancePreDestroyCallbacksInNestedClasses() { @Test void testSpecificTestInstancePreDestroyCallbackIsCalled() { - executeTestsForClass( - TestCaseWithTestSpecificTestInstancePreDestroyCallback.class).testEvents().assertStatistics( - stats -> stats.started(1).succeeded(1)); + executeTestsForClass(TestCaseWithTestSpecificTestInstancePreDestroyCallback.class).testEvents()// + .assertStatistics(stats -> stats.started(1).succeeded(1)); // @formatter:off assertThat(callSequence).containsExactly( @@ -80,8 +77,8 @@ void testSpecificTestInstancePreDestroyCallbackIsCalled() { @Test void classLifecyclePreDestroyCallbacks() { - executeTestsForClass(PerClassLifecyclePreDestroyCallbackWithTwoTestMethods.class).testEvents().assertStatistics( - stats -> stats.started(2).succeeded(2)); + executeTestsForClass(PerClassLifecyclePreDestroyCallbackWithTwoTestMethods.class).testEvents()// + .assertStatistics(stats -> stats.started(2).succeeded(2)); // @formatter:off assertThat(callSequence).containsExactly( @@ -96,6 +93,15 @@ void classLifecyclePreDestroyCallbacks() { // ------------------------------------------------------------------- + private abstract static class Destroyable { + + boolean destroyed; + + void setDestroyed() { + this.destroyed = true; + } + } + @ExtendWith(FooInstancePreDestroyCallback.class) static class OuterTestCase extends Destroyable { @@ -106,7 +112,7 @@ void beforeOuterMethod() { @Test void testOuter() { - assertFalse(isDestroyed); + assertFalse(destroyed); callSequence.add("testOuter"); } @@ -116,7 +122,7 @@ class InnerTestCase extends Destroyable { @BeforeEach void beforeInnerMethod() { - assertFalse(isDestroyed); + assertFalse(destroyed); callSequence.add("beforeInnerMethod"); } @@ -131,7 +137,7 @@ static class TestCaseWithTestSpecificTestInstancePreDestroyCallback extends Dest @BeforeEach void beforeEachMethod() { - assertFalse(isDestroyed); + assertFalse(destroyed); callSequence.add("beforeEachMethod"); } @@ -142,7 +148,7 @@ void test() { } } - @TestInstance(TestInstance.Lifecycle.PER_CLASS) + @TestInstance(PER_CLASS) @ExtendWith(FooInstancePreDestroyCallback.class) static class PerClassLifecyclePreDestroyCallbackWithTwoTestMethods extends Destroyable { @@ -162,31 +168,17 @@ void test2() { } } - static class FooInstancePreDestroyCallback extends AbstractInstancePreDestroyCallback { - - protected FooInstancePreDestroyCallback() { - super("foo"); - } - } - - static class BarInstancePreDestroyCallback extends AbstractInstancePreDestroyCallback { - - protected BarInstancePreDestroyCallback() { - super("bar"); - } - } - - static abstract class AbstractInstancePreDestroyCallback implements TestInstancePreDestroyCallback { + static abstract class AbstractTestInstancePreDestroyCallback implements TestInstancePreDestroyCallback { private final String name; - AbstractInstancePreDestroyCallback(String name) { + AbstractTestInstancePreDestroyCallback(String name) { this.name = name; } @Override public void preDestroyTestInstance(ExtensionContext context) { - assertTrue(context.getTestInstance().isPresent()); + assertThat(context.getTestInstance()).isPresent(); Object testInstance = context.getTestInstance().get(); if (testInstance instanceof Destroyable) { ((Destroyable) testInstance).setDestroyed(); @@ -195,12 +187,18 @@ public void preDestroyTestInstance(ExtensionContext context) { } } - private abstract static class Destroyable { + static class FooInstancePreDestroyCallback extends AbstractTestInstancePreDestroyCallback { - boolean isDestroyed; + FooInstancePreDestroyCallback() { + super("foo"); + } + } - void setDestroyed() { - isDestroyed = true; + static class BarInstancePreDestroyCallback extends AbstractTestInstancePreDestroyCallback { + + BarInstancePreDestroyCallback() { + super("bar"); } } + }