Drop support for test criticality (#83)

* Remove criticality support

* Keep current behaviour in token macro as default

* doc/README.md: clarify plugin versions and RF compatibility

---------

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

Author: asimell

doc/README.md

@@ -64,8 +64,8 @@ Tracker](https://issues.jenkins-ci.org/issues/?jql=project+%3D+JENKINS+AND+compo
     persist for viewing later in the logs you must configure them under
     "Advanced... -\> Other files to copy". The field accepts comma
     separated list of files and supports wildcards `*` and `?`.
-9.  Set thresholds and optionally disable thresholds for critical tests
-    only to count every test in the pass percentage.
+9.  Set thresholds and optionally include thresholds for skipped tests
+    to count every test in the pass percentage.
 
 ### Pipeline configuration
 
@@ -86,8 +86,6 @@ Tracker](https://issues.jenkins-ci.org/issues/?jql=project+%3D+JENKINS+AND+compo
    )
     ```
 
-**NOTE!** `onlyCritical` parameter has been deprecated and will be removed in the future. It has no effect on the results.
-
 ### Configuring direct links to log files
 
 #### Version 1.3.0+
@@ -142,8 +140,9 @@ Prerequisites: token-macro plugin and email-ext plugin installed.
     -   `${ROBOT_PASSPERCENTAGE, countSkippedTests}` - Expands to pass percentage
         of tests. (passed / total \* 100 %).
         `countSkippedTests` is an optional parameter that can be used to include skipped tests in the total count. Default value is `false`.
-    -   `${ROBOT_PASSRATIO}` - Expands to build result in 'passed
-        / total' format. This total includes skipped test cases.
+    -   `${ROBOT_PASSRATIO, countSkippedTests}` - Expands to build result in 'passed
+        / total' format.
+        `countSkippedTests` is an optional parameter that can be used to include skipped tests in the total count. Default value is `True`.
     -   `${ROBOT_PASSED}` - Expands to count of passed cases.
     -   `${ROBOT_REPORTLINK}` - If logfile link is configured in the Robot
         plugin this link will point to that file for the build. Else show
@@ -174,12 +173,12 @@ security vulnerabilities**.
 
 ## Robot Framework 4.x+ compatibility
 
-: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.
 
-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)
+## Robot Framework <3.0 compatibility
+
+If you want to use RF 3.x with criticality, please downgrade to the major version 4.0.0. Plugin versions 5.x support RF 3.x as well, but no longer takes criticality into account.
 
 ## Overall Screenshots
 

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

@@ -61,10 +61,6 @@ public double getOverallPassPercentage() {
 		return aggregatedResult.getPassPercentage(false);
 	}
 
-	public double getCriticalPassPercentage() {
-		return aggregatedResult.getPassPercentage(true);
-	}
-
 	public String getIconFileName() {
 		return "/plugin/robot/robot.png";
 	}
@@ -100,7 +96,6 @@ public void doGraph(StaplerRequest2 req, StaplerResponse2 rsp)
 				Boolean.parseBoolean(req.getParameter("zoomSignificant")),
 				false, Boolean.parseBoolean(req.getParameter("hd")),
 				Boolean.parseBoolean(req.getParameter("failedOnly")),
-				Boolean.parseBoolean(req.getParameter("criticalOnly")),
 				labelFormat,
 				Integer.parseInt(req.getParameter("maxBuildsToShow")));
 		g.doPng(req, rsp);
@@ -123,24 +118,6 @@ public void addResult(RobotResult result) {
 			skipped += result.getOverallSkipped();
 		}
 
-		@Deprecated
-		@Override
-		public long getCriticalPassed() {
-			return this.getOverallPassed();
-		}
-
-		@Deprecated
-		@Override
-		public long getCriticalFailed() {
-			return this.getOverallFailed();
-		}
-
-		@Deprecated
-		@Override
-		public long getCriticalTotal() {
-			return this.getOverallTotal();
-		}
-
 		@Override
 		public long getOverallPassed(){
 			return passed;

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

@@ -200,10 +200,10 @@ public double getOverallPassPercentage(){
 	}
 
 	/**
-	 * Get ratio of passed tests per total tests. Accounts for only critical tests run.
+	 * Get ratio of passed tests per total tests. Accounts for all tests run and skipped.
 	 * @return percent number
 	 */
-	public double getCriticalPassPercentage() {
+	public double getPassPercentageWithSkipped() {
 		return getResult().getPassPercentage(true);
 	}
 
@@ -282,7 +282,6 @@ public void doGraph(StaplerRequest2 req, StaplerResponse2 rsp)
 				Boolean.parseBoolean(req.getParameter("zoomSignificant")), false,
 				Boolean.parseBoolean(req.getParameter("hd")),
 				Boolean.parseBoolean(req.getParameter("failedOnly")),
-				Boolean.parseBoolean(req.getParameter("criticalOnly")),
 				labelFormat,
 				Integer.parseInt(maxBuildsReq));
 		g.doPng(req, rsp);

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

@@ -287,7 +287,6 @@ private RobotCaseResult processTest(XMLStreamReader reader, RobotSuiteResult res
 			caseResult.setLogFile(this.logFileName);
 			//parse attributes
 			caseResult.setName(reader.getAttributeValue(null, "name"));
-			//setCriticalityIfAvailable(reader, caseResult);
 			caseResult.setId(reader.getAttributeValue(null, "id"));
 			//parse test tags
 			caseResult.setDescription("");
@@ -340,7 +339,6 @@ private RobotCaseResult processTest(XMLStreamReader reader, RobotSuiteResult res
 			if (schemaVersion >= 5) {
 				caseResult.setElapsedTime(reader.getAttributeValue(null, elapsedLocalName));
 			}
-//			setCriticalityIfAvailable(reader, caseResult);
 			while(reader.hasNext()){
 				reader.next();
 				if(reader.isCharacters()){
@@ -574,13 +572,6 @@ private String processReturnBreakContinue(XMLStreamReader reader, int nestedCoun
 			return stringBuilder.toString();
 		}
 
-		private static void setCriticalityIfAvailable(XMLStreamReader reader, RobotCaseResult caseResult) {
-			String criticality = reader.getAttributeValue(null, "critical");
-			if (criticality != null) {
-				caseResult.setCritical(criticality.equals("yes"));
-			}
-		}
-
 		@Override
 		public void checkRoles(RoleChecker roleChecker) throws SecurityException {
 

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

@@ -95,7 +95,6 @@ public void doGraph(StaplerRequest2 req, StaplerResponse2 rsp)
 			+ "/graph?zoomSignificant="+Boolean.valueOf(req.getParameter("zoomSignificant"))
 			+ "&hd="+Boolean.valueOf(req.getParameter("hd"))
 			+ "&failedOnly="+Boolean.valueOf(req.getParameter("failedOnly"))
-			+ "&criticalOnly="+Boolean.valueOf(req.getParameter("criticalOnly"))
 			+ "&maxBuildsToShow="+Integer.valueOf(req.getParameter("maxBuildsToShow")));
 
 	}

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

@@ -73,7 +73,6 @@ public class RobotPublisher extends Recorder implements Serializable,
     final private boolean enableCache;
 
     //Default to true
-    private boolean onlyCritical = true;
     private boolean countSkippedTests = false;
 
     /**
@@ -87,15 +86,14 @@ public class RobotPublisher extends Recorder implements Serializable,
      * @param logFileName          Name of Robot log html
      * @param passThreshold        Threshold of test pass percentage for successful builds
      * @param unstableThreshold    Threshold of test pass percentage for unstable builds
-     * @param onlyCritical         True if only critical tests are included in pass percentage
      * @param otherFiles           Other files to be saved
      * @param enableCache          True if caching is used
      */
     @DataBoundConstructor
     public RobotPublisher(String archiveDirName, String outputPath, String outputFileName,
                           boolean disableArchiveOutput, String reportFileName, String logFileName,
                           double passThreshold, double unstableThreshold,
-                          boolean onlyCritical, boolean countSkippedTests, String otherFiles, boolean enableCache, String overwriteXAxisLabel) {
+                          boolean countSkippedTests, String otherFiles, boolean enableCache, String overwriteXAxisLabel) {
         this.archiveDirName = archiveDirName;
         this.outputPath = outputPath;
         this.outputFileName = outputFileName;
@@ -104,7 +102,6 @@ public RobotPublisher(String archiveDirName, String outputPath, String outputFil
         this.passThreshold = passThreshold;
         this.unstableThreshold = unstableThreshold;
         this.logFileName = logFileName;
-        this.onlyCritical = onlyCritical;
         this.countSkippedTests = countSkippedTests;
         this.enableCache = enableCache;
         this.overwriteXAxisLabel = overwriteXAxisLabel;
@@ -202,15 +199,6 @@ public double getUnstableThreshold() {
         return unstableThreshold;
     }
 
-    /**
-     * Gets if only critical tests should be accounted for the thresholds.
-     *
-     * @return true if only critical tests should be accounted for the thresholds
-     */
-    public boolean getOnlyCritical() {
-        return onlyCritical;
-    }
-
     /**
      * Gets if skipped tests should be counted in the thresholds.
      *
@@ -274,7 +262,6 @@ public void perform(Run<?, ?> build, @NonNull FilePath workspace, @NonNull EnvVa
         if (build.getResult() != Result.ABORTED) {
             PrintStream logger = listener.getLogger();
             logger.println(Messages.robot_publisher_started());
-            logger.println(Messages.robot_publisher_only_critical());
             logger.println(Messages.robot_publisher_parsing());
             RobotResult result;
 

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

@@ -37,7 +37,6 @@ public class RobotStep extends Step {
 	private double unstableThreshold;
 	private @CheckForNull String[] otherFiles;
 	private boolean enableCache = true;
-	private boolean onlyCritical = true;
 	private boolean countSkippedTests = false;
 	private @CheckForNull String overwriteXAxisLabel;
 
@@ -92,10 +91,6 @@ public boolean getEnableCache() {
 		return this.enableCache;
 	}
 
-	public boolean getOnlyCritical() {
-		return this.onlyCritical;
-	}
-
 	public boolean getCountSkippedTests() { return this.countSkippedTests; }
 
 	public String getOverwriteXAxisLabel() {
@@ -142,11 +137,6 @@ public void setEnableCache(boolean enableCache) {
 		this.enableCache = enableCache;
 	}
 
-	@DataBoundSetter
-	public void setOnlyCritical(boolean onlyCritical) {
-		this.onlyCritical = onlyCritical;
-	}
-
 	@DataBoundSetter
 	public void setCountSkippedTests(boolean countSkippedTests) {
 		this.countSkippedTests = countSkippedTests;

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

@@ -29,7 +29,7 @@ public class RobotStepExecution extends SynchronousNonBlockingStepExecution<Void
     @Override protected Void run() throws Exception {
     	FilePath workspace = getContext().get(FilePath.class);
         workspace.mkdirs();
-    	RobotPublisher rp = new RobotPublisher(step.getArchiveDirName(), step.getOutputPath(), step.getOutputFileName(), step.getDisableArchiveOutput(), step.getReportFileName(), step.getLogFileName(), step.getPassThreshold(), step.getUnstableThreshold(), step.getOnlyCritical(), step.getCountSkippedTests(), step.getOtherFiles(), step.getEnableCache(), step.getOverwriteXAxisLabel());
+    	RobotPublisher rp = new RobotPublisher(step.getArchiveDirName(), step.getOutputPath(), step.getOutputFileName(), step.getDisableArchiveOutput(), step.getReportFileName(), step.getLogFileName(), step.getPassThreshold(), step.getUnstableThreshold(), step.getCountSkippedTests(), step.getOtherFiles(), step.getEnableCache(), step.getOverwriteXAxisLabel());
     	rp.perform(getContext().get(Run.class), workspace, getContext().get(EnvVars.class), getContext().get(Launcher.class), getContext().get(TaskListener.class));
     	return null;
     }

src/main/java/hudson/plugins/robot/graph/RobotGraphHelper.java

@@ -36,7 +36,6 @@ public class RobotGraphHelper {
 	 * Create a test result trend graph. The graph will ignore builds with no robot results.
 	 * @param significantData True if graph has significant data
 	 * @param binarydata Binary data
-	 * @param criticalOnly True if graph only critical tests
 	 * @param hd True if you want a larger image
 	 * @param labelFormat Label format
 	 * @param rootObject The dataset will be taken from rootObject backwards.
@@ -51,7 +50,6 @@ public static RobotGraph createTestResultsGraphForTestObject(RobotTestObject roo
 																 boolean binarydata,
 																 boolean hd,
 																 boolean failedOnly,
-																 boolean criticalOnly,
 																 String labelFormat,
 																 int maxBuildsToShow) {
 		List<Number> values = new ArrayList<>();
@@ -65,14 +63,14 @@ public static RobotGraph createTestResultsGraphForTestObject(RobotTestObject roo
 			 testObject != null && buildsLeftToShow != 0;
 			 testObject = testObject.getPreviousResult(), buildsLeftToShow--)
 		{
-			Number failed =  !criticalOnly ? testObject.getFailed() : testObject.getCriticalFailed();
+			Number failed =  testObject.getFailed();
 			Number passed = 0;
 			Number skipped = 0;
 			int compareLowerBoundTo;
 			if ( failedOnly) {
 			    compareLowerBoundTo = failed.intValue();
 			} else {
-			    passed = !criticalOnly ? testObject.getPassed() : testObject.getCriticalPassed();
+			    passed = testObject.getPassed();
 			    skipped = testObject.getSkipped();
 			    compareLowerBoundTo = passed.intValue();
 			}

src/main/java/hudson/plugins/robot/model/RobotCaseComparator.java

@@ -29,11 +29,6 @@ public int compare(RobotCaseResult result1, RobotCaseResult result2) {
 		if (!result1.isPassed()) {
 			if (result2.isPassed())
 				return -1;
-			if (result1.isCritical()) {
-				if (!result2.isCritical())
-					return -1;
-			} else if (result2.isCritical())
-				return 1;
 		} else if (!result2.isPassed())
 			return 1;
 		return result1.getRelativePackageName(result1).compareTo(result2.getRelativePackageName(result2));