You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@curator.apache.org by ra...@apache.org on 2019/09/21 17:59:00 UTC
[curator] 01/01: CURATOR-541
This is an automated email from the ASF dual-hosted git repository.
randgalt pushed a commit to branch CURATOR-541
in repository https://gitbox.apache.org/repos/asf/curator.git
commit 3bef5e95ccd983e5dde221151ba835df5b0a861e
Author: randgalt <ra...@apache.org>
AuthorDate: Sat Sep 21 12:57:09 2019 -0500
CURATOR-541
The retry code in BaseClassForTests was hopelessly broken - I don't know for how long. Reworked it so that it does the right thing now (hopefully). Storing the retry count as an attribute wasn't working. The new method stores it as a field in the test class and makes sure that it's always correct. It should only ever be true when actually retrying and, given that TestNG always calls the retry method, it can be reset after a retry fails.
---
.../org/apache/curator/test/BaseClassForTests.java | 144 ++++++---------------
1 file changed, 40 insertions(+), 104 deletions(-)
diff --git a/curator-test/src/main/java/org/apache/curator/test/BaseClassForTests.java b/curator-test/src/main/java/org/apache/curator/test/BaseClassForTests.java
index f932ae4..51af821 100644
--- a/curator-test/src/main/java/org/apache/curator/test/BaseClassForTests.java
+++ b/curator-test/src/main/java/org/apache/curator/test/BaseClassForTests.java
@@ -23,7 +23,6 @@ import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.testng.IInvokedMethod;
import org.testng.IInvokedMethodListener2;
-import org.testng.IRetryAnalyzer;
import org.testng.ITestContext;
import org.testng.ITestResult;
import org.testng.annotations.AfterMethod;
@@ -31,14 +30,14 @@ import org.testng.annotations.BeforeMethod;
import org.testng.annotations.BeforeSuite;
import java.io.IOException;
import java.net.BindException;
-import java.util.Objects;
import java.util.concurrent.atomic.AtomicBoolean;
-import java.util.concurrent.atomic.AtomicInteger;
public class BaseClassForTests
{
protected volatile TestingServer server;
+
private final Logger log = LoggerFactory.getLogger(getClass());
+ private final AtomicBoolean isRetrying = new AtomicBoolean(false);
private static final String INTERNAL_PROPERTY_DONT_LOG_CONNECTION_ISSUES;
private static final String INTERNAL_PROPERTY_REMOVE_WATCHERS_IN_FOREGROUND;
@@ -85,7 +84,33 @@ public class BaseClassForTests
@BeforeSuite(alwaysRun = true)
public void beforeSuite(ITestContext context)
{
- context.getSuite().addListener(new MethodListener(log));
+ IInvokedMethodListener2 methodListener2 = new IInvokedMethodListener2()
+ {
+ @Override
+ public void beforeInvocation(IInvokedMethod method, ITestResult testResult)
+ {
+ method.getTestMethod().setRetryAnalyzer(BaseClassForTests.this::retry);
+ }
+
+ @Override
+ public void beforeInvocation(IInvokedMethod method, ITestResult testResult, ITestContext context)
+ {
+ beforeInvocation(method, testResult);
+ }
+
+ @Override
+ public void afterInvocation(IInvokedMethod method, ITestResult testResult, ITestContext context)
+ {
+ // NOP
+ }
+
+ @Override
+ public void afterInvocation(IInvokedMethod method, ITestResult testResult)
+ {
+ // NOP
+ }
+ };
+ context.getSuite().addListener(methodListener2);
}
@BeforeMethod
@@ -139,113 +164,24 @@ public class BaseClassForTests
}
}
- private static class RetryContext
- {
- final AtomicBoolean isRetrying = new AtomicBoolean(false);
- final AtomicInteger runVersion = new AtomicInteger(0);
- }
-
- private static class RetryAnalyzer implements IRetryAnalyzer
+ private boolean retry(ITestResult result)
{
- private final Logger log;
- private final RetryContext retryContext;
-
- RetryAnalyzer(Logger log, RetryContext retryContext)
+ if ( result.isSuccess() || isRetrying.get() )
{
- this.log = log;
- this.retryContext = Objects.requireNonNull(retryContext, "retryContext cannot be null");
+ isRetrying.set(false);
+ return false;
}
- @Override
- public boolean retry(ITestResult result)
+ result.setStatus(ITestResult.SKIP);
+ if ( result.getThrowable() != null )
{
- if ( result.isSuccess() || retryContext.isRetrying.get() )
- {
- retryContext.isRetrying.set(false);
- return false;
- }
-
- result.setStatus(ITestResult.SKIP);
- if ( result.getThrowable() != null )
- {
- log.error("Retrying 1 time", result.getThrowable());
- }
- else
- {
- log.error("Retrying 1 time");
- }
- retryContext.isRetrying.set(true);
- return true;
+ log.error("Retrying 1 time", result.getThrowable());
}
- }
-
- private static class MethodListener implements IInvokedMethodListener2
- {
- private final Logger log;
-
- private static final String ATTRIBUTE_NAME = "__curator";
-
- MethodListener(Logger log)
- {
- this.log = log;
- }
-
- @Override
- public void beforeInvocation(IInvokedMethod method, ITestResult testResult)
- {
- // NOP
- }
-
- @Override
- public void afterInvocation(IInvokedMethod method, ITestResult testResult)
+ else
{
- // NOP
- }
-
- @Override
- public void beforeInvocation(IInvokedMethod method, ITestResult testResult, ITestContext context)
- {
- if ( method.getTestMethod().isBeforeMethodConfiguration() )
- {
- RetryContext retryContext = (RetryContext)context.getAttribute(ATTRIBUTE_NAME);
- if ( retryContext == null )
- {
- retryContext = new RetryContext();
- context.setAttribute(ATTRIBUTE_NAME, retryContext);
- }
- }
- else if ( method.isTestMethod() )
- {
- RetryContext retryContext = (RetryContext)context.getAttribute(ATTRIBUTE_NAME);
- if ( retryContext != null )
- {
- method.getTestMethod().setRetryAnalyzer(new RetryAnalyzer(log, retryContext));
- }
- }
- }
-
- @Override
- public void afterInvocation(IInvokedMethod method, ITestResult testResult, ITestContext context)
- {
- if ( method.isTestMethod() )
- {
- RetryContext retryContext = (RetryContext)context.getAttribute(ATTRIBUTE_NAME);
- if ( retryContext == null )
- {
- log.error("No retryContext!");
- }
- else
- {
- if ( testResult.isSuccess() || (testResult.getStatus() == ITestResult.FAILURE) )
- {
- retryContext.isRetrying.set(false);
- if ( retryContext.runVersion.incrementAndGet() > 1 )
- {
- context.setAttribute(ATTRIBUTE_NAME, null);
- }
- }
- }
- }
+ log.error("Retrying 1 time");
}
+ isRetrying.set(true);
+ return true;
}
}