[kudu] branch master updated: [test] Fix handling of assume statements in Java RetryRule

[gr...@apache.org]

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

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


The following commit(s) were added to refs/heads/master by this push:
     new bf03c42  [test] Fix handling of assume statements in Java RetryRule
bf03c42 is described below

commit bf03c423db1569674e9904b2684d4d51feb00be8
Author: Grant Henke <gr...@apache.org>
AuthorDate: Mon Apr 12 13:19:10 2021 -0500

    [test] Fix handling of assume statements in Java RetryRule
    
    The java RetryRule did not treat the AssumptionViolatedException
    thrown by the various assume statements in Junit correctly. This
    can result in test failures which should actually be skipped tests
    and in the case of the flaky test reporting can result in reporting
    failed tests which were actually skipped.
    
    This patch adjusts the handling of the AssumptionViolatedException
    to skip the test correctly and mark the test as a success when
    reporting given there is no “skipped” result we can report to dist-test.
    
    I used the JUnit  TestWatcher rule which I used for reference:
    https://github.com/junit-team/junit4/blob/main/src/main/java/org/junit/rules/TestWatcher.java#L63-L65
    
    Change-Id: I638f56c8e21ed7c57ee576cbc23332e676dba7cf
    Reviewed-on: http://gerrit.cloudera.org:8080/17307
    Reviewed-by: Alexey Serbin <as...@cloudera.com>
    Tested-by: Grant Henke <gr...@apache.org>
---
 .../src/main/java/org/apache/kudu/test/junit/RetryRule.java  | 12 ++++++++++++
 .../test/java/org/apache/kudu/test/junit/TestRetryRule.java  |  9 +++++++++
 2 files changed, 21 insertions(+)

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 8698b21..f4ac3d0 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
@@ -190,6 +190,14 @@ public class RetryRule implements TestRule {
           // and we don't need the logs of successful tests.
           report(ResultReporter.Result.SUCCESS, /*logFile=*/ null);
           return;
+        }  catch (org.junit.internal.AssumptionViolatedException  e) {
+          // The test was skipped because an assumption failed.
+          // A test for which an assumption fails should not generate a test case failure.
+          //
+          // We skip the file upload; this saves space and network bandwidth,
+          // and we don't need the logs of skipped tests.
+          report(ResultReporter.Result.SUCCESS, /*logFile=*/ null);
+          return;
         } catch (Throwable t) {
           // The test failed.
           //
@@ -215,6 +223,10 @@ public class RetryRule implements TestRule {
     private void doOneAttempt(int attempt) throws Throwable {
       try {
         base.evaluate();
+      } catch (org.junit.internal.AssumptionViolatedException e) {
+        // The test was skipped because an assumption failed.
+        // A test for which an assumption fails should not generate a test case failure.
+        LOG.warn("{}: skipped due to failed assumption", humanReadableTestName, e);
       } catch (Throwable t) {
         LOG.error("{}: failed attempt {}", humanReadableTestName, attempt, t);
         throw t;
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 734107a..9617e71 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
@@ -18,6 +18,7 @@
 package org.apache.kudu.test.junit;
 
 import static org.junit.Assert.fail;
+import static org.junit.Assume.assumeTrue;
 
 import org.junit.Rule;
 import org.junit.Test;
@@ -45,4 +46,12 @@ public class TestRetryRule {
     // Pass the test (by not throwing) on the final retry.
   }
 
+  // Ensure that the RetryRule does not cause test failures when
+  // assumeTrue and other similar assumption statements are used.
+  @Test
+  public void testAssumeTrue() {
+    assumeTrue(false);
+    fail("This is unreachable!");
+  }
+
 }