From c34654266352f5a5a55924f7201c8bd3d2a841d6 Mon Sep 17 00:00:00 2001 From: westse <10818305+westse@users.noreply.github.com> Date: Sat, 29 Mar 2025 10:28:36 -0600 Subject: [PATCH 1/4] Avoid masking early engine failure Fixes #202 --- .../junit/support/testng/engine/ExecutionListener.java | 9 ++++++--- .../engine/ConfigurationMethodIntegrationTests.java | 2 +- .../FailingBeforeSuiteConfigurationMethodTestCase.java | 6 ++++++ .../FailingBeforeTestConfigurationMethodTestCase.java | 6 ++++++ 4 files changed, 19 insertions(+), 4 deletions(-) diff --git a/src/main/java/org/junit/support/testng/engine/ExecutionListener.java b/src/main/java/org/junit/support/testng/engine/ExecutionListener.java index bbf1a05..0b1a682 100644 --- a/src/main/java/org/junit/support/testng/engine/ExecutionListener.java +++ b/src/main/java/org/junit/support/testng/engine/ExecutionListener.java @@ -237,9 +237,12 @@ private static Stream throwables(Set results) { private static Throwable chain(Stream failures) { Iterator iterator = failures.iterator(); - Throwable throwable = iterator.next(); - iterator.forEachRemaining(throwable::addSuppressed); - return throwable; + Throwable throwable = null; + if (iterator.hasNext()) { + throwable = iterator.next(); + iterator.forEachRemaining(throwable::addSuppressed); + } + return throwable; } static class MethodProgress { diff --git a/src/test/java/org/junit/support/testng/engine/ConfigurationMethodIntegrationTests.java b/src/test/java/org/junit/support/testng/engine/ConfigurationMethodIntegrationTests.java index 92f4218..02ac2b8 100644 --- a/src/test/java/org/junit/support/testng/engine/ConfigurationMethodIntegrationTests.java +++ b/src/test/java/org/junit/support/testng/engine/ConfigurationMethodIntegrationTests.java @@ -74,7 +74,7 @@ void reportsFailureFromEarlyEngineLevelConfigurationMethodAsAborted(Class tes event(testClass(testClass), started()), // event(test("method:test()"), started()), // event(test("method:test()"), abortedWithReason(message("boom"))), // - event(testClass(testClass), finishedSuccessfully()), // + event(testClass(testClass), abortedWithReason()), // event(engine(), finishedWithFailure(message("boom")))); } diff --git a/src/testFixtures/java/example/configuration/methods/FailingBeforeSuiteConfigurationMethodTestCase.java b/src/testFixtures/java/example/configuration/methods/FailingBeforeSuiteConfigurationMethodTestCase.java index b7c959a..470bb19 100644 --- a/src/testFixtures/java/example/configuration/methods/FailingBeforeSuiteConfigurationMethodTestCase.java +++ b/src/testFixtures/java/example/configuration/methods/FailingBeforeSuiteConfigurationMethodTestCase.java @@ -10,6 +10,7 @@ package example.configuration.methods; +import org.testng.annotations.BeforeMethod; import org.testng.annotations.BeforeSuite; import org.testng.annotations.Test; @@ -20,6 +21,11 @@ public void beforeSuite() { throw new AssertionError("boom"); } + @BeforeMethod + public void beforeMethod() { + // never called + } + @Test public void test() { // never called diff --git a/src/testFixtures/java/example/configuration/methods/FailingBeforeTestConfigurationMethodTestCase.java b/src/testFixtures/java/example/configuration/methods/FailingBeforeTestConfigurationMethodTestCase.java index 17468cb..63a9e95 100644 --- a/src/testFixtures/java/example/configuration/methods/FailingBeforeTestConfigurationMethodTestCase.java +++ b/src/testFixtures/java/example/configuration/methods/FailingBeforeTestConfigurationMethodTestCase.java @@ -10,6 +10,7 @@ package example.configuration.methods; +import org.testng.annotations.BeforeMethod; import org.testng.annotations.BeforeTest; import org.testng.annotations.Test; @@ -20,6 +21,11 @@ public void beforeTest() { throw new AssertionError("boom"); } + @BeforeMethod + public void beforeMethod() { + // never called + } + @Test public void test() { // never called From dc179a3eddd48a71fdbf08db0d9cb8c651c741df Mon Sep 17 00:00:00 2001 From: Marc Philipp Date: Sun, 30 Mar 2025 14:30:12 +0200 Subject: [PATCH 2/4] Check for empty throwable --- .../testng/engine/AbstractIntegrationTests.java | 15 +++++++++++++++ .../ConfigurationMethodIntegrationTests.java | 3 ++- 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/src/test/java/org/junit/support/testng/engine/AbstractIntegrationTests.java b/src/test/java/org/junit/support/testng/engine/AbstractIntegrationTests.java index a3148d9..318325e 100644 --- a/src/test/java/org/junit/support/testng/engine/AbstractIntegrationTests.java +++ b/src/test/java/org/junit/support/testng/engine/AbstractIntegrationTests.java @@ -11,18 +11,24 @@ package org.junit.support.testng.engine; import static java.util.function.Predicate.isEqual; +import static org.assertj.core.api.Assertions.allOf; import static org.junit.platform.commons.util.FunctionUtils.where; +import static org.junit.platform.engine.TestExecutionResult.Status.ABORTED; import static org.junit.platform.testkit.engine.Event.byTestDescriptor; import static org.junit.platform.testkit.engine.EventConditions.container; import static org.junit.platform.testkit.engine.EventConditions.displayName; import static org.junit.platform.testkit.engine.EventConditions.event; +import static org.junit.platform.testkit.engine.EventConditions.finished; import static org.junit.platform.testkit.engine.EventConditions.uniqueIdSubstring; +import static org.junit.platform.testkit.engine.TestExecutionResultConditions.status; import java.nio.file.Path; +import java.util.Optional; import org.assertj.core.api.Condition; import org.junit.jupiter.api.io.TempDir; import org.junit.platform.engine.TestDescriptor; +import org.junit.platform.engine.TestExecutionResult; import org.junit.platform.testkit.engine.EngineTestKit; import org.junit.platform.testkit.engine.Event; @@ -48,4 +54,13 @@ static Condition legacyReportingName(String legacyReportingName) { byTestDescriptor(where(TestDescriptor::getLegacyReportingName, isEqual(legacyReportingName))), "descriptor with legacy reporting name '%s'", legacyReportingName); } + + static Condition abortedWithoutReason() { + return finished(allOf(status(ABORTED), emptyThrowable())); + } + + static Condition emptyThrowable() { + return new Condition<>(where(TestExecutionResult::getThrowable, Optional::isEmpty), "throwable is empty"); + } + } diff --git a/src/test/java/org/junit/support/testng/engine/ConfigurationMethodIntegrationTests.java b/src/test/java/org/junit/support/testng/engine/ConfigurationMethodIntegrationTests.java index 02ac2b8..3fb1423 100644 --- a/src/test/java/org/junit/support/testng/engine/ConfigurationMethodIntegrationTests.java +++ b/src/test/java/org/junit/support/testng/engine/ConfigurationMethodIntegrationTests.java @@ -10,6 +10,7 @@ package org.junit.support.testng.engine; +import static org.assertj.core.api.Assertions.allOf; import static org.assertj.core.api.Assertions.assertThat; import static org.junit.platform.engine.discovery.DiscoverySelectors.selectClass; import static org.junit.platform.testkit.engine.EventConditions.abortedWithReason; @@ -74,7 +75,7 @@ void reportsFailureFromEarlyEngineLevelConfigurationMethodAsAborted(Class tes event(testClass(testClass), started()), // event(test("method:test()"), started()), // event(test("method:test()"), abortedWithReason(message("boom"))), // - event(testClass(testClass), abortedWithReason()), // + event(testClass(testClass), abortedWithoutReason()), // event(engine(), finishedWithFailure(message("boom")))); } From 18a40c718118b18d7ce13183d343044cba6437a5 Mon Sep 17 00:00:00 2001 From: Marc Philipp Date: Sun, 30 Mar 2025 14:30:21 +0200 Subject: [PATCH 3/4] Fix formatting --- .../support/testng/engine/ExecutionListener.java | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/main/java/org/junit/support/testng/engine/ExecutionListener.java b/src/main/java/org/junit/support/testng/engine/ExecutionListener.java index 0b1a682..1c77bd0 100644 --- a/src/main/java/org/junit/support/testng/engine/ExecutionListener.java +++ b/src/main/java/org/junit/support/testng/engine/ExecutionListener.java @@ -237,12 +237,12 @@ private static Stream throwables(Set results) { private static Throwable chain(Stream failures) { Iterator iterator = failures.iterator(); - Throwable throwable = null; - if (iterator.hasNext()) { - throwable = iterator.next(); - iterator.forEachRemaining(throwable::addSuppressed); - } - return throwable; + Throwable throwable = null; + if (iterator.hasNext()) { + throwable = iterator.next(); + iterator.forEachRemaining(throwable::addSuppressed); + } + return throwable; } static class MethodProgress { From fb929433f6fff64baf89387e15fb26b3830b5e1b Mon Sep 17 00:00:00 2001 From: Marc Philipp Date: Sun, 30 Mar 2025 14:32:11 +0200 Subject: [PATCH 4/4] Remove unused import --- .../testng/engine/ConfigurationMethodIntegrationTests.java | 1 - 1 file changed, 1 deletion(-) diff --git a/src/test/java/org/junit/support/testng/engine/ConfigurationMethodIntegrationTests.java b/src/test/java/org/junit/support/testng/engine/ConfigurationMethodIntegrationTests.java index 3fb1423..d258235 100644 --- a/src/test/java/org/junit/support/testng/engine/ConfigurationMethodIntegrationTests.java +++ b/src/test/java/org/junit/support/testng/engine/ConfigurationMethodIntegrationTests.java @@ -10,7 +10,6 @@ package org.junit.support.testng.engine; -import static org.assertj.core.api.Assertions.allOf; import static org.assertj.core.api.Assertions.assertThat; import static org.junit.platform.engine.discovery.DiscoverySelectors.selectClass; import static org.junit.platform.testkit.engine.EventConditions.abortedWithReason;