Permit the setting of Build Schedule Delay to "0".

Delay not needed with Replication Events. Also fixed dependent projects feature to no longer rely on this default 3 seconds delay. [FIXED JENKINS-22812] Change-Id: Ic11f0fb7b9e43e95ae7dec71c30fe1e6fab1e8ac
jenkinsci · May 26, 2014 · e2c0e3e · e2c0e3e
1 parent 4c2d6f7
commit e2c0e3e
Show file tree

Hide file tree

Showing 11 changed files with 167 additions and 13 deletions.
diff --git a/src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/config/Config.java b/src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/config/Config.java
@@ -338,8 +338,8 @@ public void setValues(JSONObject formData) {
         buildScheduleDelay = formData.optInt(
                 "buildScheduleDelay",
                 DEFAULT_BUILD_SCHEDULE_DELAY);
-        if (buildScheduleDelay <= DEFAULT_BUILD_SCHEDULE_DELAY) {
-            buildScheduleDelay = DEFAULT_BUILD_SCHEDULE_DELAY;
+        if (buildScheduleDelay < 0) {
+            buildScheduleDelay = 0;
         }
         dynamicConfigRefreshInterval = formData.optInt(
                 "dynamicConfigRefreshInterval",

diff --git a/...n/hudson/plugins/gerrit/trigger/dependency/BecauseWaitingToEnsureOtherJobsAreInQueue.java b/...n/hudson/plugins/gerrit/trigger/dependency/BecauseWaitingToEnsureOtherJobsAreInQueue.java
@@ -0,0 +1,43 @@
+/*
+ * The MIT License
+ *
+ * Copyright 2014 Ericsson.
+ *
+ * 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
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+package com.sonyericsson.hudson.plugins.gerrit.trigger.dependency;
+
+import com.sonyericsson.hudson.plugins.gerrit.trigger.Messages;
+
+import hudson.model.queue.CauseOfBlockage;
+
+/**
+ * Build is blocked because it is waiting to ensure other jobs are in queue.
+ *
+ * @author Scott Hebert &lt;scott.hebert@ericsson.com&gt;
+ */
+public class BecauseWaitingToEnsureOtherJobsAreInQueue extends CauseOfBlockage {
+
+    @Override
+    public String getShortDescription() {
+        return Messages.WaitingToEnsureOtherJobsAreInQueue();
+    }
+
+}
diff --git a/.../sonyericsson/hudson/plugins/gerrit/trigger/dependency/DependencyQueueTaskDispatcher.java b/.../sonyericsson/hudson/plugins/gerrit/trigger/dependency/DependencyQueueTaskDispatcher.java
@@ -40,15 +40,16 @@
 import java.util.Collections;
 import java.util.StringTokenizer;
 import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.TimeUnit;
 
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import com.sonymobile.tools.gerrit.gerritevents.GerritDefaultValues;
 import com.sonymobile.tools.gerrit.gerritevents.GerritEventListener;
 import com.sonymobile.tools.gerrit.gerritevents.dto.events.GerritTriggeredEvent;
 import com.sonymobile.tools.gerrit.gerritevents.dto.GerritEvent;
 import com.sonymobile.tools.gerrit.gerritevents.GerritHandler;
-
 import com.sonyericsson.hudson.plugins.gerrit.trigger.events.lifecycle.GerritEventLifecycleListener;
 import com.sonyericsson.hudson.plugins.gerrit.trigger.events.lifecycle.GerritEventLifecycle;
 import com.sonyericsson.hudson.plugins.gerrit.trigger.hudsontrigger.GerritCause;
@@ -159,6 +160,15 @@ public CauseOfBlockage canRun(Queue.Item item) {
         }
         //logger.debug("We have dependencies on project {} : {}", p, trigger.getDependencyJobsNames());
 
+        // We ensure that we wait until other jobs have been put into queue.
+        // We use the default Gerrit Build Schedule Delay value
+        long inQueueSince = item.getInQueueSince();
+        if (System.currentTimeMillis() - inQueueSince < TimeUnit.SECONDS
+                .toMillis(GerritDefaultValues.DEFAULT_BUILD_SCHEDULE_DELAY)) {
+            logger.debug("We need to wait to ensure dependent jobs {} are in queue", event);
+            return new BecauseWaitingToEnsureOtherJobsAreInQueue();
+        }
+
         /* we really should first check for other projects which will be triggered
          * for the same event, and haven't yet. Unfortunately, this requires some kind
          * of event notification - GerritEventLifecycle like - which is not available

diff --git a/...main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/hudsontrigger/GerritTrigger.java b/...main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/hudsontrigger/GerritTrigger.java
@@ -666,7 +666,7 @@ public void notifyBuildEnded(GerritTriggeredEvent event) {
      */
     public int getBuildScheduleDelay() {
         if (isAnyServer()) {
-            int max = DEFAULT_BUILD_SCHEDULE_DELAY;
+            int max = 0;
             for (GerritServer server : PluginImpl.getInstance().getServers()) {
                 if (server.getConfig() != null) {
                     max = Math.max(max, server.getConfig().getBuildScheduleDelay());
@@ -679,7 +679,8 @@ public int getBuildScheduleDelay() {
         } else {
             int buildScheduleDelay = PluginImpl.getInstance().getServer(serverName).getConfig()
                     .getBuildScheduleDelay();
-            return Math.max(buildScheduleDelay, DEFAULT_BUILD_SCHEDULE_DELAY);
+            //check if less than zero
+            return Math.max(0, buildScheduleDelay);
         }
 
     }

diff --git a/src/main/resources/com/sonyericsson/hudson/plugins/gerrit/trigger/GerritServer/index.jelly b/src/main/resources/com/sonyericsson/hudson/plugins/gerrit/trigger/GerritServer/index.jelly
@@ -215,7 +215,7 @@
                             <f:textbox name="buildScheduleDelay"
                                        value="${it.config.buildScheduleDelay}"
                                        default="${com.sonyericsson.hudson.plugins.gerrit.gerritevents.GerritDefaultValues.DEFAULT_BUILD_SCHEDULE_DELAY}"
-                                       checkUrl="'${rootURL}/${serverURL}/positiveIntegerCheck?value='+escape(this.value)"/>
+                                       checkUrl="'${rootURL}/${serverURL}/nonNegativeIntegerCheck?value='+escape(this.value)"/>
                         </f:entry>
                         <f:entry title="${%Dynamic Config Refresh Interval}"
                                  help="/plugin/gerrit-trigger/help-DynamicTriggerConfigRefreshInterval.html">

diff --git a/src/main/resources/com/sonyericsson/hudson/plugins/gerrit/trigger/Messages.properties b/src/main/resources/com/sonyericsson/hudson/plugins/gerrit/trigger/Messages.properties
@@ -150,6 +150,8 @@ CannotAddSelfAsDependency=\
   Cannot add a project to its own set of dependencies
 WaitingForDependencyProjectsToTrigger=\
   Waiting for all projects to finish gerrit-triggering to make sure no dependencies need to build first.
+WaitingToEnsureOtherJobsAreInQueue=\
+  Waiting to ensure all dependent jobs are in queue.
 NotificationLevel_DefaultValue=\
   (Server default)
 NotificationLevel_DefaultValueFromServer=\

diff --git a/src/main/webapp/help-BuildScheduleDelay.html b/src/main/webapp/help-BuildScheduleDelay.html
@@ -1 +1,2 @@
-<p><strong>Build Schedule Delay</strong> delays the scheduling of the build by the specified number of seconds. The default and minimum <strong>Build Schedule Delay</strong> is 3 seconds. A longer delay can be needed if the changes are to be built from Gerrit mirrors, since the replication takes time.</p>
+<p><strong>Build Schedule Delay</strong> delays the scheduling of the build by the specified number of seconds. The default <strong>Build Schedule Delay</strong> is 3 seconds.
+A longer delay can be needed if the changes are to be built from Gerrit mirrors, since the replication takes time. However, if you are using replication events to trigger your jobs, you may set this value to 0 to disable the <strong>Build Schedule Delay</strong></p>
diff --git a/src/test/java/com/sonyericsson/hudson/plugins/gerrit/trigger/config/ConfigTest.java b/src/test/java/com/sonyericsson/hudson/plugins/gerrit/trigger/config/ConfigTest.java
@@ -26,6 +26,7 @@
 
 import com.sonymobile.tools.gerrit.gerritevents.dto.events.PatchsetCreated;
 import com.sonymobile.tools.gerrit.gerritevents.dto.rest.Notify;
+import com.sonymobile.tools.gerrit.gerritevents.GerritDefaultValues;
 import com.sonyericsson.hudson.plugins.gerrit.trigger.mock.Setup;
 
 import net.sf.json.JSONObject;
@@ -118,6 +119,7 @@ public void testSetValues() {
         assertEquals(6, config.getNumberOfReceivingWorkerThreads());
         assertEquals(4, config.getNumberOfSendingWorkerThreads());
         assertEquals(Notify.OWNER, config.getNotificationLevel());
+        assertEquals(GerritDefaultValues.DEFAULT_BUILD_SCHEDULE_DELAY, config.getBuildScheduleDelay());
     }
 
     //CS IGNORE MagicNumber FOR NEXT 100 LINES. REASON: Mocks tests.
@@ -155,6 +157,7 @@ public void testCopyConfig() {
                 + "\"gerritProxy\":\"\","
                 + "\"gerritUserName\":\"gerrit\","
                 + "\"numberOfSendingWorkerThreads\":\"4\","
+                + "\"buildScheduleDelay\":\"0\","
                 + "\"numberOfReceivingWorkerThreads\":\"6\"}";
         JSONObject form = (JSONObject)JSONSerializer.toJSON(formString);
         Config initialConfig = new Config(form);
@@ -194,6 +197,7 @@ public void testCopyConfig() {
         assertEquals("gerrit", config.getGerritUserName());
         assertEquals(6, config.getNumberOfReceivingWorkerThreads());
         assertEquals(4, config.getNumberOfSendingWorkerThreads());
+        assertEquals(0, config.getBuildScheduleDelay());
     }
 
     /**

diff --git a/...yericsson/hudson/plugins/gerrit/trigger/dependency/DependencyQueueTaskDispatcherTest.java b/...yericsson/hudson/plugins/gerrit/trigger/dependency/DependencyQueueTaskDispatcherTest.java
@@ -45,6 +45,7 @@
 import java.util.ArrayList;
 import java.util.Calendar;
 import java.util.List;
+import java.util.concurrent.TimeUnit;
 
 import jenkins.model.Jenkins;
 
@@ -55,6 +56,7 @@
 import org.powermock.core.classloader.annotations.PrepareForTest;
 import org.powermock.modules.junit4.PowerMockRunner;
 
+import com.sonymobile.tools.gerrit.gerritevents.GerritDefaultValues;
 import com.sonymobile.tools.gerrit.gerritevents.GerritHandler;
 import com.sonymobile.tools.gerrit.gerritevents.dto.events.GerritTriggeredEvent;
 import com.sonymobile.tools.gerrit.gerritevents.dto.events.PatchsetCreated;
@@ -70,7 +72,7 @@
  * @author Yannick Bréhon &lt;yannick.brehon@smartmatic.com&gt;
  */
 @RunWith(PowerMockRunner.class)
-@PrepareForTest({ Jenkins.class, ToGerritRunListener.class })
+@PrepareForTest({ Jenkins.class, ToGerritRunListener.class, WaitingItem.class })
 public class DependencyQueueTaskDispatcherTest {
 
     private DependencyQueueTaskDispatcher dispatcher;
@@ -287,9 +289,12 @@ private Queue.Item createItem(GerritCause gerritCause, String dependency) {
         abstractProjectDependencyMock = mock(AbstractProject.class);
         when(abstractProjectDependencyMock.getTrigger(GerritTrigger.class)).thenReturn(gerritTriggerMock);
         when(gerritTriggerMock.getDependencyJobsNames()).thenReturn(dependency);
-        /*doReturn(abstractProjectDependencyMock).when(jenkinsMock).getItem(eq("upstream"), any(Item.class),
-                Item.class);*/
         when(jenkinsMock.getItem(eq("upstream"), any(Item.class), Item.class)).thenReturn(abstractProjectDependencyMock);
-        return new WaitingItem(Calendar.getInstance(), abstractProjectMock, actions);
+        WaitingItem waitingItem = PowerMockito.spy(new WaitingItem(Calendar.getInstance(),
+                abstractProjectMock, actions));
+        when(waitingItem.getInQueueSince()).thenReturn(System.currentTimeMillis()
+                - TimeUnit.SECONDS.toMillis(GerritDefaultValues.DEFAULT_BUILD_SCHEDULE_DELAY));
+        return waitingItem;
     }
+
 }
diff --git a/...nyericsson/hudson/plugins/gerrit/trigger/extensions/GerritTriggeredBuildListenerTest.java b/...nyericsson/hudson/plugins/gerrit/trigger/extensions/GerritTriggeredBuildListenerTest.java
@@ -26,6 +26,7 @@
 import com.sonymobile.tools.gerrit.gerritevents.dto.events.GerritTriggeredEvent;
 import com.sonyericsson.hudson.plugins.gerrit.trigger.GerritServer;
 import com.sonyericsson.hudson.plugins.gerrit.trigger.PluginImpl;
+import com.sonyericsson.hudson.plugins.gerrit.trigger.config.Config;
 import com.sonyericsson.hudson.plugins.gerrit.trigger.mock.DuplicatesUtil;
 import com.sonyericsson.hudson.plugins.gerrit.trigger.mock.Setup;
 import com.sonymobile.tools.gerrit.gerritevents.mock.SshdServerMock;
@@ -100,6 +101,32 @@ public void testListenTriggeredBuild() throws Exception {
         assertTrue("Time out", buildListenerLatch.await(15, TimeUnit.SECONDS));
     }
 
+    /**
+     * Tests that {@link GrritTriggeredBuildListener} can listen triggered build with no build schedule.
+     *
+     * @throws Exception if so.
+     */
+    @LocalData
+    public void testListenTriggeredBuildWithNoBuildScheduleDelay() throws Exception {
+        ExtensionList<GerritTriggeredBuildListener> list =
+                Jenkins.getInstance().getExtensionList(GerritTriggeredBuildListener.class);
+        assertTrue("Listener has not been registered", list.size() > 0);
+
+        buildListenerLatch = new CountDownLatch(2);
+        GerritServer gerritServer = PluginImpl.getInstance().getServer(PluginImpl.DEFAULT_SERVER_NAME);
+
+        Config config = (Config)gerritServer.getConfig();
+        config.setBuildScheduleDelay(0);
+
+        gerritServer.setConfig(config);
+
+        DuplicatesUtil.createGerritTriggeredJob(this, "projectX");
+        server.waitForCommand(GERRIT_STREAM_EVENTS, 2000);
+        gerritServer.triggerEvent(Setup.createPatchsetCreated());
+
+        assertTrue("Time out", buildListenerLatch.await(15, TimeUnit.SECONDS));
+    }
+
     /**
      * A {@link GrritTriggeredBuildListener} implementation class.
      * This would be automatically registered to system by @TestExtension annotation.
@@ -121,4 +148,26 @@ public void onCompleted(Result result, GerritTriggeredEvent event, String comman
             buildListenerLatch.countDown();
         }
     }
+
+    /**
+     * A {@link GrritTriggeredBuildListener} implementation class.
+     * This would be automatically registered to system by @TestExtension annotation.
+     */
+    @TestExtension("testListenTriggeredBuildWithNoBuildScheduleDelay")
+    public static class GerritTriggeredBuildListenerImpl2 extends GerritTriggeredBuildListener {
+
+        @Override
+        public void onStarted(GerritTriggeredEvent event, String command) {
+            System.out.println("onStarted: [event] " + event.getEventType() + " [command] " + command);
+            buildListenerLatch.countDown();
+        }
+
+        @Override
+        public void onCompleted(Result result, GerritTriggeredEvent event, String command) {
+            System.out.println("onStarted: [event] " + event.getEventType()
+                    + " [result] " + result.toString()
+                    + " [command] " + command);
+            buildListenerLatch.countDown();
+        }
+    }
 }
diff --git a/.../java/com/sonyericsson/hudson/plugins/gerrit/trigger/hudsontrigger/GerritTriggerTest.java b/.../java/com/sonyericsson/hudson/plugins/gerrit/trigger/hudsontrigger/GerritTriggerTest.java
@@ -330,8 +330,47 @@ public void testScheduleWithNegativeBuildScheduleDelay() {
         doReturn("http://mock.url").when(gerritCause).getUrl();
         trigger.schedule(gerritCause, event);
         verify(project).scheduleBuild2(
-                //negative value will be reset into default value 3
-                eq(3),
+                //negative value will be reset to 0
+                eq(0),
+                same(gerritCause),
+                isA(Action.class),
+                isA(Action.class),
+                isA(Action.class),
+                isA(Action.class));
+    }
+
+    /**
+     * Tests the schedule method of GerritTrigger.
+     * It verifies that {@link AbstractProject#scheduleBuild2(int, hudson.model.Cause, hudson.model.Action...)}
+     * gets called with an negative buildScheduleDelay -20.
+     */
+    @Test
+    public void testScheduleWithNoBuildScheduleDelay() {
+        AbstractProject project = PowerMockito.mock(AbstractProject.class);
+        when(project.getFullDisplayName()).thenReturn("MockedProject");
+        when(project.getFullName()).thenReturn("MockedProject");
+        PowerMockito.mockStatic(PluginImpl.class);
+        PluginImpl plugin = PowerMockito.mock(PluginImpl.class);
+        GerritServer server = mock(GerritServer.class);
+        when(plugin.getServer(any(String.class))).thenReturn(server);
+        IGerritHudsonTriggerConfig config = Setup.createConfig();
+        config = spy(config);
+        doReturn("http://mock.url").when(config).getGerritFrontEndUrlFor(any(String.class), any(String.class));
+        when(server.getConfig()).thenReturn(config);
+        GerritHandler handler = mock(GerritHandler.class);
+        when(plugin.getHandler()).thenReturn(handler);
+        PowerMockito.when(PluginImpl.getInstance()).thenReturn(plugin);
+        when(config.getBuildScheduleDelay()).thenReturn(0);
+
+        GerritTrigger trigger = Setup.createDefaultTrigger(project);
+        when(project.getTrigger(GerritTrigger.class)).thenReturn(trigger);
+        PatchsetCreated event = Setup.createPatchsetCreated();
+        GerritCause gerritCause = new GerritCause(event, true);
+        gerritCause = spy(gerritCause);
+        doReturn("http://mock.url").when(gerritCause).getUrl();
+        trigger.schedule(gerritCause, event);
+        verify(project).scheduleBuild2(
+                eq(0),
                 same(gerritCause),
                 isA(Action.class),
                 isA(Action.class),