Refactored status handling (#320)

* background steps are considered * fixed background handling * changelog
trivago · Aug 10, 2023 · 805914e · 805914e
1 parent 70ae4dd
commit 805914e
Show file tree

Hide file tree

Showing 20 changed files with 106 additions and 121 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -7,6 +7,17 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
 
 Back to [Readme](README.md).
 
+## [3.4.0] - 2023-08-10
+
+### Fixed
+* Background steps were not considered in the overall scenario outcome [#319]
+* Background steps were not shown in the bar charts
+
+### Changed
+* Skipped scenarios are now determined to the official Cucumber guidelines 
+* Charts show small bars for steps with a time of 0 instead of none at all
+* Before and after hooks are not shown in the step charts anymore
+
 ## [3.3.1] - 2023-08-08
 
 ### Fixed
@@ -751,6 +762,7 @@ steps with status `pending` or `undefined` (default value is `false`) (#74)
 
 Initial project version on GitHub and Maven Central.
 
+[3.4.0]: https://github.com/trivago/cluecumber-report-plugin/tree/3.4.0
 [3.3.1]: https://github.com/trivago/cluecumber-report-plugin/tree/3.3.1
 [3.3.0]: https://github.com/trivago/cluecumber-report-plugin/tree/3.3.0
 [3.2.2]: https://github.com/trivago/cluecumber-report-plugin/tree/3.2.2

diff --git a/core/pom.xml b/core/pom.xml
@@ -6,13 +6,13 @@
     <modelVersion>4.0.0</modelVersion>
 
     <artifactId>cluecumber-core</artifactId>
-    <version>3.3.1</version>
+    <version>3.4.0</version>
     <packaging>jar</packaging>
 
     <parent>
         <artifactId>cluecumber-parent</artifactId>
         <groupId>com.trivago.rta</groupId>
-        <version>3.3.1</version>
+        <version>3.4.0</version>
     </parent>
 
     <name>Cluecumber Core</name>

diff --git a/engine/pom.xml b/engine/pom.xml
@@ -8,11 +8,11 @@
     <parent>
         <artifactId>cluecumber-parent</artifactId>
         <groupId>com.trivago.rta</groupId>
-        <version>3.3.1</version>
+        <version>3.4.0</version>
     </parent>
 
     <artifactId>cluecumber-engine</artifactId>
-    <version>3.3.1</version>
+    <version>3.4.0</version>
     <packaging>jar</packaging>
 
     <name>Cluecumber Engine</name>

diff --git a/engine/src/main/java/com/trivago/cluecumber/engine/constants/Status.java b/engine/src/main/java/com/trivago/cluecumber/engine/constants/Status.java
@@ -32,6 +32,7 @@
 
 import java.util.Arrays;
 import java.util.List;
+import java.util.Set;
 
 /**
  * Enum to manage all states for steps and scenarios.
@@ -65,7 +66,7 @@ public enum Status {
     /**
      * The three basic states: passed, failed and skipped.
      */
-    public static final List<Status> BASIC_STATES = Arrays.asList(Status.PASSED, Status.FAILED, Status.SKIPPED);
+    public static final List<Status> BASIC_STATES = Arrays.asList(Status.FAILED, Status.SKIPPED, Status.PASSED);
 
     private final String status;
 
@@ -79,10 +80,37 @@ public enum Status {
      * @param status The status string.
      * @return The matching {@link Status} enum.
      */
-    public static Status fromString(String status) {
+    public static Status fromString(final String status) {
         return valueOf(status.toUpperCase());
     }
 
+    public Status basicStatus() {
+        switch (this) {
+            case PASSED:
+                return Status.PASSED;
+            case FAILED:
+                return Status.FAILED;
+            default:
+                return Status.SKIPPED;
+        }
+    }
+
+    /**
+     * Return the highest status from the given list of states.
+     *
+     * @return The status string.
+     */
+    public static Status getHighestBasicState(Set<Status> allStates) {
+        for (Status basicState : BASIC_STATES) {
+            for (Status allState : allStates) {
+                if (allState.basicStatus() == basicState) {
+                    return basicState;
+                }
+            }
+        }
+        return FAILED;
+    }
+
     /**
      * Return the status string from this enum.
      *

diff --git a/engine/src/main/java/com/trivago/cluecumber/engine/json/pojo/Element.java b/engine/src/main/java/com/trivago/cluecumber/engine/json/pojo/Element.java
@@ -22,6 +22,8 @@
 import java.time.ZonedDateTime;
 import java.util.ArrayList;
 import java.util.List;
+import java.util.Set;
+import java.util.stream.Collectors;
 
 /**
  * This represents a scenarios.
@@ -358,73 +360,17 @@ public boolean isSkipped() {
      * @return The overall status.
      */
     public Status getStatus() {
-        int totalSteps = steps.size();
-
-        if (totalSteps == 0) {
-            return Status.SKIPPED;
-        }
-
-        // If any hooks fail, report the scenario as failed
-        for (ResultMatch beforeHook : before) {
-            if (beforeHook.isFailed()) {
-                return Status.FAILED;
-            }
-        }
-        for (ResultMatch afterHook : after) {
-            if (afterHook.isFailed()) {
-                return Status.FAILED;
-            }
+        Set<Status> allStates = before.stream().map(ResultMatch::getStatus).collect(Collectors.toSet());
+        backgroundSteps.stream().map(ResultMatch::getStatus).forEach(allStates::add);
+        steps.stream().map(ResultMatch::getStatus).forEach(allStates::add);
+        after.stream().map(ResultMatch::getStatus).forEach(allStates::add);
+
+        if (failOnPendingOrUndefined && allStates.size() == 1
+                && allStates.iterator().next().basicStatus() == Status.SKIPPED) {
+            return Status.FAILED;
         }
 
-        // If all steps have the same status, return this as the scenario status.
-        for (Status status : Status.BASIC_STATES) {
-            int stepsWithCertainStatusCount = 0;
-            for (Step step : steps) {
-                if (step.getConsolidatedStatus() == status) {
-                    stepsWithCertainStatusCount++;
-                }
-
-                // If any step hooks fail, report scenario as failed.
-                for (ResultMatch beforeStepHook : step.getBefore()) {
-                    if (beforeStepHook.isFailed()) {
-                        return Status.FAILED;
-                    }
-                }
-                for (ResultMatch afterStepHook : step.getAfter()) {
-                    if (afterStepHook.isFailed()) {
-                        return Status.FAILED;
-                    }
-                }
-            }
-
-            if (totalSteps == stepsWithCertainStatusCount) {
-                if (status == Status.SKIPPED) {
-                    if (failOnPendingOrUndefined) {
-                        return Status.FAILED;
-                    }
-                }
-                return status;
-            }
-        }
-
-        // If at least one step passed and the other steps are skipped, return skipped (or failed if failOnPendingOrUndefined is true).
-        if (getTotalNumberOfPassedSteps() >= 0 &&
-                (getTotalNumberOfSkippedSteps() + getTotalNumberOfPassedSteps()) == getTotalNumberOfSteps()) {
-            if (failOnPendingOrUndefined) {
-                return Status.FAILED;
-            }
-            return Status.SKIPPED;
-        }
-
-        // If all steps are skipped return skipped (or failed if failOnPendingOrUndefined is true).
-        if (getTotalNumberOfSkippedSteps() == totalSteps) {
-            if (failOnPendingOrUndefined) {
-                return Status.FAILED;
-            }
-            return Status.SKIPPED;
-        }
-
-        return Status.FAILED;
+        return Status.getHighestBasicState(allStates);
     }
 
     /**
@@ -534,7 +480,7 @@ public void setScenarioIndex(final int scenarioIndex) {
      * @return The number of steps.
      */
     public int getTotalNumberOfSteps() {
-        return getSteps().size();
+        return getAllStepsIncludingBackgroundSteps().size();
     }
 
     /**
@@ -571,7 +517,7 @@ public int getTotalNumberOfSkippedSteps() {
      * @return The number of step.
      */
     private int getNumberOfStepsWithStatus(final Status status) {
-        return (int) getSteps().stream().filter(step -> step.getConsolidatedStatus() == status).count();
+        return (int) getAllStepsIncludingBackgroundSteps().stream().filter(step -> step.getConsolidatedStatus() == status).count();
     }
 
     /**
@@ -685,8 +631,7 @@ public boolean hasStepHooksWithContent() {
      */
     public List<ResultMatch> getAllResultMatches() {
         List<ResultMatch> resultMatches = new ArrayList<>(getBefore());
-        resultMatches.addAll(getBackgroundSteps());
-        resultMatches.addAll(getSteps());
+        resultMatches.addAll(getAllStepsIncludingBackgroundSteps());
         resultMatches.addAll(getAfter());
         return resultMatches;
     }
@@ -753,4 +698,10 @@ public String getFeatureUri() {
     public void setFeatureUri(String featureUri) {
         this.featureUri = featureUri;
     }
+
+    public List<Step> getAllStepsIncludingBackgroundSteps() {
+        List<Step> combinedSteps = new ArrayList<>(backgroundSteps);
+        combinedSteps.addAll(steps);
+        return combinedSteps;
+    }
 }
diff --git a/engine/src/main/java/com/trivago/cluecumber/engine/json/pojo/ResultMatch.java b/engine/src/main/java/com/trivago/cluecumber/engine/json/pojo/ResultMatch.java
@@ -201,19 +201,8 @@ public boolean hasContent() {
      * @return The basic {@link Status} enum.
      */
     public Status getConsolidatedStatus() {
-        switch (getStatus()) {
-            case PASSED:
-                return Status.PASSED;
-            case SKIPPED:
-            case PENDING:
-            case AMBIGUOUS:
-            case UNDEFINED:
-                return Status.SKIPPED;
-            case FAILED:
-            default:
-                return Status.FAILED;
-        }
-    }
+        return getStatus().basicStatus();
+   }
 
     /**
      * Get the string of the basic status.

diff --git a/...rc/main/java/com/trivago/cluecumber/engine/rendering/pages/charts/ChartJsonConverter.java b/...rc/main/java/com/trivago/cluecumber/engine/rendering/pages/charts/ChartJsonConverter.java
@@ -17,6 +17,7 @@
 
 import com.google.gson.Gson;
 import com.google.gson.GsonBuilder;
+import com.google.gson.JsonElement;
 import com.trivago.cluecumber.engine.rendering.pages.charts.pojos.Chart;
 
 import javax.inject.Inject;

diff --git a/engine/src/main/java/com/trivago/cluecumber/engine/rendering/pages/charts/pojos/Dataset.java b/engine/src/main/java/com/trivago/cluecumber/engine/rendering/pages/charts/pojos/Dataset.java
@@ -15,6 +15,7 @@
  */
 package com.trivago.cluecumber.engine.rendering.pages.charts.pojos;
 
+import java.util.ArrayList;
 import java.util.List;
 
 

diff --git a/...ivago/cluecumber/engine/rendering/pages/pojos/pagecollections/AllStepsPageCollection.java b/...ivago/cluecumber/engine/rendering/pages/pojos/pagecollections/AllStepsPageCollection.java
@@ -20,10 +20,7 @@
 import com.trivago.cluecumber.engine.rendering.pages.pojos.ResultCount;
 import com.trivago.cluecumber.engine.rendering.pages.pojos.Times;
 
-import java.util.HashMap;
-import java.util.List;
-import java.util.Map;
-import java.util.Set;
+import java.util.*;
 
 /**
  * Page collection for the step overview page.
@@ -129,7 +126,11 @@ private void calculateStepResultCounts(final List<Report> reports) {
         if (reports == null) return;
         reports.forEach(report -> report.getElements().forEach(element -> {
             int scenarioIndex = element.getScenarioIndex();
-            element.getSteps().forEach(step -> {
+
+            List<Step> steps = new ArrayList<>(element.getBackgroundSteps());
+            steps.addAll(element.getSteps());
+
+            steps.forEach(step -> {
                 ResultCount stepResultCount = stepResultCounts.getOrDefault(step, new ResultCount());
                 updateResultCount(stepResultCount, step.getStatus());
                 stepResultCounts.put(step, stepResultCount);

diff --git a/...a/com/trivago/cluecumber/engine/rendering/pages/renderering/AllScenariosPageRenderer.java b/...a/com/trivago/cluecumber/engine/rendering/pages/renderering/AllScenariosPageRenderer.java
@@ -122,13 +122,13 @@ public String getRenderedContentByStepFilter(
 
         AllScenariosPageCollection allScenariosPageCollectionClone = getAllScenariosPageCollectionClone(allScenariosPageCollection);
         allScenariosPageCollectionClone.setStepFilter(step);
-        allScenariosPageCollectionClone.getReports().forEach(report -> {
+        for (Report report : allScenariosPageCollectionClone.getReports()) {
             List<Element> elements = report.getElements()
                     .stream()
-                    .filter(element -> element.getSteps().contains(step))
+                    .filter(element -> element.getSteps().contains(step) || element.getBackgroundSteps().contains(step))
                     .collect(Collectors.toList());
             report.setElements(elements);
-        });
+        }
 
         addChartJsonToReportDetails(allScenariosPageCollectionClone);
         return processedContent(template, allScenariosPageCollectionClone, propertyManager.getNavigationLinks());

diff --git a/.../java/com/trivago/cluecumber/engine/rendering/pages/renderering/AllStepsPageRenderer.java b/.../java/com/trivago/cluecumber/engine/rendering/pages/renderering/AllStepsPageRenderer.java
@@ -82,15 +82,15 @@ public String getRenderedContent(
         return processedContent(template, allStepsPageCollection, propertyManager.getNavigationLinks());
     }
 
-    private void addChartJsonToReportDetails(final AllStepsPageCollection allTagsPageCollection) {
+    private void addChartJsonToReportDetails(final AllStepsPageCollection allStepsPageCollection) {
 
         List<Float> passed = new ArrayList<>();
         List<Float> failed = new ArrayList<>();
         List<Float> skipped = new ArrayList<>();
         Map<String, String> urlLookup = new HashMap<>();
 
         int maximumNumberOfRuns = 0;
-        for (Map.Entry<Step, ResultCount> entry : allTagsPageCollection.getStepResultCounts().entrySet()) {
+        for (Map.Entry<Step, ResultCount> entry : allStepsPageCollection.getStepResultCounts().entrySet()) {
             urlLookup.put(
                     entry.getKey().returnNameWithArgumentPlaceholders(),
                     Settings.PAGES_DIRECTORY + Settings.STEP_SCENARIO_PAGE_FRAGMENT +
@@ -104,7 +104,7 @@ private void addChartJsonToReportDetails(final AllStepsPageCollection allTagsPag
             }
         }
 
-        List<String> keys = allTagsPageCollection.getStepResultCounts()
+        List<String> keys = allStepsPageCollection.getStepResultCounts()
                 .keySet()
                 .stream()
                 .map(Step::returnNameWithArgumentPlaceholders)
@@ -113,7 +113,7 @@ private void addChartJsonToReportDetails(final AllStepsPageCollection allTagsPag
         Chart chart =
                 new StackedBarChartBuilder(chartConfiguration)
                         .setLabels(keys)
-                        .setxAxisLabel(allTagsPageCollection.getTotalNumberOfSteps() + " Steps")
+                        .setxAxisLabel(allStepsPageCollection.getTotalNumberOfSteps() + " Steps")
                         .setyAxisStepSize(maximumNumberOfRuns)
                         .setyAxisLabel("Number of Usages")
                         .addValues(passed, Status.PASSED)
@@ -122,7 +122,7 @@ private void addChartJsonToReportDetails(final AllStepsPageCollection allTagsPag
                         .build();
 
 
-        allTagsPageCollection.getReportDetails().setChartJson(convertChartToJson(chart));
-        allTagsPageCollection.getReportDetails().setChartUrlLookup(urlLookup);
+        allStepsPageCollection.getReportDetails().setChartJson(convertChartToJson(chart));
+        allStepsPageCollection.getReportDetails().setChartUrlLookup(urlLookup);
     }
 }
-Original file line number
+Diff line change
@@ Expand Up / @@ -15,6 +15,7 @@ @@
      */
     package com.trivago.cluecumber.engine.rendering.pages.charts.pojos;
+    import java.util.ArrayList;
     import java.util.List;
@@ Expand Down @@