Skip to content
Permalink
Browse files

[JENKINS-42319] - Fix broken compareTo method of Run (#2762)

* JENKINS-42319: Fix broken compareTo method of Run

Previous implementation relied on fact that users always compare run
objects with same parent. But this is not always the case.
So, it was not safe to put runs from different parents to TreeSet, i.e.

* Make comparasion more performance friendly
  • Loading branch information...
Jimilian authored and oleg-nenashev committed Mar 18, 2017
1 parent 4fd4bd3 commit e8efc96f3d4ce3ed6dc121661423ad4a2892e2b2
Showing with 53 additions and 2 deletions.
  1. +6 −1 core/src/main/java/hudson/model/Run.java
  2. +47 −1 core/src/test/java/hudson/model/RunTest.java
@@ -413,9 +413,14 @@ public void addAction(@Nonnull Action a) {

/**
* Ordering based on build numbers.
* If numbers are equal order based on names of parent projects.
*/
public int compareTo(@Nonnull RunT that) {
return this.number - that.number;
final int res = this.number - that.number;
if (res == 0)
return this.getParent().getFullName().compareTo(that.getParent().getFullName());

return res;
}

/**
@@ -25,12 +25,13 @@
package hudson.model;

import java.io.IOException;
import hudson.model.Run.Artifact;
import java.io.File;
import java.io.PrintWriter;
import java.util.List;
import java.util.Locale;
import java.util.Set;
import java.util.TimeZone;
import java.util.TreeSet;
import java.util.concurrent.Callable;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;
@@ -212,4 +213,49 @@ public void getLogReturnsAllLines() throws Exception {
assertEquals("", logLines.get(2));
assertEquals("c3", logLines.get(3));
}

@Test
public void compareRunsFromSameJobWithDifferentNumbers() throws Exception {
final ItemGroup group = Mockito.mock(ItemGroup.class);
final Job j = Mockito.mock(Job.class);

Mockito.when(j.getParent()).thenReturn(group);
Mockito.when(group.getFullName()).thenReturn("j");
Mockito.when(j.assignBuildNumber()).thenReturn(1, 2);

Run r1 = new Run(j) {};
Run r2 = new Run(j) {};

final Set<Run> treeSet = new TreeSet<>();
treeSet.add(r1);
treeSet.add(r2);

assertTrue(r1.compareTo(r2) < 0);
assertTrue(treeSet.size() == 2);
}

@Issue("JENKINS-42319")
@Test
public void compareRunsFromDifferentParentsWithSameNumber() throws Exception {
final ItemGroup group1 = Mockito.mock(ItemGroup.class);
final ItemGroup group2 = Mockito.mock(ItemGroup.class);
final Job j1 = Mockito.mock(Job.class);
final Job j2 = Mockito.mock(Job.class);
Mockito.when(j1.getParent()).thenReturn(group1);
Mockito.when(j2.getParent()).thenReturn(group2);
Mockito.when(group1.getFullName()).thenReturn("g1");
Mockito.when(group2.getFullName()).thenReturn("g2");
Mockito.when(j1.assignBuildNumber()).thenReturn(1);
Mockito.when(j2.assignBuildNumber()).thenReturn(1);

Run r1 = new Run(j1) {};
Run r2 = new Run(j2) {};

final Set<Run> treeSet = new TreeSet<>();
treeSet.add(r1);
treeSet.add(r2);

assertTrue(r1.compareTo(r2) != 0);
assertTrue(treeSet.size() == 2);
}
}

0 comments on commit e8efc96

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