Skip to content
Permalink
Browse files

[FIXED JENKINS-23945] Test result trend graph should not load builds …

…which were not already loaded.

(cherry picked from commit 0449883)

Conflicts:
	changelog.html
  • Loading branch information
jglick authored and olivergondza committed Sep 7, 2014
1 parent 784e91b commit 8d8102036f4e52aada39db37c05025b9bb31516d
Showing with 6 additions and 4 deletions.
  1. +6 −4 core/src/main/java/hudson/tasks/test/AbstractTestResultAction.java
@@ -33,6 +33,7 @@
import java.util.Collections;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.concurrent.ConcurrentHashMap;
import jenkins.model.RunAction2;
import org.jfree.chart.ChartFactory;
@@ -167,13 +168,14 @@ public Api getApi() {
* Gets the test result of the previous build, if it's recorded, or null.
*/
public T getPreviousResult() {
return (T)getPreviousResult(getClass());
return (T)getPreviousResult(getClass(), true);
}

private <U extends AbstractTestResultAction> U getPreviousResult(Class<U> type) {
private <U extends AbstractTestResultAction> U getPreviousResult(Class<U> type, boolean eager) {
Set<Integer> loadedBuilds = eager ? null : owner.getProject()._getRuns().getLoadedBuilds().keySet();
AbstractBuild<?,?> b = owner;
while(true) {
b = b.getPreviousBuild();
b = eager || loadedBuilds.contains(b.number - /* assuming there are no gaps */1) ? b.getPreviousBuild() : null;

This comment has been minimized.

Copy link
@jtnord

jtnord Sep 23, 2014

Member

There will be gaps as jenkins will if current builds are unstable always keep the last stable around.

100 -> stable
122 -> unstable
123 -> unstable
124 -> unstable
125 -> unstable.

All will be shown by the history (with a log rotator or 5 builds) - yet the stable version will not be returned here - causing the trending to be broken)

(unless I have read this incorrectly)
I would maybe guess that this build should have a getPreviousBuildNumber() function.

This comment has been minimized.

Copy link
@jglick

jglick Sep 23, 2014

Author Member

Yes, in such a case the test result for the old stable build will not be shown in the graph. I consider this a lesser evil compared to taking the server down, though.

Unfortunately an efficient getPreviousBuildNumber would be tricky to write at the moment, though not impossible. #1379 would probably make it easier. Either way some new APIs would need to be developed which are probably not appropriate for an LTS line.

if(b==null)
return null;
U r = b.getAction(type);
@@ -256,7 +258,7 @@ private CategoryDataset buildDataSet(StaplerRequest req) {

DataSetBuilder<String,NumberOnlyBuildLabel> dsb = new DataSetBuilder<String,NumberOnlyBuildLabel>();

for( AbstractTestResultAction<?> a=this; a!=null; a=a.getPreviousResult(AbstractTestResultAction.class) ) {
for (AbstractTestResultAction<?> a = this; a != null; a = a.getPreviousResult(AbstractTestResultAction.class, false)) {
dsb.add( a.getFailCount(), "failed", new NumberOnlyBuildLabel(a.owner));
if(!failureOnly) {
dsb.add( a.getSkipCount(), "skipped", new NumberOnlyBuildLabel(a.owner));

3 comments on commit 8d81020

@jtnord

This comment has been minimized.

Copy link
Member

@jtnord jtnord replied Sep 24, 2014

Another issue looming is for those that just embed the Image in a different portal (outside of Jenkins)

e.g. ${JENKINS_URL}/job/commit/test/trend

If I understand correctly you may not get anything here as no builds may be loaded as you are not browsing using the job page which will not have loaded the state of previous jobs to show in the UI...

I can;t help feeling it would be simpler to always load the runs and to have a limit on the number that are loaded by this graph. - e.g. I want my trend graph to show the last 10 builds (assuming there are 140).

@jglick

This comment has been minimized.

Copy link
Member Author

@jglick jglick replied Sep 24, 2014

you may not get anything here as no builds may be loaded

This is also possible, though probably unlikely since just browsing a list view including the job forces the last five or six builds to be loaded.

BTW if you visit the job index page, you are almost guaranteed to get at least the last thirty or so builds (assuming they form a contiguous version range), as the build history widget loads them.

@jtnord

This comment has been minimized.

Copy link
Member

@jtnord jtnord replied Oct 28, 2014

as I thought this is not unlikely and has caused a definite regression and a major loss of functionality

see https://issues.jenkins-ci.org/browse/JENKINS-25340

Please sign in to comment.
You can’t perform that action at this time.