[JENKINS-35198] - DelegatingComputerLauncher should accept child clas…

…ses in its hooks (#2384) * JENKINS-35198 Add failing test for clear identification of the issue and future regression test * JENKINS-35198 Fix DelegatingComputerLauncher logic for filtering out subclasses as possible delegates this logic is by default, and subclasses can still allow the selection of other DelegatingComputerLauncher instances as delegates.
jenkinsci · May 29, 2016 · 15ce24a · 15ce24a
1 parent 0ae6c42
commit 15ce24a
Show file tree

Hide file tree

Showing 2 changed files with 72 additions and 1 deletion.
diff --git a/core/src/main/java/hudson/slaves/DelegatingComputerLauncher.java b/core/src/main/java/hudson/slaves/DelegatingComputerLauncher.java
@@ -74,7 +74,7 @@ public static abstract class DescriptorImpl extends Descriptor<ComputerLauncher>
         public List<Descriptor<ComputerLauncher>> getApplicableDescriptors() {
             List<Descriptor<ComputerLauncher>> r = new ArrayList<Descriptor<ComputerLauncher>>();
             for (Descriptor<ComputerLauncher> d : Functions.getComputerLauncherDescriptors()) {
-                if (DelegatingComputerLauncher.class.isInstance(d))  continue;
+                if (DelegatingComputerLauncher.class.isAssignableFrom(d.getKlass().toJavaClass()))  continue;
                 r.add(d);
             }
             return r;

diff --git a/core/src/test/java/hudson/slaves/DelegatingComputerLauncherTest.java b/core/src/test/java/hudson/slaves/DelegatingComputerLauncherTest.java
@@ -0,0 +1,71 @@
+package hudson.slaves;
+
+import hudson.DescriptorExtensionList;
+import hudson.model.Descriptor;
+import jenkins.model.Jenkins;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.powermock.api.mockito.PowerMockito;
+import org.powermock.core.classloader.annotations.PrepareForTest;
+import org.powermock.modules.junit4.PowerMockRunner;
+
+import java.util.ArrayList;
+
+import static org.junit.Assert.*;
+import static org.mockito.Matchers.eq;
+import static org.mockito.Mockito.doReturn;
+import static org.powermock.api.mockito.PowerMockito.mock;
+import static org.powermock.api.mockito.PowerMockito.when;
+
+/**
+ * @author peppelan
+ */
+@RunWith(PowerMockRunner.class)
+public class DelegatingComputerLauncherTest {
+
+    public static class DummyOne extends DelegatingComputerLauncher {
+
+        public DummyOne() {
+            super(null);
+        }
+
+        public static class DummyOneDescriptor extends DescriptorImpl {
+        }
+    }
+
+
+    public static class DummyTwo extends DelegatingComputerLauncher {
+
+        public DummyTwo() {
+            super(null);
+        }
+
+        public static class DummyTwoDescriptor extends DescriptorImpl {
+        }
+    }
+
+    // Ensure that by default a DelegatingComputerLauncher subclass doesn't advertise the option to delegate another
+    // DelegatingComputerLauncher
+    @Test
+    @PrepareForTest(Jenkins.class)
+    public void testRecursionAvoidance() {
+        PowerMockito.mockStatic(Jenkins.class);
+        Jenkins mockJenkins = mock(Jenkins.class);
+        PowerMockito.when(Jenkins.getInstance()).thenReturn(mockJenkins);
+
+        DescriptorExtensionList<ComputerLauncher, Descriptor<ComputerLauncher>> mockList =
+                mock(DescriptorExtensionList.class);
+        doReturn(mockList).when(mockJenkins).getDescriptorList(eq(ComputerLauncher.class));
+        ArrayList<Descriptor<ComputerLauncher>> returnedList = new ArrayList<>();
+
+        returnedList.add(new DummyOne.DummyOneDescriptor());
+        returnedList.add(new DummyTwo.DummyTwoDescriptor());
+
+        when(mockList.iterator()).thenReturn(returnedList.iterator());
+
+        assertTrue("DelegatingComputerLauncher should filter out other DelegatingComputerLauncher instances " +
+                   "from its descriptor's getApplicableDescriptors() method",
+                new DummyTwo.DummyTwoDescriptor().getApplicableDescriptors().isEmpty());
+    }
+
+}