Merge pull request #29 from juherr/JENKINS-32746

Fix JENKINS-32746: Change loop determination condition in order to check all loaded builds
jenkinsci · May 14, 2017 · c9c17fe · c9c17fe
2 parents 25ff107 + a956ec0
commit c9c17fe
Show file tree

Hide file tree

Showing 2 changed files with 55 additions and 4 deletions.
diff --git a/src/main/java/hudson/plugins/testng/TestNGProjectAction.java b/src/main/java/hudson/plugins/testng/TestNGProjectAction.java
@@ -13,8 +13,9 @@
 import hudson.tasks.test.TestResultProjectAction;
 import hudson.util.ChartUtil;
 import hudson.util.DataSetBuilder;
-import java.util.Set;
+import java.util.SortedMap;
 import jenkins.model.lazy.LazyBuildMixIn;
+import jenkins.model.lazy.LazyBuildMixIn.LazyLoadingJob;
 import org.jfree.chart.JFreeChart;
 import org.kohsuke.stapler.Stapler;
 import org.kohsuke.stapler.StaplerRequest;
@@ -195,9 +196,10 @@ protected void populateDataSetBuilder(DataSetBuilder<String, ChartUtil.NumberOnl
       if (!(job instanceof LazyBuildMixIn.LazyLoadingJob)) {
          return;
       }
-      Set<Integer> loadedBuilds = ((LazyBuildMixIn.LazyLoadingJob<?,?>) job).getLazyBuildMixIn()._getRuns().getLoadedBuilds().keySet(); // cf. AbstractTestResultAction.getPreviousResult(Class, false)
-      for (Run<?, ?> build = getProject().getLastBuild();
-         build != null; build = loadedBuilds.contains(build.number - 1) ? build.getPreviousCompletedBuild() : null) {
+
+      // cf. AbstractTestResultAction.getPreviousResult(Class, false)
+      SortedMap<Integer, Run<?, ?>> loadedBuilds = (SortedMap<Integer, Run<?, ?>>) ((LazyLoadingJob<?,?>) job).getLazyBuildMixIn()._getRuns().getLoadedBuilds();
+      for (Run<?, ?> build : loadedBuilds.values()) {
          ChartUtil.NumberOnlyBuildLabel label = new ChartUtil.NumberOnlyBuildLabel(build);
          TestNGTestResultBuildAction action = build.getAction(getBuildActionClass());
 

diff --git a/src/test/java/hudson/plugins/testng/TestNGProjectActionTest.java b/src/test/java/hudson/plugins/testng/TestNGProjectActionTest.java
@@ -10,6 +10,8 @@
 import hudson.plugins.testng.util.TestResultHistoryUtil;
 import hudson.tasks.test.AbstractTestResultAction;
 import hudson.tasks.test.TestResult;
+import hudson.util.ChartUtil;
+import hudson.util.DataSetBuilder;
 import org.junit.Assert;
 import org.junit.Rule;
 import org.junit.Test;
@@ -78,4 +80,51 @@ public boolean perform(AbstractBuild<?, ?> build, Launcher launcher,
         String summary = TestResultHistoryUtil.toSummary(projAction.getLastCompletedBuildAction());
         Assert.assertTrue(summary.contains("gov.nasa.jpl.FoobarTests.b"));
     }
+
+    @Test
+    // For JENKINS-32746: TestNG Results Trend graph doesn't show all build results
+    public void testHistoryRemoval() throws Exception {
+        FreeStyleProject p = r.createFreeStyleProject();
+        Publisher publisher = new Publisher();
+        publisher.setReportFilenamePattern("some.xml");
+        publisher.setEscapeTestDescp(false);
+        publisher.setEscapeExceptionMsg(true);
+        p.getPublishersList().add(publisher);
+        p.onCreatedFromScratch(); //to setup project action
+
+        p.getBuildersList().add(new TestBuilder() {
+            @Override
+            public boolean perform(AbstractBuild<?, ?> build, Launcher launcher,
+                BuildListener listener) throws InterruptedException, IOException {
+                //any testng xml will do
+                String contents = CommonUtil.getContents(Constants.TESTNG_XML_EXP_MSG_XML);
+                build.getWorkspace().child("some.xml").write(contents,"UTF-8");
+                return true;
+            }
+        });
+
+        //run build
+        int buildNumber = 5;
+        FreeStyleBuild[] builds = new FreeStyleBuild[buildNumber];
+        for (int i = 0; i < buildNumber; i++) {
+            builds[i] = p.scheduleBuild2(0).get();
+        }
+
+        TestNGProjectAction action = p.getAction(TestNGProjectAction.class);
+
+        DataSetBuilder<String, ChartUtil.NumberOnlyBuildLabel> dataSetBuilder = new DataSetBuilder<String, ChartUtil.NumberOnlyBuildLabel>();
+        action.populateDataSetBuilder(dataSetBuilder);
+
+        Assert.assertEquals(buildNumber, dataSetBuilder.build().getColumnCount());
+
+        int[] buildsToRemove = { 2, 3 };
+        for (int buildToRemove : buildsToRemove) {
+            builds[buildToRemove].delete();
+        }
+
+        dataSetBuilder = new DataSetBuilder<String, ChartUtil.NumberOnlyBuildLabel>();
+        action.populateDataSetBuilder(dataSetBuilder);
+
+        Assert.assertEquals((buildNumber - buildsToRemove.length), dataSetBuilder.build().getColumnCount());
+    }
 }