From 4ff3f04436e4a2b3d49f57cf25643c94ed4b39a4 Mon Sep 17 00:00:00 2001 From: Vincent Latombe Date: Wed, 7 Oct 2020 14:37:32 +0200 Subject: [PATCH] [JENKINS-63828] It is legal to have a CloudProvisioningListener not implementing canProvision I got confused in https://github.com/jenkinsci/jenkins/commit/3992a30efb1abf697cae8e02f9e81dd04b92b135 by Cloud#canProvision which was abstract vs. CloudProvisioningListener#canProvision which wasn't and was returning null. --- core/src/main/java/hudson/Util.java | 2 +- .../slaves/CloudProvisioningListener.java | 9 +++++--- core/src/test/java/hudson/UtilTest.java | 2 +- .../slaves/CloudProvisioningListenerTest.java | 21 +++++++++---------- 4 files changed, 18 insertions(+), 16 deletions(-) diff --git a/core/src/main/java/hudson/Util.java b/core/src/main/java/hudson/Util.java index 6db16d5b04..0b24f7713f 100644 --- a/core/src/main/java/hudson/Util.java +++ b/core/src/main/java/hudson/Util.java @@ -1480,7 +1480,7 @@ public class Util { if (isOverridden(base, derived, methodName, types)) { return supplier.get(); } else { - throw new AbstractMethodError("You must override at least one of the " + throw new AbstractMethodError("The class " + derived.getName() + " must override at least one of the " + base.getSimpleName() + "." + methodName + " methods"); } } diff --git a/core/src/main/java/hudson/slaves/CloudProvisioningListener.java b/core/src/main/java/hudson/slaves/CloudProvisioningListener.java index 7eaa77a1f5..2f27a49604 100644 --- a/core/src/main/java/hudson/slaves/CloudProvisioningListener.java +++ b/core/src/main/java/hudson/slaves/CloudProvisioningListener.java @@ -42,13 +42,16 @@ public abstract class CloudProvisioningListener implements ExtensionPoint { */ @Deprecated public CauseOfBlockage canProvision(Cloud cloud, Label label, int numExecutors) { - return Util.ifOverridden(() -> canProvision(cloud, new CloudState(label, 0), numExecutors), - CloudProvisioningListener.class, + if (Util.isOverridden(CloudProvisioningListener.class, getClass(), "canProvision", Cloud.class, CloudState.class, - int.class); + int.class)) { + return canProvision(cloud, new CloudState(label, 0), numExecutors); + } else { + return null; + } } /** diff --git a/core/src/test/java/hudson/UtilTest.java b/core/src/test/java/hudson/UtilTest.java index 87a454f573..7beb38e97b 100644 --- a/core/src/test/java/hudson/UtilTest.java +++ b/core/src/test/java/hudson/UtilTest.java @@ -598,7 +598,7 @@ public class UtilTest { AbstractMethodError error = Assert.assertThrows(AbstractMethodError.class, () -> { Util.ifOverridden(() -> true, BaseClass.class, DerivedClassFailure.class, "method"); }); - assertEquals("You must override at least one of the BaseClass.method methods", error.getMessage()); + assertEquals("The class " + DerivedClassFailure.class.getName() + " must override at least one of the BaseClass.method methods", error.getMessage()); } public static class BaseClass { diff --git a/core/src/test/java/hudson/slaves/CloudProvisioningListenerTest.java b/core/src/test/java/hudson/slaves/CloudProvisioningListenerTest.java index 46b8378527..641706f887 100644 --- a/core/src/test/java/hudson/slaves/CloudProvisioningListenerTest.java +++ b/core/src/test/java/hudson/slaves/CloudProvisioningListenerTest.java @@ -1,37 +1,36 @@ package hudson.slaves; +import hudson.model.Messages; import hudson.model.queue.CauseOfBlockage; -import org.junit.Assert; import org.junit.Test; import org.jvnet.hudson.test.Issue; +import org.jvnet.localizer.Localizable; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; public class CloudProvisioningListenerTest { - public static class CloudProvisioningListenerFailing extends CloudProvisioningListener {} + public static class CloudProvisioningListenerNoOverride extends CloudProvisioningListener {} - public static class CloudProvisioningListenerSuccess extends CloudProvisioningListener { + public static class CloudProvisioningListenerOverride extends CloudProvisioningListener { @Override public CauseOfBlockage canProvision(Cloud cloud, Cloud.CloudState state, int numExecutors) { - return null; + return CauseOfBlockage.fromMessage(Messages._Queue_InProgress()); } } @Issue("JENKINS-63828") @Test - public void failing(){ + public void noOverride(){ Cloud.CloudState state = new Cloud.CloudState(null, 0); - AbstractMethodError error = Assert.assertThrows(AbstractMethodError.class, () -> { - new CloudProvisioningListenerFailing().canProvision(null, state, 0); - }); - assertEquals("You must override at least one of the CloudProvisioningListener.canProvision methods", error.getMessage()); + assertNull(new CloudProvisioningListenerNoOverride().canProvision(null, state, 0)); } @Issue("JENKINS-63828") @Test - public void success() { + public void override() { Cloud.CloudState state = new Cloud.CloudState(null, 0); - assertNull(new CloudProvisioningListenerSuccess().canProvision(null, state, 0)); + assertNotNull(new CloudProvisioningListenerOverride().canProvision(null, state, 0)); } }