Updated Status in afterInvocation() not considered

CHANGES.txt

@@ -1,4 +1,5 @@
 Current
+Fixed: GITHUB-1600: Updated in afterInvocation() testResult.status is not used in willRetry condition (Krishnan Mahadevan)
 Fixed: GITHUB-1598: Injected types parameter and optional parameters cannot be used together. (Yehui Wang)
 Fixed: GITHUB-1594: Cannot filter by "testnames" when suite xml is a suite of suites (Krishnan Mahadevan)
 Fixed: GITHUB-1584: Can't run tests from IDEA (Krishnan Mahadevan)

src/main/java/org/testng/internal/Invoker.java

@@ -601,10 +601,10 @@ private ITestResult invokeMethod(Object instance,
       ExpectedExceptionsHolder expectedExceptionClasses
           = new ExpectedExceptionsHolder(m_annotationFinder, tm, new RegexpExpectedExceptionsHolder(m_annotationFinder, tm));
       StatusHolder holder = considerExceptions(tm, testResult, expectedExceptionClasses, failureContext);
-
-      // Run invokedMethodListeners after updating TestResult
+      int statusBeforeListenerInvocation = testResult.getStatus();
       runInvokedMethodListeners(AFTER_INVOCATION, invokedMethod, testResult);
-      handleInvocationResults(tm, testResult, failureContext, holder);
+      boolean wasResultUnaltered = statusBeforeListenerInvocation == testResult.getStatus();
+      handleInvocationResults(tm, testResult, failureContext, holder, wasResultUnaltered);
 
       // If this method has a data provider and just failed, memorize the number
       // at which it failed.
@@ -1139,14 +1139,15 @@ private StatusHolder considerExceptions(ITestNGMethod tm, ITestResult testresult
   private void handleInvocationResults(ITestNGMethod testMethod,
                                ITestResult testResult,
                                FailureContext failure,
-                               StatusHolder holder) {
+                               StatusHolder holder,
+                               boolean wasResultUnaltered) {
     //
     // Go through all the results and create a TestResult for each of them
     //
     List<ITestResult> resultsToRetry = Lists.newArrayList();
 
     Throwable ite = testResult.getThrowable();
-    int status = holder.status;
+    int status = computeTestStatusComparingTestResultAndStatusHolder(testResult, holder, wasResultUnaltered);
     boolean handled = holder.handled;
 
     IRetryAnalyzer retryAnalyzer = testMethod.getRetryAnalyzer();
@@ -1167,11 +1168,18 @@ private void handleInvocationResults(ITestNGMethod testMethod,
     }
   }
 
+  private static int computeTestStatusComparingTestResultAndStatusHolder(ITestResult testResult, StatusHolder holder,
+                                                                         boolean wasResultUnaltered) {
+    if (wasResultUnaltered) {
+      return holder.status;
+    }
+    return testResult.getStatus();
+  }
+
   private boolean isSkipExceptionAndSkip(Throwable ite) {
     return SkipException.class.isAssignableFrom(ite.getClass()) && ((SkipException) ite).isSkip();
   }
 
-
   /**
    * To reduce thread contention and also to correctly handle thread-confinement
    * this method invokes the @BeforeGroups and @AfterGroups corresponding to the current @Test method.

src/test/java/test/retryAnalyzer/RetryAnalyzerTest.java

@@ -13,6 +13,8 @@
 import test.SimpleBaseTest;
 import test.retryAnalyzer.github1519.MyListener;
 import test.retryAnalyzer.github1519.TestClassSample;
+import test.retryAnalyzer.github1600.Github1600Listener;
+import test.retryAnalyzer.github1600.Github1600TestSample;
 
 public class RetryAnalyzerTest extends SimpleBaseTest {
     @Test
@@ -41,4 +43,16 @@ public void testIfRetryIsInvokedBeforeListener() {
         tng.run();
         assertThat(TestClassSample.messages).containsExactly("afterInvocation", "retry", "afterInvocation");
     }
+
+    @Test(description = "GITHUB-1600")
+    public void testIfRetryIsInvokedBeforeListenerButHasToConsiderFailures() {
+        TestNG tng = create(Github1600TestSample.class);
+        Github1600Listener listener = new Github1600Listener();
+        TestListenerAdapter tla = new TestListenerAdapter();
+        tng.addListener((ITestNGListener) tla);
+        tng.addListener((ITestNGListener) listener);
+        tng.run();
+        assertThat(tla.getFailedTests()).hasSize(1);
+        assertThat(tla.getSkippedTests()).hasSize(1);
+    }
 }

src/test/java/test/retryAnalyzer/github1600/Github1600Analyzer.java

@@ -0,0 +1,25 @@
+package test.retryAnalyzer.github1600;
+
+import org.testng.IRetryAnalyzer;
+import org.testng.ITestResult;
+
+public class Github1600Analyzer implements IRetryAnalyzer {
+
+    static final String RETRY = "RETRY";
+    public static final String NO = "NO";
+    static final String YES = "YES";
+    private static int retryCount = 0;
+    private static final int MAX_RETRY_COUNT = 10;
+
+    @Override
+    public boolean retry(ITestResult iTestResult) {
+        String attribute = (String) iTestResult.getAttribute(RETRY);
+        if (NO.equalsIgnoreCase(attribute)) {
+            return false;
+        } else if (YES.equalsIgnoreCase(attribute) || retryCount < MAX_RETRY_COUNT) {
+            retryCount++;
+            return true;
+        }
+        return false;
+    }
+}

src/test/java/test/retryAnalyzer/github1600/Github1600Listener.java

@@ -0,0 +1,37 @@
+package test.retryAnalyzer.github1600;
+
+import org.testng.IAnnotationTransformer;
+import org.testng.IInvokedMethod;
+import org.testng.IInvokedMethodListener;
+import org.testng.IRetryAnalyzer;
+import org.testng.ITestResult;
+import org.testng.annotations.ITestAnnotation;
+
+import java.lang.reflect.Constructor;
+import java.lang.reflect.Method;
+
+public class Github1600Listener implements IInvokedMethodListener, IAnnotationTransformer {
+    @Override
+    public void beforeInvocation(IInvokedMethod iInvokedMethod, ITestResult iTestResult) {
+
+    }
+
+    @Override
+    public void afterInvocation(IInvokedMethod iInvokedMethod, ITestResult iTestResult) {
+        if (iInvokedMethod.isTestMethod()) {
+            String attribute = Github1600Analyzer.NO;
+            if (iTestResult.getStatus() == ITestResult.SUCCESS) {
+                iTestResult.setStatus(ITestResult.FAILURE);
+                attribute = Github1600Analyzer.YES;
+            }
+            iTestResult.setAttribute(Github1600Analyzer.RETRY, attribute);
+        }
+    }
+
+    @Override
+    public void transform(ITestAnnotation iTestAnnotation, Class aClass, Constructor constructor, Method method) {
+        IRetryAnalyzer retry = iTestAnnotation.getRetryAnalyzer();
+        if (retry == null)
+            iTestAnnotation.setRetryAnalyzer(Github1600Analyzer.class);
+    }
+}

src/test/java/test/retryAnalyzer/github1600/Github1600TestSample.java

@@ -0,0 +1,15 @@
+package test.retryAnalyzer.github1600;
+
+import org.testng.Assert;
+import org.testng.annotations.Test;
+
+public class Github1600TestSample {
+    private static int a = 2;
+
+    @Test
+    public void test1() {
+        Assert.assertEquals(a, 2);
+        a++;
+    }
+
+}