From 02a73a7cafbdffc81661f8a0cda74a4da306b4cc Mon Sep 17 00:00:00 2001 From: Scott Babcock Date: Thu, 17 Mar 2022 05:46:48 +0000 Subject: [PATCH] Add exception to retry notification; fix assertions (#117) --- .../automation/junit/JUnitConfig.java | 17 +++++-- .../automation/junit/MutableTest.java | 21 +++----- .../automation/junit/RetryHandler.java | 32 +++++++++---- .../junit/BeforeClassFailureTest.java | 6 +-- .../junit/ReferenceReleaseTest.java | 48 +++++++++---------- .../automation/junit/RuleFailureTest.java | 6 +-- 6 files changed, 72 insertions(+), 58 deletions(-) diff --git a/src/main/java/com/nordstrom/automation/junit/JUnitConfig.java b/src/main/java/com/nordstrom/automation/junit/JUnitConfig.java index 347c4c0..caf858d 100644 --- a/src/main/java/com/nordstrom/automation/junit/JUnitConfig.java +++ b/src/main/java/com/nordstrom/automation/junit/JUnitConfig.java @@ -25,7 +25,7 @@ public class JUnitConfig extends SettingsCore { */ public enum JUnitSettings implements SettingsCore.SettingsAPI { /** - * Global per-test timeout interval in milliseconds. + * This setting specifies the global per-test timeout interval in milliseconds. *

* name: junit.timeout.test
* default: {@code null} @@ -33,7 +33,7 @@ public enum JUnitSettings implements SettingsCore.SettingsAPI { TEST_TIMEOUT("junit.timeout.test", null), /** - * Global per-class timeout interval in milliseconds. + * This setting specifies the global per-class timeout interval in milliseconds. *

* name: junit.timeout.rule
* default: {@code null} @@ -41,13 +41,22 @@ public enum JUnitSettings implements SettingsCore.SettingsAPI { TIMEOUT_RULE("junit.timeout.rule", null), /** - * Maximum retry attempts for failed test methods. + * This setting specifies the maximum retry attempts for failed test methods. *

* name: junit.max.retry
* default: 0 */ - MAX_RETRY("junit.max.retry", "0"); + MAX_RETRY("junit.max.retry", "0"), + /** + * This setting specifies whether the exception that caused a test to fail will be logged in the notification + * that the test is being retried. + *

+ * name: junit.retry.more.info
+ * default: {@code false} + */ + RETRY_MORE_INFO("junit.retry.more.info", "false"); + private final String propertyName; private final String defaultValue; diff --git a/src/main/java/com/nordstrom/automation/junit/MutableTest.java b/src/main/java/com/nordstrom/automation/junit/MutableTest.java index 544d457..ba07ee3 100644 --- a/src/main/java/com/nordstrom/automation/junit/MutableTest.java +++ b/src/main/java/com/nordstrom/automation/junit/MutableTest.java @@ -79,21 +79,12 @@ public static MutableTest proxyFor(Method testMethod, long timeout) { } if (declared != null) { try { - Field field = LifecycleHooks.getDeclaredField(testMethod, DECLARED_ANNOTATIONS); - field.setAccessible(true); - try { - @SuppressWarnings("unchecked") - Map, Annotation> map = - (Map, Annotation>) field.get(testMethod); - MutableTest mutable = new MutableTest(declared, timeout); - map.put(Test.class, mutable); - return mutable; - } catch (IllegalArgumentException | IllegalAccessException e) { - throw new UnsupportedOperationException("Failed acquiring annotations map for method: " + testMethod, e); - } - } catch (NoSuchFieldException | SecurityException e) { - throw new UnsupportedOperationException("Failed acquiring [" + DECLARED_ANNOTATIONS - + "] field of Executable class", e); + Map, Annotation> map = LifecycleHooks.getFieldValue(testMethod, DECLARED_ANNOTATIONS); + MutableTest mutable = new MutableTest(declared, timeout); + map.put(Test.class, mutable); + return mutable; + } catch (IllegalAccessException | NoSuchFieldException | SecurityException e) { + throw new UnsupportedOperationException("Failed acquiring annotations map for method: " + testMethod, e); } } throw new IllegalArgumentException("Specified method is not a JUnit @Test: " + testMethod); diff --git a/src/main/java/com/nordstrom/automation/junit/RetryHandler.java b/src/main/java/com/nordstrom/automation/junit/RetryHandler.java index 7dc19ef..37f7552 100644 --- a/src/main/java/com/nordstrom/automation/junit/RetryHandler.java +++ b/src/main/java/com/nordstrom/automation/junit/RetryHandler.java @@ -1,5 +1,6 @@ package com.nordstrom.automation.junit; +import static com.nordstrom.automation.junit.LifecycleHooks.getConfig; import static com.nordstrom.automation.junit.LifecycleHooks.invoke; import static com.nordstrom.automation.junit.LifecycleHooks.toMapKey; @@ -119,8 +120,8 @@ public static Throwable runChildWithRetry(final Object runner, final FrameworkMe static boolean doRetry(FrameworkMethod method, Throwable thrown, AtomicInteger retryCounter) { boolean doRetry = false; if ((retryCounter.decrementAndGet() > -1) && isRetriable(method, thrown)) { - LOGGER.warn("### RETRY ### {}", method); doRetry = true; + LOGGER.warn("### RETRY ### {}", method, getThrowableToLog(thrown)); } return doRetry; } @@ -146,12 +147,23 @@ static int getMaxRetry(Object runner, final FrameworkMethod method) { // if method isn't ignored or excluded from retry attempts if (Boolean.FALSE.equals(invoke(runner, "isIgnored", method)) && (noRetryOnMethod == null) && (noRetryOnClass == null)) { // get configured maximum retry count - maxRetry = JUnitConfig.getConfig().getInteger(JUnitSettings.MAX_RETRY.key(), Integer.valueOf(0)); + maxRetry = getConfig().getInteger(JUnitSettings.MAX_RETRY.key(), Integer.valueOf(0)); } return maxRetry; } + /** + * Determine if the specified method is being retried. + * + * @param method JUnit framework method + * @return {@code true} if method is being retried; otherwise {@code false} + */ + static boolean doRetryFor(final FrameworkMethod method) { + Boolean doRetry = METHOD_TO_RETRY.get(toMapKey(method)); + return (doRetry != null) ? doRetry.booleanValue() : false; + } + /** * Determine if the specified failed test should be retried. * @@ -159,7 +171,7 @@ static int getMaxRetry(Object runner, final FrameworkMethod method) { * @param thrown exception for this failed test * @return {@code true} if test should be retried; otherwise {@code false} */ - static boolean isRetriable(final FrameworkMethod method, final Throwable thrown) { + private static boolean isRetriable(final FrameworkMethod method, final Throwable thrown) { synchronized(retryAnalyzerLoader) { for (JUnitRetryAnalyzer analyzer : retryAnalyzerLoader) { if (analyzer.retry(method, thrown)) { @@ -171,14 +183,16 @@ static boolean isRetriable(final FrameworkMethod method, final Throwable thrown) } /** - * Determine if the specified method is being retried. + * Get the {@link Throwable} to log with the retry notification. * - * @param method JUnit framework method - * @return {@code true} if method is being retried; otherwise {@code false} + * @param thrown exception that caused the test to fail + * @return if exception logging is indicated, the specified exception; otherwise {@code null} */ - static boolean doRetryFor(final FrameworkMethod method) { - Boolean doRetry = METHOD_TO_RETRY.get(toMapKey(method)); - return (doRetry != null) ? doRetry.booleanValue() : false; + private static Throwable getThrowableToLog(Throwable thrown) { + if (LOGGER.isDebugEnabled() || getConfig().getBoolean(JUnitSettings.RETRY_MORE_INFO.key())) { + return thrown; + } + return null; } } diff --git a/src/test/java/com/nordstrom/automation/junit/BeforeClassFailureTest.java b/src/test/java/com/nordstrom/automation/junit/BeforeClassFailureTest.java index 760a042..2b07d26 100644 --- a/src/test/java/com/nordstrom/automation/junit/BeforeClassFailureTest.java +++ b/src/test/java/com/nordstrom/automation/junit/BeforeClassFailureTest.java @@ -25,10 +25,10 @@ public void verifyBeforeClassExceptionHandling() { runner.addListener(checker); Result result = runner.run(BeforeClassThrowsException.class); assertFalse(result.wasSuccessful()); - assertEquals(1, result.getFailureCount(), "Incorrect failed test count"); + assertEquals(result.getFailureCount(), 1, "Incorrect failed test count"); Failure failure = result.getFailures().get(0); - assertEquals(RuntimeException.class, failure.getException().getClass(), "Incorrect exception class"); - assertEquals("Must be failed", failure.getMessage(), "Incorrect exception message"); + assertEquals(failure.getException().getClass(), RuntimeException.class, "Incorrect exception class"); + assertEquals(failure.getMessage(), "Must be failed", "Incorrect exception message"); assertTrue(failure.getDescription().isSuite(), "Failure should describe a suite"); Optional optWatcher = LifecycleHooks.getAttachedWatcher(UnitTestWatcher.class); diff --git a/src/test/java/com/nordstrom/automation/junit/ReferenceReleaseTest.java b/src/test/java/com/nordstrom/automation/junit/ReferenceReleaseTest.java index a051544..d490f0a 100644 --- a/src/test/java/com/nordstrom/automation/junit/ReferenceReleaseTest.java +++ b/src/test/java/com/nordstrom/automation/junit/ReferenceReleaseTest.java @@ -18,8 +18,8 @@ public void verifyHappyPath() { runner.addListener(checker); Result result = runner.run(AutomaticRetryPassing.class); assertTrue(result.wasSuccessful()); - assertEquals(result.getRunCount(), 1); - assertEquals(result.getFailureCount(), 0); + assertEquals(result.getRunCount(), 1, "Incorrect test run count"); + assertEquals(result.getFailureCount(), 0, "Incorrect failed test count"); checkLeakReports(checker); } @@ -31,8 +31,8 @@ public void verifyFailure() { runner.addListener(checker); Result result = runner.run(AutomaticRetryFailing.class); assertFalse(result.wasSuccessful()); - assertEquals(result.getRunCount(), 1); - assertEquals(result.getFailureCount(), 1); + assertEquals(result.getRunCount(), 1, "Incorrect test run count"); + assertEquals(result.getFailureCount(), 1, "Incorrect failed test count"); checkLeakReports(checker); } @@ -44,8 +44,8 @@ public void verifyParameterizedReferenceRelease() { runner.addListener(checker); Result result = runner.run(ReferenceReleaseParameterized.class); assertFalse(result.wasSuccessful()); - assertEquals(result.getRunCount(), 2); - assertEquals(result.getFailureCount(), 1); + assertEquals(result.getRunCount(), 2, "Incorrect test run count"); + assertEquals(result.getFailureCount(), 1, "Incorrect failed test count"); checkLeakReports(checker); } @@ -57,8 +57,8 @@ public void verifyJUnitParamsReferenceRelease() { runner.addListener(checker); Result result = runner.run(ReferenceReleaseJUnitParams.class); assertFalse(result.wasSuccessful()); - assertEquals(result.getRunCount(), 2); - assertEquals(result.getFailureCount(), 1); + assertEquals(result.getRunCount(), 2, "Incorrect test run count"); + assertEquals(result.getFailureCount(), 1, "Incorrect failed test count"); checkLeakReports(checker); } @@ -70,26 +70,26 @@ public void verifyTheoriesReferenceRelease() { runner.addListener(checker); Result result = runner.run(ReferenceReleaseTheories.class); assertFalse(result.wasSuccessful()); - assertEquals(result.getRunCount(), 3); - assertEquals(result.getFailureCount(), 2); + assertEquals(result.getRunCount(), 3, "Incorrect test run count"); + assertEquals(result.getFailureCount(), 2, "Incorrect failed test count"); checkLeakReports(checker); } static void checkLeakReports(ReferenceChecker checker) { - assertEquals(checker.reportNoAtomicTests(), 0); - assertEquals(checker.reportStatementLeaks(), 0); - assertEquals(checker.reportWatcherLeaks(), 0); - assertEquals(checker.reportAtomicTestLeaks(), 0); - assertEquals(checker.reportTargetLeaks(), 0); - assertEquals(checker.reportCallableLeaks(), 0); - assertEquals(checker.reportDescriptionLeaks(), 0); - assertEquals(checker.reportMethodLeaks(), 0); - assertEquals(checker.reportRunnerLeaks(), 0); - assertEquals(checker.reportStartFlagLeaks(), 0); - assertEquals(checker.reportNotifierLeaks(), 0); - assertEquals(checker.reportNotifyFlagLeaks(), 0); - assertEquals(checker.reportParentLeaks(), 0); - assertEquals(checker.reportRunnerStackLeak(), 0); + assertEquals(checker.reportNoAtomicTests(), 0, "Missing atomic tests detected"); + assertEquals(checker.reportStatementLeaks(), 0, "Leaked statements detected"); + assertEquals(checker.reportWatcherLeaks(), 0, "Leaked watchers detected"); + assertEquals(checker.reportAtomicTestLeaks(), 0, "Leaked atomic tests detected"); + assertEquals(checker.reportTargetLeaks(), 0, "Leaked target references detected"); + assertEquals(checker.reportCallableLeaks(), 0, "Leaked Callable references detected"); + assertEquals(checker.reportDescriptionLeaks(), 0, "Leaked Description references detected"); + assertEquals(checker.reportMethodLeaks(), 0, "Leaked Method references detected"); + assertEquals(checker.reportRunnerLeaks(), 0, "Leaked Runner references detected"); + assertEquals(checker.reportStartFlagLeaks(), 0, "Leaked 'start' flags detected"); + assertEquals(checker.reportNotifierLeaks(), 0, "Leaked Notifier referenced detected"); + assertEquals(checker.reportNotifyFlagLeaks(), 0, "Leaked 'notify' flags detected"); + assertEquals(checker.reportParentLeaks(), 0, "Leaked parent runner references detected"); + assertEquals(checker.reportRunnerStackLeak(), 0, "Leaked runner stack entries detected"); } } diff --git a/src/test/java/com/nordstrom/automation/junit/RuleFailureTest.java b/src/test/java/com/nordstrom/automation/junit/RuleFailureTest.java index 3eaa1bc..0f70a9b 100644 --- a/src/test/java/com/nordstrom/automation/junit/RuleFailureTest.java +++ b/src/test/java/com/nordstrom/automation/junit/RuleFailureTest.java @@ -18,10 +18,10 @@ public void verifyRuleExceptionHandling() { runner.addListener(checker); Result result = runner.run(RuleThrowsException.class); assertFalse(result.wasSuccessful()); - assertEquals(1, result.getFailureCount()); + assertEquals(result.getFailureCount(), 1, "Incorrect failed test count"); Failure failure = result.getFailures().get(0); - assertEquals(RuntimeException.class, failure.getException().getClass()); - assertEquals("Must be failed", failure.getMessage()); + assertEquals(failure.getException().getClass(), RuntimeException.class, "Incorrect failure exception"); + assertEquals(failure.getMessage(), "Must be failed", "Incorrect failure message"); ReferenceReleaseTest.checkLeakReports(checker); }