Merge pull request #2484 from jglick/ArtifactArchiver.excludes-JENKINS-29922

[JENKINS-29922] Correct the default value handling of ArtifactArchiver.excludes

core/src/main/java/hudson/tasks/ArtifactArchiver.java

@@ -50,6 +50,7 @@ import java.util.HashMap;
 import java.util.Map;
 import java.util.logging.Level;
 import java.util.logging.Logger;
+import javax.annotation.CheckForNull;
 
 import net.sf.json.JSONObject;
 import javax.annotation.Nonnull;
@@ -76,7 +77,7 @@ public class ArtifactArchiver extends Recorder implements SimpleBuildStep {
     /**
      * Possibly null 'excludes' pattern as in Ant.
      */
-    private String excludes = "";
+    private String excludes;
 
     @Deprecated
     private Boolean latestOnly;
@@ -154,11 +155,11 @@ public class ArtifactArchiver extends Recorder implements SimpleBuildStep {
         return artifacts;
     }
 
-    public String getExcludes() {
+    public @CheckForNull String getExcludes() {
         return excludes;
     }
 
-    @DataBoundSetter public final void setExcludes(String excludes) {
+    @DataBoundSetter public final void setExcludes(@CheckForNull String excludes) {
         this.excludes = Util.fixEmptyAndTrim(excludes);
     }
 

test/pom.xml

@@ -132,6 +132,12 @@ THE SOFTWARE.
       <version>1.2-beta-4</version>
       <scope>test</scope>
     </dependency>
+    <dependency>
+      <groupId>org.jenkins-ci.plugins</groupId>
+      <artifactId>structs</artifactId>
+      <version>1.2</version>
+      <scope>test</scope>
+    </dependency>
     <dependency>
       <groupId>org.jvnet.mock-javamail</groupId>
       <artifactId>mock-javamail</artifactId>

test/src/test/java/hudson/tasks/ArtifactArchiverTest.java

@@ -27,8 +27,8 @@ package hudson.tasks;
 import hudson.AbortException;
 import hudson.FilePath;
 import hudson.Launcher;
+import hudson.Util;
 import hudson.model.AbstractBuild;
-import hudson.model.AbstractProject;
 import hudson.model.BuildListener;
 import hudson.model.FreeStyleBuild;
 import hudson.model.FreeStyleProject;
@@ -40,6 +40,7 @@ import java.net.HttpURLConnection;
 import java.util.Collections;
 import java.util.List;
 import jenkins.util.VirtualFile;
+import org.jenkinsci.plugins.structs.describable.DescribableModel;
 import static org.junit.Assert.*;
 import static org.junit.Assume.*;
 
@@ -155,19 +156,7 @@ public class ArtifactArchiverTest {
         assertFalse(kids[0].exists());
         j.createWebClient().assertFails(b.getUrl() + "artifact/hack", HttpURLConnection.HTTP_NOT_FOUND);
     }
-    
-    private void runNewBuildAndStartUnitlIsCreated(AbstractProject project) throws InterruptedException{
-        int buildNumber = project.getNextBuildNumber();
-        project.scheduleBuild2(0);
-        int count = 0;
-        while(project.getBuildByNumber(buildNumber)==null && count<50){
-            Thread.sleep(100);
-            count ++;
-        }
-        if(project.getBuildByNumber(buildNumber)==null)
-            fail("Build " + buildNumber + " did not created.");
-    }
-    
+
     static class CreateArtifact extends TestBuilder {
         public boolean perform(AbstractBuild<?,?> build, Launcher launcher, BuildListener listener) throws IOException, InterruptedException {
             build.getWorkspace().child("f").write("content", "UTF-8");
@@ -197,8 +186,22 @@ public class ArtifactArchiverTest {
         assertFalse(project.getBuildByNumber(2).getHasArtifacts());
     }
 
-
-
+    @Issue("JENKINS-29922")
+    @Test
+    public void configRoundTrip() throws Exception {
+        ArtifactArchiver aa = new ArtifactArchiver("*.txt");
+        assertNull(Util.fixEmpty(aa.getExcludes())); // null and "" behave the same, we do not care which it is
+        assertEquals("{artifacts=*.txt}", DescribableModel.uninstantiate_(aa).toString()); // but we do care that excludes is considered to be at the default
+        aa = j.configRoundtrip(aa);
+        assertEquals("*.txt", aa.getArtifacts());
+        assertNull(Util.fixEmpty(aa.getExcludes()));
+        assertEquals("{artifacts=*.txt}", DescribableModel.uninstantiate_(aa).toString());
+        aa.setExcludes("README.txt");
+        aa = j.configRoundtrip(aa);
+        assertEquals("*.txt", aa.getArtifacts());
+        assertEquals("README.txt", aa.getExcludes());
+        assertEquals("{artifacts=*.txt, excludes=README.txt}", DescribableModel.uninstantiate_(aa).toString()); // TreeMap, so attributes will be sorted
+    }
 
     static class CreateDefaultExcludesArtifact extends TestBuilder {
         public boolean perform(AbstractBuild<?,?> build, Launcher launcher, BuildListener listener) throws IOException, InterruptedException {