This is an automated email from the ASF dual-hosted git repository.

adar pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/kudu.git

commit d929aac91831c12e7675faf2dbfd6c578996f84f
Author: Mike Percy <[email protected]>
AuthorDate: Tue Dec 4 18:57:47 2018 -0800

    java: add support for flaky test reporting
    
    This patch hooks into the existing RetryRule to report test results to
    the flaky test server inline as the tests are executed. All of the
    actual reporting logic is factored out into a separate ResultReporter class.
    
    The interface for the test reporter to pass relevant information about
    the build environment to the flaky test server is based on environment
    variables. This includes configuration and build metadata such as flaky
    test server address, git revision, build id, build host, and build type.
    
    This patch also includes a simple integration test for the reporter
    using a mocked-up flaky test server HTTP endpoint.
    
    This patch does not integrate the above functionality into the build.
    That will happen in a follow-up patch.
    
    Change-Id: I34f88363dbf52c2f7bba50fbfb5de059f88e7f74
    Reviewed-on: http://gerrit.cloudera.org:8080/12042
    Tested-by: Adar Dembo <[email protected]>
    Reviewed-by: Grant Henke <[email protected]>
---
 java/gradle/dependencies.gradle                    |   6 +
 java/kudu-test-utils/build.gradle                  |   5 +
 .../org/apache/kudu/test/CapturingLogAppender.java |   8 +-
 .../kudu/test/CapturingToFileLogAppender.java      | 177 +++++++++++++++
 .../org/apache/kudu/test/junit/ResultReporter.java | 249 +++++++++++++++++++++
 .../java/org/apache/kudu/test/junit/RetryRule.java |  88 ++++++--
 .../apache/kudu/test/junit/TestResultReporter.java | 203 +++++++++++++++++
 .../org/apache/kudu/test/junit/TestRetryRule.java  |   3 +-
 8 files changed, 721 insertions(+), 18 deletions(-)

