Test JENKINS-30178 using GitStatus.toString

Asserts that job parameter default values are available when a job is started by a notifyCommit. If the notifyCommit includes a sha1 parameter, then the changes for JENKINS-27092 fail to assign parameters their default values. Modifying the GitStatus object to be more easily tested was simpler than using a TestExtension. Should eventually replace the testing misuse of the GitStatus.toString() method with a TestExtension of GitStatus.Listener.
jenkinsci · Dec 8, 2015 · 2dfd86d · 2dfd86d · jtnord · Dec 8, 2015
1 parent 4937c6c
commit 2dfd86d
Show file tree

Hide file tree

Showing 2 changed files with 188 additions and 5 deletions.
diff --git a/src/main/java/hudson/plugins/git/GitStatus.java b/src/main/java/hudson/plugins/git/GitStatus.java
@@ -53,9 +53,62 @@ public String getUrlName() {
         return "git";
     }
 
+    private String lastURL = "";        // Required query parameter
+    private String lastBranches = null; // Optional query parameter
+    private String lastSHA1 = null;     // Optional query parameter
+    private List<ParameterValue> lastBuildParameters = null;
+    private static List<ParameterValue> lastStaticBuildParameters = null;
+
+    @Override
+    public String toString() {
+        StringBuilder s = new StringBuilder();
+
+        s.append("URL: ");
+        s.append(lastURL);
+
+        if (lastSHA1 != null) {
+            s.append(" SHA1: ");
+            s.append(lastSHA1);
+        }
+
+        if (lastBranches != null) {
+            s.append(" Branches: ");
+            s.append(lastBranches);
+        }
+
+        if (lastBuildParameters != null && !lastBuildParameters.isEmpty()) {
+            s.append(" Parameters: ");
+            for (ParameterValue buildParameter : lastBuildParameters) {
+                s.append(buildParameter.getName());
+                s.append("='");
+                s.append(buildParameter.getValue());
+                s.append("',");
+            }
+            s.delete(s.length() - 1, s.length());
+        }
+
+        if (lastStaticBuildParameters != null && !lastStaticBuildParameters.isEmpty()) {
+            s.append(" More parameters: ");
+            for (ParameterValue buildParameter : lastStaticBuildParameters) {
+                s.append(buildParameter.getName());
+                s.append("='");
+                s.append(buildParameter.getValue());
+                s.append("',");
+            }
+            s.delete(s.length() - 1, s.length());
+        }
+
+        return s.toString();
+    }
+
     public HttpResponse doNotifyCommit(HttpServletRequest request, @QueryParameter(required=true) String url,
                                        @QueryParameter(required=false) String branches,
                                        @QueryParameter(required=false) String sha1) throws ServletException, IOException {
+        lastURL = url;
+        lastBranches = branches;
+        lastSHA1 = sha1;
+        lastBuildParameters = null;
+        lastStaticBuildParameters = null;
         URIish uri;
         List<ParameterValue> buildParameters = new ArrayList<ParameterValue>();
 
@@ -71,6 +124,7 @@ public HttpResponse doNotifyCommit(HttpServletRequest request, @QueryParameter(r
                 if (entry.getValue()[0] != null)
                     buildParameters.add(new StringParameterValue(entry.getKey(), entry.getValue()[0]));
         }
+        lastBuildParameters = buildParameters;
 
         branches = Util.fixEmptyAndTrim(branches);
 
@@ -207,6 +261,8 @@ public List<ResponseContributor> onNotifyCommit(URIish uri, String sha1, List<Pa
                 LOGGER.fine("Received notification for uri = " + uri + " ; sha1 = " + sha1 + " ; branches = " + Arrays.toString(branches));
             }
 
+            lastStaticBuildParameters = null;
+            List<ParameterValue> allBuildParameters = new ArrayList<ParameterValue>(buildParameters);
             List<ResponseContributor> result = new ArrayList<ResponseContributor>();
             // run in high privilege to see all the projects anonymous users don't see.
             // this is safe because when we actually schedule a build, it's a build that can
@@ -281,12 +337,20 @@ public List<ResponseContributor> onNotifyCommit(URIish uri, String sha1, List<Pa
 
                             if (!(project instanceof AbstractProject && ((AbstractProject) project).isDisabled())) {
                                 if (!parametrizedBranchSpec && isNotEmpty(sha1)) {
+                                    /* If SHA1 and not a parameterized branch spec, then schedule build.
+                                     * NOTE: This is SCHEDULING THE BUILD, not triggering polling of the repo.
+                                     * If no SHA1 or the branch spec is parameterized, it will only poll.
+                                     */
                                     LOGGER.info("Scheduling " + project.getFullDisplayName() + " to build commit " + sha1);
                                     scmTriggerItem.scheduleBuild2(scmTriggerItem.getQuietPeriod(),
                                             new CauseAction(new CommitHookCause(sha1)),
                                             new RevisionParameterAction(sha1, matchedURL), new ParametersAction(buildParameters));
                                     result.add(new ScheduledResponseContributor(project));
                                 } else {
+                                    /* Poll the repository for changes
+                                     * NOTE: This is not scheduling the build, just polling for changes
+                                     * If the polling detects changes, it will schedule the build
+                                     */
                                     LOGGER.info("Triggering the polling of " + project.getFullDisplayName());
                                     trigger.run();
                                     result.add(new PollingScheduledResponseContributor(project));
@@ -306,6 +370,7 @@ public List<ResponseContributor> onNotifyCommit(URIish uri, String sha1, List<Pa
                                     .join(branches, ",")));
                 }
 
+                lastStaticBuildParameters = allBuildParameters;
                 return result;
             } finally {
                 SecurityContextHolder.setContext(old);

diff --git a/src/test/java/hudson/plugins/git/GitStatusTest.java b/src/test/java/hudson/plugins/git/GitStatusTest.java
@@ -1,11 +1,19 @@
 package hudson.plugins.git;
 
 import hudson.model.Action;
+import hudson.model.Cause;
+import hudson.model.FreeStyleBuild;
 import hudson.model.FreeStyleProject;
 import hudson.model.ParameterValue;
 import hudson.model.ParametersAction;
+import hudson.model.ParametersDefinitionProperty;
+import hudson.model.StringParameterDefinition;
 import hudson.plugins.git.extensions.GitSCMExtension;
+import hudson.tasks.BatchFile;
+import hudson.tasks.CommandInterpreter;
+import hudson.tasks.Shell;
 import hudson.triggers.SCMTrigger;
+import java.io.File;
 import java.net.URISyntaxException;
 import java.util.*;
 
@@ -26,12 +34,18 @@ public class GitStatusTest extends AbstractGitProject {
     private GitStatus gitStatus;
     private HttpServletRequest requestWithNoParameter;
     private HttpServletRequest requestWithParameter;
+    private String repoURL;
+    private String branch;
+    private String sha1;
 
     @Before
     public void setUp() throws Exception {
         this.gitStatus = new GitStatus();
         this.requestWithNoParameter = mock(HttpServletRequest.class);
         this.requestWithParameter = mock(HttpServletRequest.class);
+        this.repoURL = new File(".").getAbsolutePath();
+        this.branch = "**";
+        this.sha1 = "7bb68ef21dc90bd4f7b08eca876203b2e049198d";
     }
 
     @WithoutJenkins
@@ -231,20 +245,124 @@ public void testLooselyMatches() throws URISyntaxException {
             "git@github.com:jenkinsci/git-plugin.git/"
         };
         List<URIish> uris = new ArrayList<URIish>();
-        for (String repoURL : equivalentRepoURLs) {
-            uris.add(new URIish(repoURL));
+        for (String testURL : equivalentRepoURLs) {
+            uris.add(new URIish(testURL));
         }
 
         /* Extra slashes on end of URL probably should be considered equivalent,
-         * but current implementation does not consider them as loose matches 
+         * but current implementation does not consider them as loose matches
          */
-        URIish badURL = new URIish(equivalentRepoURLs[0] + "///");
+        URIish badURLTrailingSlashes = new URIish(equivalentRepoURLs[0] + "///");
+        /* Different hostname should always fail match check */
+        URIish badURLHostname = new URIish(equivalentRepoURLs[0].replace("github.com", "bitbucket.org"));
 
         for (URIish lhs : uris) {
-            assertFalse(lhs + " matches " + badURL, GitStatus.looselyMatches(lhs, badURL));
+            assertFalse(lhs + " matches trailing slashes " + badURLTrailingSlashes, GitStatus.looselyMatches(lhs, badURLTrailingSlashes));
+            assertFalse(lhs + " matches bad hostname " + badURLHostname, GitStatus.looselyMatches(lhs, badURLHostname));
             for (URIish rhs : uris) {
                 assertTrue(lhs + " and " + rhs + " didn't match", GitStatus.looselyMatches(lhs, rhs));
             }
         }
     }
+
+    private FreeStyleProject setupNotifyProject() throws Exception {
+        FreeStyleProject project = jenkins.createFreeStyleProject();
+        project.setQuietPeriod(0);
+        GitSCM git = new GitSCM(
+                Collections.singletonList(new UserRemoteConfig(repoURL, null, null, null)),
+                Collections.singletonList(new BranchSpec(branch)),
+                false, Collections.<SubmoduleConfig>emptyList(),
+                null, null,
+                Collections.<GitSCMExtension>emptyList());
+        project.setScm(git);
+        project.addTrigger(new SCMTrigger("")); // Required for GitStatus to see polling request
+        return project;
+    }
+
+    private Map<String, String[]> setupParameterMap() {
+        Map<String, String[]> parameterMap = new HashMap<String, String[]>();
+        String[] repoURLs = {repoURL};
+        parameterMap.put("url", repoURLs);
+        String[] branches = {branch};
+        parameterMap.put("branches", branches);
+        String[] hashes = {sha1};
+        parameterMap.put("sha1", hashes);
+        return parameterMap;
+    }
+
+    private Map<String, String[]> setupParameterMap(String extraValue) {
+        Map<String, String[]> parameterMap = setupParameterMap();
+        String[] extra = {extraValue};
+        parameterMap.put("extra", extra);
+        return parameterMap;
+    }
+
+    @Test
+    public void testDoNotifyCommitNoParameters() throws Exception {
+        setupNotifyProject();
+        this.gitStatus.doNotifyCommit(requestWithNoParameter, repoURL, branch, sha1);
+        assertEquals("URL: " + repoURL
+                + " SHA1: " + sha1
+                + " Branches: " + branch, this.gitStatus.toString());
+    }
+
+    @Test
+    public void testDoNotifyCommitWithExtraParameter() throws Exception {
+        setupNotifyProject();
+        String extraValue = "An-extra-value";
+        when(requestWithParameter.getParameterMap()).thenReturn(setupParameterMap(extraValue));
+        this.gitStatus.doNotifyCommit(requestWithParameter, repoURL, branch, sha1);
+        assertEquals("URL: " + repoURL
+                + " SHA1: " + sha1
+                + " Branches: " + branch
+                + " Parameters: extra='" + extraValue + "'"
+                + " More parameters: extra='" + extraValue + "'", this.gitStatus.toString());
+    }
+
+    @Test
+    public void testDoNotifyCommitWithNullValueExtraParameter() throws Exception {
+        setupNotifyProject();
+        when(requestWithParameter.getParameterMap()).thenReturn(setupParameterMap(null));
+        this.gitStatus.doNotifyCommit(requestWithParameter, repoURL, branch, sha1);
+        assertEquals("URL: " + repoURL
+                + " SHA1: " + sha1
+                + " Branches: " + branch, this.gitStatus.toString());
+    }
+
+    @Test
+    public void testDoNotifyCommitWithDefaultParameter() throws Exception {
+        // Use official repo for this single test
+        this.repoURL = "https://github.com/jenkinsci/git-plugin.git";
+        FreeStyleProject project = setupNotifyProject();
+        project.addProperty(new ParametersDefinitionProperty(
+                new StringParameterDefinition("A", "aaa"),
+                new StringParameterDefinition("C", "ccc"),
+                new StringParameterDefinition("B", "$A$C")
+        ));
+        final CommandInterpreter script = isWindows()
+                ? new BatchFile("echo %A% %B% %C%")
+                : new Shell("echo $A $B $C");
+        project.getBuildersList().add(script);
+
+        FreeStyleBuild build = project.scheduleBuild2(0, new Cause.UserCause()).get();
+
+        jenkins.assertLogContains("aaa aaaccc ccc", build);
+
+        String extraValue = "An-extra-value";
+        when(requestWithParameter.getParameterMap()).thenReturn(setupParameterMap(extraValue));
+        this.gitStatus.doNotifyCommit(requestWithParameter, repoURL, branch, sha1);
+        assertEquals("URL: " + repoURL
+                + " SHA1: " + sha1
+                + " Branches: " + branch
+                + " Parameters: extra='" + extraValue + "'"
+                + " More parameters: extra='" + extraValue + "',A='aaa',C='ccc',B='$A$C'", this.gitStatus.toString());
+    }
+
+    /**
+     * inline ${@link hudson.Functions#isWindows()} to prevent a transient
+     * remote classloader issue
+     */
+    private boolean isWindows() {
+        return File.pathSeparatorChar == ';';
+    }
 }