Applying code review suggestions

jenkinsci · Oct 8, 2021 · 9856369 · 9856369
1 parent 6b49249
commit 9856369
Show file tree

Hide file tree

Showing 3 changed files with 12 additions and 19 deletions.
diff --git a/src/main/java/hudson/plugins/s3/ClientHelper.java b/src/main/java/hudson/plugins/s3/ClientHelper.java
@@ -22,7 +22,7 @@ public class ClientHelper {
     public final static String DEFAULT_AMAZON_S3_REGION_NAME = System.getProperty(
             "hudson.plugins.s3.DEFAULT_AMAZON_S3_REGION",
             com.amazonaws.services.s3.model.Region.US_Standard.toAWSRegion().getName());
-    public static final String ENDPOINT = System.getProperty("hudson.plugins.s3.ENDPOINT", System.getenv("hudson.plugins.s3.ENDPOINT"));
+    public static final String ENDPOINT = System.getProperty("hudson.plugins.s3.ENDPOINT", System.getenv("PLUGIN_S3_ENDPOINT"));
 
     public static AmazonS3 createClient(String accessKey, String secretKey, boolean useRole, String region, ProxyConfiguration proxy)
     {
@@ -88,7 +88,7 @@ public static ClientConfiguration getClientConfiguration(@Nonnull ProxyConfigura
         } else {
             s3Endpoint = region.getServiceEndpoint(AmazonS3.ENDPOINT_PREFIX);
         }
-        Logger.getLogger(ClientHelper.class.getName()).info("ENDPOINT: " + s3Endpoint);
+        Logger.getLogger(ClientHelper.class.getName()).fine(() -> String.format("ENDPOINT: %s", s3Endpoint));
         if (shouldUseProxy(proxy, s3Endpoint)) {
             clientConfiguration.setProxyHost(proxy.name);
             clientConfiguration.setProxyPort(proxy.port);

diff --git a/src/main/java/hudson/plugins/s3/S3CopyArtifact.java b/src/main/java/hudson/plugins/s3/S3CopyArtifact.java
@@ -279,7 +279,7 @@ private boolean perform(Run<?,?> src, Run<?,?> dst, String includeFilter, String
             fingerprints.put(record.getName(), record.getFingerprint());
         }
 
-        for (Run<?,?> r : new Run[]{src, dst}) {
+        for (Run<?,?> r : new Run<?,?>[]{src, dst}) {
             if (r == null) {
                 continue;
             }

diff --git a/...st/java/hudson/plugins/s3/MiniIOTest.java → ...est/java/hudson/plugins/s3/MinIOTest.java b/...st/java/hudson/plugins/s3/MiniIOTest.java → ...est/java/hudson/plugins/s3/MinIOTest.java
@@ -55,6 +55,7 @@
 import java.time.Duration;
 import java.util.Collections;
 
+import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertTrue;
 
 public class MinIOTest {
@@ -71,7 +72,7 @@ public class MinIOTest {
 
     @Rule
     public RealJenkinsRule rr = new RealJenkinsRule().javaOptions("-Xmx256m",
-            "-Dhudson.plugins.s3.DEFAULT_AMAZON_S3_REGION=local", "-Dhudson.plugins.s3.ENDPOINT=http://" + minioServiceEndpoint);
+            "-Dhudson.plugins.s3.ENDPOINT=http://" + minioServiceEndpoint);
 
     @BeforeClass
     public static void setUpClass() throws Exception {
@@ -121,14 +122,10 @@ public static void shutDownClass() {
     public void testS3BucketPublisher() throws Throwable {
         final String endpoint = minioServiceEndpoint;
         rr.then(r -> {
-
             /*r.jenkins.setLabelString("work"); //Able to debug when running on the controller but not an agent
             r.jenkins.setNumExecutors(1);*/
-            r.createOnlineSlave(Label.get("work"), new EnvVars("hudson.plugins.s3.ENDPOINT", "http://" + endpoint));
-
+            r.createOnlineSlave(Label.get("work"), new EnvVars("PLUGIN_S3_ENDPOINT", "http://" + endpoint));
             createProfile();
-
-
             createAndRunPublisher(r);
         });
     }
@@ -173,8 +170,8 @@ private static void createProfile() {
     public void testS3CopyArtifact() throws Throwable {
         final String endpoint = minioServiceEndpoint;
         rr.then(r -> {
-            r.createOnlineSlave(Label.get("work"), new EnvVars("hudson.plugins.s3.ENDPOINT", "http://" + endpoint));
-            r.createOnlineSlave(Label.get("copy"), new EnvVars("hudson.plugins.s3.ENDPOINT", "http://" + endpoint));
+            r.createOnlineSlave(Label.get("work"), new EnvVars("PLUGIN_S3_ENDPOINT", "http://" + endpoint));
+            r.createOnlineSlave(Label.get("copy"), new EnvVars("PLUGIN_S3_ENDPOINT", "http://" + endpoint));
 
             createProfile();
             createAndRunPublisher(r);
@@ -199,15 +196,11 @@ public static class VerifyFileBuilder extends TestBuilder {
         @Override
         public boolean perform(final AbstractBuild<?, ?> build, final Launcher launcher, final BuildListener listener) throws InterruptedException, IOException {
             final FilePath child = build.getWorkspace().child("test.txt");
-            if (!child.exists()) {
-                listener.error("No test.txt in workspace!");
-                return false;
-            }
+            assertTrue("No test.txt in workspace!", child.exists());
+
             final String s = child.readToString();
-            if (!FILE_CONTENT.equals(s)) {
-                listener.error("Wrong file content! Expected \"" + FILE_CONTENT + "\" but was \"" + s + "\"");
-                return false;
-            }
+            assertEquals(FILE_CONTENT, s);
+
             return true;
         }
     }