You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by aa...@apache.org on 2021/04/16 21:21:20 UTC

[pulsar] branch master updated: [Testing] Speed up execution of flaky test group and improve test coverage of test listeners (#10249)

This is an automated email from the ASF dual-hosted git repository.

aahmed pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/pulsar.git


The following commit(s) were added to refs/heads/master by this push:
     new 52b7059  [Testing] Speed up execution of flaky test group and improve test coverage of test listeners (#10249)
52b7059 is described below

commit 52b7059bdac7120f319a3c6f8085f25a2b9de7c8
Author: Lari Hotari <lh...@users.noreply.github.com>
AuthorDate: Sat Apr 17 00:20:21 2021 +0300

    [Testing] Speed up execution of flaky test group and improve test coverage of test listeners (#10249)
    
    * Add tests for test retries
    
    * Add test for ExcludedGroups
    
    * Test retries with fail-fast mode
    
    * Decouple PulsarTestListener and FailFastNotifier
    
    * Speed up execution of flaky test group
    
    * Revert "Add test for ExcludedGroups"
    
    This reverts commit af24e5beaac40748088d6630f8c7d3632b634ce3.
    
    * Fix downloading of golang
---
 build/run_unit_group.sh                            |  4 +--
 buildtools/pom.xml                                 | 16 ++++++++++
 .../apache/pulsar/tests/AnnotationListener.java    |  3 +-
 .../org/apache/pulsar/tests/FailFastNotifier.java  |  8 ++++-
 .../apache/pulsar/tests/PulsarTestListener.java    |  1 -
 .../org/apache/pulsar/tests/RetryAnalyzer.java     |  2 +-
 .../pulsar/tests/AnnotationListenerTest.java}      | 26 +++++++---------
 .../pulsar/tests/DefaultTestRetriesTest.java}      | 35 ++++++++++++++--------
 .../docker-images/latest-version-image/Dockerfile  |  2 +-
 9 files changed, 63 insertions(+), 34 deletions(-)

diff --git a/build/run_unit_group.sh b/build/run_unit_group.sh
index 83b3f23..188c750 100755
--- a/build/run_unit_group.sh
+++ b/build/run_unit_group.sh
@@ -90,11 +90,11 @@ function broker_flaky() {
     print_testng_failures pulsar-broker/target/surefire-reports/testng-failed.xml "Quarantined test failure in" "Quarantined test failures"
   # run quarantined Replicator tests separately
   $MVN_COMMAND test -pl pulsar-broker -Dgroups='quarantine' -DexcludedGroups='' -DfailIfNoTests=false \
-    -DtestForkCount=1 -DtestReuseFork=false -Dinclude='**/Replicator*Test.java' || \
+    -DtestForkCount=2 -Dinclude='**/Replicator*Test.java' || \
     print_testng_failures pulsar-broker/target/surefire-reports/testng-failed.xml "Quarantined test failure in" "Quarantined Replicator*Test failures"
   echo "::endgroup::"
   echo "::group::Running flaky tests"
-  $MVN_TEST_COMMAND -pl pulsar-broker -Dgroups='flaky' -DtestForkCount=1 -DtestReuseFork=false
+  $MVN_TEST_COMMAND -pl pulsar-broker -Dgroups='flaky' -DtestForkCount=2
   echo "::endgroup::"
 }
 
diff --git a/buildtools/pom.xml b/buildtools/pom.xml
index 23476fa..961f50e 100644
--- a/buildtools/pom.xml
+++ b/buildtools/pom.xml
@@ -129,6 +129,22 @@
       </plugin>
       <plugin>
         <groupId>org.apache.maven.plugins</groupId>
+        <artifactId>maven-surefire-plugin</artifactId>
+        <configuration>
+          <!-- configuration for testing the test retries -->
+          <systemPropertyVariables>
+            <testRetryCount>1</testRetryCount>
+          </systemPropertyVariables>
+          <properties>
+            <property>
+              <name>listener</name>
+              <value>org.apache.pulsar.tests.PulsarTestListener,org.apache.pulsar.tests.AnnotationListener,org.apache.pulsar.tests.FailFastNotifier</value>
+            </property>
+          </properties>
+        </configuration>
+      </plugin>
+      <plugin>
+        <groupId>org.apache.maven.plugins</groupId>
         <artifactId>maven-shade-plugin</artifactId>
         <version>${maven-shade-plugin.version}</version>
         <configuration>
diff --git a/buildtools/src/main/java/org/apache/pulsar/tests/AnnotationListener.java b/buildtools/src/main/java/org/apache/pulsar/tests/AnnotationListener.java
index b2719b2..2fdd178 100644
--- a/buildtools/src/main/java/org/apache/pulsar/tests/AnnotationListener.java
+++ b/buildtools/src/main/java/org/apache/pulsar/tests/AnnotationListener.java
@@ -39,7 +39,8 @@ public class AnnotationListener implements IAnnotationTransformer {
                           Class testClass,
                           Constructor testConstructor,
                           Method testMethod) {
-        if (annotation.getRetryAnalyzerClass() == DisabledRetryAnalyzer.class) {
+        if (annotation.getRetryAnalyzerClass() == null
+                || annotation.getRetryAnalyzerClass() == DisabledRetryAnalyzer.class) {
             annotation.setRetryAnalyzer(RetryAnalyzer.class);
         }
 
diff --git a/buildtools/src/main/java/org/apache/pulsar/tests/FailFastNotifier.java b/buildtools/src/main/java/org/apache/pulsar/tests/FailFastNotifier.java
index 1224677..bbc5d4c 100644
--- a/buildtools/src/main/java/org/apache/pulsar/tests/FailFastNotifier.java
+++ b/buildtools/src/main/java/org/apache/pulsar/tests/FailFastNotifier.java
@@ -20,6 +20,7 @@ package org.apache.pulsar.tests;
 
 import org.testng.IInvokedMethod;
 import org.testng.IInvokedMethodListener;
+import org.testng.ITestListener;
 import org.testng.ITestResult;
 import org.testng.SkipException;
 
@@ -39,7 +40,7 @@ import org.testng.SkipException;
  *
  */
 public class FailFastNotifier
-        implements IInvokedMethodListener {
+        implements IInvokedMethodListener, ITestListener {
     private static final boolean FAIL_FAST_ENABLED = Boolean.valueOf(
             System.getProperty("testFailFast", "true"));
 
@@ -72,6 +73,11 @@ public class FailFastNotifier
     }
 
     @Override
+    public void onTestFailure(ITestResult result) {
+        FailFastNotifier.FailFastEventsSingleton.getInstance().setSkipOnNextTest();
+    }
+
+    @Override
     public void beforeInvocation(IInvokedMethod iInvokedMethod, ITestResult iTestResult) {
         if (FAIL_FAST_ENABLED && FailFastEventsSingleton.getInstance().isSkipAfterFailure()) {
             throw new FailFastSkipException("Skipped after failure since testFailFast system property is set.");
diff --git a/buildtools/src/main/java/org/apache/pulsar/tests/PulsarTestListener.java b/buildtools/src/main/java/org/apache/pulsar/tests/PulsarTestListener.java
index 18ab56c..dfec5dc 100644
--- a/buildtools/src/main/java/org/apache/pulsar/tests/PulsarTestListener.java
+++ b/buildtools/src/main/java/org/apache/pulsar/tests/PulsarTestListener.java
@@ -41,7 +41,6 @@ public class PulsarTestListener implements ITestListener {
 
     @Override
     public void onTestFailure(ITestResult result) {
-        FailFastNotifier.FailFastEventsSingleton.getInstance().setSkipOnNextTest();
         if (!(result.getThrowable() instanceof SkipException)) {
             System.out.format("!!!!!!!!! FAILURE-- %s.%s(%s)-------\n", result.getTestClass(),
                     result.getMethod().getMethodName(), Arrays.toString(result.getParameters()));
diff --git a/buildtools/src/main/java/org/apache/pulsar/tests/RetryAnalyzer.java b/buildtools/src/main/java/org/apache/pulsar/tests/RetryAnalyzer.java
index c790d09..4b9216c 100644
--- a/buildtools/src/main/java/org/apache/pulsar/tests/RetryAnalyzer.java
+++ b/buildtools/src/main/java/org/apache/pulsar/tests/RetryAnalyzer.java
@@ -24,7 +24,7 @@ import org.testng.util.RetryAnalyzerCount;
 
 public class RetryAnalyzer extends RetryAnalyzerCount {
     // Only try again once
-    private static final int MAX_RETRIES = Integer.parseInt(System.getProperty("testRetryCount", "1"));
+    static final int MAX_RETRIES = Integer.parseInt(System.getProperty("testRetryCount", "1"));
 
     public RetryAnalyzer() {
         setCount(MAX_RETRIES);
diff --git a/buildtools/src/main/java/org/apache/pulsar/tests/RetryAnalyzer.java b/buildtools/src/test/java/org/apache/pulsar/tests/AnnotationListenerTest.java
similarity index 62%
copy from buildtools/src/main/java/org/apache/pulsar/tests/RetryAnalyzer.java
copy to buildtools/src/test/java/org/apache/pulsar/tests/AnnotationListenerTest.java
index c790d09..5121d3d 100644
--- a/buildtools/src/main/java/org/apache/pulsar/tests/RetryAnalyzer.java
+++ b/buildtools/src/test/java/org/apache/pulsar/tests/AnnotationListenerTest.java
@@ -18,20 +18,16 @@
  */
 package org.apache.pulsar.tests;
 
-import org.testng.ITestResult;
-import org.testng.SkipException;
-import org.testng.util.RetryAnalyzerCount;
+import static org.testng.Assert.assertEquals;
+import org.testng.annotations.Test;
+import org.testng.internal.annotations.TestAnnotation;
 
-public class RetryAnalyzer extends RetryAnalyzerCount {
-    // Only try again once
-    private static final int MAX_RETRIES = Integer.parseInt(System.getProperty("testRetryCount", "1"));
-
-    public RetryAnalyzer() {
-        setCount(MAX_RETRIES);
-    }
-
-    @Override
-    public boolean retryMethod(ITestResult result) {
-        return !(result.getThrowable() instanceof SkipException);
+public class AnnotationListenerTest {
+    @Test
+    void shouldSetDefaultRetryAnalyser() {
+        AnnotationListener annotationListener = new AnnotationListener();
+        TestAnnotation testAnnotation = new TestAnnotation();
+        annotationListener.transform(testAnnotation, null, null, null);
+        assertEquals(testAnnotation.getRetryAnalyzerClass(), RetryAnalyzer.class);
     }
-}
+}
\ No newline at end of file
diff --git a/buildtools/src/main/java/org/apache/pulsar/tests/RetryAnalyzer.java b/buildtools/src/test/java/org/apache/pulsar/tests/DefaultTestRetriesTest.java
similarity index 56%
copy from buildtools/src/main/java/org/apache/pulsar/tests/RetryAnalyzer.java
copy to buildtools/src/test/java/org/apache/pulsar/tests/DefaultTestRetriesTest.java
index c790d09..dc5f9e2 100644
--- a/buildtools/src/main/java/org/apache/pulsar/tests/RetryAnalyzer.java
+++ b/buildtools/src/test/java/org/apache/pulsar/tests/DefaultTestRetriesTest.java
@@ -18,20 +18,31 @@
  */
 package org.apache.pulsar.tests;
 
-import org.testng.ITestResult;
-import org.testng.SkipException;
-import org.testng.util.RetryAnalyzerCount;
+import org.testng.annotations.Test;
 
-public class RetryAnalyzer extends RetryAnalyzerCount {
-    // Only try again once
-    private static final int MAX_RETRIES = Integer.parseInt(System.getProperty("testRetryCount", "1"));
+public class DefaultTestRetriesTest {
+    int invocationCountA;
+    int invocationCountB;
+    int invocationCountC;
 
-    public RetryAnalyzer() {
-        setCount(MAX_RETRIES);
+    @Test
+    void testMethodA() {
+        if (invocationCountA++ < 1) {
+            throw new IllegalStateException("Sample failure to trigger retry.");
+        }
     }
 
-    @Override
-    public boolean retryMethod(ITestResult result) {
-        return !(result.getThrowable() instanceof SkipException);
+    @Test
+    void testMethodB() {
+        if (invocationCountB++ < 1) {
+            throw new IllegalStateException("Sample failure to trigger retry.");
+        }
     }
-}
+
+    @Test
+    void testMethodC() {
+        if (invocationCountC++ < 1) {
+            throw new IllegalStateException("Sample failure to trigger retry.");
+        }
+    }
+}
\ No newline at end of file
diff --git a/tests/docker-images/latest-version-image/Dockerfile b/tests/docker-images/latest-version-image/Dockerfile
index 0ae1c0a..4711e9b 100644
--- a/tests/docker-images/latest-version-image/Dockerfile
+++ b/tests/docker-images/latest-version-image/Dockerfile
@@ -28,7 +28,7 @@ RUN apt-get install -y procps curl git
 
 ENV GOLANG_VERSION 1.13.3
 
-RUN curl -sSL https://storage.googleapis.com/golang/go$GOLANG_VERSION.linux-amd64.tar.gz \
+RUN curl -sSL https://golang.org/dl/go$GOLANG_VERSION.linux-amd64.tar.gz \
 		| tar -C /usr/local -xz
 
 # RUN wget https://dl.google.com/go/go1.13.3.linux-amd64.tar.gz && tar -xvf go1.13.3.linux-amd64.tar.gz && mv go /usr/local