Add a list of all Skipped tests to the Test Result report #106
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,18 +1,18 @@ | ||
| /* | ||
| * The MIT License | ||
| * | ||
| * Copyright (c) 2004-2009, Sun Microsystems, Inc., Kohsuke Kawaguchi, Daniel Dyer, Seiji Sogabe, Tom Huybrechts, Yahoo!, Inc. | ||
| * | ||
| * Permission is hereby granted, free of charge, to any person obtaining a copy | ||
| * of this software and associated documentation files (the "Software"), to deal | ||
| * in the Software without restriction, including without limitation the rights | ||
| * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell | ||
| * copies of the Software, and to permit persons to whom the Software is | ||
| * furnished to do so, subject to the following conditions: | ||
| * | ||
| * The above copyright notice and this permission notice shall be included in | ||
| * all copies or substantial portions of the Software. | ||
| * | ||
| * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR | ||
| * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, | ||
| * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE | ||
|
|
@@ -96,6 +96,13 @@ public class CaseResult extends TestResult implements Comparable<CaseResult> { | |
| */ | ||
| private /*final*/ int failedSince; | ||
|
|
||
| /** | ||
| * This test has been skipped since this build number (not id.) | ||
| * | ||
| * If {@link #isPassed() passing}, this field is left unused to 0. | ||
| */ | ||
| private /*final*/ int skippedSince; | ||
|
|
||
| private static float parseTime(Element testCase) { | ||
| String time = testCase.attributeValue("time"); | ||
| return new TimeToFloat(time).parse(); | ||
|
|
@@ -209,7 +216,7 @@ private static int halfMaxSize(Collection<CaseResult> results) { | |
| public CaseResult(SuiteResult parent, String testName, String errorStackTrace) { | ||
| this(parent, testName, errorStackTrace, ""); | ||
| } | ||
|
|
||
| public CaseResult(SuiteResult parent, String testName, String errorStackTrace, String errorDetails) { | ||
| this.className = parent == null ? "unnamed" : parent.getName(); | ||
| this.testName = testName; | ||
|
|
@@ -222,7 +229,7 @@ public CaseResult(SuiteResult parent, String testName, String errorStackTrace, S | |
| this.skipped = false; | ||
| this.skippedMessage = null; | ||
| } | ||
|
|
||
| public ClassResult getParent() { | ||
| return classResult; | ||
| } | ||
|
|
@@ -363,12 +370,12 @@ public String getPackageName() { | |
| if(idx<0) return "(root)"; | ||
| else return className.substring(0,idx); | ||
| } | ||
|
|
||
| @Override | ||
| public String getFullName() { | ||
| return className+'.'+getName(); | ||
| } | ||
|
|
||
| /** | ||
| * @since 1.515 | ||
| */ | ||
|
|
@@ -397,26 +404,52 @@ public int getPassCount() { | |
| */ | ||
| @Exported(visibility=9) | ||
| public int getFailedSince() { | ||
| // If we haven't calculated failedSince yet, and we should, do it now. | ||
| if (failedSince==0 && getFailCount()==1) { | ||
| CaseResult prev = getPreviousResult(); | ||
| if(prev!=null && !prev.isPassed()) { | ||
| this.failedSince = prev.getFailedSince(); | ||
| } | ||
| else if (getRun() != null) { | ||
| this.failedSince = getRun().getNumber(); | ||
| } else { | ||
| LOGGER.warning("trouble calculating getFailedSince. We've got prev, but no owner."); | ||
| // failedSince will be 0, which isn't correct. | ||
| } | ||
| } | ||
| return failedSince; | ||
| } | ||
|
|
||
| /** | ||
| * If this test was skipped, then return the build number | ||
| * when this test was first skipped. | ||
| */ | ||
| @Exported(visibility=9) | ||
|
There was a problem hiding this comment. Shouldn't the visibility of the skipped tests be higher than the visibility of failed tests? There was a problem hiding this comment. On second thought I think the visibility for getFailedSince and getSkippedSince should be identical. |
||
| public int getSkippedSince() { | ||
| // If we haven't calculated skippedSince yet, and we should, do it now. | ||
| if (skippedSince==0 && getSkipCount()==1) { | ||
| CaseResult prev = getPreviousResult(); | ||
| if(prev!=null && prev.isSkipped()){ | ||
| this.skippedSince = prev.getSkippedSince(); | ||
|
There was a problem hiding this comment. What happens if in There was a problem hiding this comment. You're absolutely right - when a test becomes skipped, its skippedSince counter is set to the current build number.
There was a problem hiding this comment. Unlike failedSince, skippedSince is not available in XML, so this could recursively load all the skipped builds leading to StackOverflowError like JENKINS-31660 . IMHO it would be safer to save skippedSince in XML too and assume current build is the first skipped one if no information is found in XML, avoiding recursion. (Related to #117.) There was a problem hiding this comment. Good catch! I will review this and suggest a fix There was a problem hiding this comment. @zbynek I've given this some thought.. as I understand we only parse the XML and have no control over its contents. Do you see any other way to protect against a StackOverflowError ? |
||
| } | ||
| else if (getRun() != null) { | ||
| this.skippedSince = getRun().getNumber(); | ||
| } else { | ||
| LOGGER.warning("trouble calculating getSkippedSince. We've got prev, but no owner."); | ||
| // skippedSince will be 0, which isn't correct. | ||
| } | ||
| } | ||
| return skippedSince; | ||
| } | ||
|
|
||
| public Run<?,?> getFailedSinceRun() { | ||
| return getRun().getParent().getBuildByNumber(getFailedSince()); | ||
| } | ||
|
|
||
| public Run<?,?> getSkippedSinceRun() { | ||
| return getRun().getParent().getBuildByNumber(getSkippedSince()); | ||
| } | ||
|
|
||
| /** | ||
| * Gets the number of consecutive builds (including this) | ||
| * that this test case has been failing. | ||
|
|
@@ -427,12 +460,14 @@ public Run<?,?> getFailedSinceRun() { | |
| public int getAge() { | ||
| if(isPassed()) | ||
| return 0; | ||
| else if (isFailed() && getRun() != null) | ||
| return getRun().getNumber()-getFailedSince()+1; | ||
| else if (isSkipped() && getRun() != null) { | ||
| return getRun().getNumber()-getSkippedSince()+1; | ||
| } else { | ||
| LOGGER.fine("Trying to get age of a CaseResult without an owner"); | ||
| return 0; | ||
| } | ||
| } | ||
|
|
||
| /** | ||
|
|
@@ -452,7 +487,7 @@ else if (getRun() != null) { | |
| public String getStdout() { | ||
| if(stdout!=null) return stdout; | ||
| SuiteResult sr = getSuiteResult(); | ||
| if (sr==null) return ""; | ||
| return getSuiteResult().getStdout(); | ||
| } | ||
|
|
||
|
|
@@ -477,7 +512,7 @@ public CaseResult getPreviousResult() { | |
| if(pr==null) return null; | ||
| return pr.getCase(getTransformedTestName()); | ||
| } | ||
|
|
||
| /** | ||
| * Case results have no children | ||
| * @return null | ||
|
|
@@ -486,7 +521,7 @@ public CaseResult getPreviousResult() { | |
| public TestResult findCorrespondingResult(String id) { | ||
| if (id.equals(safe(getName()))) { | ||
| return this; | ||
| } | ||
| return null; | ||
| } | ||
|
|
||
|
|
@@ -560,7 +595,7 @@ public boolean isPassed() { | |
| public boolean isSkipped() { | ||
| return skipped; | ||
| } | ||
|
|
||
| /** | ||
| * @return true if the test was not skipped and did not pass, false otherwise. | ||
| * @since 1.520 | ||
|
|
@@ -629,7 +664,7 @@ public Run<?,?> getRun() { | |
|
|
||
| public void setParentSuiteResult(SuiteResult parent) { | ||
| this.parent = parent; | ||
| } | ||
|
|
||
| public void freeze(SuiteResult parent) { | ||
| this.parent = parent; | ||
|
|
@@ -689,7 +724,7 @@ public Status getStatus() { | |
| /*package*/ void setClass(ClassResult classResult) { | ||
| this.classResult = classResult; | ||
| } | ||
|
|
||
| void replaceParent(SuiteResult parent) { | ||
| this.parent = parent; | ||
| } | ||
|
|
||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
shouldn't
/*final*/be removed?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks - removing