[Fixed JENKINS-44037] Add a new BuildData action if PreBuildMerge f…

…ails This fixes a buggy behavior where BuildData from the previous Run is used instead, which corrupts build history.
jenkinsci · May 3, 2017 · 6b9bb1b · 6b9bb1b
1 parent b0a5ebf
commit 6b9bb1b
Show file tree

Hide file tree

Showing 3 changed files with 125 additions and 5 deletions.
diff --git a/src/main/java/hudson/plugins/git/extensions/impl/PreBuildMerge.java b/src/main/java/hudson/plugins/git/extensions/impl/PreBuildMerge.java
@@ -21,6 +21,7 @@
 import org.kohsuke.stapler.DataBoundConstructor;
 
 import java.io.IOException;
+import java.util.List;
 
 import static hudson.model.Result.FAILURE;
 import hudson.model.Run;
@@ -86,12 +87,30 @@ public Revision decorateRevisionToBuild(GitSCM scm, Run<?, ?> build, GitClient g
             // record the fact that we've tried building 'rev' and it failed, or else
             // BuildChooser in future builds will pick up this same 'rev' again and we'll see the exact same merge failure
             // all over again.
-            BuildData bd = scm.getBuildData(build);
-            if(bd != null){
-                bd.saveBuild(new Build(marked,rev, build.getNumber(), FAILURE));
-            } else {
-                listener.getLogger().println("Was not possible to get build data");
+
+            // Track whether we're trying to add a duplicate BuildData, now that it's been updated with
+            // revision info for this build etc. The default assumption is that it's a duplicate.
+            BuildData buildData = scm.getBuildData(build, true);
+            boolean buildDataAlreadyPresent = false;
+            List<BuildData> actions = build.getActions(BuildData.class);
+            for (BuildData d: actions)  {
+                if (d.similarTo(buildData)) {
+                    buildDataAlreadyPresent = true;
+                    break;
+                }
+            }
+            if (!actions.isEmpty()) {
+                buildData.setIndex(actions.size()+1);
             }
+
+            // If the BuildData is not already attached to this build, add it to the build and mark that
+            // it wasn't already present, so that we add the GitTagAction and changelog after the checkout
+            // finishes.
+            if (!buildDataAlreadyPresent) {
+                build.addAction(buildData);
+            }
+
+            buildData.saveBuild(new Build(marked,rev, build.getNumber(), FAILURE));
             throw new AbortException("Branch not suitable for integration as it does not merge cleanly: " + ex.getMessage());
         }
 

diff --git a/src/test/java/hudson/plugins/git/extensions/GitSCMExtensionTest.java b/src/test/java/hudson/plugins/git/extensions/GitSCMExtensionTest.java
@@ -37,6 +37,12 @@ public void setUp() throws Exception {
 	}
 
 	protected abstract void before() throws Exception;
+
+	/**
+	 * The {@link GitSCMExtension} being tested - this will be added to the
+	 * project built in {@link #setupBasicProject(TestGitRepo)}
+	 * @return
+	 */
 	protected abstract GitSCMExtension getExtension();
 
 	protected FreeStyleBuild build(final FreeStyleProject project, final Result expectedResult) throws Exception {
@@ -47,6 +53,14 @@ protected FreeStyleBuild build(final FreeStyleProject project, final Result expe
 		return build;
 	}
 
+	/**
+	 * Create a {@link FreeStyleProject} configured with a {@link GitSCM}
+	 * building on the {@code master} branch of the provided {@code repo},
+	 * and with the extension described in {@link #getExtension()} added.
+	 * @param repo
+	 * @return
+	 * @throws Exception
+	 */
 	protected FreeStyleProject setupBasicProject(TestGitRepo repo) throws Exception {
 		GitSCMExtension extension = getExtension();
 		FreeStyleProject project = j.createFreeStyleProject(extension.getClass() + "Project");

diff --git a/src/test/java/hudson/plugins/git/extensions/impl/PreBuildMergeTest.java b/src/test/java/hudson/plugins/git/extensions/impl/PreBuildMergeTest.java
@@ -0,0 +1,87 @@
+package hudson.plugins.git.extensions.impl;
+
+import hudson.model.FreeStyleBuild;
+import hudson.model.FreeStyleProject;
+import hudson.model.Result;
+import hudson.plugins.git.GitSCM;
+import hudson.plugins.git.Revision;
+import hudson.plugins.git.TestGitRepo;
+import hudson.plugins.git.UserMergeOptions;
+import hudson.plugins.git.extensions.GitSCMExtension;
+import hudson.plugins.git.extensions.GitSCMExtensionTest;
+import hudson.plugins.git.util.BuildData;
+import org.jenkinsci.plugins.gitclient.MergeCommand;
+import org.junit.Test;
+
+import static org.hamcrest.Matchers.containsString;
+import static org.junit.Assert.*;
+
+/**
+ * @author dalvizu
+ */
+public class PreBuildMergeTest extends GitSCMExtensionTest
+{
+    private FreeStyleProject project;
+
+    private TestGitRepo repo;
+
+    private String MASTER_FILE = "commitFileBase";
+
+    public void before() throws Exception {
+        repo = new TestGitRepo("repo", tmp.newFolder(), listener);
+        project = setupBasicProject(repo);
+        // make an initial commit to master
+        repo.commit(MASTER_FILE, repo.johnDoe, "Initial Commit");
+        // create integration branch
+        repo.git.branch("integration");
+    }
+
+    @Test
+    public void testBasicPreMerge() throws Exception {
+        FreeStyleBuild firstBuild = build(project, Result.SUCCESS);
+    }
+
+    @Test
+    public void testFailedMerge() throws Exception {
+        FreeStyleBuild firstBuild = build(project, Result.SUCCESS);
+        assertEquals(GitSCM.class, project.getScm().getClass());
+        GitSCM gitSCM = (GitSCM)project.getScm();
+        BuildData buildData = gitSCM.getBuildData(firstBuild);
+        assertNotNull("Build data not found", buildData);
+        assertEquals(firstBuild.getNumber(), buildData.lastBuild.getBuildNumber());
+        Revision firstMarked = buildData.lastBuild.getMarked();
+        Revision firstRevision = buildData.lastBuild.getRevision();
+        assertNotNull(firstMarked);
+        assertNotNull(firstRevision);
+
+        // pretend we merged and published it successfully
+        repo.git.deleteBranch("integration");
+        repo.git.checkoutBranch("integration", "master");
+        repo.commit(MASTER_FILE, "new content on integration branch", repo.johnDoe, repo.johnDoe, "Commit which should fail!");
+        repo.git.checkout("master");
+
+        // make a new commit in master branch, this commit should not merge cleanly!
+        assertFalse("SCM polling should not detect any more changes after build", project.poll(listener).hasChanges());
+        String conflictSha1 = repo.commit(MASTER_FILE, "new content - expect a merge conflict!", repo.johnDoe, repo.johnDoe, "Commit which should fail!");
+        assertTrue("SCM polling should detect changes", project.poll(listener).hasChanges());
+
+        FreeStyleBuild secondBuild = build(project, Result.FAILURE);
+        assertThat(secondBuild.getLog(), containsString("Automatic merge failed; fix conflicts and then commit the result."));
+        assertEquals(secondBuild.getNumber(), gitSCM.getBuildData(secondBuild).lastBuild.getBuildNumber());
+        // buildData should mark this as built
+        assertEquals(conflictSha1, gitSCM.getBuildData(secondBuild).lastBuild.getMarked().getSha1String());
+        assertEquals(conflictSha1, gitSCM.getBuildData(secondBuild).lastBuild.getRevision().getSha1String());
+
+        // Check to see that build data is not corrupted (JENKINS-44037)
+        assertEquals(firstBuild.getNumber(), gitSCM.getBuildData(firstBuild).lastBuild.getBuildNumber());
+        assertEquals(firstMarked, gitSCM.getBuildData(firstBuild).lastBuild.getMarked());
+        assertEquals(firstRevision, gitSCM.getBuildData(firstBuild).lastBuild.getRevision());
+    }
+
+    @Override
+    protected GitSCMExtension getExtension() {
+        return new PreBuildMerge(new UserMergeOptions("origin", "integration", "default",
+                MergeCommand.GitPluginFastForwardMode.FF));
+    }
+
+}