[pulsar] branch master updated: [improve][ci] Revisit the TestNG FailFastNotifier changes to fix NPEs when a test fails (#19226)

[lhotari]

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

lhotari 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 ecd9fecf126 [improve][ci] Revisit the TestNG FailFastNotifier changes 
to fix NPEs when a test fails (#19226)
ecd9fecf126 is described below

commit ecd9fecf126c3d257468ad7e6e396d41b6a78253
Author: Lari Hotari <lhot...@users.noreply.github.com>
AuthorDate: Fri Jan 13 20:05:58 2023 +0200

    [improve][ci] Revisit the TestNG FailFastNotifier changes to fix NPEs when 
a test fails (#19226)
---
 .../org/apache/pulsar/tests/FailFastNotifier.java  | 34 ++++++++++++++--------
 1 file changed, 22 insertions(+), 12 deletions(-)

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 11ef7526b72..29879f594af 100644
--- a/buildtools/src/main/java/org/apache/pulsar/tests/FailFastNotifier.java
+++ b/buildtools/src/main/java/org/apache/pulsar/tests/FailFastNotifier.java
@@ -48,7 +48,7 @@ public class FailFastNotifier
     static class FailFastEventsSingleton {
         private static final FailFastEventsSingleton INSTANCE = new 
FailFastEventsSingleton();
 
-        private volatile boolean skipAfterFailure;
+        private volatile ITestResult firstFailure;
 
         private FailFastEventsSingleton() {
         }
@@ -57,12 +57,14 @@ public class FailFastNotifier
             return INSTANCE;
         }
 
-        public boolean isSkipAfterFailure() {
-            return skipAfterFailure;
+        public ITestResult getFirstFailure() {
+            return firstFailure;
         }
 
-        public void setSkipOnNextTest() {
-            this.skipAfterFailure = true;
+        public void testFailed(ITestResult result) {
+            if (this.firstFailure == null) {
+                this.firstFailure = result;
+            }
         }
     }
 
@@ -75,22 +77,30 @@ public class FailFastNotifier
 
     @Override
     public void onTestFailure(ITestResult result) {
-        
FailFastNotifier.FailFastEventsSingleton.getInstance().setSkipOnNextTest();
         // Hide FailFastSkipExceptions and mark the test as skipped
         if (result.getThrowable() instanceof FailFastSkipException) {
             result.setThrowable(null);
             result.setStatus(ITestResult.SKIP);
+        } else {
+            
FailFastNotifier.FailFastEventsSingleton.getInstance().testFailed(result);
         }
     }
 
     @Override
     public void beforeInvocation(IInvokedMethod iInvokedMethod, ITestResult 
iTestResult) {
-        ITestNGMethod iTestNGMethod = iInvokedMethod.getTestMethod();
-        if (FAIL_FAST_ENABLED && 
FailFastEventsSingleton.getInstance().isSkipAfterFailure()
-                && !(iTestNGMethod.isAfterMethodConfiguration()
-                || iTestNGMethod.isAfterClassConfiguration()
-                || iTestNGMethod.isAfterTestConfiguration())) {
-            throw new FailFastSkipException("Skipped after failure since 
testFailFast system property is set.");
+        if (FAIL_FAST_ENABLED) {
+            ITestResult firstFailure = 
FailFastEventsSingleton.getInstance().getFirstFailure();
+            if (firstFailure != null) {
+                ITestNGMethod iTestNGMethod = iInvokedMethod.getTestMethod();
+                // condition that ensures that cleanup methods will be called 
in the test class where the
+                // first exception happened
+                if (iTestResult.getInstance() != firstFailure.getInstance()
+                        || !(iTestNGMethod.isAfterMethodConfiguration()
+                        || iTestNGMethod.isAfterClassConfiguration()
+                        || iTestNGMethod.isAfterTestConfiguration())) {
+                    throw new FailFastSkipException("Skipped after failure 
since testFailFast system property is set.");
+                }
+            }
         }
     }