From 231f658d79a79ebe88ff8412bc3856590b0e2853 Mon Sep 17 00:00:00 2001 From: Kanstantsin Shautsou Date: Thu, 30 Apr 2015 01:58:25 +0300 Subject: [PATCH] Fix PR comments - Print publisher display name instead of class, so user can understand what publisher in UI was used and failed. - Setter for artifact archiver shows how it handle status - Minor typos - Added test for stacktraces in build log (cherry picked from commit 92734a8345b782ffd7e5f7c6f023b6228aa4c93d) --- .../main/java/hudson/model/AbstractBuild.java | 15 +++++- .../model/FreestyleJobPublisherTest.java | 47 +++++++++++++------ .../model/utils/AbortExceptionPublisher.java | 2 +- 3 files changed, 46 insertions(+), 18 deletions(-) diff --git a/core/src/main/java/hudson/model/AbstractBuild.java b/core/src/main/java/hudson/model/AbstractBuild.java index bcb08e0193..5e1785b7ba 100644 --- a/core/src/main/java/hudson/model/AbstractBuild.java +++ b/core/src/main/java/hudson/model/AbstractBuild.java @@ -712,6 +712,8 @@ public abstract class AbstractBuild

,R extends Abs * * @param phase * true for the post build processing, and false for the final "run after finished" execution. + * + * @return false if any build step failed */ protected final boolean performAllBuildSteps(BuildListener listener, Iterable buildSteps, boolean phase) throws InterruptedException, IOException { boolean r = true; @@ -725,19 +727,28 @@ public abstract class AbstractBuild

,R extends Abs } } catch (Exception e) { reportError(bs, e, listener, phase); + r = false; } catch (LinkageError e) { reportError(bs, e, listener, phase); + r = false; } } return r; } private void reportError(BuildStep bs, Throwable e, BuildListener listener, boolean phase) { - String msg = "Publisher " + bs.getClass().getName() + " aborted due to exception"; - if (!(e instanceof AbortException)){ + final String publisher = ((Publisher) bs).getDescriptor().getDisplayName(); + + if (e instanceof AbortException) { + LOGGER.log(Level.FINE, "{0} : {1} failed", new Object[] {AbstractBuild.this, publisher}); + listener.error("Publisher '" + publisher + "' failed: "); + listener.error(e.getMessage()); + } else { + String msg = "Publisher '" + publisher + "' aborted due to exception: "; e.printStackTrace(listener.error(msg)); LOGGER.log(WARNING, msg, e); } + if (phase) { setResult(Result.FAILURE); } diff --git a/test/src/test/java/hudson/model/FreestyleJobPublisherTest.java b/test/src/test/java/hudson/model/FreestyleJobPublisherTest.java index ee1105d388..feebd9cf3c 100644 --- a/test/src/test/java/hudson/model/FreestyleJobPublisherTest.java +++ b/test/src/test/java/hudson/model/FreestyleJobPublisherTest.java @@ -19,10 +19,12 @@ import java.io.IOException; import java.util.concurrent.ExecutionException; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; /** * Freestyle publishers statuses tests + * * @author Kanstantsin Shautsou */ public class FreestyleJobPublisherTest { @@ -30,7 +32,7 @@ public class FreestyleJobPublisherTest { public JenkinsRule j = new JenkinsRule(); /** - * Execute even one of publishers return false. Shows JENKINS-26964 bug. + * Execute all publishers even one of publishers return false. */ @Issue("JENKINS-26964") @Test @@ -39,51 +41,66 @@ public class FreestyleJobPublisherTest { p.getPublishersList().add(new TrueFalsePublisher(true)); // noop p.getPublishersList().add(new TrueFalsePublisher(false)); // FAIL build with false - p.getPublishersList().add(new ResultWriterPublisher("result.txt")); //catch result to file - p.getPublishersList().add(new ArtifactArchiver("result.txt", "", false)); + p.getPublishersList().add(new ResultWriterPublisher("result.txt")); // catch result to file + final ArtifactArchiver artifactArchiver = new ArtifactArchiver("result.txt"); + artifactArchiver.setOnlyIfSuccessful(false); + p.getPublishersList().add(artifactArchiver); // transfer file to build dir FreeStyleBuild b = p.scheduleBuild2(0).get(); assertEquals("Build must fail, because we used FalsePublisher", b.getResult(), Result.FAILURE); File file = new File(b.getArtifactsDir(), "result.txt"); - assertTrue("ArtifactArchiver is executed even prior publisher fails.", file.exists()); - assertTrue("Second publisher must see FAILURE status", FileUtils.readFileToString(file).equals(Result.FAILURE.toString())); + assertTrue("ArtifactArchiver is executed even prior publisher fails", file.exists()); + assertTrue("Publisher, after publisher with return false status, must see FAILURE status", + FileUtils.readFileToString(file).equals(Result.FAILURE.toString())); } /** * Execute all publishers even one of them throws AbortException. */ + @Issue("JENKINS-26964") @Test - public void testFreestyleWithExceptionPublisher() throws IOException, ExecutionException, InterruptedException { + public void testFreestyleWithExceptionPublisher() throws Exception { FreeStyleProject p = j.createFreeStyleProject(); p.getPublishersList().add(new TrueFalsePublisher(true)); // noop p.getPublishersList().add(new AbortExceptionPublisher()); // FAIL build with AbortException - p.getPublishersList().add(new ResultWriterPublisher("result.txt")); //catch result to file - p.getPublishersList().add(new ArtifactArchiver("result.txt", "", false)); + p.getPublishersList().add(new ResultWriterPublisher("result.txt")); // catch result to file + final ArtifactArchiver artifactArchiver = new ArtifactArchiver("result.txt"); + artifactArchiver.setOnlyIfSuccessful(false); + p.getPublishersList().add(artifactArchiver); // transfer file to build dir FreeStyleBuild b = p.scheduleBuild2(0).get(); - assertEquals("Build must fail, because we used FalsePublisher", b.getResult(), Result.FAILURE); + + assertEquals("Build must fail, because we used AbortExceptionPublisher", b.getResult(), Result.FAILURE); + j.assertLogNotContains("\tat", b); // log must not contain stacktrace File file = new File(b.getArtifactsDir(), "result.txt"); - assertTrue("ArtifactArchiver is executed even prior publisher fails.", file.exists()); - assertTrue("Second publisher must see FAILURE status", FileUtils.readFileToString(file).equals(Result.FAILURE.toString())); + assertTrue("ArtifactArchiver is executed even prior publisher fails", file.exists()); + assertTrue("Second publisher must see FAILURE status", + FileUtils.readFileToString(file).equals(Result.FAILURE.toString())); } /** * Execute all publishers even one of them throws any Exceptions. */ + @Issue("JENKINS-26964") @Test - public void testFreestyleWithIOExceptionPublisher() throws IOException, ExecutionException, InterruptedException { + public void testFreestyleWithIOExceptionPublisher() throws Exception { FreeStyleProject p = j.createFreeStyleProject(); p.getPublishersList().add(new TrueFalsePublisher(true)); // noop p.getPublishersList().add(new IOExceptionPublisher()); // fail with IOException p.getPublishersList().add(new ResultWriterPublisher("result.txt")); //catch result to file - p.getPublishersList().add(new ArtifactArchiver("result.txt", "", false)); + final ArtifactArchiver artifactArchiver = new ArtifactArchiver("result.txt"); + artifactArchiver.setOnlyIfSuccessful(false); + p.getPublishersList().add(artifactArchiver); // transfer file to build dir FreeStyleBuild b = p.scheduleBuild2(0).get(); + assertEquals("Build must fail, because we used FalsePublisher", b.getResult(), Result.FAILURE); + j.assertLogContains("\tat hudson.model.utils.IOExceptionPublisher", b); // log must contain stacktrace File file = new File(b.getArtifactsDir(), "result.txt"); - assertTrue("ArtifactArchiver is executed even prior publisher fails.", file.exists()); - assertTrue("Second publisher must see FAILURE status", FileUtils.readFileToString(file).equals(Result.FAILURE.toString())); + assertTrue("ArtifactArchiver is executed even prior publisher fails", file.exists()); + assertTrue("Third publisher must see FAILURE status", + FileUtils.readFileToString(file).equals(Result.FAILURE.toString())); } } diff --git a/test/src/test/java/hudson/model/utils/AbortExceptionPublisher.java b/test/src/test/java/hudson/model/utils/AbortExceptionPublisher.java index a3af0af7c2..f634a07a1f 100644 --- a/test/src/test/java/hudson/model/utils/AbortExceptionPublisher.java +++ b/test/src/test/java/hudson/model/utils/AbortExceptionPublisher.java @@ -19,7 +19,7 @@ import java.io.IOException; public class AbortExceptionPublisher extends Recorder { @Override public boolean perform(AbstractBuild build, Launcher launcher, BuildListener listener) throws InterruptedException, IOException { - throw new AbortException("Throwed AbortException from publisher!"); + throw new AbortException("Threw AbortException from publisher!"); } @Override -- GitLab