[improve][ci] Improve test fail fast: disable for non-PR builds, fail…

… parallel forked test procs (apache#19252)
teaey · Jan 17, 2023 · 69866a6 · 69866a6
1 parent 1d7f5e5
commit 69866a6
Show file tree

Hide file tree

Showing 4 changed files with 63 additions and 4 deletions.
diff --git a/build/run_integration_group.sh b/build/run_integration_group.sh
@@ -64,6 +64,20 @@ mvn_run_integration_test() {
       clean_arg="clean"
       shift
   fi
+  local use_fail_fast=1
+  if [[ "$GITHUB_ACTIONS" == "true" && "$GITHUB_EVENT_NAME" != "pull_request" ]]; then
+    use_fail_fast=0
+  fi
+  if [[ "$1" == "--no-fail-fast" ]]; then
+    use_fail_fast=0
+    shift;
+  fi
+  local failfast_args
+  if [ $use_fail_fast -eq 1 ]; then
+    failfast_args="-DtestFailFast=true -DtestFailFastFile=/tmp/test_fail_fast_killswitch.$$.$RANDOM.$(date +%s) --fail-fast"
+  else
+    failfast_args="-DtestFailFast=false --fail-at-end"
+  fi
   cd "$SCRIPT_DIR"/../tests
   modules=$(mvn_list_modules -DskipDocker "$@")
   cd ..
@@ -76,7 +90,7 @@ mvn_run_integration_test() {
   if [[ $build_only -ne 1 ]]; then
     echo "::group::Run tests for " "$@"
     # use "verify" instead of "test"
-    mvn -B -ntp -pl "$modules" -DskipDocker -DskipSourceReleaseAssembly=true -Dspotbugs.skip=true -Dlicense.skip=true -Dcheckstyle.skip=true -Drat.skip=true -DredirectTestOutputToFile=false $clean_arg verify "$@"
+    mvn -B -ntp -pl "$modules" $failfast_args -DskipDocker -DskipSourceReleaseAssembly=true -Dspotbugs.skip=true -Dlicense.skip=true -Dcheckstyle.skip=true -Drat.skip=true -DredirectTestOutputToFile=false $clean_arg verify "$@"
     echo "::endgroup::"
     set +x
     "$SCRIPT_DIR/pulsar_ci_tool.sh" move_test_reports

diff --git a/build/run_unit_group.sh b/build/run_unit_group.sh
@@ -39,10 +39,24 @@ function mvn_test() {
       target="install"
       shift
     fi
+    local use_fail_fast=1
+    if [[ "$GITHUB_ACTIONS" == "true" && "$GITHUB_EVENT_NAME" != "pull_request" ]]; then
+      use_fail_fast=0
+    fi
+    if [[ "$1" == "--no-fail-fast" ]]; then
+      use_fail_fast=0
+      shift;
+    fi
+    local failfast_args
+    if [ $use_fail_fast -eq 1 ]; then
+      failfast_args="-DtestFailFast=true -DtestFailFastFile=/tmp/test_fail_fast_killswitch.$$.$RANDOM.$(date +%s) --fail-fast"
+    else
+      failfast_args="-DtestFailFast=false --fail-at-end"
+    fi
     echo "::group::Run tests for " "$@"
     # use "verify" instead of "test" to workaround MDEP-187 issue in pulsar-functions-worker and pulsar-broker projects with the maven-dependency-plugin's copy goal
     # Error message was "Artifact has not been packaged yet. When used on reactor artifact, copy should be executed after packaging: see MDEP-187"
-    $MVN_TEST_OPTIONS $clean_arg $target $coverage_arg "$@" "${COMMANDLINE_ARGS[@]}"
+    $MVN_TEST_OPTIONS $failfast_args $clean_arg $target $coverage_arg "$@" "${COMMANDLINE_ARGS[@]}"
     echo "::endgroup::"
     set +x
     "$SCRIPT_DIR/pulsar_ci_tool.sh" move_test_reports
@@ -59,7 +73,7 @@ alias echo='{ [[ $- =~ .*x.* ]] && trace_enabled=1 || trace_enabled=0; set +x; }
 
 # Test Groups  -- start --
 function test_group_broker_group_1() {
-  mvn_test -pl pulsar-broker -Dgroups='broker' -DtestReuseFork=true -DskipAfterFailureCount=1
+  mvn_test -pl pulsar-broker -Dgroups='broker' -DtestReuseFork=true
 }
 
 function test_group_broker_group_2() {

diff --git a/buildtools/src/main/java/org/apache/pulsar/tests/FailFastNotifier.java b/buildtools/src/main/java/org/apache/pulsar/tests/FailFastNotifier.java
@@ -18,6 +18,11 @@
  */
 package org.apache.pulsar.tests;
 
+import java.io.File;
+import java.io.IOException;
+import java.nio.file.Files;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 import org.testng.IInvokedMethod;
 import org.testng.IInvokedMethodListener;
 import org.testng.ITestListener;
@@ -42,8 +47,19 @@
  */
 public class FailFastNotifier
         implements IInvokedMethodListener, ITestListener {
+    private static final Logger LOG = LoggerFactory.getLogger(FailFastNotifier.class);
+    private static final String PROPERTY_NAME_TEST_FAIL_FAST = "testFailFast";
     private static final boolean FAIL_FAST_ENABLED = Boolean.parseBoolean(
-            System.getProperty("testFailFast", "true"));
+            System.getProperty(PROPERTY_NAME_TEST_FAIL_FAST, "true"));
+
+    private static final String PROPERTY_NAME_TEST_FAIL_FAST_FILE = "testFailFastFile";
+
+    // A file that is used to communicate to other parallel forked test processes to terminate the build
+    // so that fail fast mode works with multiple forked test processes
+    private static final File FAIL_FAST_KILLSWITCH_FILE =
+            System.getProperty(PROPERTY_NAME_TEST_FAIL_FAST_FILE) != null
+                    && System.getProperty(PROPERTY_NAME_TEST_FAIL_FAST_FILE).trim().length() > 0
+                    ? new File(System.getProperty(PROPERTY_NAME_TEST_FAIL_FAST_FILE).trim()) : null;
 
     static class FailFastEventsSingleton {
         private static final FailFastEventsSingleton INSTANCE = new FailFastEventsSingleton();
@@ -64,6 +80,14 @@ public ITestResult getFirstFailure() {
         public void testFailed(ITestResult result) {
             if (this.firstFailure == null) {
                 this.firstFailure = result;
+                if (FAIL_FAST_KILLSWITCH_FILE != null && !FAIL_FAST_KILLSWITCH_FILE.exists()) {
+                    try {
+                        Files.createFile(FAIL_FAST_KILLSWITCH_FILE.toPath());
+                    } catch (IOException e) {
+                        LOG.warn("Unable to create fail fast kill switch file '"
+                                + FAIL_FAST_KILLSWITCH_FILE.getAbsolutePath() + "'", e);
+                    }
+                }
             }
         }
     }
@@ -101,6 +125,9 @@ public void beforeInvocation(IInvokedMethod iInvokedMethod, ITestResult iTestRes
                     throw new FailFastSkipException("Skipped after failure since testFailFast system property is set.");
                 }
             }
+            if (FAIL_FAST_KILLSWITCH_FILE != null && FAIL_FAST_KILLSWITCH_FILE.exists()) {
+                throw new FailFastSkipException("Skipped after failure since kill switch file exists.");
+            }
         }
     }
 

diff --git a/pom.xml b/pom.xml
@@ -108,6 +108,8 @@ flexible messaging model and an intuitive client API.</description>
     <testForkCount>4</testForkCount>
     <testRealAWS>false</testRealAWS>
     <testRetryCount>1</testRetryCount>
+    <testFailFast>true</testFailFast>
+    <testFailFastFile></testFailFastFile>
     <testJacocoAgentArgument/>
     <testHeapDumpPath>/tmp</testHeapDumpPath>
     <surefire.shutdown>kill</surefire.shutdown>
@@ -1493,6 +1495,8 @@ flexible messaging model and an intuitive client API.</description>
           <systemPropertyVariables>
             <testRealAWS>${testRealAWS}</testRealAWS>
             <testRetryCount>${testRetryCount}</testRetryCount>
+            <testFailFast>${testFailFast}</testFailFast>
+            <testFailFastFile>${testFailFastFile}</testFailFastFile>
           </systemPropertyVariables>
           <properties>
             <property>