From f53e690beab53c87e1d48f06a3c9b8b9ff1e3b88 Mon Sep 17 00:00:00 2001 From: Sam Brannen <104798+sbrannen@users.noreply.github.com> Date: Thu, 4 Jan 2024 16:23:33 +0100 Subject: [PATCH] =?UTF-8?q?Ensure=20all=20@=E2=81=A0AutoClose=20fields=20a?= =?UTF-8?q?re=20closed=20in=20@=E2=81=A0Nested=20hierarchies?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This commit ensures that the use of ThrowableCollector is properly applied in @⁠Nested test class hierarchies. Closes #3367 --- .../engine/extension/AutoCloseExtension.java | 12 +- .../engine/extension/AutoCloseTests.java | 168 ++++++++++++++++-- 2 files changed, 165 insertions(+), 15 deletions(-) diff --git a/junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/extension/AutoCloseExtension.java b/junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/extension/AutoCloseExtension.java index a4cf2bd1917..d656a640076 100644 --- a/junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/extension/AutoCloseExtension.java +++ b/junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/extension/AutoCloseExtension.java @@ -44,21 +44,23 @@ class AutoCloseExtension implements TestInstancePreDestroyCallback, AfterAllCall @Override public void preDestroyTestInstance(ExtensionContext context) { + ThrowableCollector throwableCollector = new ThrowableCollector(__ -> false); TestInstancePreDestroyCallback.preDestroyTestInstances(context, - testInstance -> closeFields(testInstance.getClass(), testInstance)); + testInstance -> closeFields(testInstance.getClass(), testInstance, throwableCollector)); + throwableCollector.assertEmpty(); } @Override public void afterAll(ExtensionContext context) { - closeFields(context.getRequiredTestClass(), null); + ThrowableCollector throwableCollector = new ThrowableCollector(__ -> false); + closeFields(context.getRequiredTestClass(), null, throwableCollector); + throwableCollector.assertEmpty(); } - private static void closeFields(Class testClass, Object testInstance) { - ThrowableCollector throwableCollector = new ThrowableCollector(__ -> false); + private static void closeFields(Class testClass, Object testInstance, ThrowableCollector throwableCollector) { Predicate predicate = (testInstance == null ? ReflectionUtils::isStatic : ReflectionUtils::isNotStatic); AnnotationUtils.findAnnotatedFields(testClass, AutoClose.class, predicate, BOTTOM_UP).forEach( field -> throwableCollector.execute(() -> closeField(field, testInstance))); - throwableCollector.assertEmpty(); } private static void closeField(Field field, Object testInstance) throws Exception { diff --git a/junit-jupiter-engine/src/test/java/org/junit/jupiter/engine/extension/AutoCloseTests.java b/junit-jupiter-engine/src/test/java/org/junit/jupiter/engine/extension/AutoCloseTests.java index 32272f09ffb..c3052c59708 100644 --- a/junit-jupiter-engine/src/test/java/org/junit/jupiter/engine/extension/AutoCloseTests.java +++ b/junit-jupiter-engine/src/test/java/org/junit/jupiter/engine/extension/AutoCloseTests.java @@ -219,23 +219,18 @@ void fieldsAreProperlyClosedWithinTestClassHierarchy() { @Test void allFieldsAreClosedIfAnyFieldThrowsAnException() { - String staticField1 = "staticField1"; - String staticField2 = "staticField2"; - String staticField3 = "staticField3"; - String field1 = "field1"; - String field2 = "field2"; - String field3 = "field3"; - // Prerequisites to ensure fields are "ordered" as expected (based on the hash codes for their names). - assertThat(staticField1.hashCode()).isLessThan(staticField2.hashCode()).isLessThan(staticField3.hashCode()); - assertThat(field1.hashCode()).isLessThan(field2.hashCode()).isLessThan(field3.hashCode()); + assertThat("staticField1".hashCode()).isLessThan("staticField2".hashCode()).isLessThan( + "staticField3".hashCode()); + assertThat("field1".hashCode()).isLessThan("field2".hashCode()).isLessThan("field3".hashCode()); Class testClass = FailingFieldsTestCase.class; EngineExecutionResults allEvents = executeTestsForClass(testClass); Events tests = allEvents.testEvents(); tests.assertStatistics(stats -> stats.succeeded(0).failed(1)); - // Verify that ALL fields were closed. + + // Verify that ALL fields were closed in the proper order. assertThat(recorder).containsExactly(// "FailingFieldsTestCase.field1.close()", // "FailingFieldsTestCase.field2.close()", // @@ -263,6 +258,108 @@ void allFieldsAreClosedIfAnyFieldThrowsAnException() { .hasSuppressedException(new RuntimeException("FailingFieldsTestCase.staticField2.close()")); } + @Test + void allFieldsAreClosedIfAnyFieldThrowsAnExceptionWithNestedTestClassesWithInstancePerMethod() { + Class enclosingTestClass = FailingFieldsEnclosingTestCase.class; + Class nestedTestClass = FailingFieldsEnclosingTestCase.NestedTestCase.class; + + EngineExecutionResults allEvents = executeTestsForClass(nestedTestClass); + Events tests = allEvents.testEvents(); + tests.assertStatistics(stats -> stats.succeeded(0).failed(1)); + + // Verify that ALL fields were closed in the proper order. + assertThat(recorder).containsExactly(// + // Results from NestedTestCase instance + "NestedTestCase.nestedField1.close()", // + "NestedTestCase.nestedField2.close()", // + // Results from FailingFieldsEnclosingTestCase instance + "FailingFieldsEnclosingTestCase.enclosingField1.close()", // + "FailingFieldsEnclosingTestCase.enclosingField2.close()", // + // Results from NestedTestCase class + "NestedTestCase.nestedStaticField1.close()", // + "NestedTestCase.nestedStaticField2.close()", // + // Results from FailingFieldsEnclosingTestCase class + "FailingFieldsEnclosingTestCase.enclosingStaticField1.close()", // + "FailingFieldsEnclosingTestCase.enclosingStaticField2.close()"// + ); + + // Test-level failures + assertThat(findFailure(tests, "nestedTest()"))// + .isExactlyInstanceOf(RuntimeException.class)// + .hasMessage("NestedTestCase.nestedField1.close()")// + .hasNoCause()// + .hasSuppressedException(new RuntimeException("FailingFieldsEnclosingTestCase.enclosingField1.close()")); + + Events containers = allEvents.containerEvents(); + containers.assertStatistics(stats -> stats.succeeded(1).failed(2)); + + // Container-level failures + assertThat(findFailure(containers, nestedTestClass.getSimpleName()))// + .isExactlyInstanceOf(RuntimeException.class)// + .hasMessage("NestedTestCase.nestedStaticField1.close()")// + .hasNoCause()// + .hasNoSuppressedExceptions(); + assertThat(findFailure(containers, enclosingTestClass.getSimpleName()))// + .isExactlyInstanceOf(RuntimeException.class)// + .hasMessage("FailingFieldsEnclosingTestCase.enclosingStaticField1.close()")// + .hasNoCause()// + .hasNoSuppressedExceptions(); + + // Reset tracking + resetTracking(); + + allEvents = executeTestsForClass(enclosingTestClass); + tests = allEvents.testEvents(); + tests.assertStatistics(stats -> stats.succeeded(0).failed(2)); + + // Verify that ALL fields were closed in the proper order. + assertThat(recorder).containsExactly(// + // Results from FailingFieldsEnclosingTestCase instance + "FailingFieldsEnclosingTestCase.enclosingField1.close()", // + "FailingFieldsEnclosingTestCase.enclosingField2.close()", // + + // Results from NestedTestCase instance + "NestedTestCase.nestedField1.close()", // + "NestedTestCase.nestedField2.close()", // + // Results from FailingFieldsEnclosingTestCase instance + "FailingFieldsEnclosingTestCase.enclosingField1.close()", // + "FailingFieldsEnclosingTestCase.enclosingField2.close()", // + // Results from NestedTestCase class + "NestedTestCase.nestedStaticField1.close()", // + "NestedTestCase.nestedStaticField2.close()", // + // Results from FailingFieldsEnclosingTestCase class + "FailingFieldsEnclosingTestCase.enclosingStaticField1.close()", // + "FailingFieldsEnclosingTestCase.enclosingStaticField2.close()"// + ); + + // Test-level failures + assertThat(findFailure(tests, "enclosingTest()"))// + .isExactlyInstanceOf(RuntimeException.class)// + .hasMessage("FailingFieldsEnclosingTestCase.enclosingField1.close()")// + .hasNoCause()// + .hasNoSuppressedExceptions(); + assertThat(findFailure(tests, "nestedTest()"))// + .isExactlyInstanceOf(RuntimeException.class)// + .hasMessage("NestedTestCase.nestedField1.close()")// + .hasNoCause()// + .hasSuppressedException(new RuntimeException("FailingFieldsEnclosingTestCase.enclosingField1.close()")); + + containers = allEvents.containerEvents(); + containers.assertStatistics(stats -> stats.succeeded(1).failed(2)); + + // Container-level failures + assertThat(findFailure(containers, nestedTestClass.getSimpleName()))// + .isExactlyInstanceOf(RuntimeException.class)// + .hasMessage("NestedTestCase.nestedStaticField1.close()")// + .hasNoCause()// + .hasNoSuppressedExceptions(); + assertThat(findFailure(containers, enclosingTestClass.getSimpleName()))// + .isExactlyInstanceOf(RuntimeException.class)// + .hasMessage("FailingFieldsEnclosingTestCase.enclosingStaticField1.close()")// + .hasNoCause()// + .hasNoSuppressedExceptions(); + } + private Throwable findFailure(Events tests, String displayName) { return findExecution(tests, displayName)// .getTerminationInfo().getExecutionResult().getThrowable().orElseThrow(); @@ -518,6 +615,57 @@ void test() { } } + static class FailingFieldsEnclosingTestCase { + + @AutoClose + static AutoCloseable enclosingStaticField1; + + @AutoClose + static AutoCloseable enclosingStaticField2; + + @AutoClose + final AutoCloseable enclosingField1 = new AutoCloseSpy("enclosingField1", true); + + @AutoClose + final AutoCloseable enclosingField2 = new AutoCloseSpy("enclosingField2", false); + + @BeforeAll + static void setup() { + enclosingStaticField1 = new AutoCloseSpy("enclosingStaticField1", true); + enclosingStaticField2 = new AutoCloseSpy("enclosingStaticField2", false); + } + + @Test + void enclosingTest() { + } + + @Nested + class NestedTestCase { + + @AutoClose + static AutoCloseable nestedStaticField1; + + @AutoClose + static AutoCloseable nestedStaticField2; + + @AutoClose + final AutoCloseable nestedField1 = new AutoCloseSpy("nestedField1", true); + + @AutoClose + final AutoCloseable nestedField2 = new AutoCloseSpy("nestedField2", false); + + @BeforeAll + static void setup() { + nestedStaticField1 = new AutoCloseSpy("nestedStaticField1", true); + nestedStaticField2 = new AutoCloseSpy("nestedStaticField2", false); + } + + @Test + void nestedTest() { + } + } + } + static class AutoCloseSpy implements AutoCloseable, Runnable { private final String prefix;