Repository: authorjapps/zerocode
Issue: Retry report showing wrong scenario status if finally the step has passed

From Slack:
Cyril EDME:
Hi, 
i would like some feedback on a use case before post any ticket.
Did someone created a scenario with a first step that trigger an asynchrone treatment then assert it with a second step (sync, by REST API using the retry policy) ?
In the end it works, my second step first attempt fail but the second succeed, perfect. But in the report it's recorded as fail (even if the test is green), which is incorrect. The detail of the steps with the first attempt in fail status is correct tho.

***

 But in the report it's recorded as fail (even if the test is green), which is incorrect.
@ Cyril EDME, the issue you are facing is the report issue when retry mechanism is used?
e.g.
```
"retry":{
   "max": 5
   "delay": 500
}
```
Even if let's say first 4 retries failed and 5th one passed, the test goes green(expected), but report shows FAILED (unexpected)?

Is the report missing the 5th entry which has PASSED ?

Can you paste here a result of just one test-scenario run's
Console log : with all retry entries
CSV report : what it has captured
HTML file 

Just for 1 test-scenario please which will be simple to analyze and less noisy!

***

The reporting code is below which marks the Scenario as failed if it has at least 1 step failure.
> org.jsmart.zerocode.core.report.ZeroCodeReportGeneratorImpl#generateExtentReport

This behaviour is correct in a normal usecase, but not for a retry-step usecase.

Possible fix:
- Check if one of the retries has passed, then only mark the scenario as PASSED.
  - Basically if one of the retries succeeds, then it doesn't retry any more. e.g. if max retry is 5, and the 3rd once succeeds, then it comes out from that step(does not retry for 4th and 5th time) and continues for the next step.
- Then see if the Extent report is correctly rendering this i.e. marking the Scenario as PASSED

