From 28c001a22cc8648e0283a4edf2e28b9d281422b7 Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Mon, 20 Aug 2018 15:55:47 -0400 Subject: [PATCH 01/33] Preparing for external storage. --- .mvn/extensions.xml | 7 +++++ .mvn/maven.config | 2 ++ pom.xml | 12 ++++---- .../tasks/junit/storage/package-info.java | 28 +++++++++++++++++++ 4 files changed, 44 insertions(+), 5 deletions(-) create mode 100644 .mvn/extensions.xml create mode 100644 .mvn/maven.config create mode 100644 src/main/java/hudson/tasks/junit/storage/package-info.java diff --git a/.mvn/extensions.xml b/.mvn/extensions.xml new file mode 100644 index 000000000..1fb5b5c65 --- /dev/null +++ b/.mvn/extensions.xml @@ -0,0 +1,7 @@ + + + io.jenkins.tools.incrementals + git-changelist-maven-extension + 1.0-beta-5 + + diff --git a/.mvn/maven.config b/.mvn/maven.config new file mode 100644 index 000000000..2a0299c48 --- /dev/null +++ b/.mvn/maven.config @@ -0,0 +1,2 @@ +-Pconsume-incrementals +-Pmight-produce-incrementals diff --git a/pom.xml b/pom.xml index f4b6162f8..fca6f8ad0 100644 --- a/pom.xml +++ b/pom.xml @@ -3,18 +3,20 @@ org.jenkins-ci.plugins plugin - 3.9 + 3.19 junit - 1.25-SNAPSHOT + ${revision}${changelist} hpi JUnit Plugin Allows JUnit-format test results to be published. http://wiki.jenkins-ci.org/display/JENKINS/JUnit+Plugin - 2.7.3 - 7 + 1.25 + -SNAPSHOT + 2.121.3 + 8 false 2.37 2.14 @@ -29,7 +31,7 @@ scm:git:git://github.com/jenkinsci/${project.artifactId}-plugin.git scm:git:git@github.com:jenkinsci/${project.artifactId}-plugin.git https://github.com/jenkinsci/${project.artifactId}-plugin - HEAD + ${scmTag} diff --git a/src/main/java/hudson/tasks/junit/storage/package-info.java b/src/main/java/hudson/tasks/junit/storage/package-info.java new file mode 100644 index 000000000..a8176545c --- /dev/null +++ b/src/main/java/hudson/tasks/junit/storage/package-info.java @@ -0,0 +1,28 @@ +/* + * The MIT License + * + * Copyright 2018 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. + */ + +/** + * A pluggable storage layer for JUnit-format test results. + */ +package hudson.tasks.junit.storage; From ea8e7f5074543a64f35d9918ec741b35517f2e0e Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Fri, 24 Aug 2018 17:12:14 -0400 Subject: [PATCH 02/33] Thinking. --- src/main/java/hudson/tasks/junit/History.java | 2 + .../java/hudson/tasks/junit/JUnitParser.java | 1 + .../java/hudson/tasks/junit/TestResult.java | 1 + .../junit/storage/TestResultStorage.java | 41 +++++++++++++++++++ .../tasks/test/AbstractTestResultAction.java | 1 + 5 files changed, 46 insertions(+) create mode 100644 src/main/java/hudson/tasks/junit/storage/TestResultStorage.java diff --git a/src/main/java/hudson/tasks/junit/History.java b/src/main/java/hudson/tasks/junit/History.java index ec70e00cf..583dcc13f 100644 --- a/src/main/java/hudson/tasks/junit/History.java +++ b/src/main/java/hudson/tasks/junit/History.java @@ -77,6 +77,7 @@ public boolean historyAvailable() { public List getList(int start, int end) { List list = new ArrayList(); + // TODO this must not stand end = Math.min(end, testObject.getRun().getParent().getBuilds().size()); for (Run b: testObject.getRun().getParent().getBuilds().subList(start, end)) { if (b.isBuilding()) continue; @@ -245,6 +246,7 @@ public String generateToolTip(CategoryDataset dataset, int row, } class ChartLabel implements Comparable { + // TODO allow use of a simplified label that does not force Run loading TestResult o; String url; public ChartLabel(TestResult o) { diff --git a/src/main/java/hudson/tasks/junit/JUnitParser.java b/src/main/java/hudson/tasks/junit/JUnitParser.java index 8aaa2e181..5669db373 100644 --- a/src/main/java/hudson/tasks/junit/JUnitParser.java +++ b/src/main/java/hudson/tasks/junit/JUnitParser.java @@ -154,6 +154,7 @@ public TestResult invoke(File ws, VirtualChannel channel) throws IOException { throw new AbortException(Messages.JUnitResultArchiver_NoTestReportFound()); } } + // TODO allow TestResultStorage to intercept here return result; } } diff --git a/src/main/java/hudson/tasks/junit/TestResult.java b/src/main/java/hudson/tasks/junit/TestResult.java index 118128580..ac5a83d10 100644 --- a/src/main/java/hudson/tasks/junit/TestResult.java +++ b/src/main/java/hudson/tasks/junit/TestResult.java @@ -679,6 +679,7 @@ public AbstractTestResultAction getParentAction() { */ @Override public void tally() { + // TODO allow TestResultStorage to cancel this /// Empty out data structures // TODO: free children? memmory leak? suitesByName = new HashMap(); diff --git a/src/main/java/hudson/tasks/junit/storage/TestResultStorage.java b/src/main/java/hudson/tasks/junit/storage/TestResultStorage.java new file mode 100644 index 000000000..6a4e85a31 --- /dev/null +++ b/src/main/java/hudson/tasks/junit/storage/TestResultStorage.java @@ -0,0 +1,41 @@ +/* + * The MIT License + * + * Copyright 2018 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 hudson.tasks.junit.storage; + +import hudson.ExtensionPoint; + +/** + * Allows test results to be saved and loaded from an external storage service. + */ +public interface TestResultStorage extends ExtensionPoint { + + // TODO intercept parseResult + + // TODO produce a hudson.tasks.junit.TestResult for a Job × buildNumber; populate full data, or load it lazily? + + // TODO substitute trend graph for /job/*/ or /job/*/test/?width=800&height=600 from TestResultProjectAction/{index,floatingBox} + // TODO substitute count/duration graph for /job/*/*/testReport/pkg/SomeTest/method/history/ and similar from History/index + +} diff --git a/src/main/java/hudson/tasks/test/AbstractTestResultAction.java b/src/main/java/hudson/tasks/test/AbstractTestResultAction.java index 859d34c55..be7d92fbf 100644 --- a/src/main/java/hudson/tasks/test/AbstractTestResultAction.java +++ b/src/main/java/hudson/tasks/test/AbstractTestResultAction.java @@ -336,6 +336,7 @@ private Area calcDefaultSize() { private CategoryDataset buildDataSet(StaplerRequest req) { boolean failureOnly = Boolean.valueOf(req.getParameter("failureOnly")); + // TODO stop using ChartUtil.NumberOnlyBuildLabel as it forces loading of a Run; create a plainer Comparable DataSetBuilder dsb = new DataSetBuilder(); int cap = Integer.getInteger(AbstractTestResultAction.class.getName() + ".test.trend.max", Integer.MAX_VALUE); From d39d5c3d1f0cb494dbab2ec9f9f79d2a7833d364 Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Mon, 27 Aug 2018 15:06:56 -0400 Subject: [PATCH 03/33] Skeleton of smoke test. --- pom.xml | 22 ++++ .../junit/storage/TestResultStorageTest.java | 109 ++++++++++++++++++ 2 files changed, 131 insertions(+) create mode 100644 src/test/java/hudson/tasks/junit/storage/TestResultStorageTest.java diff --git a/pom.xml b/pom.xml index fca6f8ad0..f00a49a42 100644 --- a/pom.xml +++ b/pom.xml @@ -140,5 +140,27 @@ 2.13 test + + org.jenkins-ci.plugins + database + 1.5 + test + + + antlr + antlr + + + javax.validation + validation-api + + + + + org.jenkins-ci.plugins + database-h2 + 1.1 + test + diff --git a/src/test/java/hudson/tasks/junit/storage/TestResultStorageTest.java b/src/test/java/hudson/tasks/junit/storage/TestResultStorageTest.java new file mode 100644 index 000000000..a5fa43f63 --- /dev/null +++ b/src/test/java/hudson/tasks/junit/storage/TestResultStorageTest.java @@ -0,0 +1,109 @@ +/* + * The MIT License + * + * Copyright 2018 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 hudson.tasks.junit.storage; + +import hudson.model.Label; +import hudson.model.Result; +import java.sql.Connection; +import java.sql.PreparedStatement; +import java.sql.ResultSet; +import java.sql.ResultSetMetaData; +import java.sql.SQLException; +import org.jenkinsci.plugins.database.GlobalDatabaseConfiguration; +import org.jenkinsci.plugins.database.h2.LocalH2Database; +import org.jenkinsci.plugins.workflow.cps.CpsFlowDefinition; +import org.jenkinsci.plugins.workflow.job.WorkflowJob; +import static org.junit.Assert.*; +import org.junit.Before; +import org.junit.ClassRule; +import org.junit.Ignore; +import org.junit.Rule; +import org.junit.Test; +import org.jvnet.hudson.test.BuildWatcher; +import org.jvnet.hudson.test.JenkinsRule; +import org.jvnet.hudson.test.TestExtension; + +public class TestResultStorageTest { + + @ClassRule public static BuildWatcher buildWatcher = new BuildWatcher(); + + @Rule public JenkinsRule r = new JenkinsRule(); + + /** Need {@link LocalH2Database#getAutoServer} so that {@link Impl} can make connections from an agent JVM. */ + @Before public void autoServer() throws Exception { + LocalH2Database database = (LocalH2Database) GlobalDatabaseConfiguration.get().getDatabase(); + GlobalDatabaseConfiguration.get().setDatabase(new LocalH2Database(database.getPath(), true)); + } + + @Ignore("TODO") + @Test public void smokes() throws Exception { + r.createSlave(Label.get("remote")); + WorkflowJob p = r.createProject(WorkflowJob.class, "p"); + p.setDefinition(new CpsFlowDefinition( + "node('remote') {\n" + + " writeFile file: 'x.xml', text: ''''''\n" + + " junit 'x.xml'\n" + + "}", true)); + r.assertBuildStatus(Result.UNSTABLE, p.scheduleBuild2(0)); + // TODO verify that there is no junitResult.xml on disk + // TODO query TestResultAction in various ways + try (Connection connection = GlobalDatabaseConfiguration.get().getDatabase().getDataSource().getConnection(); + PreparedStatement statement = connection.prepareStatement("SELECT * FROM INFORMATION_SCHEMA.TABLES"); + ResultSet result = statement.executeQuery()) { + printResultSet(result); + } + // TODO dump, then later verify, DB structure + fail("TODO"); + } + + // https://gist.github.com/mikbuch/299568988fa7997cb28c7c84309232b1 + private static void printResultSet(ResultSet rs) throws SQLException { + ResultSetMetaData rsmd = rs.getMetaData(); + int columnsNumber = rsmd.getColumnCount(); + for (int i = 1; i <= columnsNumber; i++) { + if (i > 1) { + System.out.print(" | "); + } + System.out.print(rsmd.getColumnName(i)); + } + System.out.println(); + while (rs.next()) { + for (int i = 1; i <= columnsNumber; i++) { + if (i > 1) { + System.out.print(" | "); + } + System.out.print(rs.getString(i)); + } + System.out.println(); + } + } + + @TestExtension public static class Impl implements TestResultStorage { + + // TODO use XStream to remote the [LocalH2]Database + + } + +} From 67bf5858f0e74ad330d87f6d403e593341de77df Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Mon, 27 Aug 2018 17:42:43 -0400 Subject: [PATCH 04/33] Now successfully writing basic test results to a database from the agent side. --- .../java/hudson/tasks/junit/JUnitParser.java | 20 +- .../tasks/junit/JUnitResultArchiver.java | 2 + .../junit/storage/TestResultStorage.java | 30 ++- .../junit/storage/TestResultStorageTest.java | 179 ++++++++++++++++-- 4 files changed, 208 insertions(+), 23 deletions(-) diff --git a/src/main/java/hudson/tasks/junit/JUnitParser.java b/src/main/java/hudson/tasks/junit/JUnitParser.java index 5669db373..bb044faff 100644 --- a/src/main/java/hudson/tasks/junit/JUnitParser.java +++ b/src/main/java/hudson/tasks/junit/JUnitParser.java @@ -30,6 +30,7 @@ import hudson.model.AbstractBuild; import hudson.model.Run; import hudson.remoting.VirtualChannel; +import hudson.tasks.junit.storage.TestResultStorage; import java.io.IOException; import java.io.File; @@ -40,7 +41,6 @@ import org.apache.tools.ant.DirectoryScanner; import javax.annotation.CheckForNull; -import javax.annotation.Nonnull; /** * Parse some JUnit xml files and generate a TestResult containing all the @@ -108,11 +108,11 @@ public TestResult parseResult(String testResultLocations, Run build, Pipeli final long buildTime = build.getTimestamp().getTimeInMillis(); final long timeOnMaster = System.currentTimeMillis(); - // [BUG 3123310] TODO - Test Result Refactor: review and fix TestDataPublisher/TestAction subsystem] - // also get code that deals with testDataPublishers from JUnitResultArchiver.perform + TestResultStorage storage = TestResultStorage.find(); + TestResultStorage.RemotePublisher publisher = storage == null ? null : storage.createRemotePublisher(build, listener); return workspace.act(new ParseResultCallable(testResultLocations, buildTime, timeOnMaster, keepLongStdio, - allowEmptyResults, pipelineTestDetails)); + allowEmptyResults, pipelineTestDetails, publisher)); } private static final class ParseResultCallable extends MasterToSlaveFileCallable { @@ -122,16 +122,18 @@ private static final class ParseResultCallable extends MasterToSlaveFileCallable private final boolean keepLongStdio; private final boolean allowEmptyResults; private final PipelineTestDetails pipelineTestDetails; + private final @CheckForNull TestResultStorage.RemotePublisher publisher; private ParseResultCallable(String testResults, long buildTime, long nowMaster, boolean keepLongStdio, boolean allowEmptyResults, - PipelineTestDetails pipelineTestDetails) { + PipelineTestDetails pipelineTestDetails, TestResultStorage.RemotePublisher publisher) { this.buildTime = buildTime; this.testResults = testResults; this.nowMaster = nowMaster; this.keepLongStdio = keepLongStdio; this.allowEmptyResults = allowEmptyResults; this.pipelineTestDetails = pipelineTestDetails; + this.publisher = publisher; } public TestResult invoke(File ws, VirtualChannel channel) throws IOException { @@ -154,8 +156,12 @@ public TestResult invoke(File ws, VirtualChannel channel) throws IOException { throw new AbortException(Messages.JUnitResultArchiver_NoTestReportFound()); } } - // TODO allow TestResultStorage to intercept here - return result; + if (publisher != null) { + publisher.publish(result); + return new TestResult(); // ignored anyway + } else { + return result; + } } } diff --git a/src/main/java/hudson/tasks/junit/JUnitResultArchiver.java b/src/main/java/hudson/tasks/junit/JUnitResultArchiver.java index ba4189ecc..10bf2bb35 100644 --- a/src/main/java/hudson/tasks/junit/JUnitResultArchiver.java +++ b/src/main/java/hudson/tasks/junit/JUnitResultArchiver.java @@ -189,8 +189,10 @@ public static TestResultAction parseAndAttach(@Nonnull JUnitTask task, PipelineT listener.getLogger().println(Messages.JUnitResultArchiver_ResultIsEmpty()); return null; } + /* TODO some of this logic is already in ParseResultCallable; the rest should be moved there // most likely a configuration error in the job - e.g. false pattern to match the JUnit result files throw new AbortException(Messages.JUnitResultArchiver_ResultIsEmpty()); + */ } // TODO: Move into JUnitParser [BUG 3123310] diff --git a/src/main/java/hudson/tasks/junit/storage/TestResultStorage.java b/src/main/java/hudson/tasks/junit/storage/TestResultStorage.java index 6a4e85a31..964507048 100644 --- a/src/main/java/hudson/tasks/junit/storage/TestResultStorage.java +++ b/src/main/java/hudson/tasks/junit/storage/TestResultStorage.java @@ -24,18 +24,46 @@ package hudson.tasks.junit.storage; +import hudson.ExtensionList; import hudson.ExtensionPoint; +import hudson.FilePath; +import hudson.Launcher; +import hudson.model.Run; +import hudson.model.TaskListener; +import hudson.tasks.junit.JUnitParser; +import hudson.tasks.junit.TestResult; +import hudson.tasks.test.PipelineTestDetails; +import java.io.IOException; +import javax.annotation.CheckForNull; +import org.jenkinsci.remoting.SerializableOnlyOverRemoting; /** * Allows test results to be saved and loaded from an external storage service. */ public interface TestResultStorage extends ExtensionPoint { - // TODO intercept parseResult + /** + * Runs during {@link JUnitParser#parseResult(String, Run, PipelineTestDetails, FilePath, Launcher, TaskListener)}. + */ + RemotePublisher createRemotePublisher(Run build, TaskListener listener) throws IOException; + + /** + * Remotable hook to perform test result publishing. + */ + interface RemotePublisher extends SerializableOnlyOverRemoting { + void publish(TestResult result) throws IOException; + } // TODO produce a hudson.tasks.junit.TestResult for a Job × buildNumber; populate full data, or load it lazily? // TODO substitute trend graph for /job/*/ or /job/*/test/?width=800&height=600 from TestResultProjectAction/{index,floatingBox} // TODO substitute count/duration graph for /job/*/*/testReport/pkg/SomeTest/method/history/ and similar from History/index + // TODO decide whether to bother making AbstractTestResultAction.descriptions and/or testData pluggable + + static @CheckForNull TestResultStorage find() { + ExtensionList all = ExtensionList.lookup(TestResultStorage.class); + return all.isEmpty() ? null : all.iterator().next(); + } + } diff --git a/src/test/java/hudson/tasks/junit/storage/TestResultStorageTest.java b/src/test/java/hudson/tasks/junit/storage/TestResultStorageTest.java index a5fa43f63..16f6a01a3 100644 --- a/src/test/java/hudson/tasks/junit/storage/TestResultStorageTest.java +++ b/src/test/java/hudson/tasks/junit/storage/TestResultStorageTest.java @@ -24,17 +24,32 @@ package hudson.tasks.junit.storage; +import com.thoughtworks.xstream.XStream; import hudson.model.Label; import hudson.model.Result; +import hudson.model.Run; +import hudson.model.TaskListener; +import hudson.slaves.DumbSlave; +import hudson.tasks.junit.CaseResult; +import hudson.tasks.junit.SuiteResult; +import hudson.tasks.junit.TestResult; +import hudson.tasks.junit.TestResultAction; +import java.io.IOException; import java.sql.Connection; import java.sql.PreparedStatement; import java.sql.ResultSet; import java.sql.ResultSetMetaData; import java.sql.SQLException; +import java.sql.Statement; +import java.sql.Types; +import java.util.List; +import org.jenkinsci.plugins.database.Database; import org.jenkinsci.plugins.database.GlobalDatabaseConfiguration; import org.jenkinsci.plugins.database.h2.LocalH2Database; import org.jenkinsci.plugins.workflow.cps.CpsFlowDefinition; import org.jenkinsci.plugins.workflow.job.WorkflowJob; +import org.jenkinsci.plugins.workflow.job.WorkflowRun; +import org.jenkinsci.remoting.SerializableOnlyOverRemoting; import static org.junit.Assert.*; import org.junit.Before; import org.junit.ClassRule; @@ -51,7 +66,10 @@ public class TestResultStorageTest { @Rule public JenkinsRule r = new JenkinsRule(); - /** Need {@link LocalH2Database#getAutoServer} so that {@link Impl} can make connections from an agent JVM. */ + /** + * Need {@link LocalH2Database#getAutoServer} so that {@link Impl} can make connections from an agent JVM. + * @see Automatic Mixed Mode + */ @Before public void autoServer() throws Exception { LocalH2Database database = (LocalH2Database) GlobalDatabaseConfiguration.get().getDatabase(); GlobalDatabaseConfiguration.get().setDatabase(new LocalH2Database(database.getPath(), true)); @@ -59,23 +77,160 @@ public class TestResultStorageTest { @Ignore("TODO") @Test public void smokes() throws Exception { - r.createSlave(Label.get("remote")); + DumbSlave remote = r.createOnlineSlave(Label.get("remote")); + //((Channel) remote.getChannel()).addListener(new LoggingChannelListener(Logger.getLogger(TestResultStorageTest.class.getName()), Level.INFO)); WorkflowJob p = r.createProject(WorkflowJob.class, "p"); p.setDefinition(new CpsFlowDefinition( "node('remote') {\n" + - " writeFile file: 'x.xml', text: ''''''\n" + + " writeFile file: 'x.xml', text: ''''''\n" + " junit 'x.xml'\n" + "}", true)); - r.assertBuildStatus(Result.UNSTABLE, p.scheduleBuild2(0)); - // TODO verify that there is no junitResult.xml on disk - // TODO query TestResultAction in various ways + WorkflowRun b = p.scheduleBuild2(0).get(); + try (Connection connection = GlobalDatabaseConfiguration.get().getDatabase().getDataSource().getConnection(); + ResultSet result = connection.getMetaData().getTables(null, null, null, new String[] {"TABLE"})) { + printResultSet(result); + } try (Connection connection = GlobalDatabaseConfiguration.get().getDatabase().getDataSource().getConnection(); - PreparedStatement statement = connection.prepareStatement("SELECT * FROM INFORMATION_SCHEMA.TABLES"); + PreparedStatement statement = connection.prepareStatement("SELECT * FROM " + Impl.CASE_RESULTS_TABLE); ResultSet result = statement.executeQuery()) { printResultSet(result); } - // TODO dump, then later verify, DB structure - fail("TODO"); + r.assertBuildStatus(Result.UNSTABLE, b); + TestResultAction a = b.getAction(TestResultAction.class); + assertNotNull(a); + assertEquals(1, a.getFailCount()); + List failedTests = a.getFailedTests(); + assertEquals(1, failedTests.size()); + assertEquals("Klazz", failedTests.get(0).getClassName()); + assertEquals("test1", failedTests.get(0).getName()); + assertEquals("failure", failedTests.get(0).getErrorDetails()); + // TODO more detailed Java queries incl. PackageResult / ClassResult + // TODO verify that there is no junitResult.xml on disk + // TODO verify that build.xml#//hudson.tasks.junit.TestResultAction is (mostly?) empty + // TODO verify structure + } + + @TestExtension public static class Impl implements TestResultStorage { + + static final String CASE_RESULTS_TABLE = "caseResults"; + + private final ConnectionSupplier connectionSupplier = new LocalConnectionSupplier(); + + @Override public RemotePublisher createRemotePublisher(Run build, TaskListener listener) throws IOException { + try { + connectionSupplier.connection(); // make sure we start a local server and create table first + } catch (SQLException x) { + throw new IOException(x); + } + return new RemotePublisherImpl(build.getParent().getFullName(), build.getNumber(), listener); + } + + private static class RemotePublisherImpl implements RemotePublisher { + + private final String job; + private final int build; + private final TaskListener listener; + // TODO keep the same supplier and thus Connection open across builds, so long as the database config remains unchanged + private final ConnectionSupplier connectionSupplier; + + RemotePublisherImpl(String job, int build, TaskListener listener) { + this.job = job; + this.build = build; + this.listener = listener; + connectionSupplier = new RemoteConnectionSupplier(); + } + + @Override public void publish(TestResult result) throws IOException { + try { + Connection connection = connectionSupplier.connection(); + try (PreparedStatement statement = connection.prepareStatement("INSERT INTO " + CASE_RESULTS_TABLE + " (job, build, suite, className, testName, errorDetails) VALUES (?, ?, ?, ?, ?, ?)")) { + for (SuiteResult suiteResult : result.getSuites()) { + for (CaseResult caseResult : suiteResult.getCases()) { + statement.setString(1, job); + statement.setInt(2, build); + statement.setString(3, suiteResult.getName()); + statement.setString(4, caseResult.getClassName()); + statement.setString(5, caseResult.getName()); + String errorDetails = caseResult.getErrorDetails(); + if (errorDetails != null) { + statement.setString(6, errorDetails); + } else { + statement.setNull(6, Types.VARCHAR); + } + statement.executeUpdate(); + } + } + } + } catch (SQLException x) { + throw new IOException(x); + } + } + + } + + static abstract class ConnectionSupplier { // TODO AutoCloseable + + private transient Connection connection; + + protected abstract Database database(); + + protected void initialize(Connection connection) throws SQLException {} + + synchronized Connection connection() throws SQLException { + if (connection == null) { + Connection _connection = database().getDataSource().getConnection(); + initialize(_connection); + connection = _connection; + } + return connection; + } + + } + + static class LocalConnectionSupplier extends ConnectionSupplier { + + @Override protected Database database() { + return GlobalDatabaseConfiguration.get().getDatabase(); + } + + @Override protected void initialize(Connection connection) throws SQLException { + boolean exists = false; + try (ResultSet rs = connection.getMetaData().getTables(null, null, CASE_RESULTS_TABLE, new String[] {"TABLE"})) { + while (rs.next()) { + if (rs.getString("TABLE_NAME").equalsIgnoreCase(CASE_RESULTS_TABLE)) { + exists = true; + break; + } + } + } + if (!exists) { + try (Statement statement = connection.createStatement()) { + // TODO this and joined tables: skipped, skippedMessage, errorStackTrace, stdout, stderr, duration, nodeId, enclosingBlocks, enclosingBlockNames, etc. + statement.execute("CREATE TABLE " + CASE_RESULTS_TABLE + "(job varchar(255), build int, suite varchar(255), className varchar(255), testName varchar(255), errorDetails varchar(255))"); + } + } + } + + } + + /** + * Ensures a {@link LocalH2Database} configuration can be sent to an agent. + */ + static class RemoteConnectionSupplier extends ConnectionSupplier implements SerializableOnlyOverRemoting { + + private static final XStream XSTREAM = new XStream(); + private final String databaseXml; + + RemoteConnectionSupplier() { + databaseXml = XSTREAM.toXML(GlobalDatabaseConfiguration.get().getDatabase()); + } + + @Override protected Database database() { + return (Database) XSTREAM.fromXML(databaseXml); + } + + } + } // https://gist.github.com/mikbuch/299568988fa7997cb28c7c84309232b1 @@ -100,10 +255,4 @@ private static void printResultSet(ResultSet rs) throws SQLException { } } - @TestExtension public static class Impl implements TestResultStorage { - - // TODO use XStream to remote the [LocalH2]Database - - } - } From c500c1e3b10d248f5df3fc8032e7de2e6133fb98 Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Tue, 28 Aug 2018 16:31:39 -0400 Subject: [PATCH 05/33] Initial work on master-side queries. --- .../java/hudson/tasks/junit/TestResult.java | 21 +++++++++++ .../hudson/tasks/junit/TestResultAction.java | 24 ++++++++++-- .../tasks/junit/storage/TestResultImpl.java | 37 +++++++++++++++++++ .../junit/storage/TestResultStorage.java | 2 +- .../junit/storage/TestResultStorageTest.java | 28 +++++++++++++- 5 files changed, 105 insertions(+), 7 deletions(-) create mode 100644 src/main/java/hudson/tasks/junit/storage/TestResultImpl.java diff --git a/src/main/java/hudson/tasks/junit/TestResult.java b/src/main/java/hudson/tasks/junit/TestResult.java index ac5a83d10..98f76ac45 100644 --- a/src/main/java/hudson/tasks/junit/TestResult.java +++ b/src/main/java/hudson/tasks/junit/TestResult.java @@ -26,6 +26,7 @@ import hudson.AbortException; import hudson.Util; import hudson.model.Run; +import hudson.tasks.junit.storage.TestResultImpl; import hudson.tasks.test.AbstractTestResultAction; import hudson.tasks.test.PipelineTestDetails; import hudson.tasks.test.PipelineBlockWithTests; @@ -44,6 +45,7 @@ import java.util.List; import java.util.Map; import java.util.TreeMap; +import javax.annotation.CheckForNull; import org.apache.tools.ant.DirectoryScanner; import org.dom4j.DocumentException; @@ -60,6 +62,8 @@ */ public final class TestResult extends MetaTabulatedResult { + private final @CheckForNull TestResultImpl impl; + /** * List of all {@link SuiteResult}s in this test. * This is the core data structure to be persisted in the disk. @@ -118,6 +122,7 @@ public TestResult() { */ public TestResult(boolean keepLongStdio) { this.keepLongStdio = keepLongStdio; + impl = null; } @Deprecated @@ -140,9 +145,15 @@ public TestResult(long buildTime, DirectoryScanner results, boolean keepLongStdi public TestResult(long buildTime, DirectoryScanner results, boolean keepLongStdio, PipelineTestDetails pipelineTestDetails) throws IOException { this.keepLongStdio = keepLongStdio; + impl = null; parse(buildTime, results, pipelineTestDetails); } + public TestResult(TestResultImpl impl) { + this.impl = impl; + keepLongStdio = false; // irrelevant + } + public TestObject getParent() { return parent; } @@ -344,6 +355,9 @@ public void parse(File reportFile) throws IOException { * @since 1.22 */ public void parse(File reportFile, PipelineTestDetails pipelineTestDetails) throws IOException { + if (impl != null) { + throw new IllegalStateException("Cannot reparse using a pluggable impl"); + } try { for (SuiteResult suiteResult : SuiteResult.parse(reportFile, keepLongStdio, pipelineTestDetails)) add(suiteResult); @@ -446,6 +460,9 @@ public int getPassCount() { @Exported(visibility=999) @Override public int getFailCount() { + if (impl != null) { + return impl.getFailCount(); + } if(failedTests==null) return 0; else @@ -649,6 +666,9 @@ public TestResult getResultByNode(@Nonnull String nodeId) { @Nonnull public TestResult getResultByNodes(@Nonnull List nodeIds) { + if (impl != null) { + return impl.getResultByNodes(nodeIds); + } TestResult result = new TestResult(); for (String n : nodeIds) { List suites = suitesByNode.get(n); @@ -732,6 +752,7 @@ public void tally() { * and then freeze can be called again. */ public void freeze(TestResultAction parent) { + assert impl == null; this.parentAction = parent; if(suitesByName==null) { // freeze for the first time diff --git a/src/main/java/hudson/tasks/junit/TestResultAction.java b/src/main/java/hudson/tasks/junit/TestResultAction.java index ab1dd6468..6edeb9379 100644 --- a/src/main/java/hudson/tasks/junit/TestResultAction.java +++ b/src/main/java/hudson/tasks/junit/TestResultAction.java @@ -33,6 +33,7 @@ import hudson.model.Job; import hudson.model.Run; import hudson.model.TaskListener; +import hudson.tasks.junit.storage.TestResultStorage; import hudson.tasks.test.AbstractTestResultAction; import hudson.tasks.test.TestObject; import hudson.tasks.test.TestResultProjectAction; @@ -49,6 +50,8 @@ import java.util.List; import java.util.logging.Level; import java.util.logging.Logger; +import javax.annotation.CheckForNull; +import javax.annotation.Nullable; import jenkins.tasks.SimpleBuildStep; /** @@ -64,11 +67,12 @@ public class TestResultAction extends AbstractTestResultAction implements StaplerProxy, SimpleBuildStep.LastBuildAction { private transient WeakReference result; + /** null only if there is a {@link TestResultStorage} */ + private @Nullable Integer failCount; + private @Nullable Integer skipCount; // Hudson < 1.25 didn't set these fields, so use Integer // so that we can distinguish between 0 tests vs not-computed-yet. - private int failCount; - private int skipCount; - private Integer totalCount; + private @CheckForNull Integer totalCount; private Double healthScaleFactor; private List testData = new ArrayList(); @@ -82,7 +86,9 @@ public TestResultAction(AbstractBuild owner, TestResult result, BuildListener li */ public TestResultAction(Run owner, TestResult result, TaskListener listener) { super(owner); - setResult(result, listener); + if (TestResultStorage.find() == null) { + setResult(result, listener); + } } @Deprecated @@ -105,6 +111,7 @@ public TestResultAction(TestResult result, BuildListener listener) { * @since 1.2-beta-1 */ public synchronized void setResult(TestResult result, TaskListener listener) { + assert TestResultStorage.find() == null; result.freeze(this); totalCount = result.getTotalCount(); @@ -132,7 +139,12 @@ private XmlFile getDataFile() { return new XmlFile(XSTREAM, new File(run.getRootDir(), "junitResult.xml")); } + @Override public synchronized TestResult getResult() { + TestResultStorage storage = TestResultStorage.find(); + if (storage != null) { + return new TestResult(storage.load(run.getParent().getFullName(), run.getNumber())); + } TestResult r; if(result==null) { r = load(); @@ -155,6 +167,10 @@ public synchronized TestResult getResult() { @Override public synchronized int getFailCount() { + TestResultStorage storage = TestResultStorage.find(); + if (storage != null) { + return new TestResult(storage.load(run.getParent().getFullName(), run.getNumber())).getFailCount(); + } if(totalCount==null) getResult(); // this will compute the result return failCount; diff --git a/src/main/java/hudson/tasks/junit/storage/TestResultImpl.java b/src/main/java/hudson/tasks/junit/storage/TestResultImpl.java new file mode 100644 index 000000000..22530d923 --- /dev/null +++ b/src/main/java/hudson/tasks/junit/storage/TestResultImpl.java @@ -0,0 +1,37 @@ +/* + * The MIT License + * + * Copyright 2018 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 hudson.tasks.junit.storage; + +import hudson.tasks.junit.TestResult; +import java.util.List; +import javax.annotation.Nonnull; + +/** + * Pluggable implementation of {@link TestResult}. + */ +public interface TestResultImpl { + int getFailCount(); + @Nonnull TestResult getResultByNodes(@Nonnull List nodeIds); +} diff --git a/src/main/java/hudson/tasks/junit/storage/TestResultStorage.java b/src/main/java/hudson/tasks/junit/storage/TestResultStorage.java index 964507048..3fac6bed3 100644 --- a/src/main/java/hudson/tasks/junit/storage/TestResultStorage.java +++ b/src/main/java/hudson/tasks/junit/storage/TestResultStorage.java @@ -54,7 +54,7 @@ interface RemotePublisher extends SerializableOnlyOverRemoting { void publish(TestResult result) throws IOException; } - // TODO produce a hudson.tasks.junit.TestResult for a Job × buildNumber; populate full data, or load it lazily? + TestResultImpl load(String job, int build); // TODO substitute trend graph for /job/*/ or /job/*/test/?width=800&height=600 from TestResultProjectAction/{index,floatingBox} // TODO substitute count/duration graph for /job/*/*/testReport/pkg/SomeTest/method/history/ and similar from History/index diff --git a/src/test/java/hudson/tasks/junit/storage/TestResultStorageTest.java b/src/test/java/hudson/tasks/junit/storage/TestResultStorageTest.java index 16f6a01a3..c13e91b8e 100644 --- a/src/test/java/hudson/tasks/junit/storage/TestResultStorageTest.java +++ b/src/test/java/hudson/tasks/junit/storage/TestResultStorageTest.java @@ -106,8 +106,8 @@ public class TestResultStorageTest { assertEquals("failure", failedTests.get(0).getErrorDetails()); // TODO more detailed Java queries incl. PackageResult / ClassResult // TODO verify that there is no junitResult.xml on disk - // TODO verify that build.xml#//hudson.tasks.junit.TestResultAction is (mostly?) empty - // TODO verify structure + // TODO verify that build.xml#//hudson.tasks.junit.TestResultAction is empty except for healthScaleFactor and testData + // TODO verify table structure } @TestExtension public static class Impl implements TestResultStorage { @@ -125,6 +125,30 @@ public class TestResultStorageTest { return new RemotePublisherImpl(build.getParent().getFullName(), build.getNumber(), listener); } + @Override public TestResultImpl load(String job, int build) { + return new TestResultImpl() { + @Override public int getFailCount() { + try { + Connection connection = connectionSupplier.connection; + try (PreparedStatement statement = connection.prepareStatement("SELECT COUNT(*) FROM " + Impl.CASE_RESULTS_TABLE + " WHERE job = ? and build = ? and errorDetails IS NOT NULL")) { + statement.setString(1, job); + statement.setInt(2, build); + try (ResultSet result = statement.executeQuery()) { + result.next(); + return result.getInt(1); + } + } + } catch (SQLException x) { + x.printStackTrace(); + return 0; + } + } + @Override public TestResult getResultByNodes(List nodeIds) { + return new TestResult(this); // TODO + } + }; + } + private static class RemotePublisherImpl implements RemotePublisher { private final String job; From b52397b80b712abaee3aafdaad7faa065799c6a8 Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Tue, 28 Aug 2018 16:54:09 -0400 Subject: [PATCH 06/33] More test coverage. --- .../java/hudson/tasks/junit/JUnitParser.java | 1 + .../junit/storage/TestResultStorage.java | 2 +- .../junit/storage/TestResultStorageTest.java | 61 ++++++++++++++----- 3 files changed, 48 insertions(+), 16 deletions(-) diff --git a/src/main/java/hudson/tasks/junit/JUnitParser.java b/src/main/java/hudson/tasks/junit/JUnitParser.java index bb044faff..b3a80716c 100644 --- a/src/main/java/hudson/tasks/junit/JUnitParser.java +++ b/src/main/java/hudson/tasks/junit/JUnitParser.java @@ -159,6 +159,7 @@ public TestResult invoke(File ws, VirtualChannel channel) throws IOException { if (publisher != null) { publisher.publish(result); return new TestResult(); // ignored anyway + // TODO at least send back a TestResultSummary to avoid requiring JUnitResultsStepExecution to repeatedly query storage } else { return result; } diff --git a/src/main/java/hudson/tasks/junit/storage/TestResultStorage.java b/src/main/java/hudson/tasks/junit/storage/TestResultStorage.java index 3fac6bed3..47665b154 100644 --- a/src/main/java/hudson/tasks/junit/storage/TestResultStorage.java +++ b/src/main/java/hudson/tasks/junit/storage/TestResultStorage.java @@ -59,7 +59,7 @@ interface RemotePublisher extends SerializableOnlyOverRemoting { // TODO substitute trend graph for /job/*/ or /job/*/test/?width=800&height=600 from TestResultProjectAction/{index,floatingBox} // TODO substitute count/duration graph for /job/*/*/testReport/pkg/SomeTest/method/history/ and similar from History/index - // TODO decide whether to bother making AbstractTestResultAction.descriptions and/or testData pluggable + // for now, AbstractTestResultAction.descriptions and testData are not pluggable static @CheckForNull TestResultStorage find() { ExtensionList all = ExtensionList.lookup(TestResultStorage.class); diff --git a/src/test/java/hudson/tasks/junit/storage/TestResultStorageTest.java b/src/test/java/hudson/tasks/junit/storage/TestResultStorageTest.java index c13e91b8e..f6feab539 100644 --- a/src/test/java/hudson/tasks/junit/storage/TestResultStorageTest.java +++ b/src/test/java/hudson/tasks/junit/storage/TestResultStorageTest.java @@ -24,6 +24,7 @@ package hudson.tasks.junit.storage; +import com.google.common.collect.ImmutableSet; import com.thoughtworks.xstream.XStream; import hudson.model.Label; import hudson.model.Result; @@ -34,7 +35,10 @@ import hudson.tasks.junit.SuiteResult; import hudson.tasks.junit.TestResult; import hudson.tasks.junit.TestResultAction; +import java.io.ByteArrayInputStream; +import java.io.File; import java.io.IOException; +import java.nio.charset.StandardCharsets; import java.sql.Connection; import java.sql.PreparedStatement; import java.sql.ResultSet; @@ -43,6 +47,10 @@ import java.sql.Statement; import java.sql.Types; import java.util.List; +import java.util.Set; +import java.util.TreeSet; +import javax.xml.parsers.DocumentBuilderFactory; +import org.apache.commons.io.FileUtils; import org.jenkinsci.plugins.database.Database; import org.jenkinsci.plugins.database.GlobalDatabaseConfiguration; import org.jenkinsci.plugins.database.h2.LocalH2Database; @@ -53,12 +61,15 @@ import static org.junit.Assert.*; import org.junit.Before; import org.junit.ClassRule; -import org.junit.Ignore; import org.junit.Rule; import org.junit.Test; import org.jvnet.hudson.test.BuildWatcher; import org.jvnet.hudson.test.JenkinsRule; import org.jvnet.hudson.test.TestExtension; +import org.w3c.dom.Document; +import org.w3c.dom.Element; +import org.w3c.dom.Node; +import org.w3c.dom.NodeList; public class TestResultStorageTest { @@ -75,7 +86,6 @@ public class TestResultStorageTest { GlobalDatabaseConfiguration.get().setDatabase(new LocalH2Database(database.getPath(), true)); } - @Ignore("TODO") @Test public void smokes() throws Exception { DumbSlave remote = r.createOnlineSlave(Label.get("remote")); //((Channel) remote.getChannel()).addListener(new LoggingChannelListener(Logger.getLogger(TestResultStorageTest.class.getName()), Level.INFO)); @@ -95,19 +105,40 @@ public class TestResultStorageTest { ResultSet result = statement.executeQuery()) { printResultSet(result); } - r.assertBuildStatus(Result.UNSTABLE, b); - TestResultAction a = b.getAction(TestResultAction.class); - assertNotNull(a); - assertEquals(1, a.getFailCount()); - List failedTests = a.getFailedTests(); - assertEquals(1, failedTests.size()); - assertEquals("Klazz", failedTests.get(0).getClassName()); - assertEquals("test1", failedTests.get(0).getName()); - assertEquals("failure", failedTests.get(0).getErrorDetails()); - // TODO more detailed Java queries incl. PackageResult / ClassResult - // TODO verify that there is no junitResult.xml on disk - // TODO verify that build.xml#//hudson.tasks.junit.TestResultAction is empty except for healthScaleFactor and testData // TODO verify table structure + r.assertBuildStatus(Result.UNSTABLE, b); + assertFalse(new File(b.getRootDir(), "junitResult.xml").isFile()); + { + String buildXml = FileUtils.readFileToString(new File(b.getRootDir(), "build.xml")); + Document doc = DocumentBuilderFactory.newInstance().newDocumentBuilder().parse(new ByteArrayInputStream(buildXml.getBytes(StandardCharsets.UTF_8))); + NodeList testResultActionList = doc.getElementsByTagName("hudson.tasks.junit.TestResultAction"); + assertEquals(buildXml, 1, testResultActionList.getLength()); + Element testResultAction = (Element) testResultActionList.item(0); + NodeList childNodes = testResultAction.getChildNodes(); + Set childNames = new TreeSet<>(); + for (int i = 0; i < childNodes.getLength(); i++) { + Node item = childNodes.item(i); + if (item instanceof Element) { + childNames.add(((Element) item).getTagName()); + } + } + assertEquals(buildXml, ImmutableSet.of("healthScaleFactor", "testData", "descriptions"), childNames); + } + // TODO verify that at this point no master-side queries have been executed + { + TestResultAction a = b.getAction(TestResultAction.class); + assertNotNull(a); + assertEquals(1, a.getFailCount()); + /* TODO implement: + List failedTests = a.getFailedTests(); + assertEquals(1, failedTests.size()); + assertEquals("Klazz", failedTests.get(0).getClassName()); + assertEquals("test1", failedTests.get(0).getName()); + assertEquals("failure", failedTests.get(0).getErrorDetails()); + */ + // TODO more detailed Java queries incl. PackageResult / ClassResult + // TODO test healthScaleFactor, descriptions + } } @TestExtension public static class Impl implements TestResultStorage { @@ -129,7 +160,7 @@ public class TestResultStorageTest { return new TestResultImpl() { @Override public int getFailCount() { try { - Connection connection = connectionSupplier.connection; + Connection connection = connectionSupplier.connection(); try (PreparedStatement statement = connection.prepareStatement("SELECT COUNT(*) FROM " + Impl.CASE_RESULTS_TABLE + " WHERE job = ? and build = ? and errorDetails IS NOT NULL")) { statement.setString(1, job); statement.setInt(2, build); From 96c738e993ead35e67481bf96cce599ac334a9aa Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Tue, 28 Aug 2018 16:59:59 -0400 Subject: [PATCH 07/33] Oops, was building against older LTS lines. --- Jenkinsfile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Jenkinsfile b/Jenkinsfile index a676810fb..a229fa517 100644 --- a/Jenkinsfile +++ b/Jenkinsfile @@ -1 +1 @@ -buildPlugin(jenkinsVersions: [null, '2.60.1']) +buildPlugin() From 7541ad4d67c12052ae688379a3d7fc6c5b22fe3a Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Tue, 28 Aug 2018 17:06:20 -0400 Subject: [PATCH 08/33] FindBugs --- src/main/java/hudson/tasks/junit/TestResult.java | 2 ++ src/main/java/hudson/tasks/junit/TestResultAction.java | 3 +-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/main/java/hudson/tasks/junit/TestResult.java b/src/main/java/hudson/tasks/junit/TestResult.java index 98f76ac45..3dbe5f561 100644 --- a/src/main/java/hudson/tasks/junit/TestResult.java +++ b/src/main/java/hudson/tasks/junit/TestResult.java @@ -23,6 +23,7 @@ */ package hudson.tasks.junit; +import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; import hudson.AbortException; import hudson.Util; import hudson.model.Run; @@ -62,6 +63,7 @@ */ public final class TestResult extends MetaTabulatedResult { + @SuppressFBWarnings(value = "SE_BAD_FIELD", justification = "We do not expect TestResult to be serialized when this field is set.") private final @CheckForNull TestResultImpl impl; /** diff --git a/src/main/java/hudson/tasks/junit/TestResultAction.java b/src/main/java/hudson/tasks/junit/TestResultAction.java index 6edeb9379..bd458c0e8 100644 --- a/src/main/java/hudson/tasks/junit/TestResultAction.java +++ b/src/main/java/hudson/tasks/junit/TestResultAction.java @@ -50,7 +50,6 @@ import java.util.List; import java.util.logging.Level; import java.util.logging.Logger; -import javax.annotation.CheckForNull; import javax.annotation.Nullable; import jenkins.tasks.SimpleBuildStep; @@ -72,7 +71,7 @@ public class TestResultAction extends AbstractTestResultAction private @Nullable Integer skipCount; // Hudson < 1.25 didn't set these fields, so use Integer // so that we can distinguish between 0 tests vs not-computed-yet. - private @CheckForNull Integer totalCount; + private @Nullable Integer totalCount; private Double healthScaleFactor; private List testData = new ArrayList(); From 3b1bbfe9bd44d2cd317c04364cbbf4e6087762ce Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Wed, 29 Aug 2018 06:22:17 -0400 Subject: [PATCH 09/33] If you are going to patch a third-party library without repackaging/shading, please please do not pick a different groupId. --- pom.xml | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index f00a49a42..9cff4fed4 100644 --- a/pom.xml +++ b/pom.xml @@ -145,7 +145,7 @@ database 1.5 test - + antlr antlr @@ -154,6 +154,10 @@ javax.validation validation-api + + dom4j + dom4j + From 8e4aca0638f8483b05ee02de482937c20364d9a6 Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Wed, 29 Aug 2018 17:14:29 -0400 Subject: [PATCH 10/33] Threading the TestResultSummary back from the agent so that the junit step need not query the implementation just to determine UNSTABLE status. --- pom.xml | 2 +- .../java/hudson/tasks/junit/JUnitParser.java | 74 ++++++++++++++----- .../tasks/junit/JUnitResultArchiver.java | 67 +++++++++++++++-- .../hudson/tasks/junit/TestResultSummary.java | 1 + .../pipeline/JUnitResultsStepExecution.java | 21 ++---- .../junit/storage/TestResultStorage.java | 6 +- .../junit/storage/TestResultStorageTest.java | 31 +++++--- 7 files changed, 146 insertions(+), 56 deletions(-) diff --git a/pom.xml b/pom.xml index 9cff4fed4..982d3c511 100644 --- a/pom.xml +++ b/pom.xml @@ -54,7 +54,7 @@ org.jenkins-ci.plugins.workflow workflow-step-api - 2.12 + 2.16 org.jenkins-ci.plugins.workflow diff --git a/src/main/java/hudson/tasks/junit/JUnitParser.java b/src/main/java/hudson/tasks/junit/JUnitParser.java index b3a80716c..60575870f 100644 --- a/src/main/java/hudson/tasks/junit/JUnitParser.java +++ b/src/main/java/hudson/tasks/junit/JUnitParser.java @@ -34,14 +34,13 @@ import java.io.IOException; import java.io.File; +import java.util.concurrent.atomic.AtomicReference; import jenkins.MasterToSlaveFileCallable; import org.apache.tools.ant.types.FileSet; import org.apache.tools.ant.DirectoryScanner; -import javax.annotation.CheckForNull; - /** * Parse some JUnit xml files and generate a TestResult containing all the * results parsed. @@ -101,42 +100,52 @@ public TestResult parseResult(String testResultLocations, Run build, FilePa return parseResult(testResultLocations, build, null, workspace, launcher, listener); } + @Deprecated @Override public TestResult parseResult(String testResultLocations, Run build, PipelineTestDetails pipelineTestDetails, FilePath workspace, Launcher launcher, TaskListener listener) throws InterruptedException, IOException { + return parseResult(testResultLocations, build, pipelineTestDetails, workspace, launcher, listener, new AtomicReference<>()); + } + + public TestResult parseResult(String testResultLocations, Run build, PipelineTestDetails pipelineTestDetails, + FilePath workspace, Launcher launcher, TaskListener listener, AtomicReference summary) + throws InterruptedException, IOException { final long buildTime = build.getTimestamp().getTimeInMillis(); final long timeOnMaster = System.currentTimeMillis(); TestResultStorage storage = TestResultStorage.find(); - TestResultStorage.RemotePublisher publisher = storage == null ? null : storage.createRemotePublisher(build, listener); - - return workspace.act(new ParseResultCallable(testResultLocations, buildTime, timeOnMaster, keepLongStdio, - allowEmptyResults, pipelineTestDetails, publisher)); + if (storage == null) { + listener.getLogger().println(Messages.JUnitResultArchiver_Recording()); + TestResult result = workspace.act(new DirectParseResultCallable(testResultLocations, buildTime, timeOnMaster, keepLongStdio, allowEmptyResults, pipelineTestDetails)); + summary.set(new TestResultSummary(result)); + return result; + } else { + summary.set(workspace.act(new StorageParseResultCallable(testResultLocations, buildTime, timeOnMaster, keepLongStdio, allowEmptyResults, pipelineTestDetails, storage.createRemotePublisher(build), listener))); + return new TestResult(); // ignored anyway + } } - private static final class ParseResultCallable extends MasterToSlaveFileCallable { + private static abstract class ParseResultCallable extends MasterToSlaveFileCallable { private final long buildTime; private final String testResults; private final long nowMaster; private final boolean keepLongStdio; private final boolean allowEmptyResults; private final PipelineTestDetails pipelineTestDetails; - private final @CheckForNull TestResultStorage.RemotePublisher publisher; private ParseResultCallable(String testResults, long buildTime, long nowMaster, boolean keepLongStdio, boolean allowEmptyResults, - PipelineTestDetails pipelineTestDetails, TestResultStorage.RemotePublisher publisher) { + PipelineTestDetails pipelineTestDetails) { this.buildTime = buildTime; this.testResults = testResults; this.nowMaster = nowMaster; this.keepLongStdio = keepLongStdio; this.allowEmptyResults = allowEmptyResults; this.pipelineTestDetails = pipelineTestDetails; - this.publisher = publisher; } - public TestResult invoke(File ws, VirtualChannel channel) throws IOException { + public T invoke(File ws, VirtualChannel channel) throws IOException { final long nowSlave = System.currentTimeMillis(); FileSet fs = Util.createFileSet(ws, testResults); @@ -156,14 +165,43 @@ public TestResult invoke(File ws, VirtualChannel channel) throws IOException { throw new AbortException(Messages.JUnitResultArchiver_NoTestReportFound()); } } - if (publisher != null) { - publisher.publish(result); - return new TestResult(); // ignored anyway - // TODO at least send back a TestResultSummary to avoid requiring JUnitResultsStepExecution to repeatedly query storage - } else { - return result; - } + return handle(result); } + + protected abstract T handle(TestResult result) throws IOException; + + } + + private static final class DirectParseResultCallable extends ParseResultCallable { + + DirectParseResultCallable(String testResults, long buildTime, long nowMaster, boolean keepLongStdio, boolean allowEmptyResults, PipelineTestDetails pipelineTestDetails) { + super(testResults, buildTime, nowMaster, keepLongStdio, allowEmptyResults, pipelineTestDetails); + } + + @Override + protected TestResult handle(TestResult result) throws IOException { + return result; + } + + } + + private static final class StorageParseResultCallable extends ParseResultCallable { + + private final TestResultStorage.RemotePublisher publisher; + private final TaskListener listener; + + StorageParseResultCallable(String testResults, long buildTime, long nowMaster, boolean keepLongStdio, boolean allowEmptyResults, PipelineTestDetails pipelineTestDetails, TestResultStorage.RemotePublisher publisher, TaskListener listener) { + super(testResults, buildTime, nowMaster, keepLongStdio, allowEmptyResults, pipelineTestDetails); + this.publisher = publisher; + this.listener = listener; + } + + @Override + protected TestResultSummary handle(TestResult result) throws IOException { + publisher.publish(result, listener); + return new TestResultSummary(result); + } + } } diff --git a/src/main/java/hudson/tasks/junit/JUnitResultArchiver.java b/src/main/java/hudson/tasks/junit/JUnitResultArchiver.java index 10bf2bb35..f5b9de177 100644 --- a/src/main/java/hudson/tasks/junit/JUnitResultArchiver.java +++ b/src/main/java/hudson/tasks/junit/JUnitResultArchiver.java @@ -53,6 +53,7 @@ import java.io.IOException; import java.util.Collections; import java.util.List; +import java.util.concurrent.atomic.AtomicReference; import javax.annotation.Nonnull; import jenkins.tasks.SimpleBuildStep; import org.kohsuke.stapler.DataBoundSetter; @@ -121,6 +122,7 @@ public JUnitResultArchiver( setAllowEmptyResults(false); } + @Deprecated private TestResult parse(String expandedTestResults, Run run, @Nonnull FilePath workspace, Launcher launcher, TaskListener listener) throws IOException, InterruptedException { @@ -128,12 +130,20 @@ private TestResult parse(String expandedTestResults, Run run, @Nonnull File } + @Deprecated private static TestResult parse(@Nonnull JUnitTask task, PipelineTestDetails pipelineTestDetails, String expandedTestResults, Run run, @Nonnull FilePath workspace, Launcher launcher, TaskListener listener) throws IOException, InterruptedException { + return parse(task, pipelineTestDetails, expandedTestResults, run, workspace, launcher, listener, new AtomicReference<>()); + } + + private static TestResult parse(@Nonnull JUnitTask task, PipelineTestDetails pipelineTestDetails, + String expandedTestResults, Run run, @Nonnull FilePath workspace, + Launcher launcher, TaskListener listener, AtomicReference summary) + throws IOException, InterruptedException { return new JUnitParser(task.isKeepLongStdio(), task.isAllowEmptyResults()) - .parseResult(expandedTestResults, run, pipelineTestDetails, workspace, launcher, listener); + .parseResult(expandedTestResults, run, pipelineTestDetails, workspace, launcher, listener, summary); } @Deprecated @@ -150,12 +160,13 @@ protected TestResult parse(String expandedTestResults, AbstractBuild build, Laun @Override public void perform(Run build, FilePath workspace, Launcher launcher, TaskListener listener) throws InterruptedException, IOException { - TestResultAction action = parseAndAttach(this, null, build, workspace, launcher, listener); - - if (action != null && action.getResult().getFailCount() > 0) + if (parseAndSummarize(this, null, build, workspace, launcher, listener).getFailCount() > 0) { build.setResult(Result.UNSTABLE); + } } + /** @deprecated use {@link #parseAndSummarize} instead */ + @Deprecated public static TestResultAction parseAndAttach(@Nonnull JUnitTask task, PipelineTestDetails pipelineTestDetails, Run build, FilePath workspace, Launcher launcher, TaskListener listener) throws InterruptedException, IOException { @@ -189,10 +200,8 @@ public static TestResultAction parseAndAttach(@Nonnull JUnitTask task, PipelineT listener.getLogger().println(Messages.JUnitResultArchiver_ResultIsEmpty()); return null; } - /* TODO some of this logic is already in ParseResultCallable; the rest should be moved there // most likely a configuration error in the job - e.g. false pattern to match the JUnit result files throw new AbortException(Messages.JUnitResultArchiver_ResultIsEmpty()); - */ } // TODO: Move into JUnitParser [BUG 3123310] @@ -215,6 +224,52 @@ public static TestResultAction parseAndAttach(@Nonnull JUnitTask task, PipelineT } } + public static TestResultSummary parseAndSummarize(@Nonnull JUnitTask task, PipelineTestDetails pipelineTestDetails, + Run build, FilePath workspace, Launcher launcher, TaskListener listener) + throws InterruptedException, IOException { + final String testResults = build.getEnvironment(listener).expand(task.getTestResults()); + + AtomicReference summary = new AtomicReference<>(); + TestResult result = parse(task, pipelineTestDetails, testResults, build, workspace, launcher, listener, summary); + + synchronized (build) { + // TODO can the build argument be omitted now, or is it used prior to the call to addAction? + TestResultAction action = build.getAction(TestResultAction.class); + boolean appending; + if (action == null) { + appending = false; + action = new TestResultAction(build, result, listener); + } else { + appending = true; + result.freeze(action); + action.mergeResult(result, listener); + } + action.setHealthScaleFactor(task.getHealthScaleFactor()); // overwrites previous value if appending + if (summary.get().getTotalCount() == 0 && /* maybe a secondary effect */ build.getResult() != Result.FAILURE) { + assert task.isAllowEmptyResults(); + listener.getLogger().println(Messages.JUnitResultArchiver_ResultIsEmpty()); + } + + // TODO: Move into JUnitParser [BUG 3123310] + if (task.getTestDataPublishers() != null) { + for (TestDataPublisher tdp : task.getTestDataPublishers()) { + Data d = tdp.contributeTestData(build, workspace, launcher, listener, result); + if (d != null) { + action.addData(d); + } + } + } + + if (appending) { + build.save(); + } else { + build.addAction(action); + } + + return summary.get(); + } + } + /** * Not actually used, but left for backward compatibility * @param ds Directory scanner. diff --git a/src/main/java/hudson/tasks/junit/TestResultSummary.java b/src/main/java/hudson/tasks/junit/TestResultSummary.java index e97239d52..502cc7302 100644 --- a/src/main/java/hudson/tasks/junit/TestResultSummary.java +++ b/src/main/java/hudson/tasks/junit/TestResultSummary.java @@ -13,6 +13,7 @@ public class TestResultSummary implements Serializable { private int passCount; private int totalCount; + @Deprecated public TestResultSummary() { } diff --git a/src/main/java/hudson/tasks/junit/pipeline/JUnitResultsStepExecution.java b/src/main/java/hudson/tasks/junit/pipeline/JUnitResultsStepExecution.java index 22dbc750b..8f96157a6 100644 --- a/src/main/java/hudson/tasks/junit/pipeline/JUnitResultsStepExecution.java +++ b/src/main/java/hudson/tasks/junit/pipeline/JUnitResultsStepExecution.java @@ -6,7 +6,6 @@ import hudson.model.Run; import hudson.model.TaskListener; import hudson.tasks.junit.JUnitResultArchiver; -import hudson.tasks.junit.TestResultAction; import hudson.tasks.junit.TestResultSummary; import hudson.tasks.test.PipelineTestDetails; import java.util.ArrayList; @@ -46,22 +45,12 @@ protected TestResultSummary run() throws Exception { pipelineTestDetails.setNodeId(nodeId); pipelineTestDetails.setEnclosingBlocks(getEnclosingBlockIds(enclosingBlocks)); pipelineTestDetails.setEnclosingBlockNames(getEnclosingBlockNames(enclosingBlocks)); - try { - TestResultAction testResultAction = JUnitResultArchiver.parseAndAttach(step, pipelineTestDetails, run, workspace, launcher, listener); - - if (testResultAction != null) { - // TODO: Once JENKINS-43995 lands, update this to set the step status instead of the entire build. - if (testResultAction.getResult().getFailCount() > 0) { - run.setResult(Result.UNSTABLE); - } - return new TestResultSummary(testResultAction.getResult().getResultByNode(nodeId)); - } - } catch (Exception e) { - listener.getLogger().println(e.getMessage()); - throw e; + TestResultSummary summary = JUnitResultArchiver.parseAndSummarize(step, pipelineTestDetails, run, workspace, launcher, listener); + // TODO: Once JENKINS-43995 lands, update this to set the step status instead of the entire build. + if (summary.getFailCount() > 0) { + run.setResult(Result.UNSTABLE); } - - return new TestResultSummary(); + return summary; } /** diff --git a/src/main/java/hudson/tasks/junit/storage/TestResultStorage.java b/src/main/java/hudson/tasks/junit/storage/TestResultStorage.java index 47665b154..a9036041b 100644 --- a/src/main/java/hudson/tasks/junit/storage/TestResultStorage.java +++ b/src/main/java/hudson/tasks/junit/storage/TestResultStorage.java @@ -43,15 +43,15 @@ public interface TestResultStorage extends ExtensionPoint { /** - * Runs during {@link JUnitParser#parseResult(String, Run, PipelineTestDetails, FilePath, Launcher, TaskListener)}. + * Runs during {@link JUnitParser#parseResult(String, Run, PipelineTestDetails, FilePath, Launcher, TaskListener, AtomicReference)}. */ - RemotePublisher createRemotePublisher(Run build, TaskListener listener) throws IOException; + RemotePublisher createRemotePublisher(Run build) throws IOException; /** * Remotable hook to perform test result publishing. */ interface RemotePublisher extends SerializableOnlyOverRemoting { - void publish(TestResult result) throws IOException; + void publish(TestResult result, TaskListener listener) throws IOException; } TestResultImpl load(String job, int build); diff --git a/src/test/java/hudson/tasks/junit/storage/TestResultStorageTest.java b/src/test/java/hudson/tasks/junit/storage/TestResultStorageTest.java index f6feab539..7d251ae1a 100644 --- a/src/test/java/hudson/tasks/junit/storage/TestResultStorageTest.java +++ b/src/test/java/hudson/tasks/junit/storage/TestResultStorageTest.java @@ -92,14 +92,13 @@ public class TestResultStorageTest { WorkflowJob p = r.createProject(WorkflowJob.class, "p"); p.setDefinition(new CpsFlowDefinition( "node('remote') {\n" + + // TODO test skips " writeFile file: 'x.xml', text: ''''''\n" + - " junit 'x.xml'\n" + + " def s = junit 'x.xml'\n" + + // TODO test repeated publishing + " echo(/summary: fail=$s.failCount skip=$s.skipCount pass=$s.passCount total=$s.totalCount/)\n" + "}", true)); WorkflowRun b = p.scheduleBuild2(0).get(); - try (Connection connection = GlobalDatabaseConfiguration.get().getDatabase().getDataSource().getConnection(); - ResultSet result = connection.getMetaData().getTables(null, null, null, new String[] {"TABLE"})) { - printResultSet(result); - } try (Connection connection = GlobalDatabaseConfiguration.get().getDatabase().getDataSource().getConnection(); PreparedStatement statement = connection.prepareStatement("SELECT * FROM " + Impl.CASE_RESULTS_TABLE); ResultSet result = statement.executeQuery()) { @@ -107,6 +106,7 @@ public class TestResultStorageTest { } // TODO verify table structure r.assertBuildStatus(Result.UNSTABLE, b); + r.assertLogContains("summary: fail=1 skip=0 pass=1 total=2", b); assertFalse(new File(b.getRootDir(), "junitResult.xml").isFile()); { String buildXml = FileUtils.readFileToString(new File(b.getRootDir(), "build.xml")); @@ -124,7 +124,7 @@ public class TestResultStorageTest { } assertEquals(buildXml, ImmutableSet.of("healthScaleFactor", "testData", "descriptions"), childNames); } - // TODO verify that at this point no master-side queries have been executed + Impl.queriesPermitted = true; { TestResultAction a = b.getAction(TestResultAction.class); assertNotNull(a); @@ -145,20 +145,25 @@ public class TestResultStorageTest { static final String CASE_RESULTS_TABLE = "caseResults"; + static boolean queriesPermitted; + private final ConnectionSupplier connectionSupplier = new LocalConnectionSupplier(); - @Override public RemotePublisher createRemotePublisher(Run build, TaskListener listener) throws IOException { + @Override public RemotePublisher createRemotePublisher(Run build) throws IOException { try { connectionSupplier.connection(); // make sure we start a local server and create table first } catch (SQLException x) { throw new IOException(x); } - return new RemotePublisherImpl(build.getParent().getFullName(), build.getNumber(), listener); + return new RemotePublisherImpl(build.getParent().getFullName(), build.getNumber()); } @Override public TestResultImpl load(String job, int build) { return new TestResultImpl() { @Override public int getFailCount() { + if (!queriesPermitted) { + throw new IllegalStateException("Should not have been running any queries yet"); + } try { Connection connection = connectionSupplier.connection(); try (PreparedStatement statement = connection.prepareStatement("SELECT COUNT(*) FROM " + Impl.CASE_RESULTS_TABLE + " WHERE job = ? and build = ? and errorDetails IS NOT NULL")) { @@ -184,21 +189,20 @@ private static class RemotePublisherImpl implements RemotePublisher { private final String job; private final int build; - private final TaskListener listener; // TODO keep the same supplier and thus Connection open across builds, so long as the database config remains unchanged private final ConnectionSupplier connectionSupplier; - RemotePublisherImpl(String job, int build, TaskListener listener) { + RemotePublisherImpl(String job, int build) { this.job = job; this.build = build; - this.listener = listener; connectionSupplier = new RemoteConnectionSupplier(); } - @Override public void publish(TestResult result) throws IOException { + @Override public void publish(TestResult result, TaskListener listener) throws IOException { try { Connection connection = connectionSupplier.connection(); try (PreparedStatement statement = connection.prepareStatement("INSERT INTO " + CASE_RESULTS_TABLE + " (job, build, suite, className, testName, errorDetails) VALUES (?, ?, ?, ?, ?, ?)")) { + int count = 0; for (SuiteResult suiteResult : result.getSuites()) { for (CaseResult caseResult : suiteResult.getCases()) { statement.setString(1, job); @@ -213,8 +217,10 @@ private static class RemotePublisherImpl implements RemotePublisher { statement.setNull(6, Types.VARCHAR); } statement.executeUpdate(); + count++; } } + listener.getLogger().printf("Saved %d test cases into database.%n", count); } } catch (SQLException x) { throw new IOException(x); @@ -262,6 +268,7 @@ static class LocalConnectionSupplier extends ConnectionSupplier { try (Statement statement = connection.createStatement()) { // TODO this and joined tables: skipped, skippedMessage, errorStackTrace, stdout, stderr, duration, nodeId, enclosingBlocks, enclosingBlockNames, etc. statement.execute("CREATE TABLE " + CASE_RESULTS_TABLE + "(job varchar(255), build int, suite varchar(255), className varchar(255), testName varchar(255), errorDetails varchar(255))"); + // TODO indices } } } From 2d10039bbcbf760d2286f2fde75d3fa714b71792 Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Wed, 29 Aug 2018 17:35:59 -0400 Subject: [PATCH 11/33] We do not want to attach a TestResultAction when there is an empty result. --- src/main/java/hudson/tasks/junit/JUnitResultArchiver.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/hudson/tasks/junit/JUnitResultArchiver.java b/src/main/java/hudson/tasks/junit/JUnitResultArchiver.java index f5b9de177..ceeb40d50 100644 --- a/src/main/java/hudson/tasks/junit/JUnitResultArchiver.java +++ b/src/main/java/hudson/tasks/junit/JUnitResultArchiver.java @@ -262,7 +262,7 @@ public static TestResultSummary parseAndSummarize(@Nonnull JUnitTask task, Pipel if (appending) { build.save(); - } else { + } else if (summary.get().getTotalCount() > 0) { build.addAction(action); } From 049a9fe8139c08437c7d83ef1765397c28ecc69b Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Thu, 30 Aug 2018 10:22:27 -0400 Subject: [PATCH 12/33] [JENKINS-48250] Removing bogus test from #93. 8e4aca0638f8483b05ee02de482937c20364d9a6 already removed the improper exception handling in JUnitResultsStepExecution. --- .../junit/pipeline/JUnitResultsStepTest.java | 21 ------------------- 1 file changed, 21 deletions(-) diff --git a/src/test/java/hudson/tasks/junit/pipeline/JUnitResultsStepTest.java b/src/test/java/hudson/tasks/junit/pipeline/JUnitResultsStepTest.java index b39d4e619..0c8a81608 100644 --- a/src/test/java/hudson/tasks/junit/pipeline/JUnitResultsStepTest.java +++ b/src/test/java/hudson/tasks/junit/pipeline/JUnitResultsStepTest.java @@ -15,13 +15,10 @@ import hudson.tasks.junit.TestResultAction; import hudson.tasks.junit.TestResultTest; import hudson.tasks.test.PipelineBlockWithTests; -import org.hamcrest.CoreMatchers; import org.jenkinsci.plugins.workflow.actions.LabelAction; -import org.jenkinsci.plugins.workflow.actions.LogAction; import org.jenkinsci.plugins.workflow.actions.ThreadNameAction; import org.jenkinsci.plugins.workflow.cps.CpsFlowDefinition; import org.jenkinsci.plugins.workflow.cps.SnippetizerTester; -import org.jenkinsci.plugins.workflow.cps.nodes.StepAtomNode; import org.jenkinsci.plugins.workflow.cps.nodes.StepStartNode; import org.jenkinsci.plugins.workflow.flow.FlowExecution; import org.jenkinsci.plugins.workflow.graph.FlowNode; @@ -39,7 +36,6 @@ import org.kohsuke.stapler.DataBoundConstructor; import javax.annotation.Nullable; -import java.io.ByteArrayOutputStream; import java.io.IOException; import java.util.ArrayList; import java.util.Arrays; @@ -49,7 +45,6 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; -import static org.junit.Assert.assertThat; import static org.junit.Assert.fail; public class JUnitResultsStepTest { @@ -89,22 +84,6 @@ public void emptyFails() throws Exception { WorkflowRun r = j.scheduleBuild2(0).waitForStart(); rule.assertBuildStatus(Result.FAILURE, rule.waitForCompletion(r)); rule.assertLogContains("ERROR: " + Messages.JUnitResultArchiver_NoTestReportFound(), r); - FlowExecution execution = r.getExecution(); - DepthFirstScanner scanner = new DepthFirstScanner(); - FlowNode f = scanner.findFirstMatch(execution, new Predicate() { - @Override - public boolean apply(@Nullable FlowNode input) { - return input instanceof StepAtomNode && - ((StepAtomNode) input).getDescriptor() instanceof JUnitResultsStep.DescriptorImpl; - } - }); - assertNotNull(f); - LogAction logAction = f.getPersistentAction(LogAction.class); - assertNotNull(logAction); - ByteArrayOutputStream baos = new ByteArrayOutputStream(); - logAction.getLogText().writeRawLogTo(0, baos); - String log = baos.toString(); - assertThat(log, CoreMatchers.containsString(Messages.JUnitResultArchiver_NoTestReportFound())); } @Test From 6e704e892c55f80d336e45177978be6b76803ce7 Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Thu, 30 Aug 2018 10:50:23 -0400 Subject: [PATCH 13/33] The TestResultSummary constructor silently fails on an untallied/unfrozen TestResult. --- src/main/java/hudson/tasks/junit/JUnitParser.java | 5 ++--- src/main/java/hudson/tasks/junit/JUnitResultArchiver.java | 4 ++++ 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/src/main/java/hudson/tasks/junit/JUnitParser.java b/src/main/java/hudson/tasks/junit/JUnitParser.java index 60575870f..fba740b85 100644 --- a/src/main/java/hudson/tasks/junit/JUnitParser.java +++ b/src/main/java/hudson/tasks/junit/JUnitParser.java @@ -117,13 +117,12 @@ public TestResult parseResult(String testResultLocations, Run build, Pipeli TestResultStorage storage = TestResultStorage.find(); if (storage == null) { listener.getLogger().println(Messages.JUnitResultArchiver_Recording()); - TestResult result = workspace.act(new DirectParseResultCallable(testResultLocations, buildTime, timeOnMaster, keepLongStdio, allowEmptyResults, pipelineTestDetails)); - summary.set(new TestResultSummary(result)); - return result; + return workspace.act(new DirectParseResultCallable(testResultLocations, buildTime, timeOnMaster, keepLongStdio, allowEmptyResults, pipelineTestDetails)); } else { summary.set(workspace.act(new StorageParseResultCallable(testResultLocations, buildTime, timeOnMaster, keepLongStdio, allowEmptyResults, pipelineTestDetails, storage.createRemotePublisher(build), listener))); return new TestResult(); // ignored anyway } + // TODO awkward; maybe clearer to split into distinct methods, one returning TestResult, one taking TestResultStorage and returning TestResultSummary } private static abstract class ParseResultCallable extends MasterToSlaveFileCallable { diff --git a/src/main/java/hudson/tasks/junit/JUnitResultArchiver.java b/src/main/java/hudson/tasks/junit/JUnitResultArchiver.java index ceeb40d50..884ee870a 100644 --- a/src/main/java/hudson/tasks/junit/JUnitResultArchiver.java +++ b/src/main/java/hudson/tasks/junit/JUnitResultArchiver.java @@ -244,6 +244,10 @@ public static TestResultSummary parseAndSummarize(@Nonnull JUnitTask task, Pipel result.freeze(action); action.mergeResult(result, listener); } + if (summary.get() == null) { + // Cannot do this in parseResult since the result has not yet been frozen. + summary.set(new TestResultSummary(result)); + } action.setHealthScaleFactor(task.getHealthScaleFactor()); // overwrites previous value if appending if (summary.get().getTotalCount() == 0 && /* maybe a secondary effect */ build.getResult() != Result.FAILURE) { assert task.isAllowEmptyResults(); From 5665a2c6cebb7a77a13d3ffedc863c762a6fce9c Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Thu, 30 Aug 2018 15:28:59 -0400 Subject: [PATCH 14/33] Making TestResultSummary more robust by detecting an untallied/unfrozen TestResult. --- src/main/java/hudson/tasks/junit/TestResultSummary.java | 7 +++++++ .../java/hudson/tasks/junit/JUnitResultArchiverTest.java | 3 ++- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/src/main/java/hudson/tasks/junit/TestResultSummary.java b/src/main/java/hudson/tasks/junit/TestResultSummary.java index 502cc7302..53ac2b104 100644 --- a/src/main/java/hudson/tasks/junit/TestResultSummary.java +++ b/src/main/java/hudson/tasks/junit/TestResultSummary.java @@ -22,6 +22,13 @@ public TestResultSummary(TestResult result) { this.skipCount = result.getSkipCount(); this.passCount = result.getPassCount(); this.totalCount = result.getTotalCount(); + if (totalCount == 0) { + for (SuiteResult suite : result.getSuites()) { + if (!suite.getCases().isEmpty()) { + throw new IllegalArgumentException("Attempt to construct TestResultSummary from TestResult without calling tally/freeze"); + } + } + } } @Whitelisted diff --git a/src/test/java/hudson/tasks/junit/JUnitResultArchiverTest.java b/src/test/java/hudson/tasks/junit/JUnitResultArchiverTest.java index 1152a404b..7229d0d35 100644 --- a/src/test/java/hudson/tasks/junit/JUnitResultArchiverTest.java +++ b/src/test/java/hudson/tasks/junit/JUnitResultArchiverTest.java @@ -133,7 +133,8 @@ public class JUnitResultArchiverTest { basic(); } - private void assertTestResults(FreeStyleBuild build) { + private void assertTestResults(FreeStyleBuild build) throws Exception { + j.assertBuildStatus(Result.UNSTABLE, build); TestResultAction testResultAction = build.getAction(TestResultAction.class); assertNotNull("no TestResultAction", testResultAction); From c6a4223e9f7a0ba273286dfa35edd6a52cdd5222 Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Thu, 30 Aug 2018 15:58:07 -0400 Subject: [PATCH 15/33] Simplified distinction between methods returning TestResult vs. TestResultSummary. --- .../java/hudson/tasks/junit/JUnitParser.java | 35 +++++----------- .../tasks/junit/JUnitResultArchiver.java | 40 ++++++++++--------- 2 files changed, 33 insertions(+), 42 deletions(-) diff --git a/src/main/java/hudson/tasks/junit/JUnitParser.java b/src/main/java/hudson/tasks/junit/JUnitParser.java index fba740b85..b6c57509d 100644 --- a/src/main/java/hudson/tasks/junit/JUnitParser.java +++ b/src/main/java/hudson/tasks/junit/JUnitParser.java @@ -34,7 +34,6 @@ import java.io.IOException; import java.io.File; -import java.util.concurrent.atomic.AtomicReference; import jenkins.MasterToSlaveFileCallable; @@ -100,29 +99,17 @@ public TestResult parseResult(String testResultLocations, Run build, FilePa return parseResult(testResultLocations, build, null, workspace, launcher, listener); } - @Deprecated @Override public TestResult parseResult(String testResultLocations, Run build, PipelineTestDetails pipelineTestDetails, FilePath workspace, Launcher launcher, TaskListener listener) throws InterruptedException, IOException { - return parseResult(testResultLocations, build, pipelineTestDetails, workspace, launcher, listener, new AtomicReference<>()); + return workspace.act(new DirectParseResultCallable(testResultLocations, build, keepLongStdio, allowEmptyResults, pipelineTestDetails)); } - public TestResult parseResult(String testResultLocations, Run build, PipelineTestDetails pipelineTestDetails, - FilePath workspace, Launcher launcher, TaskListener listener, AtomicReference summary) + public TestResultSummary summarizeResult(String testResultLocations, Run build, PipelineTestDetails pipelineTestDetails, + FilePath workspace, Launcher launcher, TaskListener listener, TestResultStorage storage) throws InterruptedException, IOException { - final long buildTime = build.getTimestamp().getTimeInMillis(); - final long timeOnMaster = System.currentTimeMillis(); - - TestResultStorage storage = TestResultStorage.find(); - if (storage == null) { - listener.getLogger().println(Messages.JUnitResultArchiver_Recording()); - return workspace.act(new DirectParseResultCallable(testResultLocations, buildTime, timeOnMaster, keepLongStdio, allowEmptyResults, pipelineTestDetails)); - } else { - summary.set(workspace.act(new StorageParseResultCallable(testResultLocations, buildTime, timeOnMaster, keepLongStdio, allowEmptyResults, pipelineTestDetails, storage.createRemotePublisher(build), listener))); - return new TestResult(); // ignored anyway - } - // TODO awkward; maybe clearer to split into distinct methods, one returning TestResult, one taking TestResultStorage and returning TestResultSummary + return workspace.act(new StorageParseResultCallable(testResultLocations, build, keepLongStdio, allowEmptyResults, pipelineTestDetails, storage.createRemotePublisher(build), listener)); } private static abstract class ParseResultCallable extends MasterToSlaveFileCallable { @@ -133,12 +120,12 @@ private static abstract class ParseResultCallable extends MasterToSlaveFileCa private final boolean allowEmptyResults; private final PipelineTestDetails pipelineTestDetails; - private ParseResultCallable(String testResults, long buildTime, long nowMaster, + private ParseResultCallable(String testResults, Run build, boolean keepLongStdio, boolean allowEmptyResults, PipelineTestDetails pipelineTestDetails) { - this.buildTime = buildTime; + this.buildTime = build.getTimestamp().getTimeInMillis(); this.testResults = testResults; - this.nowMaster = nowMaster; + this.nowMaster = System.currentTimeMillis(); this.keepLongStdio = keepLongStdio; this.allowEmptyResults = allowEmptyResults; this.pipelineTestDetails = pipelineTestDetails; @@ -173,8 +160,8 @@ public T invoke(File ws, VirtualChannel channel) throws IOException { private static final class DirectParseResultCallable extends ParseResultCallable { - DirectParseResultCallable(String testResults, long buildTime, long nowMaster, boolean keepLongStdio, boolean allowEmptyResults, PipelineTestDetails pipelineTestDetails) { - super(testResults, buildTime, nowMaster, keepLongStdio, allowEmptyResults, pipelineTestDetails); + DirectParseResultCallable(String testResults, Run build, boolean keepLongStdio, boolean allowEmptyResults, PipelineTestDetails pipelineTestDetails) { + super(testResults, build, keepLongStdio, allowEmptyResults, pipelineTestDetails); } @Override @@ -189,8 +176,8 @@ private static final class StorageParseResultCallable extends ParseResultCallabl private final TestResultStorage.RemotePublisher publisher; private final TaskListener listener; - StorageParseResultCallable(String testResults, long buildTime, long nowMaster, boolean keepLongStdio, boolean allowEmptyResults, PipelineTestDetails pipelineTestDetails, TestResultStorage.RemotePublisher publisher, TaskListener listener) { - super(testResults, buildTime, nowMaster, keepLongStdio, allowEmptyResults, pipelineTestDetails); + StorageParseResultCallable(String testResults, Run build, boolean keepLongStdio, boolean allowEmptyResults, PipelineTestDetails pipelineTestDetails, TestResultStorage.RemotePublisher publisher, TaskListener listener) { + super(testResults, build, keepLongStdio, allowEmptyResults, pipelineTestDetails); this.publisher = publisher; this.listener = listener; } diff --git a/src/main/java/hudson/tasks/junit/JUnitResultArchiver.java b/src/main/java/hudson/tasks/junit/JUnitResultArchiver.java index 884ee870a..6fb505d4a 100644 --- a/src/main/java/hudson/tasks/junit/JUnitResultArchiver.java +++ b/src/main/java/hudson/tasks/junit/JUnitResultArchiver.java @@ -41,6 +41,7 @@ import hudson.tasks.Publisher; import hudson.tasks.Recorder; import hudson.tasks.junit.TestResultAction.Data; +import hudson.tasks.junit.storage.TestResultStorage; import hudson.tasks.test.PipelineTestDetails; import hudson.util.DescribableList; import hudson.util.FormValidation; @@ -53,7 +54,6 @@ import java.io.IOException; import java.util.Collections; import java.util.List; -import java.util.concurrent.atomic.AtomicReference; import javax.annotation.Nonnull; import jenkins.tasks.SimpleBuildStep; import org.kohsuke.stapler.DataBoundSetter; @@ -130,20 +130,12 @@ private TestResult parse(String expandedTestResults, Run run, @Nonnull File } - @Deprecated private static TestResult parse(@Nonnull JUnitTask task, PipelineTestDetails pipelineTestDetails, String expandedTestResults, Run run, @Nonnull FilePath workspace, Launcher launcher, TaskListener listener) throws IOException, InterruptedException { - return parse(task, pipelineTestDetails, expandedTestResults, run, workspace, launcher, listener, new AtomicReference<>()); - } - - private static TestResult parse(@Nonnull JUnitTask task, PipelineTestDetails pipelineTestDetails, - String expandedTestResults, Run run, @Nonnull FilePath workspace, - Launcher launcher, TaskListener listener, AtomicReference summary) - throws IOException, InterruptedException { return new JUnitParser(task.isKeepLongStdio(), task.isAllowEmptyResults()) - .parseResult(expandedTestResults, run, pipelineTestDetails, workspace, launcher, listener, summary); + .parseResult(expandedTestResults, run, pipelineTestDetails, workspace, launcher, listener); } @Deprecated @@ -227,10 +219,22 @@ public static TestResultAction parseAndAttach(@Nonnull JUnitTask task, PipelineT public static TestResultSummary parseAndSummarize(@Nonnull JUnitTask task, PipelineTestDetails pipelineTestDetails, Run build, FilePath workspace, Launcher launcher, TaskListener listener) throws InterruptedException, IOException { + TestResultStorage storage = TestResultStorage.find(); + if (storage == null) { + listener.getLogger().println(Messages.JUnitResultArchiver_Recording()); + } // else let storage decide what to print + final String testResults = build.getEnvironment(listener).expand(task.getTestResults()); - AtomicReference summary = new AtomicReference<>(); - TestResult result = parse(task, pipelineTestDetails, testResults, build, workspace, launcher, listener, summary); + TestResult result; + TestResultSummary summary; + if (storage == null) { + result = parse(task, pipelineTestDetails, testResults, build, workspace, launcher, listener); + summary = null; // see below + } else { + result = new TestResult(); // irrelevant + summary = new JUnitParser(task.isKeepLongStdio(), task.isAllowEmptyResults()).summarizeResult(testResults, build, pipelineTestDetails, workspace, launcher, listener, storage); + } synchronized (build) { // TODO can the build argument be omitted now, or is it used prior to the call to addAction? @@ -244,12 +248,12 @@ public static TestResultSummary parseAndSummarize(@Nonnull JUnitTask task, Pipel result.freeze(action); action.mergeResult(result, listener); } - if (summary.get() == null) { - // Cannot do this in parseResult since the result has not yet been frozen. - summary.set(new TestResultSummary(result)); + if (summary == null) { + // Cannot do this above since the result has not yet been frozen. + summary = new TestResultSummary(result); } action.setHealthScaleFactor(task.getHealthScaleFactor()); // overwrites previous value if appending - if (summary.get().getTotalCount() == 0 && /* maybe a secondary effect */ build.getResult() != Result.FAILURE) { + if (summary.getTotalCount() == 0 && /* maybe a secondary effect */ build.getResult() != Result.FAILURE) { assert task.isAllowEmptyResults(); listener.getLogger().println(Messages.JUnitResultArchiver_ResultIsEmpty()); } @@ -266,11 +270,11 @@ public static TestResultSummary parseAndSummarize(@Nonnull JUnitTask task, Pipel if (appending) { build.save(); - } else if (summary.get().getTotalCount() > 0) { + } else if (summary.getTotalCount() > 0) { build.addAction(action); } - return summary.get(); + return summary; } } From ad6696380d8a0baec99792e069119c8b1e220f75 Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Thu, 30 Aug 2018 16:25:57 -0400 Subject: [PATCH 16/33] How was this plugin getting a test dep on an older version of itself?! --- pom.xml | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/pom.xml b/pom.xml index 982d3c511..d9dc67220 100644 --- a/pom.xml +++ b/pom.xml @@ -114,6 +114,12 @@ workflow-basic-steps 2.6 test + + + org.jenkins-ci.plugins + junit + + org.jenkins-ci.plugins.workflow From 79b1937c09dcf51d954c8e4fdd3078c4daeeb3fe Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Thu, 30 Aug 2018 16:33:41 -0400 Subject: [PATCH 17/33] Supporting repeated archiving, and reporting on total, pass, and skip counts. --- .../tasks/junit/JUnitResultArchiver.java | 10 ++- .../java/hudson/tasks/junit/TestResult.java | 14 ++++ .../hudson/tasks/junit/TestResultAction.java | 8 ++ .../tasks/junit/storage/TestResultImpl.java | 3 + .../junit/storage/TestResultStorageTest.java | 74 +++++++++++++++---- 5 files changed, 90 insertions(+), 19 deletions(-) diff --git a/src/main/java/hudson/tasks/junit/JUnitResultArchiver.java b/src/main/java/hudson/tasks/junit/JUnitResultArchiver.java index 6fb505d4a..84a4068f4 100644 --- a/src/main/java/hudson/tasks/junit/JUnitResultArchiver.java +++ b/src/main/java/hudson/tasks/junit/JUnitResultArchiver.java @@ -224,7 +224,7 @@ public static TestResultSummary parseAndSummarize(@Nonnull JUnitTask task, Pipel listener.getLogger().println(Messages.JUnitResultArchiver_Recording()); } // else let storage decide what to print - final String testResults = build.getEnvironment(listener).expand(task.getTestResults()); + String testResults = build.getEnvironment(listener).expand(task.getTestResults()); TestResult result; TestResultSummary summary; @@ -245,10 +245,13 @@ public static TestResultSummary parseAndSummarize(@Nonnull JUnitTask task, Pipel action = new TestResultAction(build, result, listener); } else { appending = true; - result.freeze(action); - action.mergeResult(result, listener); + if (storage == null) { + result.freeze(action); + action.mergeResult(result, listener); + } } if (summary == null) { + assert storage == null; // Cannot do this above since the result has not yet been frozen. summary = new TestResultSummary(result); } @@ -258,7 +261,6 @@ public static TestResultSummary parseAndSummarize(@Nonnull JUnitTask task, Pipel listener.getLogger().println(Messages.JUnitResultArchiver_ResultIsEmpty()); } - // TODO: Move into JUnitParser [BUG 3123310] if (task.getTestDataPublishers() != null) { for (TestDataPublisher tdp : task.getTestDataPublishers()) { Data d = tdp.contributeTestData(build, workspace, launcher, listener, result); diff --git a/src/main/java/hudson/tasks/junit/TestResult.java b/src/main/java/hudson/tasks/junit/TestResult.java index 3dbe5f561..4e6b20722 100644 --- a/src/main/java/hudson/tasks/junit/TestResult.java +++ b/src/main/java/hudson/tasks/junit/TestResult.java @@ -456,6 +456,9 @@ public float getDuration() { @Exported(visibility=999) @Override public int getPassCount() { + if (impl != null) { + return impl.getPassCount(); + } return totalTests-getFailCount()-getSkipCount(); } @@ -474,8 +477,19 @@ public int getFailCount() { @Exported(visibility=999) @Override public int getSkipCount() { + if (impl != null) { + return impl.getSkipCount(); + } return skippedTestsCounter; } + + @Override + public int getTotalCount() { + if (impl != null) { + return impl.getTotalCount(); + } + return super.getTotalCount(); + } /** * Returns true if this doesn't have any any test results. diff --git a/src/main/java/hudson/tasks/junit/TestResultAction.java b/src/main/java/hudson/tasks/junit/TestResultAction.java index bd458c0e8..156131fef 100644 --- a/src/main/java/hudson/tasks/junit/TestResultAction.java +++ b/src/main/java/hudson/tasks/junit/TestResultAction.java @@ -177,6 +177,10 @@ public synchronized int getFailCount() { @Override public synchronized int getSkipCount() { + TestResultStorage storage = TestResultStorage.find(); + if (storage != null) { + return new TestResult(storage.load(run.getParent().getFullName(), run.getNumber())).getSkipCount(); + } if(totalCount==null) getResult(); // this will compute the result return skipCount; @@ -184,6 +188,10 @@ public synchronized int getSkipCount() { @Override public synchronized int getTotalCount() { + TestResultStorage storage = TestResultStorage.find(); + if (storage != null) { + return new TestResult(storage.load(run.getParent().getFullName(), run.getNumber())).getTotalCount(); + } if(totalCount==null) getResult(); // this will compute the result return totalCount; diff --git a/src/main/java/hudson/tasks/junit/storage/TestResultImpl.java b/src/main/java/hudson/tasks/junit/storage/TestResultImpl.java index 22530d923..fb18ec96a 100644 --- a/src/main/java/hudson/tasks/junit/storage/TestResultImpl.java +++ b/src/main/java/hudson/tasks/junit/storage/TestResultImpl.java @@ -33,5 +33,8 @@ */ public interface TestResultImpl { int getFailCount(); + int getSkipCount(); + int getPassCount(); + int getTotalCount(); @Nonnull TestResult getResultByNodes(@Nonnull List nodeIds); } diff --git a/src/test/java/hudson/tasks/junit/storage/TestResultStorageTest.java b/src/test/java/hudson/tasks/junit/storage/TestResultStorageTest.java index 7d251ae1a..d0c6fc9c9 100644 --- a/src/test/java/hudson/tasks/junit/storage/TestResultStorageTest.java +++ b/src/test/java/hudson/tasks/junit/storage/TestResultStorageTest.java @@ -26,6 +26,7 @@ import com.google.common.collect.ImmutableSet; import com.thoughtworks.xstream.XStream; +import hudson.Util; import hudson.model.Label; import hudson.model.Result; import hudson.model.Run; @@ -92,11 +93,18 @@ public class TestResultStorageTest { WorkflowJob p = r.createProject(WorkflowJob.class, "p"); p.setDefinition(new CpsFlowDefinition( "node('remote') {\n" + - // TODO test skips - " writeFile file: 'x.xml', text: ''''''\n" + + " writeFile file: 'x.xml', text: '''" + + "" + + "" + + "" + + "'''\n" + " def s = junit 'x.xml'\n" + - // TODO test repeated publishing " echo(/summary: fail=$s.failCount skip=$s.skipCount pass=$s.passCount total=$s.totalCount/)\n" + + " writeFile file: 'x.xml', text: '''" + + "" + + "'''\n" + + " s = junit 'x.xml'\n" + + " echo(/next summary: fail=$s.failCount skip=$s.skipCount pass=$s.passCount total=$s.totalCount/)\n" + "}", true)); WorkflowRun b = p.scheduleBuild2(0).get(); try (Connection connection = GlobalDatabaseConfiguration.get().getDatabase().getDataSource().getConnection(); @@ -106,7 +114,8 @@ public class TestResultStorageTest { } // TODO verify table structure r.assertBuildStatus(Result.UNSTABLE, b); - r.assertLogContains("summary: fail=1 skip=0 pass=1 total=2", b); + r.assertLogContains("summary: fail=1 skip=1 pass=1 total=3", b); + r.assertLogContains("next summary: fail=1 skip=0 pass=0 total=1", b); assertFalse(new File(b.getRootDir(), "junitResult.xml").isFile()); { String buildXml = FileUtils.readFileToString(new File(b.getRootDir(), "build.xml")); @@ -128,13 +137,22 @@ public class TestResultStorageTest { { TestResultAction a = b.getAction(TestResultAction.class); assertNotNull(a); - assertEquals(1, a.getFailCount()); + assertEquals(2, a.getFailCount()); + assertEquals(1, a.getSkipCount()); + assertEquals(4, a.getTotalCount()); + assertEquals(2, a.getResult().getFailCount()); + assertEquals(1, a.getResult().getSkipCount()); + assertEquals(4, a.getResult().getTotalCount()); + assertEquals(1, a.getResult().getPassCount()); /* TODO implement: List failedTests = a.getFailedTests(); - assertEquals(1, failedTests.size()); + assertEquals(2, failedTests.size()); assertEquals("Klazz", failedTests.get(0).getClassName()); assertEquals("test1", failedTests.get(0).getName()); assertEquals("failure", failedTests.get(0).getErrorDetails()); + assertEquals("another.Klazz", failedTests.get(1).getClassName()); + assertEquals("test1", failedTests.get(1).getName()); + assertEquals("another failure", failedTests.get(1).getErrorDetails()); */ // TODO more detailed Java queries incl. PackageResult / ClassResult // TODO test healthScaleFactor, descriptions @@ -158,15 +176,27 @@ public class TestResultStorageTest { return new RemotePublisherImpl(build.getParent().getFullName(), build.getNumber()); } + @FunctionalInterface + private interface Querier { + T run(Connection connection) throws SQLException; + } @Override public TestResultImpl load(String job, int build) { return new TestResultImpl() { - @Override public int getFailCount() { + private T query(Querier querier, T dflt) { if (!queriesPermitted) { throw new IllegalStateException("Should not have been running any queries yet"); } try { Connection connection = connectionSupplier.connection(); - try (PreparedStatement statement = connection.prepareStatement("SELECT COUNT(*) FROM " + Impl.CASE_RESULTS_TABLE + " WHERE job = ? and build = ? and errorDetails IS NOT NULL")) { + return querier.run(connection); + } catch (SQLException x) { + x.printStackTrace(); + return dflt; + } + } + private int getCaseCount(String and) { + return query(connection -> { + try (PreparedStatement statement = connection.prepareStatement("SELECT COUNT(*) FROM " + Impl.CASE_RESULTS_TABLE + " WHERE job = ? AND build = ?" + and)) { statement.setString(1, job); statement.setInt(2, build); try (ResultSet result = statement.executeQuery()) { @@ -174,10 +204,19 @@ public class TestResultStorageTest { return result.getInt(1); } } - } catch (SQLException x) { - x.printStackTrace(); - return 0; - } + }, 0); + } + @Override public int getFailCount() { + return getCaseCount(" AND errorDetails IS NOT NULL"); + } + @Override public int getSkipCount() { + return getCaseCount(" AND skipped IS NOT NULL"); + } + @Override public int getPassCount() { + return getCaseCount(" AND errorDetails IS NULL AND skipped IS NULL"); + } + @Override public int getTotalCount() { + return getCaseCount(""); } @Override public TestResult getResultByNodes(List nodeIds) { return new TestResult(this); // TODO @@ -201,7 +240,7 @@ private static class RemotePublisherImpl implements RemotePublisher { @Override public void publish(TestResult result, TaskListener listener) throws IOException { try { Connection connection = connectionSupplier.connection(); - try (PreparedStatement statement = connection.prepareStatement("INSERT INTO " + CASE_RESULTS_TABLE + " (job, build, suite, className, testName, errorDetails) VALUES (?, ?, ?, ?, ?, ?)")) { + try (PreparedStatement statement = connection.prepareStatement("INSERT INTO " + CASE_RESULTS_TABLE + " (job, build, suite, className, testName, errorDetails, skipped) VALUES (?, ?, ?, ?, ?, ?, ?)")) { int count = 0; for (SuiteResult suiteResult : result.getSuites()) { for (CaseResult caseResult : suiteResult.getCases()) { @@ -216,6 +255,11 @@ private static class RemotePublisherImpl implements RemotePublisher { } else { statement.setNull(6, Types.VARCHAR); } + if (caseResult.isSkipped()) { + statement.setString(7, Util.fixNull(caseResult.getSkippedMessage())); + } else { + statement.setNull(7, Types.VARCHAR); + } statement.executeUpdate(); count++; } @@ -266,8 +310,8 @@ static class LocalConnectionSupplier extends ConnectionSupplier { } if (!exists) { try (Statement statement = connection.createStatement()) { - // TODO this and joined tables: skipped, skippedMessage, errorStackTrace, stdout, stderr, duration, nodeId, enclosingBlocks, enclosingBlockNames, etc. - statement.execute("CREATE TABLE " + CASE_RESULTS_TABLE + "(job varchar(255), build int, suite varchar(255), className varchar(255), testName varchar(255), errorDetails varchar(255))"); + // TODO this and joined tables: errorStackTrace, stdout, stderr, duration, nodeId, enclosingBlocks, enclosingBlockNames, etc. + statement.execute("CREATE TABLE " + CASE_RESULTS_TABLE + "(job varchar(255), build int, suite varchar(255), className varchar(255), testName varchar(255), errorDetails varchar(255), skipped varchar(255))"); // TODO indices } } From 2f0e93f6b80b532075f3b3f9dde5d1b41eeecf12 Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Thu, 30 Aug 2018 16:35:41 -0400 Subject: [PATCH 18/33] Javadoc --- .../java/hudson/tasks/junit/storage/TestResultStorage.java | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/main/java/hudson/tasks/junit/storage/TestResultStorage.java b/src/main/java/hudson/tasks/junit/storage/TestResultStorage.java index a9036041b..ea8a040f3 100644 --- a/src/main/java/hudson/tasks/junit/storage/TestResultStorage.java +++ b/src/main/java/hudson/tasks/junit/storage/TestResultStorage.java @@ -26,13 +26,10 @@ import hudson.ExtensionList; import hudson.ExtensionPoint; -import hudson.FilePath; -import hudson.Launcher; import hudson.model.Run; import hudson.model.TaskListener; import hudson.tasks.junit.JUnitParser; import hudson.tasks.junit.TestResult; -import hudson.tasks.test.PipelineTestDetails; import java.io.IOException; import javax.annotation.CheckForNull; import org.jenkinsci.remoting.SerializableOnlyOverRemoting; @@ -43,7 +40,7 @@ public interface TestResultStorage extends ExtensionPoint { /** - * Runs during {@link JUnitParser#parseResult(String, Run, PipelineTestDetails, FilePath, Launcher, TaskListener, AtomicReference)}. + * Runs during {@link JUnitParser#summarizeResult}. */ RemotePublisher createRemotePublisher(Run build) throws IOException; From d01433974a99c4567141b6b792e75fc182958437 Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Tue, 25 Sep 2018 11:51:47 -0400 Subject: [PATCH 19/33] Preparing more tests. --- .../java/hudson/tasks/junit/CaseResult.java | 27 +++++++++++++++++++ .../junit/storage/TestResultStorageTest.java | 2 ++ 2 files changed, 29 insertions(+) diff --git a/src/main/java/hudson/tasks/junit/CaseResult.java b/src/main/java/hudson/tasks/junit/CaseResult.java index 10c479066..d11ff49e6 100644 --- a/src/main/java/hudson/tasks/junit/CaseResult.java +++ b/src/main/java/hudson/tasks/junit/CaseResult.java @@ -643,6 +643,33 @@ public void freeze(SuiteResult parent) { } } + @Override + public String toString() { + StringBuilder b = new StringBuilder("CaseResult{className=").append(className).append(", testName=").append(testName); + if (errorDetails != null) { + b.append(", errorDetails=").append(errorDetails); + } + if (errorStackTrace != null) { + b.append(", errorStackTrace=").append(errorStackTrace); + } + if (skipped) { + b.append(", skipped=true"); + } + if (skippedMessage != null) { + b.append(", skippedMessage=").append(skippedMessage); + } + if (duration != 0.0f) { + b.append(", duration=").append(duration); + } + if (stdout != null) { + b.append(", stdout=").append(stdout); + } + if (stderr != null) { + b.append(", stderr=").append(stderr); + } + return b.append('}').toString(); + } + public int compareTo(CaseResult that) { if (this == that) { return 0; diff --git a/src/test/java/hudson/tasks/junit/storage/TestResultStorageTest.java b/src/test/java/hudson/tasks/junit/storage/TestResultStorageTest.java index d0c6fc9c9..e58a71101 100644 --- a/src/test/java/hudson/tasks/junit/storage/TestResultStorageTest.java +++ b/src/test/java/hudson/tasks/junit/storage/TestResultStorageTest.java @@ -145,6 +145,7 @@ public class TestResultStorageTest { assertEquals(4, a.getResult().getTotalCount()); assertEquals(1, a.getResult().getPassCount()); /* TODO implement: + assertEquals("[CaseResult{className=Klazz, testName=test1, errorDetails=failure}, CaseResult{className=another.Klazz, testName=test1, errorDetails=another failure}]", a.getFailedTests().toString()); List failedTests = a.getFailedTests(); assertEquals(2, failedTests.size()); assertEquals("Klazz", failedTests.get(0).getClassName()); @@ -154,6 +155,7 @@ public class TestResultStorageTest { assertEquals("test1", failedTests.get(1).getName()); assertEquals("another failure", failedTests.get(1).getErrorDetails()); */ + // TODO passedTests, skippedTests // TODO more detailed Java queries incl. PackageResult / ClassResult // TODO test healthScaleFactor, descriptions } From 656e083a13e74da2218cb2fbe638851dff24be58 Mon Sep 17 00:00:00 2001 From: Tim Jacomb Date: Tue, 11 Aug 2020 08:24:51 +0100 Subject: [PATCH 20/33] Finished implementing existing test --- .../java/hudson/tasks/junit/CaseResult.java | 65 +++++++++++-------- .../java/hudson/tasks/junit/SuiteResult.java | 2 +- .../java/hudson/tasks/junit/TestResult.java | 3 + .../hudson/tasks/junit/TestResultAction.java | 3 +- .../tasks/junit/storage/TestResultImpl.java | 2 + .../junit/storage/TestResultStorageTest.java | 54 +++++++++++++-- 6 files changed, 94 insertions(+), 35 deletions(-) diff --git a/src/main/java/hudson/tasks/junit/CaseResult.java b/src/main/java/hudson/tasks/junit/CaseResult.java index c76cca2f3..71da691ca 100644 --- a/src/main/java/hudson/tasks/junit/CaseResult.java +++ b/src/main/java/hudson/tasks/junit/CaseResult.java @@ -101,6 +101,45 @@ private static float parseTime(Element testCase) { return new TimeToFloat(time).parse(); } + + /** + * Used to create a fake failure, when Hudson fails to load data from XML files. + * Public since 1.526. + * + * @param parent Parent result. + * @param testName Test name. + * @param errorStackTrace Error stack trace. + */ + public CaseResult(SuiteResult parent, String testName, String errorStackTrace) { + this(parent, testName, errorStackTrace, ""); + } + + public CaseResult(SuiteResult parent, String testName, String errorStackTrace, String errorDetails) { + this.className = parent == null ? "unnamed" : parent.getName(); + this.testName = testName; + this.errorStackTrace = errorStackTrace; + this.errorDetails = errorDetails; + this.parent = parent; + this.stdout = null; + this.stderr = null; + this.duration = 0.0f; + this.skipped = false; + this.skippedMessage = null; + } + + public CaseResult(SuiteResult parent, String className, String testName, String errorDetails, String stdOut) { + this.className = className; + this.testName = testName; + this.errorStackTrace = null; + this.errorDetails = errorDetails; + this.parent = parent; + this.stdout = stdOut; + this.stderr = null; + this.duration = 0.0f; + this.skipped = false; + this.skippedMessage = null; + } + CaseResult(SuiteResult parent, Element testCase, String testClassName, boolean keepLongStdio) { // schema for JUnit report XML format is not available in Ant, // so I don't know for sure what means what. @@ -197,32 +236,6 @@ private static int halfMaxSize(Collection results) { return HALF_MAX_SIZE; } - - /** - * Used to create a fake failure, when Hudson fails to load data from XML files. - * Public since 1.526. - * - * @param parent Parent result. - * @param testName Test name. - * @param errorStackTrace Error stack trace. - */ - public CaseResult(SuiteResult parent, String testName, String errorStackTrace) { - this(parent, testName, errorStackTrace, ""); - } - - public CaseResult(SuiteResult parent, String testName, String errorStackTrace, String errorDetails) { - this.className = parent == null ? "unnamed" : parent.getName(); - this.testName = testName; - this.errorStackTrace = errorStackTrace; - this.errorDetails = errorDetails; - this.parent = parent; - this.stdout = null; - this.stderr = null; - this.duration = 0.0f; - this.skipped = false; - this.skippedMessage = null; - } - public ClassResult getParent() { return classResult; } diff --git a/src/main/java/hudson/tasks/junit/SuiteResult.java b/src/main/java/hudson/tasks/junit/SuiteResult.java index e0e0525fc..b18059129 100644 --- a/src/main/java/hudson/tasks/junit/SuiteResult.java +++ b/src/main/java/hudson/tasks/junit/SuiteResult.java @@ -112,7 +112,7 @@ public final class SuiteResult implements Serializable { /** * @since 1.22 */ - SuiteResult(String name, String stdout, String stderr, @CheckForNull PipelineTestDetails pipelineTestDetails) { + public SuiteResult(String name, String stdout, String stderr, @CheckForNull PipelineTestDetails pipelineTestDetails) { this.name = name; this.stderr = stderr; this.stdout = stdout; diff --git a/src/main/java/hudson/tasks/junit/TestResult.java b/src/main/java/hudson/tasks/junit/TestResult.java index 18cb108ad..8e37f6c34 100644 --- a/src/main/java/hudson/tasks/junit/TestResult.java +++ b/src/main/java/hudson/tasks/junit/TestResult.java @@ -504,6 +504,9 @@ public boolean isEmpty() { @Override public List getFailedTests() { + if (impl != null) { + return impl.getFailedTests(); + } return failedTests; } diff --git a/src/main/java/hudson/tasks/junit/TestResultAction.java b/src/main/java/hudson/tasks/junit/TestResultAction.java index 8f18cbeaf..607a61b2e 100644 --- a/src/main/java/hudson/tasks/junit/TestResultAction.java +++ b/src/main/java/hudson/tasks/junit/TestResultAction.java @@ -208,7 +208,8 @@ public void setHealthScaleFactor(double healthScaleFactor) { @Override public List getFailedTests() { - return getResult().getFailedTests(); + TestResult result = getResult(); + return result.getFailedTests(); } @Override diff --git a/src/main/java/hudson/tasks/junit/storage/TestResultImpl.java b/src/main/java/hudson/tasks/junit/storage/TestResultImpl.java index fb18ec96a..1f0fcf4ab 100644 --- a/src/main/java/hudson/tasks/junit/storage/TestResultImpl.java +++ b/src/main/java/hudson/tasks/junit/storage/TestResultImpl.java @@ -24,6 +24,7 @@ package hudson.tasks.junit.storage; +import hudson.tasks.junit.CaseResult; import hudson.tasks.junit.TestResult; import java.util.List; import javax.annotation.Nonnull; @@ -36,5 +37,6 @@ public interface TestResultImpl { int getSkipCount(); int getPassCount(); int getTotalCount(); + List getFailedTests(); @Nonnull TestResult getResultByNodes(@Nonnull List nodeIds); } diff --git a/src/test/java/hudson/tasks/junit/storage/TestResultStorageTest.java b/src/test/java/hudson/tasks/junit/storage/TestResultStorageTest.java index e58a71101..0379cf3f3 100644 --- a/src/test/java/hudson/tasks/junit/storage/TestResultStorageTest.java +++ b/src/test/java/hudson/tasks/junit/storage/TestResultStorageTest.java @@ -47,6 +47,8 @@ import java.sql.SQLException; import java.sql.Statement; import java.sql.Types; +import java.util.ArrayList; +import java.util.Collections; import java.util.List; import java.util.Set; import java.util.TreeSet; @@ -59,6 +61,9 @@ import org.jenkinsci.plugins.workflow.job.WorkflowJob; import org.jenkinsci.plugins.workflow.job.WorkflowRun; import org.jenkinsci.remoting.SerializableOnlyOverRemoting; + +import static java.util.Collections.emptyList; +import static java.util.Collections.singletonList; import static org.junit.Assert.*; import org.junit.Before; import org.junit.ClassRule; @@ -144,7 +149,6 @@ public class TestResultStorageTest { assertEquals(1, a.getResult().getSkipCount()); assertEquals(4, a.getResult().getTotalCount()); assertEquals(1, a.getResult().getPassCount()); - /* TODO implement: assertEquals("[CaseResult{className=Klazz, testName=test1, errorDetails=failure}, CaseResult{className=another.Klazz, testName=test1, errorDetails=another failure}]", a.getFailedTests().toString()); List failedTests = a.getFailedTests(); assertEquals(2, failedTests.size()); @@ -154,7 +158,7 @@ public class TestResultStorageTest { assertEquals("another.Klazz", failedTests.get(1).getClassName()); assertEquals("test1", failedTests.get(1).getName()); assertEquals("another failure", failedTests.get(1).getErrorDetails()); - */ + // TODO passedTests, skippedTests // TODO more detailed Java queries incl. PackageResult / ClassResult // TODO test healthScaleFactor, descriptions @@ -203,23 +207,59 @@ private int getCaseCount(String and) { statement.setInt(2, build); try (ResultSet result = statement.executeQuery()) { result.next(); - return result.getInt(1); + int anInt = result.getInt(1); + return anInt; } } }, 0); } + + private List getCaseResult(String column) { + return query(connection -> { + try (PreparedStatement statement = connection.prepareStatement("SELECT suite, testname, classname, errordetails FROM " + Impl.CASE_RESULTS_TABLE + " WHERE job = ? AND build = ? AND " + column + " IS NOT NULL")) { + statement.setString(1, job); + statement.setInt(2, build); + try (ResultSet result = statement.executeQuery()) { + + List results = new ArrayList<>(); + while (result.next()) { + String testName = result.getString("testname"); + String errorDetails = result.getString("errordetails"); + String suite = result.getString("suite"); + String className = result.getString("classname"); + + SuiteResult suiteResult = new SuiteResult(suite, null, null, null); + results.add(new CaseResult(suiteResult, className, testName, errorDetails, null)); + } + return results; + } + } + }, emptyList()); + } + @Override public int getFailCount() { - return getCaseCount(" AND errorDetails IS NOT NULL"); + int caseCount = getCaseCount(" AND errorDetails IS NOT NULL"); + return caseCount; } @Override public int getSkipCount() { - return getCaseCount(" AND skipped IS NOT NULL"); + int caseCount = getCaseCount(" AND skipped IS NOT NULL"); + return caseCount; } @Override public int getPassCount() { - return getCaseCount(" AND errorDetails IS NULL AND skipped IS NULL"); + int caseCount = getCaseCount(" AND errorDetails IS NULL AND skipped IS NULL"); + return caseCount; } @Override public int getTotalCount() { - return getCaseCount(""); + int caseCount = getCaseCount(""); + return caseCount; } + + @Override + public List getFailedTests() { + List errordetails = getCaseResult("errordetails"); + return errordetails; + } + @Override public TestResult getResultByNodes(List nodeIds) { return new TestResult(this); // TODO } From 805709efb11a33dd22d7d8ba58ece03c098cf46d Mon Sep 17 00:00:00 2001 From: Tim Jacomb Date: Tue, 11 Aug 2020 09:03:41 +0100 Subject: [PATCH 21/33] Skipped and passed --- .../java/hudson/tasks/junit/CaseResult.java | 9 +++-- .../java/hudson/tasks/junit/TestResult.java | 8 ++++ .../tasks/junit/storage/TestResultImpl.java | 2 + .../junit/storage/TestResultStorageTest.java | 40 +++++++++++++++---- 4 files changed, 48 insertions(+), 11 deletions(-) diff --git a/src/main/java/hudson/tasks/junit/CaseResult.java b/src/main/java/hudson/tasks/junit/CaseResult.java index 71da691ca..8fb396859 100644 --- a/src/main/java/hudson/tasks/junit/CaseResult.java +++ b/src/main/java/hudson/tasks/junit/CaseResult.java @@ -127,17 +127,18 @@ public CaseResult(SuiteResult parent, String testName, String errorStackTrace, S this.skippedMessage = null; } - public CaseResult(SuiteResult parent, String className, String testName, String errorDetails, String stdOut) { + public CaseResult(SuiteResult parent, String className, String testName, String errorDetails, String skippedMessage) { this.className = className; this.testName = testName; this.errorStackTrace = null; this.errorDetails = errorDetails; this.parent = parent; - this.stdout = stdOut; + this.stdout = null; this.stderr = null; this.duration = 0.0f; - this.skipped = false; - this.skippedMessage = null; + + this.skipped = skippedMessage != null; + this.skippedMessage = skippedMessage; } CaseResult(SuiteResult parent, Element testCase, String testClassName, boolean keepLongStdio) { diff --git a/src/main/java/hudson/tasks/junit/TestResult.java b/src/main/java/hudson/tasks/junit/TestResult.java index 8e37f6c34..6a9eb0ce2 100644 --- a/src/main/java/hudson/tasks/junit/TestResult.java +++ b/src/main/java/hudson/tasks/junit/TestResult.java @@ -517,6 +517,10 @@ public List getFailedTests() { */ @Override public synchronized List getPassedTests() { + if (impl != null) { + return impl.getPassedTests(); + } + if(passedTests == null){ passedTests = new ArrayList(); for(SuiteResult s : suites) { @@ -538,6 +542,10 @@ public synchronized List getPassedTests() { */ @Override public synchronized List getSkippedTests() { + if (impl != null) { + return impl.getSkippedTests(); + } + if(skippedTests == null){ skippedTests = new ArrayList(); for(SuiteResult s : suites) { diff --git a/src/main/java/hudson/tasks/junit/storage/TestResultImpl.java b/src/main/java/hudson/tasks/junit/storage/TestResultImpl.java index 1f0fcf4ab..7879b9637 100644 --- a/src/main/java/hudson/tasks/junit/storage/TestResultImpl.java +++ b/src/main/java/hudson/tasks/junit/storage/TestResultImpl.java @@ -38,5 +38,7 @@ public interface TestResultImpl { int getPassCount(); int getTotalCount(); List getFailedTests(); + List getSkippedTests(); + List getPassedTests(); @Nonnull TestResult getResultByNodes(@Nonnull List nodeIds); } diff --git a/src/test/java/hudson/tasks/junit/storage/TestResultStorageTest.java b/src/test/java/hudson/tasks/junit/storage/TestResultStorageTest.java index 0379cf3f3..a6f1fbca4 100644 --- a/src/test/java/hudson/tasks/junit/storage/TestResultStorageTest.java +++ b/src/test/java/hudson/tasks/junit/storage/TestResultStorageTest.java @@ -63,7 +63,6 @@ import org.jenkinsci.remoting.SerializableOnlyOverRemoting; import static java.util.Collections.emptyList; -import static java.util.Collections.singletonList; import static org.junit.Assert.*; import org.junit.Before; import org.junit.ClassRule; @@ -158,8 +157,18 @@ public class TestResultStorageTest { assertEquals("another.Klazz", failedTests.get(1).getClassName()); assertEquals("test1", failedTests.get(1).getName()); assertEquals("another failure", failedTests.get(1).getErrorDetails()); + + List skippedTests = a.getSkippedTests(); + assertEquals(1, skippedTests.size()); + assertEquals("other.Klazz", skippedTests.get(0).getClassName()); + assertEquals("test3", skippedTests.get(0).getName()); + assertEquals("Not actually run.", skippedTests.get(0).getSkippedMessage()); + + List passedTests = a.getPassedTests(); + assertEquals(1, passedTests.size()); + assertEquals("Klazz", passedTests.get(0).getClassName()); + assertEquals("test2", passedTests.get(0).getName()); - // TODO passedTests, skippedTests // TODO more detailed Java queries incl. PackageResult / ClassResult // TODO test healthScaleFactor, descriptions } @@ -213,23 +222,24 @@ private int getCaseCount(String and) { } }, 0); } - - private List getCaseResult(String column) { + + private List retrieveCaseResult(String whereCondition) { return query(connection -> { - try (PreparedStatement statement = connection.prepareStatement("SELECT suite, testname, classname, errordetails FROM " + Impl.CASE_RESULTS_TABLE + " WHERE job = ? AND build = ? AND " + column + " IS NOT NULL")) { + try (PreparedStatement statement = connection.prepareStatement("SELECT suite, testname, classname, errordetails, skipped FROM " + Impl.CASE_RESULTS_TABLE + " WHERE job = ? AND build = ? AND " + whereCondition)) { statement.setString(1, job); statement.setInt(2, build); try (ResultSet result = statement.executeQuery()) { - + List results = new ArrayList<>(); while (result.next()) { String testName = result.getString("testname"); String errorDetails = result.getString("errordetails"); String suite = result.getString("suite"); String className = result.getString("classname"); + String skipped = result.getString("skipped"); SuiteResult suiteResult = new SuiteResult(suite, null, null, null); - results.add(new CaseResult(suiteResult, className, testName, errorDetails, null)); + results.add(new CaseResult(suiteResult, className, testName, errorDetails, skipped)); } return results; } @@ -237,6 +247,10 @@ private List getCaseResult(String column) { }, emptyList()); } + private List getCaseResult(String column) { + return retrieveCaseResult(column + " IS NOT NULL"); + } + @Override public int getFailCount() { int caseCount = getCaseCount(" AND errorDetails IS NOT NULL"); return caseCount; @@ -260,6 +274,18 @@ public List getFailedTests() { return errordetails; } + @Override + public List getSkippedTests() { + List errordetails = getCaseResult("skipped"); + return errordetails; + } + + @Override + public List getPassedTests() { + List errordetails = retrieveCaseResult("errordetails IS NULL AND skipped IS NULL"); + return errordetails; + } + @Override public TestResult getResultByNodes(List nodeIds) { return new TestResult(this); // TODO } From a87088d636336842be8de37a6c674c629d77ac06 Mon Sep 17 00:00:00 2001 From: Tim Jacomb Date: Tue, 11 Aug 2020 15:10:18 +0100 Subject: [PATCH 22/33] Add package result methods --- .../hudson/tasks/junit/PackageResult.java | 14 ++- .../java/hudson/tasks/junit/TestResult.java | 11 ++- .../tasks/junit/storage/TestResultImpl.java | 6 ++ .../junit/storage/TestResultStorageTest.java | 95 ++++++++++++++++--- 4 files changed, 110 insertions(+), 16 deletions(-) diff --git a/src/main/java/hudson/tasks/junit/PackageResult.java b/src/main/java/hudson/tasks/junit/PackageResult.java index 007d64013..70f612c32 100644 --- a/src/main/java/hudson/tasks/junit/PackageResult.java +++ b/src/main/java/hudson/tasks/junit/PackageResult.java @@ -49,7 +49,7 @@ public final class PackageResult extends MetaTabulatedResult implements Comparab private final hudson.tasks.junit.TestResult parent; private float duration; - PackageResult(hudson.tasks.junit.TestResult parent, String packageName) { + public PackageResult(hudson.tasks.junit.TestResult parent, String packageName) { this.packageName = packageName; this.parent = parent; } @@ -175,6 +175,10 @@ public boolean hasChildren() { * sort order */ public List getFailedTests() { + if (parent.getPluggableStorage() != null) { + return parent.getPluggableStorage().getFailedTestsByPackage(packageName); + } + List r = new ArrayList(); for (ClassResult clr : classes.values()) { for (CaseResult cr : clr.getChildren()) { @@ -204,6 +208,10 @@ public List getFailedTestsSortedByAge() { */ @Override public List getPassedTests() { + if (parent.getPluggableStorage() != null) { + return parent.getPluggableStorage().getPassedTestsByPackage(packageName); + } + List r = new ArrayList(); for (ClassResult clr : classes.values()) { for (CaseResult cr : clr.getChildren()) { @@ -223,6 +231,10 @@ public List getPassedTests() { */ @Override public List getSkippedTests() { + if (parent.getPluggableStorage() != null) { + return parent.getPluggableStorage().getSkippedTestsByPackage(packageName); + } + List r = new ArrayList(); for (ClassResult clr : classes.values()) { for (CaseResult cr : clr.getChildren()) { diff --git a/src/main/java/hudson/tasks/junit/TestResult.java b/src/main/java/hudson/tasks/junit/TestResult.java index 6a9eb0ce2..92d47b991 100644 --- a/src/main/java/hudson/tasks/junit/TestResult.java +++ b/src/main/java/hudson/tasks/junit/TestResult.java @@ -156,6 +156,11 @@ public TestResult(TestResultImpl impl) { keepLongStdio = false; // irrelevant } + @CheckForNull + public TestResultImpl getPluggableStorage() { + return impl; + } + public TestObject getParent() { return parent; } @@ -679,6 +684,10 @@ public Object getDynamic(String token, StaplerRequest req, StaplerResponse rsp) } public PackageResult byPackage(String packageName) { + if (impl != null) { + return impl.getPackageResult(packageName); + } + return byPackages.get(packageName); } @@ -754,7 +763,7 @@ public void tally() { cr.setParentAction(this.parentAction); cr.setParentSuiteResult(s); cr.tally(); - String pkg = cr.getPackageName(), spkg = safe(pkg); + String pkg = cr.getPackageName(), spkg = safe(pkg); PackageResult pr = byPackage(spkg); if(pr==null) byPackages.put(spkg,pr=new PackageResult(this,pkg)); diff --git a/src/main/java/hudson/tasks/junit/storage/TestResultImpl.java b/src/main/java/hudson/tasks/junit/storage/TestResultImpl.java index 7879b9637..6159fa815 100644 --- a/src/main/java/hudson/tasks/junit/storage/TestResultImpl.java +++ b/src/main/java/hudson/tasks/junit/storage/TestResultImpl.java @@ -25,6 +25,7 @@ package hudson.tasks.junit.storage; import hudson.tasks.junit.CaseResult; +import hudson.tasks.junit.PackageResult; import hudson.tasks.junit.TestResult; import java.util.List; import javax.annotation.Nonnull; @@ -38,7 +39,12 @@ public interface TestResultImpl { int getPassCount(); int getTotalCount(); List getFailedTests(); + List getFailedTestsByPackage(String packageName); List getSkippedTests(); + List getSkippedTestsByPackage(String packageName); List getPassedTests(); + List getPassedTestsByPackage(String packageName); + PackageResult getPackageResult(String packageName); @Nonnull TestResult getResultByNodes(@Nonnull List nodeIds); + } diff --git a/src/test/java/hudson/tasks/junit/storage/TestResultStorageTest.java b/src/test/java/hudson/tasks/junit/storage/TestResultStorageTest.java index a6f1fbca4..3544f5142 100644 --- a/src/test/java/hudson/tasks/junit/storage/TestResultStorageTest.java +++ b/src/test/java/hudson/tasks/junit/storage/TestResultStorageTest.java @@ -33,6 +33,7 @@ import hudson.model.TaskListener; import hudson.slaves.DumbSlave; import hudson.tasks.junit.CaseResult; +import hudson.tasks.junit.PackageResult; import hudson.tasks.junit.SuiteResult; import hudson.tasks.junit.TestResult; import hudson.tasks.junit.TestResultAction; @@ -50,6 +51,7 @@ import java.util.ArrayList; import java.util.Collections; import java.util.List; +import java.util.Objects; import java.util.Set; import java.util.TreeSet; import javax.xml.parsers.DocumentBuilderFactory; @@ -63,6 +65,7 @@ import org.jenkinsci.remoting.SerializableOnlyOverRemoting; import static java.util.Collections.emptyList; +import static java.util.Objects.requireNonNull; import static org.junit.Assert.*; import org.junit.Before; import org.junit.ClassRule; @@ -111,9 +114,9 @@ public class TestResultStorageTest { " echo(/next summary: fail=$s.failCount skip=$s.skipCount pass=$s.passCount total=$s.totalCount/)\n" + "}", true)); WorkflowRun b = p.scheduleBuild2(0).get(); - try (Connection connection = GlobalDatabaseConfiguration.get().getDatabase().getDataSource().getConnection(); - PreparedStatement statement = connection.prepareStatement("SELECT * FROM " + Impl.CASE_RESULTS_TABLE); - ResultSet result = statement.executeQuery()) { + try (Connection connection = requireNonNull(GlobalDatabaseConfiguration.get().getDatabase()).getDataSource().getConnection(); + PreparedStatement statement = connection.prepareStatement("SELECT * FROM " + Impl.CASE_RESULTS_TABLE); + ResultSet result = statement.executeQuery()) { printResultSet(result); } // TODO verify table structure @@ -169,6 +172,24 @@ public class TestResultStorageTest { assertEquals("Klazz", passedTests.get(0).getClassName()); assertEquals("test2", passedTests.get(0).getName()); + PackageResult another = a.getResult().byPackage("another"); + List packageFailedTests = another.getFailedTests(); + assertEquals(1, packageFailedTests.size()); + assertEquals("another.Klazz", packageFailedTests.get(0).getClassName()); + + PackageResult other = a.getResult().byPackage("other"); + List packageSkippedTests = other.getSkippedTests(); + assertEquals(1, packageSkippedTests.size()); + assertEquals("other.Klazz", packageSkippedTests.get(0).getClassName()); + assertEquals("Not actually run.", packageSkippedTests.get(0).getSkippedMessage()); + + PackageResult root = a.getResult().byPackage("(root)"); + List rootPassedTests = root.getPassedTests(); + assertEquals(1, rootPassedTests.size()); + assertEquals("Klazz", rootPassedTests.get(0).getClassName()); + +// r.pause(); + // TODO test result summary i.e. failure content // TODO more detailed Java queries incl. PackageResult / ClassResult // TODO test healthScaleFactor, descriptions } @@ -205,8 +226,7 @@ private T query(Querier querier, T dflt) { Connection connection = connectionSupplier.connection(); return querier.run(connection); } catch (SQLException x) { - x.printStackTrace(); - return dflt; + throw new RuntimeException(x); } } private int getCaseCount(String and) { @@ -246,7 +266,43 @@ private List retrieveCaseResult(String whereCondition) { } }, emptyList()); } - + + @Override + public PackageResult getPackageResult(String packageName) { + return new PackageResult(new TestResult(this), packageName); + } + + @Override + public List getFailedTestsByPackage(String packageName) { + return getByPackage(packageName, "AND errorDetails IS NOT NULL"); + } + + private List getByPackage(String packageName, String filter) { + return query(connection -> { + try (PreparedStatement statement = connection.prepareStatement("SELECT suite, testname, classname, errordetails, skipped FROM " + Impl.CASE_RESULTS_TABLE + " WHERE job = ? AND build = ? AND package = ? " + filter)) { + statement.setString(1, job); + statement.setInt(2, build); + statement.setString(3, packageName); + try (ResultSet result = statement.executeQuery()) { + + List results = new ArrayList<>(); + while (result.next()) { + String testName = result.getString("testname"); + String errorDetails = result.getString("errordetails"); + String suite = result.getString("suite"); + String className = result.getString("classname"); + String skipped = result.getString("skipped"); + + SuiteResult suiteResult = new SuiteResult(suite, null, null, null); + results.add(new CaseResult(suiteResult, className, testName, errorDetails, skipped)); + } + return results; + } + } + }, emptyList()); + } + + private List getCaseResult(String column) { return retrieveCaseResult(column + " IS NOT NULL"); } @@ -280,12 +336,22 @@ public List getSkippedTests() { return errordetails; } + @Override + public List getSkippedTestsByPackage(String packageName) { + return getByPackage(packageName, "AND skipped IS NOT NULL"); + } + @Override public List getPassedTests() { List errordetails = retrieveCaseResult("errordetails IS NULL AND skipped IS NULL"); return errordetails; } + @Override + public List getPassedTestsByPackage(String packageName) { + return getByPackage(packageName, "AND errordetails IS NULL AND skipped IS NULL"); + } + @Override public TestResult getResultByNodes(List nodeIds) { return new TestResult(this); // TODO } @@ -308,25 +374,26 @@ private static class RemotePublisherImpl implements RemotePublisher { @Override public void publish(TestResult result, TaskListener listener) throws IOException { try { Connection connection = connectionSupplier.connection(); - try (PreparedStatement statement = connection.prepareStatement("INSERT INTO " + CASE_RESULTS_TABLE + " (job, build, suite, className, testName, errorDetails, skipped) VALUES (?, ?, ?, ?, ?, ?, ?)")) { + try (PreparedStatement statement = connection.prepareStatement("INSERT INTO " + CASE_RESULTS_TABLE + " (job, build, suite, package, className, testName, errorDetails, skipped) VALUES (?, ?, ?, ?, ?, ?, ?, ?)")) { int count = 0; for (SuiteResult suiteResult : result.getSuites()) { for (CaseResult caseResult : suiteResult.getCases()) { statement.setString(1, job); statement.setInt(2, build); statement.setString(3, suiteResult.getName()); - statement.setString(4, caseResult.getClassName()); - statement.setString(5, caseResult.getName()); + statement.setString(4, caseResult.getPackageName()); + statement.setString(5, caseResult.getClassName()); + statement.setString(6, caseResult.getName()); String errorDetails = caseResult.getErrorDetails(); if (errorDetails != null) { - statement.setString(6, errorDetails); + statement.setString(7, errorDetails); } else { - statement.setNull(6, Types.VARCHAR); + statement.setNull(7, Types.VARCHAR); } if (caseResult.isSkipped()) { - statement.setString(7, Util.fixNull(caseResult.getSkippedMessage())); + statement.setString(8, Util.fixNull(caseResult.getSkippedMessage())); } else { - statement.setNull(7, Types.VARCHAR); + statement.setNull(8, Types.VARCHAR); } statement.executeUpdate(); count++; @@ -379,7 +446,7 @@ static class LocalConnectionSupplier extends ConnectionSupplier { if (!exists) { try (Statement statement = connection.createStatement()) { // TODO this and joined tables: errorStackTrace, stdout, stderr, duration, nodeId, enclosingBlocks, enclosingBlockNames, etc. - statement.execute("CREATE TABLE " + CASE_RESULTS_TABLE + "(job varchar(255), build int, suite varchar(255), className varchar(255), testName varchar(255), errorDetails varchar(255), skipped varchar(255))"); + statement.execute("CREATE TABLE " + CASE_RESULTS_TABLE + "(job varchar(255), build int, suite varchar(255), package varchar(255), className varchar(255), testName varchar(255), errorDetails varchar(255), skipped varchar(255))"); // TODO indices } } From 0e8a000bd75d74dcbcb22bc3858a4dabf443141c Mon Sep 17 00:00:00 2001 From: Tim Jacomb Date: Wed, 12 Aug 2020 07:18:02 +0100 Subject: [PATCH 23/33] getFailedSince run --- .../java/hudson/tasks/junit/CaseResult.java | 12 ++- .../tasks/junit/storage/TestResultImpl.java | 3 + .../junit/storage/TestResultStorageTest.java | 74 ++++++++++++++++++- 3 files changed, 87 insertions(+), 2 deletions(-) diff --git a/src/main/java/hudson/tasks/junit/CaseResult.java b/src/main/java/hudson/tasks/junit/CaseResult.java index 8fb396859..523010de9 100644 --- a/src/main/java/hudson/tasks/junit/CaseResult.java +++ b/src/main/java/hudson/tasks/junit/CaseResult.java @@ -24,6 +24,8 @@ package hudson.tasks.junit; import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; +import hudson.tasks.junit.storage.TestResultImpl; +import hudson.tasks.junit.storage.TestResultStorage; import hudson.util.TextFile; import org.apache.commons.collections.iterators.ReverseListIterator; import org.apache.commons.io.FileUtils; @@ -34,6 +36,7 @@ import hudson.tasks.test.TestResult; import org.dom4j.Element; +import org.kohsuke.stapler.Stapler; import org.kohsuke.stapler.export.Exported; import javax.annotation.CheckForNull; @@ -127,7 +130,7 @@ public CaseResult(SuiteResult parent, String testName, String errorStackTrace, S this.skippedMessage = null; } - public CaseResult(SuiteResult parent, String className, String testName, String errorDetails, String skippedMessage) { + public CaseResult(SuiteResult parent, String className, String testName, String errorDetails, String skippedMessage) { this.className = className; this.testName = testName; this.errorStackTrace = null; @@ -441,6 +444,13 @@ else if (getRun() != null) { } public Run getFailedSinceRun() { + TestResultStorage storage = TestResultStorage.find(); + if (storage != null) { + Run run = Stapler.getCurrentRequest().findAncestorObject(Run.class); + TestResultImpl pluggableStorage = storage.load(run.getParent().getFullName(), run.getNumber()); + return pluggableStorage.getFailedSinceRun(this); + } + return getRun().getParent().getBuildByNumber(getFailedSince()); } diff --git a/src/main/java/hudson/tasks/junit/storage/TestResultImpl.java b/src/main/java/hudson/tasks/junit/storage/TestResultImpl.java index 6159fa815..65bfcb27a 100644 --- a/src/main/java/hudson/tasks/junit/storage/TestResultImpl.java +++ b/src/main/java/hudson/tasks/junit/storage/TestResultImpl.java @@ -24,6 +24,7 @@ package hudson.tasks.junit.storage; +import hudson.model.Run; import hudson.tasks.junit.CaseResult; import hudson.tasks.junit.PackageResult; import hudson.tasks.junit.TestResult; @@ -45,6 +46,8 @@ public interface TestResultImpl { List getPassedTests(); List getPassedTestsByPackage(String packageName); PackageResult getPackageResult(String packageName); + + Run getFailedSinceRun(CaseResult caseResult); @Nonnull TestResult getResultByNodes(@Nonnull List nodeIds); } diff --git a/src/test/java/hudson/tasks/junit/storage/TestResultStorageTest.java b/src/test/java/hudson/tasks/junit/storage/TestResultStorageTest.java index 3544f5142..1c378eb79 100644 --- a/src/test/java/hudson/tasks/junit/storage/TestResultStorageTest.java +++ b/src/test/java/hudson/tasks/junit/storage/TestResultStorageTest.java @@ -27,6 +27,7 @@ import com.google.common.collect.ImmutableSet; import com.thoughtworks.xstream.XStream; import hudson.Util; +import hudson.model.Job; import hudson.model.Label; import hudson.model.Result; import hudson.model.Run; @@ -55,6 +56,7 @@ import java.util.Set; import java.util.TreeSet; import javax.xml.parsers.DocumentBuilderFactory; +import jenkins.model.Jenkins; import org.apache.commons.io.FileUtils; import org.jenkinsci.plugins.database.Database; import org.jenkinsci.plugins.database.GlobalDatabaseConfiguration; @@ -74,6 +76,8 @@ import org.jvnet.hudson.test.BuildWatcher; import org.jvnet.hudson.test.JenkinsRule; import org.jvnet.hudson.test.TestExtension; +import org.kohsuke.stapler.Stapler; +import org.kohsuke.stapler.StaplerRequest; import org.w3c.dom.Document; import org.w3c.dom.Element; import org.w3c.dom.Node; @@ -190,8 +194,11 @@ public class TestResultStorageTest { // r.pause(); // TODO test result summary i.e. failure content - // TODO more detailed Java queries incl. PackageResult / ClassResult + // TODO getFailedSinceRun, TestResult#getChildren, TestObject#getTestResultAction + // TODO more detailed Java queries incl. ClassResult + // TODO CaseResult#getRun: In getOwner(), suiteResult.getParent() is null // TODO test healthScaleFactor, descriptions + // TODO historyAvailable(History.java:72) } } @@ -272,6 +279,71 @@ public PackageResult getPackageResult(String packageName) { return new PackageResult(new TestResult(this), packageName); } + @Override + public Run getFailedSinceRun(CaseResult caseResult) { + return query(connection -> { + int lastPassingBuildNumber; + Job theJob = Objects.requireNonNull(Jenkins.get().getItemByFullName(job, Job.class)); + try (PreparedStatement statement = connection.prepareStatement( + "SELECT build " + + "FROM " + Impl.CASE_RESULTS_TABLE + " " + + "WHERE job = ? " + + "AND build < ? " + + "AND suite = ? " + + "AND package = ? " + + "AND classname = ? " + + "AND testname = ? " + + "AND errordetails IS NULL " + + "ORDER BY BUILD DESC " + + "LIMIT 1" + )) { + statement.setString(1, job); + statement.setInt(2, build); + statement.setString(3, caseResult.getSuiteResult().getName()); + statement.setString(4, caseResult.getPackageName()); + statement.setString(5, caseResult.getClassName()); + statement.setString(6, caseResult.getName()); + try (ResultSet result = statement.executeQuery()) { + boolean hasPassed = result.next(); + if (!hasPassed) { + return theJob.getBuildByNumber(1); + } + + lastPassingBuildNumber = result.getInt("build"); + } + } + try (PreparedStatement statement = connection.prepareStatement( + "SELECT build " + + "FROM " + Impl.CASE_RESULTS_TABLE + " " + + "WHERE job = ? " + + "AND build > ? " + + "AND suite = ? " + + "AND package = ? " + + "AND classname = ? " + + "AND testname = ? " + + "AND errordetails is NOT NULL " + + "ORDER BY BUILD ASC " + + "LIMIT 1" + ) + ) { + statement.setString(1, job); + statement.setInt(2, lastPassingBuildNumber); + statement.setString(3, caseResult.getSuiteResult().getName()); + statement.setString(4, caseResult.getPackageName()); + statement.setString(5, caseResult.getClassName()); + statement.setString(6, caseResult.getName()); + + try (ResultSet result = statement.executeQuery()) { + result.next(); + + int firstFailingBuildAfterPassing = result.getInt("build"); + return theJob.getBuildByNumber(firstFailingBuildAfterPassing); + } + } + }, null); + + } + @Override public List getFailedTestsByPackage(String packageName) { return getByPackage(packageName, "AND errorDetails IS NOT NULL"); From f9e629c75039e9ac5c17707ddb7c91937c8e00fb Mon Sep 17 00:00:00 2001 From: Tim Jacomb Date: Wed, 12 Aug 2020 07:42:29 +0100 Subject: [PATCH 24/33] Add getByPackage --- .../java/hudson/tasks/junit/TestResult.java | 5 +++++ .../tasks/junit/storage/TestResultImpl.java | 1 + .../junit/storage/TestResultStorageTest.java | 22 ++++++++++++++++++- 3 files changed, 27 insertions(+), 1 deletion(-) diff --git a/src/main/java/hudson/tasks/junit/TestResult.java b/src/main/java/hudson/tasks/junit/TestResult.java index 92d47b991..526618ddc 100644 --- a/src/main/java/hudson/tasks/junit/TestResult.java +++ b/src/main/java/hudson/tasks/junit/TestResult.java @@ -647,6 +647,11 @@ public boolean isPassed() { @Override public Collection getChildren() { + if (impl != null) { + List allPackageResults = impl.getAllPackageResults(); + return allPackageResults; + } + return byPackages.values(); } diff --git a/src/main/java/hudson/tasks/junit/storage/TestResultImpl.java b/src/main/java/hudson/tasks/junit/storage/TestResultImpl.java index 65bfcb27a..642609ae6 100644 --- a/src/main/java/hudson/tasks/junit/storage/TestResultImpl.java +++ b/src/main/java/hudson/tasks/junit/storage/TestResultImpl.java @@ -46,6 +46,7 @@ public interface TestResultImpl { List getPassedTests(); List getPassedTestsByPackage(String packageName); PackageResult getPackageResult(String packageName); + List getAllPackageResults(); Run getFailedSinceRun(CaseResult caseResult); @Nonnull TestResult getResultByNodes(@Nonnull List nodeIds); diff --git a/src/test/java/hudson/tasks/junit/storage/TestResultStorageTest.java b/src/test/java/hudson/tasks/junit/storage/TestResultStorageTest.java index 1c378eb79..b60c52730 100644 --- a/src/test/java/hudson/tasks/junit/storage/TestResultStorageTest.java +++ b/src/test/java/hudson/tasks/junit/storage/TestResultStorageTest.java @@ -192,7 +192,7 @@ public class TestResultStorageTest { assertEquals(1, rootPassedTests.size()); assertEquals("Klazz", rootPassedTests.get(0).getClassName()); -// r.pause(); + r.pause(); // TODO test result summary i.e. failure content // TODO getFailedSinceRun, TestResult#getChildren, TestObject#getTestResultAction // TODO more detailed Java queries incl. ClassResult @@ -274,6 +274,26 @@ private List retrieveCaseResult(String whereCondition) { }, emptyList()); } + @Override + public List getAllPackageResults() { + return query(connection -> { + try (PreparedStatement statement = connection.prepareStatement("SELECT DISTINCT package FROM " + Impl.CASE_RESULTS_TABLE + " WHERE job = ? AND build = ?")) { + statement.setString(1, job); + statement.setInt(2, build); + try (ResultSet result = statement.executeQuery()) { + + List results = new ArrayList<>(); + while (result.next()) { + String packageName = result.getString("package"); + + results.add(new PackageResult(new TestResult(this), packageName)); + } + return results; + } + } + }, emptyList()); + } + @Override public PackageResult getPackageResult(String packageName) { return new PackageResult(new TestResult(this), packageName); From 2fd51f8d78249f7f295e0e7e3db92f8031f2d2c8 Mon Sep 17 00:00:00 2001 From: Tim Jacomb Date: Wed, 12 Aug 2020 07:57:31 +0100 Subject: [PATCH 25/33] Add a fallback --- src/main/java/hudson/tasks/test/TestObject.java | 5 +++++ .../hudson/tasks/junit/storage/TestResultStorageTest.java | 1 + 2 files changed, 6 insertions(+) diff --git a/src/main/java/hudson/tasks/test/TestObject.java b/src/main/java/hudson/tasks/test/TestObject.java index ab1a69919..21b394d54 100644 --- a/src/main/java/hudson/tasks/test/TestObject.java +++ b/src/main/java/hudson/tasks/test/TestObject.java @@ -234,6 +234,11 @@ public AbstractTestResultAction getTestResultAction() { if (owner != null) { return owner.getAction(AbstractTestResultAction.class); } else { + Run run = Stapler.getCurrentRequest().findAncestorObject(Run.class); + if (run != null) { + AbstractTestResultAction action = run.getAction(AbstractTestResultAction.class); + return action; + } LOGGER.warning("owner is null when trying to getTestResultAction."); return null; } diff --git a/src/test/java/hudson/tasks/junit/storage/TestResultStorageTest.java b/src/test/java/hudson/tasks/junit/storage/TestResultStorageTest.java index b60c52730..087fab195 100644 --- a/src/test/java/hudson/tasks/junit/storage/TestResultStorageTest.java +++ b/src/test/java/hudson/tasks/junit/storage/TestResultStorageTest.java @@ -199,6 +199,7 @@ public class TestResultStorageTest { // TODO CaseResult#getRun: In getOwner(), suiteResult.getParent() is null // TODO test healthScaleFactor, descriptions // TODO historyAvailable(History.java:72) + // TODO getByPackage UI isn't working in test report } } From 23af1da3a25dbf215e93f036c3600f10630809d3 Mon Sep 17 00:00:00 2001 From: Tim Jacomb Date: Fri, 14 Aug 2020 20:55:22 +0100 Subject: [PATCH 26/33] More hardening --- src/main/java/hudson/tasks/test/TestObject.java | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/main/java/hudson/tasks/test/TestObject.java b/src/main/java/hudson/tasks/test/TestObject.java index 21b394d54..b379b8a64 100644 --- a/src/main/java/hudson/tasks/test/TestObject.java +++ b/src/main/java/hudson/tasks/test/TestObject.java @@ -189,8 +189,11 @@ public String getRelativePathFrom(TestObject it) { // Now the build Run myBuild = cur.getRun(); if (myBuild ==null) { - LOGGER.warning("trying to get relative path, but we can't determine the build that owns this result."); - return ""; // this won't take us to the right place, but it also won't 404. + myBuild = Stapler.getCurrentRequest().findAncestorObject(Run.class); + if (myBuild == null) { + LOGGER.warning("trying to get relative path, but we can't determine the build that owns this result."); + return ""; // this won't take us to the right place, but it also won't 404. + } } buf.insert(0,'/'); buf.insert(0,myBuild.getUrl()); From 019ae4d01dabf49f8a0499fb7c3221ad197feeb4 Mon Sep 17 00:00:00 2001 From: Tim Jacomb Date: Sat, 15 Aug 2020 09:59:49 +0100 Subject: [PATCH 27/33] Test results now work in report --- .../java/hudson/tasks/junit/CaseResult.java | 2 +- .../java/hudson/tasks/junit/ClassResult.java | 15 +++- .../hudson/tasks/junit/PackageResult.java | 17 ++++- .../java/hudson/tasks/junit/SuiteResult.java | 2 +- .../tasks/junit/storage/TestResultImpl.java | 11 +++ .../java/hudson/tasks/test/TestObject.java | 11 ++- .../junit/pipeline/JUnitResultsStepTest.java | 1 + .../junit/storage/TestResultStorageTest.java | 73 +++++++++++++++++-- 8 files changed, 111 insertions(+), 21 deletions(-) diff --git a/src/main/java/hudson/tasks/junit/CaseResult.java b/src/main/java/hudson/tasks/junit/CaseResult.java index 523010de9..1f8c09d29 100644 --- a/src/main/java/hudson/tasks/junit/CaseResult.java +++ b/src/main/java/hudson/tasks/junit/CaseResult.java @@ -744,7 +744,7 @@ public Status getStatus() { } } - /*package*/ void setClass(ClassResult classResult) { + public void setClass(ClassResult classResult) { this.classResult = classResult; } diff --git a/src/main/java/hudson/tasks/junit/ClassResult.java b/src/main/java/hudson/tasks/junit/ClassResult.java index 38d5c8347..aa888563d 100644 --- a/src/main/java/hudson/tasks/junit/ClassResult.java +++ b/src/main/java/hudson/tasks/junit/ClassResult.java @@ -24,9 +24,12 @@ package hudson.tasks.junit; import hudson.model.Run; +import hudson.tasks.junit.storage.TestResultImpl; +import hudson.tasks.junit.storage.TestResultStorage; import hudson.tasks.test.TabulatedResult; import hudson.tasks.test.TestResult; import hudson.tasks.test.TestObject; +import org.kohsuke.stapler.Stapler; import org.kohsuke.stapler.StaplerRequest; import org.kohsuke.stapler.StaplerResponse; import org.kohsuke.stapler.export.Exported; @@ -52,7 +55,7 @@ public final class ClassResult extends TabulatedResult implements Comparable run = Stapler.getCurrentRequest().findAncestorObject(Run.class); + TestResultImpl pluggableStorage = storage.load(run.getParent().getFullName(), run.getNumber()); + c = pluggableStorage.getCaseResult(name); + } else { + c = getCaseResult(name); + } if (c != null) { return c; } else { diff --git a/src/main/java/hudson/tasks/junit/PackageResult.java b/src/main/java/hudson/tasks/junit/PackageResult.java index 70f612c32..5cff1a260 100644 --- a/src/main/java/hudson/tasks/junit/PackageResult.java +++ b/src/main/java/hudson/tasks/junit/PackageResult.java @@ -24,8 +24,11 @@ package hudson.tasks.junit; import hudson.model.Run; +import hudson.tasks.junit.storage.TestResultImpl; +import hudson.tasks.junit.storage.TestResultStorage; import hudson.tasks.test.MetaTabulatedResult; import hudson.tasks.test.TestResult; +import org.kohsuke.stapler.Stapler; import org.kohsuke.stapler.StaplerRequest; import org.kohsuke.stapler.StaplerResponse; import org.kohsuke.stapler.export.Exported; @@ -144,11 +147,19 @@ public int getSkipCount() { @Override public Object getDynamic(String name, StaplerRequest req, StaplerResponse rsp) { - ClassResult result = getClassResult(name); + TestResultStorage storage = TestResultStorage.find(); + ClassResult result; + if (storage != null) { + Run run = Stapler.getCurrentRequest().findAncestorObject(Run.class); + TestResultImpl pluggableStorage = storage.load(run.getParent().getFullName(), run.getNumber()); + result = pluggableStorage.getClassResult(name); + } else { + result = getClassResult(name); + } if (result != null) { - return result; + return result; } else { - return super.getDynamic(name, req, rsp); + return super.getDynamic(name, req, rsp); } } diff --git a/src/main/java/hudson/tasks/junit/SuiteResult.java b/src/main/java/hudson/tasks/junit/SuiteResult.java index b18059129..1e4e3f62b 100644 --- a/src/main/java/hudson/tasks/junit/SuiteResult.java +++ b/src/main/java/hudson/tasks/junit/SuiteResult.java @@ -446,7 +446,7 @@ public Set getClassNames() { * which requires a non-null parent. * @param parent */ - void setParent(hudson.tasks.junit.TestResult parent) { + public void setParent(hudson.tasks.junit.TestResult parent) { this.parent = parent; } diff --git a/src/main/java/hudson/tasks/junit/storage/TestResultImpl.java b/src/main/java/hudson/tasks/junit/storage/TestResultImpl.java index 642609ae6..d28181963 100644 --- a/src/main/java/hudson/tasks/junit/storage/TestResultImpl.java +++ b/src/main/java/hudson/tasks/junit/storage/TestResultImpl.java @@ -26,9 +26,11 @@ import hudson.model.Run; import hudson.tasks.junit.CaseResult; +import hudson.tasks.junit.ClassResult; import hudson.tasks.junit.PackageResult; import hudson.tasks.junit.TestResult; import java.util.List; +import java.util.Map; import javax.annotation.Nonnull; /** @@ -51,4 +53,13 @@ public interface TestResultImpl { Run getFailedSinceRun(CaseResult caseResult); @Nonnull TestResult getResultByNodes(@Nonnull List nodeIds); + // These methods don't take into account context of packages + // so could easily lookup the wrong test + // they are used in the classic view for test results + ClassResult getClassResult(String name); + CaseResult getCaseResult(String name); + // end dodgy methods with no context + } + +// junit/(root)/Klazz/test1/ diff --git a/src/main/java/hudson/tasks/test/TestObject.java b/src/main/java/hudson/tasks/test/TestObject.java index b379b8a64..4770cc5e0 100644 --- a/src/main/java/hudson/tasks/test/TestObject.java +++ b/src/main/java/hudson/tasks/test/TestObject.java @@ -189,11 +189,8 @@ public String getRelativePathFrom(TestObject it) { // Now the build Run myBuild = cur.getRun(); if (myBuild ==null) { - myBuild = Stapler.getCurrentRequest().findAncestorObject(Run.class); - if (myBuild == null) { - LOGGER.warning("trying to get relative path, but we can't determine the build that owns this result."); - return ""; // this won't take us to the right place, but it also won't 404. - } + LOGGER.warning("trying to get relative path, but we can't determine the build that owns this result."); + return ""; // this won't take us to the right place, but it also won't 404. } buf.insert(0,'/'); buf.insert(0,myBuild.getUrl()); @@ -214,7 +211,9 @@ public String getRelativePathFrom(TestObject it) { return ""; } - buf.insert(0, '/'); + if (!jenkinsUrl.endsWith("/")) { + buf.insert(0, '/'); + } buf.insert(0, jenkinsUrl); } diff --git a/src/test/java/hudson/tasks/junit/pipeline/JUnitResultsStepTest.java b/src/test/java/hudson/tasks/junit/pipeline/JUnitResultsStepTest.java index 18ec445c9..f6cf9a392 100644 --- a/src/test/java/hudson/tasks/junit/pipeline/JUnitResultsStepTest.java +++ b/src/test/java/hudson/tasks/junit/pipeline/JUnitResultsStepTest.java @@ -155,6 +155,7 @@ public void twoSteps() throws Exception { FilePath secondTestFile = ws.child("second-result.xml"); secondTestFile.copyFrom(TestResultTest.class.getResource("junit-report-2874.xml")); + rule.pause(); WorkflowRun r = rule.buildAndAssertSuccess(j); TestResultAction action = r.getAction(TestResultAction.class); assertNotNull(action); diff --git a/src/test/java/hudson/tasks/junit/storage/TestResultStorageTest.java b/src/test/java/hudson/tasks/junit/storage/TestResultStorageTest.java index 087fab195..d23a5a14f 100644 --- a/src/test/java/hudson/tasks/junit/storage/TestResultStorageTest.java +++ b/src/test/java/hudson/tasks/junit/storage/TestResultStorageTest.java @@ -34,6 +34,7 @@ import hudson.model.TaskListener; import hudson.slaves.DumbSlave; import hudson.tasks.junit.CaseResult; +import hudson.tasks.junit.ClassResult; import hudson.tasks.junit.PackageResult; import hudson.tasks.junit.SuiteResult; import hudson.tasks.junit.TestResult; @@ -50,7 +51,6 @@ import java.sql.Statement; import java.sql.Types; import java.util.ArrayList; -import java.util.Collections; import java.util.List; import java.util.Objects; import java.util.Set; @@ -76,8 +76,6 @@ import org.jvnet.hudson.test.BuildWatcher; import org.jvnet.hudson.test.JenkinsRule; import org.jvnet.hudson.test.TestExtension; -import org.kohsuke.stapler.Stapler; -import org.kohsuke.stapler.StaplerRequest; import org.w3c.dom.Document; import org.w3c.dom.Element; import org.w3c.dom.Node; @@ -253,7 +251,7 @@ private int getCaseCount(String and) { private List retrieveCaseResult(String whereCondition) { return query(connection -> { - try (PreparedStatement statement = connection.prepareStatement("SELECT suite, testname, classname, errordetails, skipped FROM " + Impl.CASE_RESULTS_TABLE + " WHERE job = ? AND build = ? AND " + whereCondition)) { + try (PreparedStatement statement = connection.prepareStatement("SELECT suite, package, testname, classname, errordetails, skipped FROM " + Impl.CASE_RESULTS_TABLE + " WHERE job = ? AND build = ? AND " + whereCondition)) { statement.setString(1, job); statement.setInt(2, build); try (ResultSet result = statement.executeQuery()) { @@ -261,13 +259,17 @@ private List retrieveCaseResult(String whereCondition) { List results = new ArrayList<>(); while (result.next()) { String testName = result.getString("testname"); + String packageName = result.getString("package"); String errorDetails = result.getString("errordetails"); String suite = result.getString("suite"); String className = result.getString("classname"); String skipped = result.getString("skipped"); SuiteResult suiteResult = new SuiteResult(suite, null, null, null); - results.add(new CaseResult(suiteResult, className, testName, errorDetails, skipped)); + suiteResult.setParent(new TestResult(this)); + CaseResult caseResult = new CaseResult(suiteResult, className, testName, errorDetails, skipped); + caseResult.setClass(new ClassResult(new PackageResult(new TestResult(this), packageName), className)); + results.add(caseResult); } return results; } @@ -300,6 +302,29 @@ public PackageResult getPackageResult(String packageName) { return new PackageResult(new TestResult(this), packageName); } + @Override + public ClassResult getClassResult(String name) { + return query(connection -> { + try (PreparedStatement statement = connection.prepareStatement("SELECT package, classname FROM " + Impl.CASE_RESULTS_TABLE + " WHERE job = ? AND build = ? AND classname = ?")) { + statement.setString(1, job); + statement.setInt(2, build); + statement.setString(3, name); + try (ResultSet result = statement.executeQuery()) { + + if (result.next()) { + String packageName = result.getString("package"); + String className = result.getString("classname"); + + PackageResult packageResult = new PackageResult(new TestResult(this), packageName); + return new ClassResult(packageResult, className); + } + return null; + } + } + }, null); + + } + @Override public Run getFailedSinceRun(CaseResult caseResult) { return query(connection -> { @@ -387,6 +412,7 @@ private List getByPackage(String packageName, String filter) { String skipped = result.getString("skipped"); SuiteResult suiteResult = new SuiteResult(suite, null, null, null); + suiteResult.setParent(new TestResult(this)); results.add(new CaseResult(suiteResult, className, testName, errorDetails, skipped)); } return results; @@ -396,10 +422,41 @@ private List getByPackage(String packageName, String filter) { } - private List getCaseResult(String column) { + private List getCaseResults(String column) { return retrieveCaseResult(column + " IS NOT NULL"); } + @Override + public CaseResult getCaseResult(String testName) { + return query(connection -> { + try (PreparedStatement statement = connection.prepareStatement("SELECT suite, testname, package, classname, errordetails, skipped FROM " + Impl.CASE_RESULTS_TABLE + " WHERE job = ? AND build = ? AND testname = ?")) { + statement.setString(1, job); + statement.setInt(2, build); + statement.setString(3, testName); + try (ResultSet result = statement.executeQuery()) { + + CaseResult caseResult = null; + if (result.next()) { + String resultTestName = result.getString("testname"); + String errorDetails = result.getString("errordetails"); + String packageName = result.getString("package"); + String suite = result.getString("suite"); + String className = result.getString("classname"); + String skipped = result.getString("skipped"); + + SuiteResult suiteResult = new SuiteResult(suite, null, null, null); + suiteResult.setParent(new TestResult(this)); + caseResult = new CaseResult(suiteResult, className, resultTestName, errorDetails, skipped); + caseResult.setClass(new ClassResult(new PackageResult(new TestResult(this), packageName), className)); + } + return caseResult; + } + } + }, null); + + + } + @Override public int getFailCount() { int caseCount = getCaseCount(" AND errorDetails IS NOT NULL"); return caseCount; @@ -419,13 +476,13 @@ private List getCaseResult(String column) { @Override public List getFailedTests() { - List errordetails = getCaseResult("errordetails"); + List errordetails = getCaseResults("errordetails"); return errordetails; } @Override public List getSkippedTests() { - List errordetails = getCaseResult("skipped"); + List errordetails = getCaseResults("skipped"); return errordetails; } From 3645b64d1567cd496229e0f2f3410946efbf8c14 Mon Sep 17 00:00:00 2001 From: Tim Jacomb Date: Sat, 15 Aug 2020 10:07:50 +0100 Subject: [PATCH 28/33] Update src/test/java/hudson/tasks/junit/pipeline/JUnitResultsStepTest.java --- .../java/hudson/tasks/junit/pipeline/JUnitResultsStepTest.java | 1 - 1 file changed, 1 deletion(-) diff --git a/src/test/java/hudson/tasks/junit/pipeline/JUnitResultsStepTest.java b/src/test/java/hudson/tasks/junit/pipeline/JUnitResultsStepTest.java index f6cf9a392..18ec445c9 100644 --- a/src/test/java/hudson/tasks/junit/pipeline/JUnitResultsStepTest.java +++ b/src/test/java/hudson/tasks/junit/pipeline/JUnitResultsStepTest.java @@ -155,7 +155,6 @@ public void twoSteps() throws Exception { FilePath secondTestFile = ws.child("second-result.xml"); secondTestFile.copyFrom(TestResultTest.class.getResource("junit-report-2874.xml")); - rule.pause(); WorkflowRun r = rule.buildAndAssertSuccess(j); TestResultAction action = r.getAction(TestResultAction.class); assertNotNull(action); From d53b8dc94ff4e982cdc933a1458c46a1ca3b8443 Mon Sep 17 00:00:00 2001 From: Tim Jacomb Date: Sat, 15 Aug 2020 10:08:41 +0100 Subject: [PATCH 29/33] Update src/test/java/hudson/tasks/junit/storage/TestResultStorageTest.java --- .../java/hudson/tasks/junit/storage/TestResultStorageTest.java | 1 - 1 file changed, 1 deletion(-) diff --git a/src/test/java/hudson/tasks/junit/storage/TestResultStorageTest.java b/src/test/java/hudson/tasks/junit/storage/TestResultStorageTest.java index d23a5a14f..18bea0c47 100644 --- a/src/test/java/hudson/tasks/junit/storage/TestResultStorageTest.java +++ b/src/test/java/hudson/tasks/junit/storage/TestResultStorageTest.java @@ -190,7 +190,6 @@ public class TestResultStorageTest { assertEquals(1, rootPassedTests.size()); assertEquals("Klazz", rootPassedTests.get(0).getClassName()); - r.pause(); // TODO test result summary i.e. failure content // TODO getFailedSinceRun, TestResult#getChildren, TestObject#getTestResultAction // TODO more detailed Java queries incl. ClassResult From 3f3eacb67e7579a81b69387af9d52d0106958cbf Mon Sep 17 00:00:00 2001 From: Tim Jacomb Date: Sat, 15 Aug 2020 10:08:55 +0100 Subject: [PATCH 30/33] Remove debug line --- src/main/java/hudson/tasks/junit/storage/TestResultImpl.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/main/java/hudson/tasks/junit/storage/TestResultImpl.java b/src/main/java/hudson/tasks/junit/storage/TestResultImpl.java index d28181963..1e0e3df7b 100644 --- a/src/main/java/hudson/tasks/junit/storage/TestResultImpl.java +++ b/src/main/java/hudson/tasks/junit/storage/TestResultImpl.java @@ -61,5 +61,3 @@ public interface TestResultImpl { // end dodgy methods with no context } - -// junit/(root)/Klazz/test1/ From 970ca5147c866c2324e0473078d96caa9bc23cee Mon Sep 17 00:00:00 2001 From: Tim Jacomb Date: Sat, 15 Aug 2020 12:15:48 +0100 Subject: [PATCH 31/33] Apparently toString on a stapler dispatched object is a bad idea Some tracing call ends up calling the toString and it breaks.. --- .../java/hudson/tasks/junit/CaseResult.java | 29 +------------------ .../junit/storage/TestResultStorageTest.java | 1 - 2 files changed, 1 insertion(+), 29 deletions(-) diff --git a/src/main/java/hudson/tasks/junit/CaseResult.java b/src/main/java/hudson/tasks/junit/CaseResult.java index 1f8c09d29..f7735670d 100644 --- a/src/main/java/hudson/tasks/junit/CaseResult.java +++ b/src/main/java/hudson/tasks/junit/CaseResult.java @@ -673,34 +673,7 @@ public void freeze(SuiteResult parent) { // some old test data doesn't have failedSince value set, so for those compute them. recomputeFailedSinceIfNeeded(); } - - @Override - public String toString() { - StringBuilder b = new StringBuilder("CaseResult{className=").append(className).append(", testName=").append(testName); - if (errorDetails != null) { - b.append(", errorDetails=").append(errorDetails); - } - if (errorStackTrace != null) { - b.append(", errorStackTrace=").append(errorStackTrace); - } - if (skipped) { - b.append(", skipped=true"); - } - if (skippedMessage != null) { - b.append(", skippedMessage=").append(skippedMessage); - } - if (duration != 0.0f) { - b.append(", duration=").append(duration); - } - if (stdout != null) { - b.append(", stdout=").append(stdout); - } - if (stderr != null) { - b.append(", stderr=").append(stderr); - } - return b.append('}').toString(); - } - + public int compareTo(CaseResult that) { if (this == that) { return 0; diff --git a/src/test/java/hudson/tasks/junit/storage/TestResultStorageTest.java b/src/test/java/hudson/tasks/junit/storage/TestResultStorageTest.java index 18bea0c47..89bbe1a37 100644 --- a/src/test/java/hudson/tasks/junit/storage/TestResultStorageTest.java +++ b/src/test/java/hudson/tasks/junit/storage/TestResultStorageTest.java @@ -153,7 +153,6 @@ public class TestResultStorageTest { assertEquals(1, a.getResult().getSkipCount()); assertEquals(4, a.getResult().getTotalCount()); assertEquals(1, a.getResult().getPassCount()); - assertEquals("[CaseResult{className=Klazz, testName=test1, errorDetails=failure}, CaseResult{className=another.Klazz, testName=test1, errorDetails=another failure}]", a.getFailedTests().toString()); List failedTests = a.getFailedTests(); assertEquals(2, failedTests.size()); assertEquals("Klazz", failedTests.get(0).getClassName()); From 385d2beed9177668f42e9fb461e2d73c84631e40 Mon Sep 17 00:00:00 2001 From: Tim Jacomb Date: Sat, 15 Aug 2020 13:55:02 +0100 Subject: [PATCH 32/33] Fix results not getting counted if no failures --- src/main/java/hudson/tasks/junit/TestResultSummary.java | 3 +++ .../tasks/junit/pipeline/JUnitResultsStepExecution.java | 6 +----- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/src/main/java/hudson/tasks/junit/TestResultSummary.java b/src/main/java/hudson/tasks/junit/TestResultSummary.java index 53ac2b104..0b8224177 100644 --- a/src/main/java/hudson/tasks/junit/TestResultSummary.java +++ b/src/main/java/hudson/tasks/junit/TestResultSummary.java @@ -3,6 +3,8 @@ import org.jenkinsci.plugins.scriptsecurity.sandbox.whitelists.Whitelisted; import java.io.Serializable; +import org.kohsuke.accmod.Restricted; +import org.kohsuke.accmod.restrictions.DoNotUse; /** * Summary of test results that can be used in Pipeline scripts. @@ -14,6 +16,7 @@ public class TestResultSummary implements Serializable { private int totalCount; @Deprecated + @Restricted(DoNotUse.class) public TestResultSummary() { } diff --git a/src/main/java/hudson/tasks/junit/pipeline/JUnitResultsStepExecution.java b/src/main/java/hudson/tasks/junit/pipeline/JUnitResultsStepExecution.java index 59762218f..e3431588b 100644 --- a/src/main/java/hudson/tasks/junit/pipeline/JUnitResultsStepExecution.java +++ b/src/main/java/hudson/tasks/junit/pipeline/JUnitResultsStepExecution.java @@ -6,8 +6,6 @@ import hudson.model.Run; import hudson.model.TaskListener; import hudson.tasks.junit.JUnitResultArchiver; -import hudson.tasks.junit.TestResult; -import hudson.tasks.junit.TestResultAction; import hudson.tasks.junit.TestResultSummary; import hudson.tasks.test.PipelineTestDetails; import java.util.ArrayList; @@ -58,15 +56,13 @@ protected TestResultSummary run() throws Exception { assert run != null; run.setResult(Result.UNSTABLE); } - return summary; } + return summary; } catch (Exception e) { assert listener != null; listener.getLogger().println(e.getMessage()); throw e; } - - return new TestResultSummary(); } /** From 5189e6bd158c2d6e92fae651b620f6a154854e5a Mon Sep 17 00:00:00 2001 From: Tim Jacomb Date: Sat, 15 Aug 2020 16:13:42 +0100 Subject: [PATCH 33/33] Fix spotbugs --- .../hudson/tasks/junit/JUnitResultArchiver.java | 2 +- .../java/hudson/tasks/junit/PackageResult.java | 15 +++++++++------ .../junit/pipeline/JUnitResultsStepExecution.java | 5 +++-- 3 files changed, 13 insertions(+), 9 deletions(-) diff --git a/src/main/java/hudson/tasks/junit/JUnitResultArchiver.java b/src/main/java/hudson/tasks/junit/JUnitResultArchiver.java index 525ca2536..9eeab671e 100644 --- a/src/main/java/hudson/tasks/junit/JUnitResultArchiver.java +++ b/src/main/java/hudson/tasks/junit/JUnitResultArchiver.java @@ -218,7 +218,7 @@ public static TestResultAction parseAndAttach(@Nonnull JUnitTask task, PipelineT } public static TestResultSummary parseAndSummarize(@Nonnull JUnitTask task, PipelineTestDetails pipelineTestDetails, - Run build, FilePath workspace, Launcher launcher, TaskListener listener) + Run build, FilePath workspace, Launcher launcher, TaskListener listener) throws InterruptedException, IOException { TestResultStorage storage = TestResultStorage.find(); if (storage == null) { diff --git a/src/main/java/hudson/tasks/junit/PackageResult.java b/src/main/java/hudson/tasks/junit/PackageResult.java index 5cff1a260..f9ed1202f 100644 --- a/src/main/java/hudson/tasks/junit/PackageResult.java +++ b/src/main/java/hudson/tasks/junit/PackageResult.java @@ -186,8 +186,9 @@ public boolean hasChildren() { * sort order */ public List getFailedTests() { - if (parent.getPluggableStorage() != null) { - return parent.getPluggableStorage().getFailedTestsByPackage(packageName); + TestResultImpl pluggableStorage = parent.getPluggableStorage(); + if (pluggableStorage != null) { + return pluggableStorage.getFailedTestsByPackage(packageName); } List r = new ArrayList(); @@ -219,8 +220,9 @@ public List getFailedTestsSortedByAge() { */ @Override public List getPassedTests() { - if (parent.getPluggableStorage() != null) { - return parent.getPluggableStorage().getPassedTestsByPackage(packageName); + TestResultImpl pluggableStorage = parent.getPluggableStorage(); + if (pluggableStorage != null) { + return pluggableStorage.getPassedTestsByPackage(packageName); } List r = new ArrayList(); @@ -242,8 +244,9 @@ public List getPassedTests() { */ @Override public List getSkippedTests() { - if (parent.getPluggableStorage() != null) { - return parent.getPluggableStorage().getSkippedTestsByPackage(packageName); + TestResultImpl pluggableStorage = parent.getPluggableStorage(); + if (pluggableStorage != null) { + return pluggableStorage.getSkippedTestsByPackage(packageName); } List r = new ArrayList(); diff --git a/src/main/java/hudson/tasks/junit/pipeline/JUnitResultsStepExecution.java b/src/main/java/hudson/tasks/junit/pipeline/JUnitResultsStepExecution.java index e3431588b..63c221c78 100644 --- a/src/main/java/hudson/tasks/junit/pipeline/JUnitResultsStepExecution.java +++ b/src/main/java/hudson/tasks/junit/pipeline/JUnitResultsStepExecution.java @@ -20,6 +20,8 @@ import org.jenkinsci.plugins.workflow.steps.StepDescriptor; import org.jenkinsci.plugins.workflow.steps.SynchronousNonBlockingStepExecution; +import static java.util.Objects.requireNonNull; + public class JUnitResultsStepExecution extends SynchronousNonBlockingStepExecution { private transient final JUnitResultsStep step; @@ -33,7 +35,7 @@ public JUnitResultsStepExecution(@Nonnull JUnitResultsStep step, StepContext con protected TestResultSummary run() throws Exception { FilePath workspace = getContext().get(FilePath.class); workspace.mkdirs(); - Run run = getContext().get(Run.class); + Run run = requireNonNull(getContext().get(Run.class)); TaskListener listener = getContext().get(TaskListener.class); Launcher launcher = getContext().get(Launcher.class); FlowNode node = getContext().get(FlowNode.class); @@ -53,7 +55,6 @@ protected TestResultSummary run() throws Exception { int testFailures = summary.getFailCount(); if (testFailures > 0) { node.addOrReplaceAction(new WarningAction(Result.UNSTABLE).withMessage(testFailures + " tests failed")); - assert run != null; run.setResult(Result.UNSTABLE); } }