diff --git a/java/gradle/dependencies.gradle b/java/gradle/dependencies.gradle
index 181a146..5b945cc 100755
--- a/java/gradle/dependencies.gradle
+++ b/java/gradle/dependencies.gradle
@@ -39,7 +39,9 @@ versions += [
     hamcrest       : "2.1",
     hdrhistogram   : "2.1.11",
     hive           : "2.3.4",
+    httpClient     : "4.5.7",
     jepsen         : "0.1.5",
+    jetty          : "9.4.15.v20190215",
     jsr305         : "3.0.2",
     junit          : "4.12",
     log4j          : "1.2.17",
@@ -94,7 +96,11 @@ libs += [
     hdrhistogram         : 
"org.hdrhistogram:HdrHistogram:$versions.hdrhistogram",
     hiveMetastore        : "org.apache.hive:hive-metastore:$versions.hive",
     hiveMetastoreTest    : 
"org.apache.hive:hive-metastore:$versions.hive:tests",
+    httpClient           : 
"org.apache.httpcomponents:httpclient:$versions.httpClient",
+    httpMime             : 
"org.apache.httpcomponents:httpmime:$versions.httpClient",
     jepsen               : "jepsen:jepsen:$versions.jepsen",
+    jetty                : "org.eclipse.jetty:jetty-server:$versions.jetty",
+    jettyServlet         : "org.eclipse.jetty:jetty-servlet:$versions.jetty",
     jsr305               : "com.google.code.findbugs:jsr305:$versions.jsr305",
     junit                : "junit:junit:$versions.junit",
     log4j                : "log4j:log4j:$versions.log4j",
diff --git a/java/kudu-test-utils/build.gradle 
b/java/kudu-test-utils/build.gradle
index e172c2a..9db3651 100644
--- a/java/kudu-test-utils/build.gradle
+++ b/java/kudu-test-utils/build.gradle
@@ -21,6 +21,8 @@ dependencies {
   compile project(path: ":kudu-client")
   compile libs.commonsIo
   compile libs.guava
+  compile libs.httpClient
+  compile libs.httpMime
   compile libs.osdetector
 
   compileUnshaded libs.junit
@@ -32,6 +34,9 @@ dependencies {
 
   optional libs.jsr305
   optional libs.yetusAnnotations
+
+  testCompile libs.jetty
+  testCompile libs.jettyServlet
 }
 
 // kudu-test-utils has no public Javadoc.
diff --git 
a/java/kudu-test-utils/src/main/java/org/apache/kudu/test/CapturingLogAppender.java
 
b/java/kudu-test-utils/src/main/java/org/apache/kudu/test/CapturingLogAppender.java
index 81c9bc9..056ef80 100644
--- 
a/java/kudu-test-utils/src/main/java/org/apache/kudu/test/CapturingLogAppender.java
+++ 
b/java/kudu-test-utils/src/main/java/org/apache/kudu/test/CapturingLogAppender.java
@@ -36,8 +36,9 @@ import org.apache.yetus.audience.InterfaceStability;
 @InterfaceAudience.Private
 @InterfaceStability.Unstable
 public class CapturingLogAppender extends AppenderSkeleton {
+  private static final Layout LAYOUT = new SimpleLayout();
+
   private StringBuilder appended = new StringBuilder();
-  private static final Layout layout = new SimpleLayout();
 
   @Override
   public void close() {
@@ -50,13 +51,16 @@ public class CapturingLogAppender extends AppenderSkeleton {
 
   @Override
   protected void append(LoggingEvent event) {
-    appended.append(layout.format(event));
+    appended.append(LAYOUT.format(event));
     if (event.getThrowableInformation() != null) {
       appended.append(Throwables.getStackTraceAsString(
           event.getThrowableInformation().getThrowable())).append("\n");
     }
   }
 
+  /**
+   * @return all of the appended messages captured thus far, joined together.
+   */
   public String getAppendedText() {
     return appended.toString();
   }
diff --git 
a/java/kudu-test-utils/src/main/java/org/apache/kudu/test/CapturingToFileLogAppender.java
 
b/java/kudu-test-utils/src/main/java/org/apache/kudu/test/CapturingToFileLogAppender.java
new file mode 100644
index 0000000..6eea54a
--- /dev/null
+++ 
b/java/kudu-test-utils/src/main/java/org/apache/kudu/test/CapturingToFileLogAppender.java
@@ -0,0 +1,177 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package org.apache.kudu.test;
+
+import java.io.BufferedWriter;
+import java.io.Closeable;
+import java.io.File;
+import java.io.FileOutputStream;
+import java.io.IOException;
+import java.io.OutputStream;
+import java.io.OutputStreamWriter;
+import java.io.Writer;
+import java.util.zip.GZIPOutputStream;
+
+import com.google.common.base.Throwables;
+import org.apache.commons.io.IOUtils;
+import org.apache.log4j.AppenderSkeleton;
+import org.apache.log4j.Layout;
+import org.apache.log4j.Logger;
+import org.apache.log4j.PatternLayout;
+import org.apache.log4j.spi.LoggingEvent;
+import org.apache.yetus.audience.InterfaceAudience;
+import org.apache.yetus.audience.InterfaceStability;
+
+import static java.nio.charset.StandardCharsets.UTF_8;
+
+/**
+ * Test utility which wraps Log4j and captures all messages logged while
+ * attached, storing them in an (optionally gzipped) temporary file.
+ *
+ * The typical lifecycle is as follows:
+ *
+ * constructor: temporary file is created and opened.
+ * append():    a new log event is captured. It may or may not be flushed to 
disk.
+ * finish():    all events previously captured in append() are now guaranteed 
to
+ *              be on disk and visible to readers. No more events may be 
appended.
+ * close():     the temporary file is deleted.
+ */
[email protected]
[email protected]
+public class CapturingToFileLogAppender extends AppenderSkeleton implements 
AutoCloseable {
+  // This is the standard layout used in Kudu tests.
+  private static final Layout LAYOUT = new PatternLayout(
+      "%d{HH:mm:ss.SSS} [%p - %t] (%F:%L) %m%n");
+
+  private File outputFile;
+  private Writer outputFileWriter;
+
+  /**
+   * Creates a new appender. The temporary file is created immediately; it may
+   * be obtained via getOutputFile().
+   *
+   * Appended messages are buffered; they must be flushed to disk via finish().
+   *
+   * @param useGzip whether to gzip-compress messages when appended
+   */
+  public CapturingToFileLogAppender(boolean useGzip) throws IOException {
+    outputFile = File.createTempFile("captured_output", ".txt.gz");
+    try {
+      OutputStream os = new FileOutputStream(outputFile.getPath());
+      try {
+        if (useGzip) {
+          os = new GZIPOutputStream(os);
+        }
+
+        // As per the recommendation in OutputStreamWriter's Javadoc, we wrap 
in a
+        // BufferedWriter to buffer up character conversions.
+        outputFileWriter = new BufferedWriter(new OutputStreamWriter(os, 
UTF_8));
+      } catch (Throwable t) {
+        IOUtils.closeQuietly(os);
+        throw t;
+      }
+    } catch (Throwable t) {
+      outputFile.delete();
+      throw t;
+    }
+  }
+
+  @Override
+  public void close() {
+    // Just do the cleanup; we don't care about exceptions/logging.
+
+    if (outputFileWriter != null) {
+      IOUtils.closeQuietly(outputFileWriter);
+      outputFileWriter = null;
+    }
+    if (outputFile != null) {
+      outputFile.delete();
+      outputFile = null;
+    }
+  }
+
+  @Override
+  public boolean requiresLayout() {
+    return false;
+  }
+
+  @Override
+  protected void append(LoggingEvent event) {
+    assert outputFileWriter != null;
+    try {
+      outputFileWriter.write(LAYOUT.format(event));
+      if (event.getThrowableInformation() != null) {
+        outputFileWriter.write(Throwables.getStackTraceAsString(
+            event.getThrowableInformation().getThrowable()));
+        outputFileWriter.write("\n");
+      }
+    } catch (IOException e) {
+      throw new RuntimeException(e);
+    }
+  }
+
+  /**
+   * Flushes any buffered appended events to the on-disk temporary file and
+   * closes it.
+   *
+   * After calling this function, all appended events will be visible to new
+   * readers.
+   *
+   * @throws IOException if close failed
+   */
+  public void finish() throws IOException {
+    // As per the Writer contract, this will also flush the output stream as
+    // well as the compressor (if gzip-compression is used).
+    //
+    // Why close() and not flush()? It turns out to be remarkably hard to
+    // flush a GZIPOutputStream [1]. At the very least it also requires calling
+    // finish(), which is not a generic OutputStream method. But for our use
+    // case (multiple append() calls followed by a single file access) it's
+    // easier to just close() when we're done appending.
+    //
+    // 1. 
https://stackoverflow.com/questions/3640080/force-flush-on-a-gzipoutputstream-in-java
+    //
+    outputFileWriter.close();
+    outputFileWriter = null;
+  }
+
+  /**
+   * @return the temporary file opened in the appender's constructor
+   */
+  public File getOutputFile() {
+    return outputFile;
+  }
+
+  /**
+   * Temporarily attach the capturing appender to the Log4j root logger.
+   * This can be used in a 'try-with-resources' block:
+   * <code>
+   *   try (Closeable c = capturer.attach()) {
+   *     ...
+   *   }
+   * </code>
+   */
+  public Closeable attach() {
+    Logger.getRootLogger().addAppender(this);
+    return new Closeable() {
+      @Override
+      public void close() throws IOException {
+        Logger.getRootLogger().removeAppender(CapturingToFileLogAppender.this);
+      }
+    };
+  }
+}
diff --git 
a/java/kudu-test-utils/src/main/java/org/apache/kudu/test/junit/ResultReporter.java
 
b/java/kudu-test-utils/src/main/java/org/apache/kudu/test/junit/ResultReporter.java
new file mode 100644
index 0000000..483b6ca
--- /dev/null
+++ 
b/java/kudu-test-utils/src/main/java/org/apache/kudu/test/junit/ResultReporter.java
@@ -0,0 +1,249 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package org.apache.kudu.test.junit;
+
+import java.io.BufferedReader;
+import java.io.File;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.InputStreamReader;
+import java.util.ArrayList;
+import java.util.List;
+
+import com.google.common.base.Joiner;
+import com.google.common.collect.ImmutableList;
+import org.apache.http.StatusLine;
+import org.apache.http.util.EntityUtils;
+import org.apache.http.client.methods.CloseableHttpResponse;
+import org.apache.http.client.methods.HttpPost;
+import org.apache.http.entity.ContentType;
+import org.apache.http.entity.mime.MultipartEntityBuilder;
+import org.apache.http.impl.client.CloseableHttpClient;
+import org.apache.http.impl.client.HttpClients;
+import org.apache.yetus.audience.InterfaceAudience;
+import org.apache.yetus.audience.InterfaceStability;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import static java.nio.charset.StandardCharsets.UTF_8;
+
+/** Class to report test results to the flaky test server. */
[email protected]
[email protected]
+public class ResultReporter {
+  public enum Result {
+    SUCCESS,
+    FAILURE
+  }
+
+  public static class Options {
+    private boolean reportResults = true;
+    private String httpEndpoint;
+    private String buildTag;
+    private String revision;
+    private String hostname;
+    private String buildConfig;
+
+    public Options reportResults(boolean reportResults) {
+      this.reportResults = reportResults;
+      return this;
+    }
+    public Options httpEndpoint(String httpEndpoint) {
+      this.httpEndpoint = httpEndpoint;
+      return this;
+    }
+    public Options buildTag(String buildTag) {
+      this.buildTag = buildTag;
+      return this;
+    }
+    public Options revision(String revision) {
+      this.revision = revision;
+      return this;
+    }
+    public Options hostname(String hostname) {
+      this.hostname = hostname;
+      return this;
+    }
+    public Options buildConfig(String buildConfig) {
+      this.buildConfig = buildConfig;
+      return this;
+    }
+  }
+
+  private static final Logger LOG = 
LoggerFactory.getLogger(ResultReporter.class);
+  private static final String KUDU_REPORT_TEST_RESULTS_VAR = 
"KUDU_REPORT_TEST_RESULTS";
+  private static final String TEST_RESULT_SERVER_VAR = "TEST_RESULT_SERVER";
+  private static final String BUILD_TAG_VAR = "BUILD_TAG";
+  private static final String GIT_REVISION_VAR = "GIT_REVISION";
+  private static final String BUILD_CONFIG_VAR = "BUILD_CONFIG";
+
+  private final Options options;
+
+  public ResultReporter() {
+    this(new Options()
+        .reportResults(isReportingConfigured())
+        .httpEndpoint(getEnvStringWithDefault(TEST_RESULT_SERVER_VAR,
+                                              "localhost:8080"))
+        .buildTag(System.getenv(BUILD_TAG_VAR))
+        .revision(System.getenv(GIT_REVISION_VAR))
+        .buildConfig(System.getenv(BUILD_CONFIG_VAR))
+        .hostname(getLocalHostname()));
+  }
+
+  @InterfaceAudience.LimitedPrivate("Test")
+  ResultReporter(Options options) {
+    this.options = options;
+  }
+
+  private static boolean isVarSetAndNonEmpty(String name) {
+    String var = System.getenv(name);
+    return var != null && !var.equals("");
+  }
+
+  private static boolean areRequiredReportingVarsSetAndNonEmpty() {
+    return isVarSetAndNonEmpty(BUILD_TAG_VAR) &&
+           isVarSetAndNonEmpty(GIT_REVISION_VAR) &&
+           isVarSetAndNonEmpty(BUILD_CONFIG_VAR);
+  }
+
+  private static String reportingVarDump() {
+    List<String> vars = new ArrayList<>();
+    for (String var : ImmutableList.of(TEST_RESULT_SERVER_VAR,
+                                       BUILD_TAG_VAR,
+                                       GIT_REVISION_VAR,
+                                       BUILD_CONFIG_VAR)) {
+      vars.add(var + ": \"" + System.getenv(var) + "\"");
+    }
+    return Joiner.on(", ").join(vars);
+  }
+
+  private static boolean isReportingConfigured() {
+    if (getEnvIntegerWithDefault(KUDU_REPORT_TEST_RESULTS_VAR, 0) == 0) {
+      return false;
+    }
+    if (!areRequiredReportingVarsSetAndNonEmpty()) {
+      throw new IllegalStateException("Not all required variables are set: " +
+                                      reportingVarDump());
+    }
+    return true;
+  }
+
+  private static String getEnvStringWithDefault(String name,
+                                                String defaultValue) {
+    String value = System.getenv(name);
+    if (value == null || value.isEmpty()) {
+      return defaultValue;
+    }
+    return value;
+  }
+
+  private static int getEnvIntegerWithDefault(String name, int defaultValue) {
+    return Integer.parseInt(getEnvStringWithDefault(
+        name, String.valueOf(defaultValue)));
+  }
+
+  /**
+   * Invokes the `hostname` UNIX utility to retrieve the machine's hostname.
+   *
+   * Note: this is not the same as InetAddress.getLocalHost().getHostName(),
+   * which performs a reverse DNS lookup and may return a different result,
+   * depending on the machine's networking configuration. The equivalent C++
+   * code uses `hostname`, so it's important we do the same here for parity.
+   *
+   * @returns the local hostname
+   */
+  @InterfaceAudience.LimitedPrivate("Test")
+  static String getLocalHostname() {
+    ProcessBuilder pb = new ProcessBuilder("hostname");
+    try {
+      Process p = pb.start();
+      try (InputStreamReader isr = new InputStreamReader(p.getInputStream(), 
UTF_8);
+           BufferedReader br = new BufferedReader(isr)) {
+        int rv = p.waitFor();
+        if (rv != 0) {
+          throw new IllegalStateException(String.format(
+              "Process 'hostname' exited with exit status %d", rv));
+        }
+        return br.readLine();
+      }
+    } catch (InterruptedException | IOException e) {
+      throw new RuntimeException(e);
+    }
+  }
+
+  /**
+   * Reports a test result to the flaky test server.
+   *
+   * @param testName the display name of the JUnit test
+   * @param result success or failure
+   * @param logFile optionally, file containing log messages generated by the 
test
+   * @throws IOException if test reporting failed
+   */
+  public void reportResult(String testName, Result result, File logFile)
+      throws IOException {
+    if (!options.reportResults) return;
+
+    try (CloseableHttpClient client = HttpClients.createDefault()) {
+      HttpPost post = new HttpPost("http://"; + options.httpEndpoint + 
"/add_result");
+
+      // Set up the request with all form parts.
+      MultipartEntityBuilder meb = MultipartEntityBuilder.create();
+      // In the backend, the BUILD_TAG field is called 'build_id', but we 
can't use
+      // that as an env variable because it'd collide with Jenkins' BUILD_ID.
+      meb.addTextBody("build_id", options.buildTag);
+      meb.addTextBody("hostname", options.hostname);
+      meb.addTextBody("revision", options.revision);
+      meb.addTextBody("build_config", options.buildConfig);
+      meb.addTextBody("test_name", testName);
+      // status=0 indicates success, status=1 indicates failure.
+      meb.addTextBody("status", Integer.toString(result == Result.SUCCESS ? 0 
: 1));
+      if (logFile != null) {
+        meb.addBinaryBody("log", logFile, ContentType.APPLICATION_OCTET_STREAM,
+                          testName + ".txt.gz");
+      }
+      post.setEntity(meb.build());
+
+      // Send the request and process the response.
+      try (CloseableHttpResponse resp = client.execute(post)) {
+        StatusLine sl = resp.getStatusLine();
+        if (sl.getStatusCode() != 200) {
+          throw new IOException("Bad response from server: " + 
sl.getStatusCode() + ": " +
+                                EntityUtils.toString(resp.getEntity(), UTF_8));
+        }
+      }
+    }
+  }
+
+  /**
+   * Same as {@link #reportResult(String, Result)} but never throws an 
exception.
+   * Logs a warning message on failure.
+   */
+  public void tryReportResult(String testName, Result result, File logFile) {
+    try {
+      reportResult(testName, result, logFile);
+    } catch (IOException ex) {
+      LOG.warn("Failed to record test result for {} as {}", testName, result, 
ex);
+    }
+  }
+
+  /**
+   * @return whether result reporting is enabled for this reporter
+   */
+  public boolean isReportingEnabled() {
+    return options.reportResults;
+  }
+}
diff --git 
a/java/kudu-test-utils/src/main/java/org/apache/kudu/test/junit/RetryRule.java 
b/java/kudu-test-utils/src/main/java/org/apache/kudu/test/junit/RetryRule.java
index cf1bab7..31d8a95 100644
--- 
a/java/kudu-test-utils/src/main/java/org/apache/kudu/test/junit/RetryRule.java
+++ 
b/java/kudu-test-utils/src/main/java/org/apache/kudu/test/junit/RetryRule.java
@@ -16,6 +16,7 @@
 // under the License.
 package org.apache.kudu.test.junit;
 
+import org.apache.kudu.test.CapturingToFileLogAppender;
 import org.apache.yetus.audience.InterfaceAudience;
 import org.apache.yetus.audience.InterfaceStability;
 import org.junit.rules.TestRule;
@@ -23,32 +24,43 @@ import org.junit.runner.Description;
 import org.junit.runners.model.Statement;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
+import java.io.Closeable;
+import java.io.File;
+import java.io.IOException;
 
 /**
- * A JUnit rule to retry failed tests.
- * We use this with Gradle because it doesn't support
- * Surefire/Failsafe rerunFailingTestsCount like Maven does. We use the system
- * property rerunFailingTestsCount to mimic the maven arguments closely.
+ * JUnit rule to retry failed tests.
+ *
+ * The number of retries is controlled by the "rerunFailingTestsCount" system
+ * property, mimicking Surefire in that regard.
+ *
+ * By default will use ResultReporter to report success/failure of each test
+ * attempt to an external server; this may be skipped if desired.
  */
 @InterfaceAudience.Private
 @InterfaceStability.Unstable
 public class RetryRule implements TestRule {
 
+  private static final String RETRY_PROP = "rerunFailingTestsCount";
+
   private static final Logger LOG = LoggerFactory.getLogger(RetryRule.class);
+
   private final int retryCount;
+  private final ResultReporter reporter;
 
   public RetryRule() {
-    this(Integer.getInteger("rerunFailingTestsCount", 0));
+    this(Integer.getInteger(RETRY_PROP, 0), /*skipReporting=*/ false);
   }
 
-  // Visible for testing.
-  RetryRule(int retryCount) {
+  @InterfaceAudience.LimitedPrivate("Test")
+  RetryRule(int retryCount, boolean skipReporting) {
     this.retryCount = retryCount;
+    this.reporter = skipReporting ? null : new ResultReporter();
   }
 
   @Override
   public Statement apply(Statement base, Description description) {
-    return new RetryStatement(base, description, retryCount);
+    return new RetryStatement(base, description, retryCount, reporter);
   }
 
   private static class RetryStatement extends Statement {
@@ -56,11 +68,57 @@ public class RetryRule implements TestRule {
     private final Statement base;
     private final Description description;
     private final int retryCount;
+    private final ResultReporter reporter;
+    private final String humanReadableTestName;
 
-    RetryStatement(Statement base, Description description, int retryCount) {
+    RetryStatement(Statement base, Description description,
+                   int retryCount, ResultReporter reporter) {
       this.base = base;
       this.description = description;
       this.retryCount = retryCount;
+      this.reporter = reporter;
+      this.humanReadableTestName = description.getClassName() + "." + 
description.getMethodName();
+    }
+
+    private void report(ResultReporter.Result result, File logFile) {
+      reporter.tryReportResult(humanReadableTestName, result, logFile);
+    }
+
+    private void doOneAttemptAndReport(int attempt) throws Throwable {
+      try (CapturingToFileLogAppender capturer =
+           new CapturingToFileLogAppender(/*useGzip=*/ true)) {
+        try {
+          try (Closeable c = capturer.attach()) {
+            base.evaluate();
+          }
+
+          // The test succeeded.
+          //
+          // We skip the file upload; this saves space and network bandwidth,
+          // and we don't need the logs of successful tests.
+          report(ResultReporter.Result.SUCCESS, /*logFile=*/ null);
+          return;
+        } catch (Throwable t) {
+          // The test failed.
+          //
+          // Before reporting, capture the failing exception too.
+          try (Closeable c = capturer.attach()) {
+            LOG.error("{}: failed attempt {}", humanReadableTestName, attempt, 
t);
+          }
+          capturer.finish();
+          report(ResultReporter.Result.FAILURE, capturer.getOutputFile());
+          throw t;
+        }
+      }
+    }
+
+    private void doOneAttempt(int attempt) throws Throwable {
+      try {
+        base.evaluate();
+      } catch (Throwable t) {
+        LOG.error("{}: failed attempt {}", humanReadableTestName, attempt, t);
+        throw t;
+      }
     }
 
     @Override
@@ -70,17 +128,17 @@ public class RetryRule implements TestRule {
       do {
         attempt++;
         try {
-          base.evaluate();
+          if (reporter != null && reporter.isReportingEnabled()) {
+            doOneAttemptAndReport(attempt);
+          } else {
+            doOneAttempt(attempt);
+          }
           return;
-
         } catch (Throwable t) {
-          // To retry, we catch the exception from evaluate(), log an error, 
and loop.
-          // We retain and rethrow the last failure if all attempts fail.
           lastException = t;
-          LOG.error("{}: failed attempt {}", description.getDisplayName(), 
attempt, t);
         }
       } while (attempt <= retryCount);
-      LOG.error("{}: giving up after {} attempts", 
description.getDisplayName(), attempt);
+      LOG.error("{}: giving up after {} attempts", humanReadableTestName, 
attempt);
       throw lastException;
     }
   }
diff --git 
a/java/kudu-test-utils/src/test/java/org/apache/kudu/test/junit/TestResultReporter.java
 
b/java/kudu-test-utils/src/test/java/org/apache/kudu/test/junit/TestResultReporter.java
new file mode 100644
index 0000000..bad2aad
--- /dev/null
+++ 
b/java/kudu-test-utils/src/test/java/org/apache/kudu/test/junit/TestResultReporter.java
@@ -0,0 +1,203 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package org.apache.kudu.test.junit;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertTrue;
+
+import org.apache.commons.io.IOUtils;
+import org.eclipse.jetty.server.Server;
+import org.eclipse.jetty.servlet.ServletContextHandler;
+import org.eclipse.jetty.servlet.ServletHolder;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import javax.servlet.MultipartConfigElement;
+import javax.servlet.ServletException;
+import javax.servlet.http.HttpServlet;
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
+import javax.servlet.http.Part;
+import java.io.File;
+import java.io.FileOutputStream;
+import java.io.IOException;
+import java.net.InetSocketAddress;
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.Iterator;
+import java.util.List;
+import java.util.Map;
+
+import com.google.common.base.Joiner;
+import com.google.common.collect.ImmutableList;
+
+import static java.nio.charset.StandardCharsets.UTF_8;
+
+/** Unit test for ResultReporter. */
+public class TestResultReporter {
+
+  /** Record of a specific test run. */
+  private static class TestRecord {
+    public String testName;
+    public String buildTag;
+    public String revision;
+    public String hostname;
+    public String buildConfig;
+    public int status;
+    public String log;
+
+    public TestRecord(Map<String, String> params) {
+      testName = params.get("test_name");
+      buildTag = params.get("build_id");
+      revision = params.get("revision");
+      hostname = params.get("hostname");
+      buildConfig = params.get("build_config");
+      status = Integer.parseInt(params.get("status"));
+      log = params.get("log");
+    }
+
+    @Override
+    public String toString() {
+      List<String> required = ImmutableList.of(
+          testName, buildTag, revision, hostname, buildConfig, 
Integer.toString(status));
+      List<String> all = new ArrayList<>(required);
+      if (log != null) {
+        all.add(log);
+      }
+      return Joiner.on(" ").join(all);
+    }
+  }
+
+  /**
+   * Mock implementation of the flaky test server.
+   *
+   * Must be a servlet (not just a Jetty handler) to support multipart forms.
+   */
+  private static class MockFlakyTestServlet extends HttpServlet {
+    private static final Logger LOG = 
LoggerFactory.getLogger(MockFlakyTestServlet.class);
+    private final List<TestRecord> records = new ArrayList<>();
+
+    public List<TestRecord> getRecords() {
+      return records;
+    }
+
+    @Override
+    protected void doPost(HttpServletRequest request,
+                          HttpServletResponse response) throws IOException, 
ServletException {
+      LOG.debug("Handling request {}: ", request);
+
+      // Process the form parts into key/value pairs.
+      Map<String, String> params = new HashMap<>();
+      for (Part p : request.getParts()) {
+        params.put(p.getName(), IOUtils.toString(p.getInputStream(), UTF_8));
+      }
+
+      // We're done processing the request.
+      records.add(new TestRecord(params));
+      response.setContentType("text/html; charset=utf-8");
+      response.setStatus(HttpServletResponse.SC_OK);
+    }
+  }
+
+  private static final Logger LOGGER = 
LoggerFactory.getLogger(TestResultReporter.class);
+  private static final String BIND_ADDR = "127.0.0.1";
+  private Server server;
+  private MockFlakyTestServlet flakyTestServlet;
+
+  @Before
+  public void setup() throws Exception {
+    flakyTestServlet = new MockFlakyTestServlet();
+
+    // This Enterprise Java nonsense is to enable multipart form submission. 
The
+    // servlet is configured to only spill parts to disk if they exceed 1 MB in
+    // size, which isn't a concern for this test.
+    ServletContextHandler context = new 
ServletContextHandler(ServletContextHandler.SESSIONS);
+    context.setContextPath("/");
+    ServletHolder holder = new ServletHolder(flakyTestServlet);
+    holder.getRegistration().setMultipartConfig(new MultipartConfigElement(
+        "",            // location
+        1024 * 1024,   // maxFileSize
+        1024 * 1024,   // maxRequestSize
+        1024 * 1024)); // fileSizeThreshold
+    context.addServlet(holder, "/*");
+
+    server = new Server(new InetSocketAddress(BIND_ADDR, 0));
+    server.setHandler(context);
+    server.start();
+  }
+
+  @After
+  public void teardown() throws Exception {
+    server.stop();
+    server.join();
+  }
+
+  @Test
+  public void testRoundTrip() throws IOException {
+    ResultReporter.Options options = new ResultReporter.Options();
+    assertNotNull(server);
+    assertTrue(server.isStarted());
+    assertNotNull(server.getURI());
+    options.httpEndpoint(BIND_ADDR + ":" + server.getURI().getPort())
+           .buildTag("shark")
+           .revision("do")
+           .hostname("do-do")
+           .buildConfig("do-do-do");
+    ResultReporter.Result[] expectedResults = {
+        ResultReporter.Result.SUCCESS, ResultReporter.Result.FAILURE };
+    String[] testNames = { "baby", "mommy", "daddy"};
+    String logFormat = "%s: a log message";
+    ResultReporter reporter = new ResultReporter(options);
+    int expectedRecords = 0;
+    for (ResultReporter.Result result : expectedResults) {
+      for (String testName : testNames) {
+        File tempLogFile = null;
+        if (result == ResultReporter.Result.FAILURE) {
+          tempLogFile = File.createTempFile("test_log", ".txt");
+          tempLogFile.deleteOnExit();
+          FileOutputStream fos = new FileOutputStream(tempLogFile);
+          IOUtils.write(String.format(logFormat, testName), fos, UTF_8);
+        }
+        reporter.reportResult(testName, result, tempLogFile);
+        expectedRecords++;
+      }
+    }
+    assertEquals(expectedRecords, flakyTestServlet.getRecords().size());
+    Iterator<TestRecord> iterator = flakyTestServlet.getRecords().iterator();
+    for (ResultReporter.Result result : expectedResults) {
+      for (String testName : testNames) {
+        assertTrue(iterator.hasNext());
+        TestRecord record = iterator.next();
+        LOGGER.info(record.toString());
+        assertEquals(testName, record.testName);
+        assertEquals(result == ResultReporter.Result.SUCCESS ? 0 : 1, 
record.status);
+        assertEquals(result == ResultReporter.Result.FAILURE ?
+                     String.format(logFormat, testName) : null, record.log);
+      }
+    }
+  }
+
+  @Test
+  public void testHostName() {
+    // Just tests that this doesn't throw an exception.
+    LOGGER.info(ResultReporter.getLocalHostname());
+  }
+}
diff --git 
a/java/kudu-test-utils/src/test/java/org/apache/kudu/test/junit/TestRetryRule.java
 
b/java/kudu-test-utils/src/test/java/org/apache/kudu/test/junit/TestRetryRule.java
index 92693db..b5a3d29 100644
--- 
a/java/kudu-test-utils/src/test/java/org/apache/kudu/test/junit/TestRetryRule.java
+++ 
b/java/kudu-test-utils/src/test/java/org/apache/kudu/test/junit/TestRetryRule.java
@@ -29,8 +29,9 @@ public class TestRetryRule {
   // an assertion exception.
   private int failures = 0;
 
+  // We skip flaky test reporting for this test because it is designed to fail.
   @Rule
-  public RetryRule retryRule = new RetryRule(MAX_FAILURES);
+  public RetryRule retryRule = new RetryRule(MAX_FAILURES, /*skipReporting=*/ 
true);
 
   // Ensure that the RetryRule prevents test failures as long as we don't 
exceed MAX_FAILURES
   // failures.

Reply via email to