Cleanup tests and improve build times (#628)

* Cleanup tests and improve build times * Remove unused test deps * Remove redundant slavetemplate tests Co-authored-by: res0nance <res0nance@users.noreply.github.com>
jenkinsci · Jun 5, 2021 · e23d17b · e23d17b
1 parent 9770b6e
commit e23d17b
Show file tree

Hide file tree

Showing 16 changed files with 191 additions and 587 deletions.
diff --git a/pom.xml b/pom.xml
@@ -185,42 +185,16 @@ THE SOFTWARE.
             <artifactId>workflow-durable-task-step</artifactId>
             <scope>test</scope>
         </dependency>
-        <dependency>
-            <groupId>org.jenkins-ci.plugins.workflow</groupId>
-            <artifactId>workflow-support</artifactId>
-            <classifier>tests</classifier>
-            <scope>test</scope>
-        </dependency>
         <dependency>
             <groupId>org.jenkins-ci.plugins.workflow</groupId>
             <artifactId>workflow-job</artifactId>
             <scope>test</scope>
         </dependency>
-        <dependency>
-            <groupId>org.jenkins-ci.plugins.workflow</groupId>
-            <artifactId>workflow-basic-steps</artifactId>
-            <scope>test</scope>
-        </dependency>
         <dependency>
             <groupId>org.jenkins-ci.plugins.workflow</groupId>
             <artifactId>workflow-api</artifactId>
             <scope>test</scope>
         </dependency>
-        <dependency>
-            <groupId>org.jenkins-ci.plugins</groupId>
-            <artifactId>script-security</artifactId>
-            <scope>test</scope>
-        </dependency>
-        <dependency>
-            <groupId>org.jenkins-ci.plugins</groupId>
-            <artifactId>scm-api</artifactId>
-            <scope>test</scope>
-        </dependency>
-        <dependency>
-            <groupId>org.jenkins-ci.plugins</groupId>
-            <artifactId>structs</artifactId>
-            <scope>test</scope>
-        </dependency>
         <dependency>
             <groupId>io.jenkins</groupId>
             <artifactId>configuration-as-code</artifactId>
@@ -255,13 +229,6 @@ THE SOFTWARE.
                 </exclusion>
             </exclusions>
         </dependency>
-        <!-- Upper bound deps -->
-        <dependency>
-            <groupId>org.slf4j</groupId>
-            <artifactId>slf4j-api</artifactId>
-            <version>1.7.30</version>
-            <scope>test</scope>
-        </dependency>
         <dependency>
             <groupId>org.kohsuke</groupId>
             <artifactId>libzfs</artifactId>

diff --git a/src/main/java/hudson/plugins/ec2/SlaveTemplate.java b/src/main/java/hudson/plugins/ec2/SlaveTemplate.java
@@ -572,7 +572,7 @@ public Set<String> parseSecurityGroups() {
         if (securityGroups == null || "".equals(securityGroups.trim())) {
             return Collections.emptySet();
         } else {
-            return new HashSet<String>(Arrays.asList(securityGroups.split("\\s*,\\s*")));
+            return new HashSet<>(Arrays.asList(securityGroups.split("\\s*,\\s*")));
         }
     }
 
@@ -684,6 +684,9 @@ public String getidleTerminationMinutes() {
     }
 
     public Set<LabelAtom> getLabelSet() {
+        if (labelSet == null) {
+            labelSet = Label.parse(labels);
+        }
         return labelSet;
     }
 
@@ -1636,9 +1639,11 @@ public EC2AbstractSlave attach(String instanceId, TaskListener listener) throws
      * Initializes data structure that we don't persist.
      */
     protected Object readResolve() {
-        Jenkins.get().checkPermission(Jenkins.ADMINISTER);
+        Jenkins j = Jenkins.getInstanceOrNull();
+         if (j != null) {
+             j.checkPermission(Jenkins.ADMINISTER);
+         }
 
-        labelSet = Label.parse(labels);
         securityGroupSet = parseSecurityGroups();
 
         /**

diff --git a/src/main/java/hudson/plugins/ec2/UnixData.java b/src/main/java/hudson/plugins/ec2/UnixData.java
@@ -28,7 +28,10 @@ public UnixData(String rootCommandPrefix, String slaveCommandPrefix, String slav
     }
 
     protected Object readResolve() {
-        Jenkins.get().checkPermission(Jenkins.ADMINISTER);
+        Jenkins j = Jenkins.getInstanceOrNull();
+        if (j != null) {
+            j.checkPermission(Jenkins.ADMINISTER);
+        }
         return this;
     }
 

diff --git a/src/test/java/hudson/plugins/ec2/AmazonEC2CloudTest.java b/src/test/java/hudson/plugins/ec2/AmazonEC2CloudTest.java
@@ -80,7 +80,6 @@ public void testConfigRoundtrip() throws Exception {
 
     @Test
     public void testAmazonEC2FactoryGetInstance() throws Exception {
-        r.configRoundtrip();
         AmazonEC2Cloud cloud = r.jenkins.clouds.get(AmazonEC2Cloud.class);
         AmazonEC2 connection = cloud.connect();
         Assert.assertNotNull(connection);
@@ -164,11 +163,7 @@ public void testCustomSshCredentialTypes() throws IOException {
     }
 
     private HtmlForm getConfigForm() throws IOException, SAXException {
-        if (Jenkins.getVersion().isNewerThanOrEqualTo(new VersionNumber("2.205"))) {
-            return r.createWebClient().goTo("configureClouds").getFormByName("config");
-        } else {
-            return r.createWebClient().goTo("configure").getFormByName("config");
-        }
+        return r.createWebClient().goTo("configureClouds").getFormByName("config");
     }
 
 }
diff --git a/src/test/java/hudson/plugins/ec2/AmazonEC2CloudUnitTest.java b/src/test/java/hudson/plugins/ec2/AmazonEC2CloudUnitTest.java
@@ -117,4 +117,16 @@ public void testSpotInstanceCount() throws Exception {
         // Should equal number of spot instance requests + 1 for spot nodes not having a spot instance request
         assertEquals(numberOfSpotInstanceRequests+1, n);
     }
+
+    @Test
+    public void testCNPartition() {
+        assertEquals(EC2Cloud.getAwsPartitionHostForService("cn-northwest-1", "ec2"), "ec2.cn-northwest-1.amazonaws.com.cn");
+        assertEquals(EC2Cloud.getAwsPartitionHostForService("cn-northwest-1", "s3"), "s3.cn-northwest-1.amazonaws.com.cn");
+    }
+
+    @Test
+    public void testNormalPartition() {
+        assertEquals(EC2Cloud.getAwsPartitionHostForService("us-east-1", "ec2"), "ec2.us-east-1.amazonaws.com");
+        assertEquals(EC2Cloud.getAwsPartitionHostForService("us-east-1", "s3"), "s3.us-east-1.amazonaws.com");
+    }
 }
diff --git a/src/test/java/hudson/plugins/ec2/AmazonPartitionEC2CloudTest.java b/src/test/java/hudson/plugins/ec2/AmazonPartitionEC2CloudTest.java
diff --git a/src/test/java/hudson/plugins/ec2/EC2InstanceTypesTest.java b/src/test/java/hudson/plugins/ec2/EC2InstanceTypesTest.java
diff --git a/src/test/java/hudson/plugins/ec2/EC2OndemandSlaveTest.java b/src/test/java/hudson/plugins/ec2/EC2OndemandSlaveTest.java
@@ -22,5 +22,4 @@ public void testSpecifyMode() throws Exception {
         EC2OndemandSlave slaveExclusive = new EC2OndemandSlave("name", "instanceId", "description", "remoteFS", 1, "labelString", Node.Mode.EXCLUSIVE, "initScript", "tmpDir", Collections.emptyList(), "remoteAdmin", "jvmopts", false, "30", "publicDNS", "privateDNS", Collections.emptyList(), "cloudName", false,  0, new UnixData("a", null, null, "b"), ConnectionStrategy.PRIVATE_IP, -1);
         assertEquals(Node.Mode.EXCLUSIVE, slaveExclusive.getMode());
     }
-
 }
diff --git a/src/test/java/hudson/plugins/ec2/EC2PrivateKeyTest.java b/src/test/java/hudson/plugins/ec2/EC2PrivateKeyTest.java
@@ -23,14 +23,13 @@
  */
 package hudson.plugins.ec2;
 
-import org.junit.Rule;
+import com.amazonaws.AmazonClientException;
+import org.junit.Before;
 import org.junit.Test;
 import org.jvnet.hudson.test.Issue;
-import org.jvnet.hudson.test.JenkinsRule;
 
 import java.io.IOException;
-
-import com.amazonaws.AmazonClientException;
+import java.security.Security;
 
 import static org.junit.Assert.assertEquals;
 
@@ -39,8 +38,11 @@
  */
 public class EC2PrivateKeyTest {
 
-    @Rule
-    public JenkinsRule r = new JenkinsRule();
+    @Before
+    public void before() {
+        // Add provider manually to avoid requiring jenkinsrule
+        Security.addProvider(new org.bouncycastle.jce.provider.BouncyCastleProvider());
+    }
 
     private EC2PrivateKey getPrivateKey() {
         return new EC2PrivateKey("-----BEGIN RSA PRIVATE KEY-----\n"

diff --git a/src/test/java/hudson/plugins/ec2/EC2RetentionStrategyTest.java b/src/test/java/hudson/plugins/ec2/EC2RetentionStrategyTest.java
@@ -12,14 +12,11 @@
 import hudson.slaves.NodeProperty;
 import hudson.slaves.OfflineCause;
 import jenkins.util.NonLocalizable;
-import net.sf.json.JSONObject;
 import org.junit.Rule;
 import org.junit.Test;
 import org.jvnet.hudson.test.JenkinsRule;
-import org.testcontainers.shaded.org.bouncycastle.jce.provider.BouncyCastleProvider;
 import org.jvnet.hudson.test.LoggerRule;
 
-import java.security.Security;
 import java.time.Clock;
 import java.time.Duration;
 import java.time.Instant;
@@ -52,12 +49,12 @@ public class EC2RetentionStrategyTest {
 
     final AtomicBoolean idleTimeoutCalled = new AtomicBoolean(false);
     final AtomicBoolean terminateCalled = new AtomicBoolean(false);
-    private static ZoneId zoneId = ZoneId.systemDefault();
+    private static final ZoneId zoneId = ZoneId.systemDefault();
 
     @Test
     public void testOnBillingHourRetention() throws Exception {
-        List<int[]> upTime = new ArrayList<int[]>();
-        List<Boolean> expected = new ArrayList<Boolean>();
+        List<int[]> upTime = new ArrayList<>();
+        List<Boolean> expected = new ArrayList<>();
         upTime.add(new int[] { 58, 0 });
         expected.add(true);
         upTime.add(new int[] { 57, 59 });
@@ -144,8 +141,8 @@ public boolean isConnecting() {
     @Test
     public void testOnUsageCountRetention() throws Exception {
         EC2RetentionStrategy rs = new EC2RetentionStrategy("0");
-        List<Integer> usageCounts = new ArrayList<Integer>();
-        List<Boolean> expected = new ArrayList<Boolean>();
+        List<Integer> usageCounts = new ArrayList<>();
+        List<Boolean> expected = new ArrayList<>();
         usageCounts.add(5);
         expected.add(false);
 
@@ -168,7 +165,6 @@ public void testOnUsageCountRetention() throws Exception {
                     assertEquals("Expected " + usageCount + " to be " + expected.get(i), (boolean) expected.get(i), terminateCalled.get());
                 }
             }
-
         }
     }
 
@@ -184,13 +180,12 @@ public String getEc2Type() {
                 return null;
             }
         };
-        EC2Computer computer = new EC2Computer(slave) {
+        return new EC2Computer(slave) {
             @Override
             public EC2AbstractSlave getNode() {
                 return slave;
             }
         };
-        return computer;
     }
 
     /**
@@ -230,9 +225,9 @@ public void testTerminateOfflineComputerIfNotConnecting() throws Exception {
 
     @Test
     public void testInternalCheckRespectsWait() throws Exception {
-        List<Boolean> expected = new ArrayList<Boolean>();
+        List<Boolean> expected = new ArrayList<>();
         EC2Computer computer = computerWithIdleTime(0, 0);
-        List<int[]> upTimeAndCheckAfter = new ArrayList<int[]>();
+        List<int[]> upTimeAndCheckAfter = new ArrayList<>();
 
         upTimeAndCheckAfter.add(new int[] { 0, -1 });
         expected.add(true);
@@ -253,7 +248,6 @@ public void testInternalCheckRespectsWait() throws Exception {
             if (i > 0) {
                 Clock clock = Clock.fixed(now.plusSeconds(startingUptime), zoneId);
                 rs = new EC2RetentionStrategy("1", clock, nextCheckAfter);
-
             } else {
                 rs = new EC2RetentionStrategy("1");
             }