[FIXED JENKINS-22631] Do not just warn about multiple builds with the…

… same number; note this in the official problem list, and offer to fix it.
jenkinsci · Apr 15, 2014 · deb90a4 · deb90a4
1 parent a33a73a
commit deb90a4
Show file tree

Hide file tree

Showing 5 changed files with 47 additions and 19 deletions.
diff --git a/changelog.html b/changelog.html
@@ -63,6 +63,9 @@
   <li class=bug>
     Linkage errors in notifiers could leak workspace leases.
     (<a href="https://issues.jenkins-ci.org/browse/JENKINS-21622">issue 21622</a>)
+  <li class=bug>
+    Better correction of the anomalous condition that several builds of a job specify the same number.
+    (<a href="https://issues.jenkins-ci.org/browse/JENKINS-22631">issue 22631</a>)
   <li class=bug>
     Fix a bug which only showed the first detail part for radio buttons.
     (<a href="https://issues.jenkins-ci.org/browse/JENKINS-22583">issue 22583</a>)

diff --git a/core/src/main/java/jenkins/diagnostics/ooom/BuildPtr.java b/core/src/main/java/jenkins/diagnostics/ooom/BuildPtr.java
@@ -6,7 +6,6 @@
 
 import java.io.File;
 import java.io.IOException;
-import java.util.Comparator;
 
 /**
  * ID and build number of one build.
@@ -48,20 +47,6 @@ public String toString() {
     }
 
 
-    static final Comparator<BuildPtr> BY_NUMBER = new Comparator<BuildPtr>() {
-        @Override
-        public int compare(BuildPtr o1, BuildPtr o2) {
-            return o1.n - o2.n;
-        }
-    };
-
-    static final Comparator<BuildPtr> BY_ID = new Comparator<BuildPtr>() {
-        @Override
-        public int compare(BuildPtr o1, BuildPtr o2) {
-            return o1.id.compareTo(o2.id);
-        }
-    };
-
     /**
      * If this build and that build are inconsistent, in that
      * their numbers and timestamps are ordering in the wrong direction.
@@ -103,6 +88,6 @@ public void fix(TaskListener listener) throws IOException, InterruptedException
 
     @Override
     public int compareTo(BuildPtr that) {
-        return this.n - that.n;
+        return this.id.compareTo(that.id);
     }
 }
diff --git a/core/src/main/java/jenkins/diagnostics/ooom/Problem.java b/core/src/main/java/jenkins/diagnostics/ooom/Problem.java
@@ -146,7 +146,7 @@ protected void inspect() {
             }
 
             byId = new ArrayList<BuildPtr>(byN);
-            Collections.sort(byId, BuildPtr.BY_ID);
+            Collections.sort(byId);
             i=0;
             for (BuildPtr b : byId) {
                 b.posByID = i++;
@@ -254,8 +254,10 @@ protected SortedMap<Integer,BuildPtr> scan() {
                         BuildPtr b = new BuildPtr(job,build,n);
 
                         BuildPtr o = builds.put(n, b);
-                        if (o!=null)
-                            LOGGER.warning("Multiple builds have the same number: "+o+" vs "+b);
+                        if (o != null) {
+                            LOGGER.log(WARNING, "Multiple builds have the same number: {0} vs. {1}", new Object[] {o, b});
+                            offenders.add(o);
+                        }
                     }
                 } catch (XPathExpressionException e) {
                     LOGGER.log(WARNING, "Failed to inspect "+build, e);

diff --git a/test/src/test/groovy/jenkins/diagnostics/ooom/OutOfOrderBuildDetectorTest.groovy b/test/src/test/groovy/jenkins/diagnostics/ooom/OutOfOrderBuildDetectorTest.groovy
@@ -1,10 +1,14 @@
 package jenkins.diagnostics.ooom
 
 import com.gargoylesoftware.htmlunit.html.HtmlPage
+import hudson.model.FreeStyleProject
 import hudson.model.Job
+import hudson.model.TaskListener
 import hudson.util.StreamTaskListener
+import static org.junit.Assert.*;
 import org.junit.Rule
 import org.junit.Test
+import org.jvnet.hudson.test.Bug
 import org.jvnet.hudson.test.JenkinsRule
 import org.jvnet.hudson.test.recipes.LocalData
 
@@ -103,4 +107,38 @@ class OutOfOrderBuildDetectorTest {
         j.assertBuildStatusSuccess(f.scheduleBuild2(0));
         assert Problem.find(f)==null;
     }
+
+    @Bug(22631)
+    @LocalData
+    @Test public void buildNumberClash() throws Exception {
+        j.jenkins.injector.injectMembers(this);
+        FreeStyleProject p = j.jenkins.getItemByFullName("problematic", FreeStyleProject.class);
+        assertNotNull(p);
+        ByteArrayOutputStream baos = new ByteArrayOutputStream();
+        TaskListener l = new StreamTaskListener(baos);
+        oobd.execute(l, 0);
+        String log = baos.toString();
+        String idA = "2014-04-15_11-22-11";
+        String idB = "2014-04-15_11-22-14";
+        assertEquals(idB, p.getBuildByNumber(2).getId());
+        assertEquals(3, p.getLastBuild().getNumber());
+        File dir = p.getBuildDir();
+        File buildDirA = new File(dir, idA);
+        File buildDirB = new File(dir, idB);
+        BuildPtr b2A = new BuildPtr(p, buildDirA, 2);
+        BuildPtr b2B = new BuildPtr(p, buildDirB, 2);
+        String expected = "[" + b2A + "]";
+        assertTrue("Should see " + expected + " in:\n" + log, log.contains(expected));
+        baos = new ByteArrayOutputStream();
+        l = new StreamTaskListener(baos);
+        oobm.fix(l);
+        log = baos.toString();
+        assertTrue(buildDirB.isDirectory());
+        assertTrue("Should see " + buildDirA + " in:\n" + log, log.contains(buildDirA.toString()));
+        File dest = new File(new File(p.getRootDir(), "outOfOrderBuilds"), idA);
+        assertTrue("Should see " + dest + " in:\n" + log, log.contains(dest.toString()));
+        assertFalse(buildDirA.isDirectory());
+        assertTrue(dest.isDirectory());
+    }
+
 }
diff --git a/.../test/resources/jenkins/diagnostics/ooom/OutOfOrderBuildDetectorTest/buildNumberClash.zip b/.../test/resources/jenkins/diagnostics/ooom/OutOfOrderBuildDetectorTest/buildNumberClash.zip