JENKINS-72538 - SKIP status configurable when calculating all tests (#77)

* Deprecate criticalPassed and criticalFailed

* Deprecate more criticalPassed/Failed stuff

* Remove critical tests from the latest results table

* Add possibility to optionally include skip status when calculating pass
percentage

* Update readme to match current behaviour

* Update src/main/resources/hudson/plugins/robot/RobotPublisher/config.properties

Co-authored-by: Tatu Kairi <[email protected]>

* Update src/test/java/hudson/plugins/robot/model/RobotResultTest.java

Co-authored-by: Tatu Kairi <[email protected]>

* Update src/test/java/hudson/plugins/robot/model/RobotResultTest.java

Co-authored-by: Tatu Kairi <[email protected]>

* Update src/test/java/hudson/plugins/robot/tokens/RobotPassTokenMacroTest.java

Co-authored-by: Tatu Kairi <[email protected]>

* Update src/test/java/hudson/plugins/robot/tokens/RobotPassRatioTokenMacroTest.java

Co-authored-by: Tatu Kairi <[email protected]>

* Update src/test/java/hudson/plugins/robot/tokens/RobotPassPercentageTokenMacroTest.java

Co-authored-by: Tatu Kairi <[email protected]>

* Update src/test/java/hudson/plugins/robot/model/RobotResultTest.java

Co-authored-by: Tatu Kairi <[email protected]>

* Update src/main/resources/hudson/plugins/robot/RobotStep/config.properties

Co-authored-by: Tatu Kairi <[email protected]>

* Add unit tests to check new getPassPercentage behaviour

* Update doc/README.md

Co-authored-by: Tatu Kairi <[email protected]>

---------

Co-authored-by: Tatu Kairi <[email protected]>

Author: asimell

doc/README.md

@@ -178,22 +178,14 @@ and [here](https://content-security-policy.com/) how to change you CSP settings
 but be aware that **Changing CSP settings will potentially expose you Jenkins instance for
 security vulnerabilities**.
 
-### Robot Framework 4.x compatibility
+### Robot Framework 4.x+ compatibility
 
-The plugin supports both Robot Framework 3.x and 4.x output files. However, in order to support both, the plugin
-shows some extra information for both. [In Robot Framework 4.0 test criticality was removed and "SKIP" status was added](https://github.com/robotframework/robotframework/blob/master/doc/releasenotes/rf-4.0.rst). So for 3.x the
-results overview will show a `Skipped` column, which will always be 0 and for Robot Framework 4.x output files
-the `Critical tests` row will always be 0.
+:heavy_exclamation_mark: As we're preparing to drop support for RF 3.x, the `onlyCritical` flag has been deprecated and no
+longer has any effect. It will be removed in the future, but for now it's available for the transition period.
+A new flag `countSkippedTests` has been added to the pipeline step to allow users to choose whether to count skipped
+tests in the pass percentage calculation.
 
-Skipped tests aren't taken into account when calculating pass percentage, but they are calculated to the total
-amount of tests.
-
-Because criticality was removed in Robot Framework 4.0, having the `Use thresholds for critical tests only` checkbox
-checked will always result in a passing step (because pass percentage is always considered to be 100% when there are
-0 tests). In order to have set build status correctly, you **must** uncheck the checkbox or use `onlyCritical: false`
-in your pipeline when you call `robot`.
-
-We are planning of dropping support for Robot Framework 3.x and earlier some time in distant future.
+The plugin still supports RF 3.x, but no longer takes criticality into account. If you want to use RF 3.x with criticality, please downgrade to the last major version (4.0.0)
 
 ### Overall Screenshots
 

src/main/java/hudson/plugins/robot/AggregatedRobotAction.java

@@ -109,33 +109,34 @@ public static class AggregatedRobotResult extends RobotResult {
 
 		private static final long serialVersionUID = 1L;
 		private final transient AggregatedRobotAction parent;
-		private int passed, failed, skipped, criticalPassed, criticalFailed;
+		private int passed, failed, skipped;
 
 		public AggregatedRobotResult(AggregatedRobotAction parent) {
 			this.parent = parent;
 		}
 
 		public void addResult(RobotResult result) {
-			criticalFailed += result.getCriticalFailed();
-			criticalPassed += result.getCriticalPassed();
 			failed += result.getOverallFailed();
 			passed += result.getOverallPassed();
 			skipped += result.getOverallSkipped();
 		}
 
+		@Deprecated
 		@Override
 		public long getCriticalPassed() {
-			return criticalPassed;
+			return this.getOverallPassed();
 		}
 
+		@Deprecated
 		@Override
 		public long getCriticalFailed() {
-			return criticalFailed;
+			return this.getOverallFailed();
 		}
 
+		@Deprecated
 		@Override
 		public long getCriticalTotal() {
-			return criticalFailed + criticalPassed;
+			return this.getOverallTotal();
 		}
 
 		@Override

src/main/java/hudson/plugins/robot/RobotBuildAction.java

@@ -65,6 +65,8 @@ public class RobotBuildAction extends AbstractTestResultAction<RobotBuildAction>
 	private RobotResult result;
 	private String xAxisLabel;
 
+	private boolean countSkippedTests;
+
 	static {
 		XSTREAM.alias("result",RobotResult.class);
 		XSTREAM.alias("suite",RobotSuiteResult.class);
@@ -83,7 +85,8 @@ public class RobotBuildAction extends AbstractTestResultAction<RobotBuildAction>
 	 * @param enableCache Whether we want to enable caching or not
 	 */
 	public RobotBuildAction(Run<?, ?> build, RobotResult result,
-			String outputPath, TaskListener listener, String logFileLink, String logHtmlLink, boolean enableCache, String xAxisLabel) {
+			String outputPath, TaskListener listener, String logFileLink, String logHtmlLink, boolean enableCache, String xAxisLabel,
+							boolean countSkippedTests) {
 		super();
 		super.onAttached(build);
 		this.build = build;
@@ -92,6 +95,7 @@ public RobotBuildAction(Run<?, ?> build, RobotResult result,
 		this.logHtmlLink = logHtmlLink;
 		this.enableCache = enableCache;
 		this.xAxisLabel = xAxisLabel;
+		this.countSkippedTests = countSkippedTests;
 		setResult(result, listener);
 	}
 
@@ -333,4 +337,12 @@ public String getUrlName() {
 	public List<RobotCaseResult> getAllTests() {
 		return getResult().getAllCases();
 	}
+
+	public boolean isCountSkippedTests() {
+		return countSkippedTests;
+	}
+
+	public void setCountSkippedTests(boolean countSkippedTests) {
+		this.countSkippedTests = countSkippedTests;
+	}
 }

src/main/java/hudson/plugins/robot/RobotParser.java

@@ -293,7 +293,7 @@ private RobotCaseResult processTest(XMLStreamReader reader, RobotSuiteResult res
 			caseResult.setLogFile(this.logFileName);
 			//parse attributes
 			caseResult.setName(reader.getAttributeValue(null, "name"));
-			setCriticalityIfAvailable(reader, caseResult);
+			//setCriticalityIfAvailable(reader, caseResult);
 			caseResult.setId(reader.getAttributeValue(null, "id"));
 			//parse test tags
 			caseResult.setDescription("");
@@ -346,7 +346,7 @@ private RobotCaseResult processTest(XMLStreamReader reader, RobotSuiteResult res
 			if (schemaVersion >= 5) {
 				caseResult.setElapsedTime(reader.getAttributeValue(null, elapsedLocalName));
 			}
-			setCriticalityIfAvailable(reader, caseResult);
+//			setCriticalityIfAvailable(reader, caseResult);
 			while(reader.hasNext()){
 				reader.next();
 				if(reader.isCharacters()){