[FIXED JENKINS-4617]

this and the previous commit constitutes the fix.
Also added a test case.

changelog.html

@@ -61,6 +61,9 @@ Upcoming changes</a>
   <li class=bug>
     Debian init script gives false positives for port already in use
     (<a href="http://issues.jenkins-ci.org/browse/JENKINS-9281">issue 9281</a>)
+  <li class=bug>
+    "include culprits" should treat unstable and failure as the same
+    (<a href="http://issues.jenkins-ci.org/browse/JENKINS-4617">issue 4617</a>)
   <li class=rfe>
     Added two new CLI commands "wait-node-online" and "wait-node-offline" to block until a slave becomes online/offline.
   <li class=rfe>

core/src/main/java/hudson/model/AbstractBuild.java

@@ -269,7 +269,7 @@ public abstract class AbstractBuild<P extends AbstractProject<P,R>,R extends Abs
             R p = getPreviousCompletedBuild();
             if (p !=null && isBuilding()) {
                 Result pr = p.getResult();
-                if (pr!=null && pr.isWorseThan(Result.UNSTABLE)) {
+                if (pr!=null && pr.isWorseThan(Result.SUCCESS)) {
                     // we are still building, so this is just the current latest information,
                     // but we seems to be failing so far, so inherit culprits from the previous build.
                     // isBuilding() check is to avoid recursion when loading data from old Hudson, which doesn't record

test/src/main/java/org/jvnet/hudson/test/FakeChangeLogSCM.java

+/*
+ * The MIT License
+ *
+ * Copyright (c) 2011, CloudBees, Inc.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+package org.jvnet.hudson.test;
+
+import hudson.FilePath;
+import hudson.Launcher;
+import hudson.model.AbstractBuild;
+import hudson.model.AbstractProject;
+import hudson.model.BuildListener;
+import hudson.model.InvisibleAction;
+import hudson.model.JobProperty;
+import hudson.model.User;
+import hudson.scm.ChangeLogParser;
+import hudson.scm.ChangeLogSet;
+import hudson.scm.ChangeLogSet.Entry;
+import hudson.scm.NullSCM;
+import org.xml.sax.SAXException;
+
+import java.io.File;
+import java.io.IOException;
+import java.io.Serializable;
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.Iterator;
+import java.util.List;
+
+/**
+ * Fake SCM implementation that can report arbitrary commits from arbitrary users.
+ *
+ * @author Kohsuke Kawaguchi
+ */
+public class FakeChangeLogSCM extends NullSCM {
+
+    /**
+     * Changes to be reported in the next build.
+     */
+    private List<EntryImpl> entries = new ArrayList<EntryImpl>();
+
+    public EntryImpl addChange() {
+        EntryImpl e = new EntryImpl();
+        entries.add(e);
+        return e;
+    }
+
+    @Override
+    public boolean checkout(AbstractBuild<?, ?> build, Launcher launcher, FilePath remoteDir, BuildListener listener, File changeLogFile) throws IOException, InterruptedException {
+        new FilePath(changeLogFile).touch(0);
+        build.addAction(new ChangelogAction(entries));
+        entries = new ArrayList<EntryImpl>();
+        return true;
+    }
+
+    @Override
+    public ChangeLogParser createChangeLogParser() {
+        return new FakeChangeLogParser();
+    }
+
+    public static class ChangelogAction extends InvisibleAction {
+        private final List<EntryImpl> entries;
+
+        public ChangelogAction(List<EntryImpl> entries) {
+            this.entries = entries;
+        }
+    }
+
+    public static class FakeChangeLogParser extends ChangeLogParser {
+        @Override
+        public FakeChangeLogSet parse(AbstractBuild build, File changelogFile) throws IOException, SAXException {
+            return new FakeChangeLogSet(build, build.getAction(ChangelogAction.class).entries);
+        }
+    }
+
+    public static class FakeChangeLogSet extends ChangeLogSet<EntryImpl> {
+        private List<EntryImpl> entries;
+
+        public FakeChangeLogSet(AbstractBuild<?, ?> build, List<EntryImpl> entries) {
+            super(build);
+            this.entries = entries;
+        }
+
+        @Override
+        public boolean isEmptySet() {
+            return entries.isEmpty();
+        }
+
+        public Iterator<EntryImpl> iterator() {
+            return entries.iterator();
+        }
+    }
+
+    public static class EntryImpl extends Entry {
+        private String msg = "some commit message";
+        private String author = "someone";
+
+        public EntryImpl withAuthor(String author) {
+            this.author = author;
+            return this;
+        }
+
+        public EntryImpl withMsg(String msg) {
+            this.msg = msg;
+            return this;
+        }
+
+        @Override
+        public String getMsg() {
+            return msg;
+        }
+
+        @Override
+        public User getAuthor() {
+            return User.get(author);
+        }
+
+        @Override
+        public Collection<String> getAffectedPaths() {
+            return Collections.singleton("path");
+        }
+    }
+}

test/src/test/groovy/hudson/model/AbstractBuildTest.groovy

@@ -30,6 +30,10 @@ import hudson.slaves.EnvironmentVariablesNodeProperty.Entry
 import junit.framework.Assert
 import org.jvnet.hudson.test.CaptureEnvironmentBuilder
 import org.jvnet.hudson.test.GroovyHudsonTestCase
+import org.jvnet.hudson.test.ExtractResourceWithChangesSCM
+import org.jvnet.hudson.test.FakeChangeLogSCM
+import org.jvnet.hudson.test.FailureBuilder
+import org.jvnet.hudson.test.UnstableBuilder
 
 public class AbstractBuildTest extends GroovyHudsonTestCase {
 	void testVariablesResolved() {
@@ -61,4 +65,53 @@ public class AbstractBuildTest extends GroovyHudsonTestCase {
         println "Output:\n"+rsp.webResponse.contentAsString
         assertTrue(rsp.webResponse.contentAsString.contains(out));
     }
+
+    def assertCulprits(AbstractBuild b, Collection<String> expectedIds) {
+        assertEquals(expectedIds as Set, b.culprits*.id as Set);
+    }
+
+    void testCulprits() {
+
+        def p = createFreeStyleProject();
+        def scm = new FakeChangeLogSCM()
+        p.scm = scm
+
+        // 1st build, successful, no culprits
+        scm.addChange().withAuthor("alice")
+        def b = assertBuildStatus(Result.SUCCESS,p.scheduleBuild2(0).get())
+        assertCulprits(b,["alice"])
+
+        // 2nd build
+        scm.addChange().withAuthor("bob")
+        p.buildersList.add(new FailureBuilder())
+        b = assertBuildStatus(Result.FAILURE,p.scheduleBuild2(0).get())
+        assertCulprits(b,["bob"])
+
+        // 3rd build. bob continues to be in culprit
+        scm.addChange().withAuthor("charlie")
+        b = assertBuildStatus(Result.FAILURE,p.scheduleBuild2(0).get())
+        assertCulprits(b,["bob","charlie"])
+
+        // 4th build, unstable. culprit list should continue
+        scm.addChange().withAuthor("dave")
+        p.buildersList.replaceBy([new UnstableBuilder()])
+        b = assertBuildStatus(Result.UNSTABLE,p.scheduleBuild2(0).get())
+        assertCulprits(b,["bob","charlie","dave"])
+
+        // 5th build, unstable. culprit list should continue
+        scm.addChange().withAuthor("eve")
+        b = assertBuildStatus(Result.UNSTABLE,p.scheduleBuild2(0).get())
+        assertCulprits(b,["bob","charlie","dave","eve"])
+
+        // 6th build, success, accumulation continues up to this point
+        scm.addChange().withAuthor("fred")
+        p.buildersList.replaceBy([])
+        b = assertBuildStatus(Result.SUCCESS,p.scheduleBuild2(0).get())
+        assertCulprits(b,["bob","charlie","dave","eve","fred"])
+
+        // 7th build, back to empty culprits
+        scm.addChange().withAuthor("george")
+        b = assertBuildStatus(Result.SUCCESS,p.scheduleBuild2(0).get())
+        assertCulprits(b,["george"])
+    }
 }