[Download and unzip this - retry html and csv report.zip](https://github.com/authorjapps/zerocode/files/4585271/retry.html.and.csv.report.zip)


AC1:
===
Mark the scenario as `Pass`(Green) if any of the retries succeeds for the retry-step(s) and all other steps in this scenario have passed.

AC2:
===
Remove the `redundant failed retry steps` from extent-report view. This will leave with the final outcome of the step(wither Passed or Failed) with just the 1 entry which is the correct for the HTML report view.
In case the test-engineers need to see the failed retry steps, then they can look at the logs in the `target` folder.

AC3:
===
If AC2 solution is implemented, then remove the `redundant failed retry steps` from CSV records to make both consistent with each other.

AC4(Tech-Debt):
=============
Create a tech-debt ticket to raise an issue with `Extent Project` to allow overriding the final scenario status(green or Pass in this case) even if the scenario has a failed step. This seems like not possible currently.
First check in the `Extent Slack channel` prior to raise as issue.



Changed files:
core/src/main/java/org/jsmart/zerocode/core/report/ZeroCodeReportGeneratorImpl.java
core/src/test/java/org/jsmart/zerocode/core/report/ZeroCodeReportGeneratorImplTest.java
core/src/main/java/org/jsmart/zerocode/core/domain/reports/ZeroCodeReportStep.java
core/src/main/java/org/jsmart/zerocode/core/domain/reports/ZeroCodeExecResult.java

diff --git a/core/src/main/java/org/jsmart/zerocode/core/domain/reports/ZeroCodeExecResult.java b/core/src/main/java/org/jsmart/zerocode/core/domain/reports/ZeroCodeExecResult.java
index faab8c7d..ae17016b 100644
--- a/core/src/main/java/org/jsmart/zerocode/core/domain/reports/ZeroCodeExecResult.java
+++ b/core/src/main/java/org/jsmart/zerocode/core/domain/reports/ZeroCodeExecResult.java
@@ -35,6 +35,10 @@ public class ZeroCodeExecResult {
         return steps;
     }
 
+    public void setSteps(List<ZeroCodeReportStep> steps) {
+        this.steps = steps;
+    }
+
     @Override
     public String toString() {
         return "ZeroCodeExecResult{" +
diff --git a/core/src/main/java/org/jsmart/zerocode/core/domain/reports/ZeroCodeReportStep.java b/core/src/main/java/org/jsmart/zerocode/core/domain/reports/ZeroCodeReportStep.java
index ddfb08fd..07d6c6f2 100644
--- a/core/src/main/java/org/jsmart/zerocode/core/domain/reports/ZeroCodeReportStep.java
+++ b/core/src/main/java/org/jsmart/zerocode/core/domain/reports/ZeroCodeReportStep.java
@@ -61,6 +61,12 @@ public class ZeroCodeReportStep {
         this.customLog = customLog;
     }
 
+
+    public  ZeroCodeReportStep(String correlationId,String result){
+        this(null,null,null,null,correlationId,null,null,null,null,result,null,null,null,null);
+    }
+
+
     public Integer getLoop() {
         return loop;
     }
diff --git a/core/src/main/java/org/jsmart/zerocode/core/report/ZeroCodeReportGeneratorImpl.java b/core/src/main/java/org/jsmart/zerocode/core/report/ZeroCodeReportGeneratorImpl.java
index dc3e6cc9..bd5b6668 100644
--- a/core/src/main/java/org/jsmart/zerocode/core/report/ZeroCodeReportGeneratorImpl.java
+++ b/core/src/main/java/org/jsmart/zerocode/core/report/ZeroCodeReportGeneratorImpl.java
@@ -18,7 +18,9 @@ import org.jsmart.zerocode.core.domain.builders.HighChartColumnHtmlBuilder;
 import org.jsmart.zerocode.core.domain.builders.ZeroCodeChartKeyValueArrayBuilder;
 import org.jsmart.zerocode.core.domain.builders.ZeroCodeChartKeyValueBuilder;
 import org.jsmart.zerocode.core.domain.builders.ZeroCodeCsvReportBuilder;
+import org.jsmart.zerocode.core.domain.reports.ZeroCodeExecResult;
 import org.jsmart.zerocode.core.domain.reports.ZeroCodeReport;
+import org.jsmart.zerocode.core.domain.reports.ZeroCodeReportStep;
 import org.jsmart.zerocode.core.domain.reports.chart.HighChartColumnHtml;
 import org.jsmart.zerocode.core.domain.reports.csv.ZeroCodeCsvReport;
 import org.slf4j.Logger;
@@ -28,10 +30,7 @@ import java.io.File;
 import java.io.IOException;
 import java.time.LocalDateTime;
 import java.time.ZoneId;
-import java.util.ArrayList;
-import java.util.Arrays;
-import java.util.Date;
-import java.util.List;
+import java.util.*;
 import java.util.stream.Collectors;
 
 import static java.util.Collections.emptyList;
@@ -82,10 +81,26 @@ public class ZeroCodeReportGeneratorImpl implements ZeroCodeReportGenerator {
         this.mapper = mapper;
     }
 
+    /**
+     * Gets unique steps from a scenario. In case of retries, the steps have same correlation id and if
+     * one of the retries is successful, we include it in the result(not the one which failed).
+     * In a normal case(without retry), both PASS and FAIL will be included as usual.
+     * @param steps
+     * @return
+     */
+     List<ZeroCodeReportStep> getUniqueSteps(List<ZeroCodeReportStep> steps){
+        Map<String,ZeroCodeReportStep> result = new HashMap<>();
+        steps.forEach(step->{
+            result.merge(step.getCorrelationId(), step,
+                    (s1, s2) -> RESULT_PASS.equals(s1.getResult()) ? s1 : s2);
+        });
+        return new ArrayList<>(result.values());
+    }
+
     @Override
     public void generateExtentReport() {
 
-        if(interactiveHtmlReportDisabled){
+        if (interactiveHtmlReportDisabled) {
             return;
         }
 
@@ -100,22 +115,22 @@ public class ZeroCodeReportGeneratorImpl implements ZeroCodeReportGenerator {
                 test.assignCategory(DEFAULT_REGRESSION_CATEGORY);
 
                 test.assignAuthor(optionalAuthor(thisScenario.getScenarioName()));
-
-                thisScenario.getSteps().forEach(thisStep -> {
+                List<ZeroCodeReportStep> thisScenarioUniqueSteps = getUniqueSteps(thisScenario.getSteps());
+                thisScenarioUniqueSteps.forEach(thisStep -> {
                     test.getModel().setStartTime(utilDateOf(thisStep.getRequestTimeStamp()));
                     test.getModel().setEndTime(utilDateOf(thisStep.getResponseTimeStamp()));
 
                     final Status testStatus = thisStep.getResult().equals(RESULT_PASS) ? Status.PASS : Status.FAIL;
-					
+
                     ExtentTest step = test.createNode(thisStep.getName(), TEST_STEP_CORRELATION_ID + " " + thisStep.getCorrelationId());
-                 
-                    if(testStatus.equals(Status.PASS)) {
-                    	step.pass(thisStep.getResult());
-                    }else {
-                    	step.info(MarkupHelper.createCodeBlock(thisStep.getOperation() + "\\t" + thisStep.getUrl()));
-                    	step.info(MarkupHelper.createCodeBlock(thisStep.getRequest(), CodeLanguage.JSON));
+
+                    if (testStatus.equals(Status.PASS)) {
+                        step.pass(thisStep.getResult());
+                    } else {
+                        step.info(MarkupHelper.createCodeBlock(thisStep.getOperation() + "\\t" + thisStep.getUrl()));
+                        step.info(MarkupHelper.createCodeBlock(thisStep.getRequest(), CodeLanguage.JSON));
                         step.info(MarkupHelper.createCodeBlock(thisStep.getResponse(), CodeLanguage.JSON));
-                    	step.fail(MarkupHelper.createCodeBlock("Reason:\\n" + thisStep.getAssertions()));
+                        step.fail(MarkupHelper.createCodeBlock("Reason:\\n" + thisStep.getAssertions()));
                     }
                     extentReports.flush();
                 });
@@ -133,13 +148,13 @@ public class ZeroCodeReportGeneratorImpl implements ZeroCodeReportGenerator {
         // (might be disabled by current runner)
         // Then it's good to link it to that spike report.
         // ------------------------------------------------
-        if(spikeChartReportEnabled || spikeChartFileName != null){
+        if (spikeChartReportEnabled || spikeChartFileName != null) {
             final String reportName = getReportName();
 
             String linkCodeToTargetSpikeChartHtml =
                     String.format("<code>&nbsp;&nbsp;<a href='%s' style=\\"color: #006; background: #ff6;\\"> %s </a></code>",
-                    spikeChartFileName,
-                    LINK_LABEL_NAME);
+                            spikeChartFileName,
+                            LINK_LABEL_NAME);
 
             ExtentReportsFactory.reportName(reportName + linkCodeToTargetSpikeChartHtml);
         }
@@ -148,33 +163,33 @@ public class ZeroCodeReportGeneratorImpl implements ZeroCodeReportGenerator {
     protected String optionalAuthor(String scenarioName) {
         String authorName = substringBetween(scenarioName, AUTHOR_MARKER, AUTHOR_MARKER);
 
-        if(authorName == null){
+        if (authorName == null) {
             authorName = substringBetween(scenarioName, AUTHOR_MARKER, ",");
         }
 
-        if(authorName == null){
+        if (authorName == null) {
             authorName = substringBetween(scenarioName, AUTHOR_MARKER, " ");
         }
 
-        if(authorName == null){
+        if (authorName == null) {
             authorName = scenarioName.substring(scenarioName.lastIndexOf(AUTHOR_MARKER) + AUTHOR_MARKER.length());
         }
 
-        if(scenarioName.lastIndexOf(AUTHOR_MARKER) == -1 || StringUtils.isEmpty(authorName)){
+        if (scenarioName.lastIndexOf(AUTHOR_MARKER) == -1 || StringUtils.isEmpty(authorName)) {
             authorName = ANONYMOUS_AUTHOR;
         }
 
         return authorName;
     }
-    
+
     protected String onlyScenarioName(String scenarioName) {
-    	
-    	int index = scenarioName.indexOf(AUTHOR_MARKER);
-    	if(index == -1) {
-    		return scenarioName;
-    	}else {
-    		return scenarioName.substring(0, index -1); 
-    	}
+
+        int index = scenarioName.indexOf(AUTHOR_MARKER);
+        if (index == -1) {
+            return scenarioName;
+        } else {
+            return scenarioName.substring(0, index - 1);
+        }
     }
 
     @Override
@@ -198,7 +213,7 @@ public class ZeroCodeReportGeneratorImpl implements ZeroCodeReportGenerator {
         /*
          * Generate: Spike Chart using HighChart
          */
-        if(spikeChartReportEnabled){
+        if (spikeChartReportEnabled) {
             HighChartColumnHtml highChartColumnHtml = convertCsvRowsToHighChartData(zeroCodeCsvFlattenedRows);
             generateHighChartReport(highChartColumnHtml);
         }
@@ -331,7 +346,11 @@ public class ZeroCodeReportGeneratorImpl implements ZeroCodeReportGenerator {
                     }
                 })
                 .collect(Collectors.toList());
-
+        for (ZeroCodeReport zeroCodeReport : scenarioReports) {
+            for (ZeroCodeExecResult zeroCodeExecResult : zeroCodeReport.getResults()) {
+                zeroCodeExecResult.setSteps(getUniqueSteps(zeroCodeExecResult.getSteps()));
+            }
+        }
         return scenarioReports;
     }
 
@@ -342,7 +361,7 @@ public class ZeroCodeReportGeneratorImpl implements ZeroCodeReportGenerator {
             return name.endsWith(".json");
         });
 
-        if(files == null || files.length == 0){
+        if (files == null || files.length == 0) {
 
             LOGGER.error("\\n\\t\\t\\t************\\nNow files were found in folder:{}, hence could not proceed. " +
                     "\\n(If this was intentional, then you can safely ignore this error)" +
diff --git a/core/src/test/java/org/jsmart/zerocode/core/report/ZeroCodeReportGeneratorImplTest.java b/core/src/test/java/org/jsmart/zerocode/core/report/ZeroCodeReportGeneratorImplTest.java
index b05c464f..469272bc 100644
--- a/core/src/test/java/org/jsmart/zerocode/core/report/ZeroCodeReportGeneratorImplTest.java
+++ b/core/src/test/java/org/jsmart/zerocode/core/report/ZeroCodeReportGeneratorImplTest.java
@@ -2,14 +2,21 @@ package org.jsmart.zerocode.core.report;
 
 import com.fasterxml.jackson.databind.ObjectMapper;
 import org.jsmart.zerocode.core.di.provider.ObjectMapperProvider;
+import org.jsmart.zerocode.core.domain.reports.ZeroCodeReportStep;
 import org.junit.Before;
 import org.junit.Ignore;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.ExpectedException;
 
+import java.util.ArrayList;
+import java.util.List;
+
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.core.Is.is;
+import static org.jsmart.zerocode.core.constants.ZeroCodeReportConstants.RESULT_FAIL;
+import static org.jsmart.zerocode.core.constants.ZeroCodeReportConstants.RESULT_PASS;
+import static org.junit.Assert.assertEquals;
 
 public class ZeroCodeReportGeneratorImplTest {
 
@@ -76,4 +83,38 @@ public class ZeroCodeReportGeneratorImplTest {
 
     }
 
+    @Test
+    public void testGettingUniqueStepsForMultipleRetries(){
+        List<ZeroCodeReportStep> steps = new ArrayList<ZeroCodeReportStep>(){
+            {
+                add(new ZeroCodeReportStep("testCorrelationId",RESULT_PASS));
+                add(new ZeroCodeReportStep("testCorrelationId",RESULT_FAIL));
+            }
+        };
+        List<ZeroCodeReportStep> uniqueSteps = zeroCodeReportGenerator.getUniqueSteps(steps);
+        assertEquals(uniqueSteps.size() , 1);
+        assertEquals(uniqueSteps.get(0).getResult(),RESULT_PASS);
+    }
+
+    @Test
+    public void testGettingUniqueStepsForNoRetries(){
+        List<ZeroCodeReportStep> steps = new ArrayList<ZeroCodeReportStep>(){
+            {
+                add(new ZeroCodeReportStep("testCorrelationId1",RESULT_PASS));
+                add(new ZeroCodeReportStep("testCorrelationId2",RESULT_FAIL));
+                add(new ZeroCodeReportStep("testCorrelationId3",RESULT_FAIL));
+            }
+        };
+        List<ZeroCodeReportStep> uniqueSteps = zeroCodeReportGenerator.getUniqueSteps(steps);
+        assertEquals(uniqueSteps.size() , 3);
+        assertEquals(uniqueSteps.stream().filter(step->step.getResult().equals(RESULT_PASS)).count(),1);
+        assertEquals(uniqueSteps.stream().filter(step->step.getResult().equals(RESULT_FAIL)).count(),2);
+
+        assertThat(uniqueSteps.get(0).getCorrelationId(),is("testCorrelationId3")); // order different to original
+        assertThat(uniqueSteps.get(1).getCorrelationId(),is("testCorrelationId1")); // order different to original
+        assertThat(uniqueSteps.get(2).getCorrelationId(),is("testCorrelationId2")); // order different to original
+
+        // order different to original: Not really an issue, as the CSV can be sorted ACS or DESC by a an external tool
+    }
+
 }
\\ No newline at end of file