[FIXED JENKINS-30751] Use same logic everywhere to say whether a test…

… is a failure or not
jenkinsci · Oct 9, 2015 · a1da20b · a1da20b
1 parent 96365ab
commit a1da20b
Show file tree

Hide file tree

Showing 4 changed files with 49 additions and 48 deletions.
diff --git a/TODO.txt b/TODO.txt
@@ -0,0 +1,5 @@
+- fix performance issues
+- TapTestResultResult and TapResult could be merged?
+- look for code duplication
+- remove TapStreamResult#getFailedTests2()
+- look for open tasks
diff --git a/src/main/java/org/tap4j/plugin/TapResult.java b/src/main/java/org/tap4j/plugin/TapResult.java
@@ -42,7 +42,6 @@
 import org.tap4j.consumer.TapConsumerFactory;
 import org.tap4j.model.BailOut;
 import org.tap4j.model.Comment;
-import org.tap4j.model.Directive;
 import org.tap4j.model.Plan;
 import org.tap4j.model.TestResult;
 import org.tap4j.model.TestSet;
@@ -52,8 +51,6 @@
 import org.tap4j.plugin.util.Constants;
 import org.tap4j.plugin.util.DiagnosticUtil;
 import org.tap4j.plugin.util.Util;
-import org.tap4j.util.DirectiveValues;
-import org.tap4j.util.StatusValues;
 
 import hudson.FilePath;
 import hudson.model.AbstractBuild;
@@ -178,9 +175,9 @@ public void tally() {
 				this.skipped += testResults.size();
 			} else {
 				for (TestResult testResult : testResults) {
-					if (isSkipped(testResult)) {
+					if (Util.isSkipped(testResult)) {
 						skipped += 1;
-					} else if (isFailure(testResult)) {
+					} else if (Util.isFailure(testResult, todoIsFailure)) {
 						failed += 1;
 					} else {
 						passed += 1;
@@ -256,30 +253,6 @@ public float getDuration() {
         return this.duration;
     }
 
-	private boolean isSkipped(TestResult testResult) {
-		boolean r = false;
-		Directive directive = testResult.getDirective();
-		if (directive != null
-				&& directive.getDirectiveValue() == DirectiveValues.SKIP) {
-			r = true;
-		}
-		return r;
-	}
-
-	private boolean isFailure(TestResult testResult) {
-		boolean r = false;
-		Directive directive = testResult.getDirective();
-		StatusValues status = testResult.getStatus();
-		if (directive != null) {
-			if(directive.getDirectiveValue() == DirectiveValues.TODO && todoIsFailure != null && true == todoIsFailure) {
-				r = true;
-			}
-		} else if (status != null && status == StatusValues.NOT_OK) {
-			r = true;
-		}
-		return r;
-	}
-
 	/**
 	 * Called from TapResult/index.jelly
 	 */

diff --git a/src/main/java/org/tap4j/plugin/model/TapTestResultResult.java b/src/main/java/org/tap4j/plugin/model/TapTestResultResult.java
@@ -23,13 +23,6 @@
  */
 package org.tap4j.plugin.model;
 
-import hudson.Functions;
-import hudson.model.Item;
-import hudson.model.AbstractBuild;
-import hudson.tasks.test.AbstractTestResultAction;
-import hudson.tasks.test.TestObject;
-import hudson.tasks.test.TestResult;
-
 import java.io.StringWriter;
 import java.io.UnsupportedEncodingException;
 import java.net.URLEncoder;
@@ -38,17 +31,23 @@
 import java.util.Map;
 import java.util.logging.Logger;
 
-import jenkins.model.Jenkins;
-
 import org.apache.commons.lang.StringUtils;
 import org.kohsuke.stapler.Stapler;
 import org.kohsuke.stapler.StaplerRequest;
 import org.tap4j.model.Comment;
 import org.tap4j.model.Directive;
 import org.tap4j.model.TestSet;
 import org.tap4j.plugin.TapResult;
+import org.tap4j.plugin.util.Util;
 import org.tap4j.util.DirectiveValues;
-import org.tap4j.util.StatusValues;
+
+import hudson.Functions;
+import hudson.model.AbstractBuild;
+import hudson.model.Item;
+import hudson.tasks.test.AbstractTestResultAction;
+import hudson.tasks.test.TestObject;
+import hudson.tasks.test.TestResult;
+import jenkins.model.Jenkins;
 
 /**
  * 
@@ -139,22 +138,18 @@ public String getName() {
 	}
 
 	public String getStatus() {
-		return this.tapTestResult.getStatus() == StatusValues.OK ? "OK" : "NOT OK";
+		boolean failure = Util.isFailure(this.tapTestResult, todoIsFailure);
+		return failure ? "NOT OK" : "OK";
 	}
 
 	public String getSkip() {
-		String skip = "No";
-		Directive directive = this.tapTestResult.getDirective();
-		if(directive != null) {
-			if(directive.getDirectiveValue() == DirectiveValues.SKIP) {
-				skip = "Yes";
-			}
-		}
-		return skip;
+		boolean skip = Util.isSkipped(this.tapTestResult);
+		return skip ? "Yes" : "No";
 	}
 
 	public String getTodo() {
 		String todo = "No";
+		// TODO: not consistent with the other methods in TapResult
 		Directive directive = this.tapTestResult.getDirective();
 		if(directive != null) {
 			if(directive.getDirectiveValue() == DirectiveValues.TODO) {

diff --git a/src/main/java/org/tap4j/plugin/util/Util.java b/src/main/java/org/tap4j/plugin/util/Util.java
@@ -23,6 +23,11 @@
  */
 package org.tap4j.plugin.util;
 
+import org.tap4j.model.Directive;
+import org.tap4j.model.TestResult;
+import org.tap4j.util.DirectiveValues;
+import org.tap4j.util.StatusValues;
+
 /**
  * Utility methods used by tap-plugin.
  */
@@ -54,4 +59,27 @@ public static String normalizeFolders(String workspace, String relative) {
 		return relative;
 	}
 
+	public static boolean isSkipped(TestResult testResult) {
+		boolean r = false;
+		Directive directive = testResult.getDirective();
+		if (directive != null
+				&& directive.getDirectiveValue() == DirectiveValues.SKIP) {
+			r = true;
+		}
+		return r;
+	}
+
+	public static boolean isFailure(TestResult testResult, Boolean todoIsFailure) {
+		boolean r = false;
+		Directive directive = testResult.getDirective();
+		StatusValues status = testResult.getStatus();
+		if (directive != null) {
+			if(directive.getDirectiveValue() == DirectiveValues.TODO && todoIsFailure != null && true == todoIsFailure) {
+				r = true;
+			}
+		} else if (status != null && status == StatusValues.NOT_OK) {
+			r = true;
+		}
+		return r;
+	}
 }