You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by ho...@apache.org on 2019/10/07 17:42:05 UTC

[lucene-solr] branch master updated: LUCENE-8999: LuceneTestCase.expectThrows now propogates assert/assumption failures up to the test w/o wrapping in a new assertion failure unless the caller has explicitly expected them

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

hossman pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/lucene-solr.git


The following commit(s) were added to refs/heads/master by this push:
     new 4d0afd4  LUCENE-8999: LuceneTestCase.expectThrows now propogates assert/assumption failures up to the test w/o wrapping in a new assertion failure unless the caller has explicitly expected them
4d0afd4 is described below

commit 4d0afd4afffaeeb652da097ab2c2d44af8cd5083
Author: Chris Hostetter <ho...@apache.org>
AuthorDate: Mon Oct 7 10:41:57 2019 -0700

    LUCENE-8999: LuceneTestCase.expectThrows now propogates assert/assumption failures up to the test w/o wrapping in a new assertion failure unless the caller has explicitly expected them
---
 lucene/CHANGES.txt                                 |   3 +
 .../org/apache/lucene/util/LuceneTestCase.java     | 152 +++++++++++---------
 .../org/apache/lucene/util/TestExpectThrows.java   | 155 +++++++++++++++++++++
 3 files changed, 244 insertions(+), 66 deletions(-)

diff --git a/lucene/CHANGES.txt b/lucene/CHANGES.txt
index 0c6b96a..fdb8402 100644
--- a/lucene/CHANGES.txt
+++ b/lucene/CHANGES.txt
@@ -189,6 +189,9 @@ Other
 
 * LUCENE-8998: Fix OverviewImplTest.testIsOptimized reproducible failure. (Tomoko Uchida)
 
+* LUCENE-8999: LuceneTestCase.expectThrows now propogates assert/assumption failures up to the test
+  w/o wrapping in a new assertion failure unless the caller has explicitly expected them (hossman)
+
 ======================= Lucene 8.2.0 =======================
 
 API Changes
diff --git a/lucene/test-framework/src/java/org/apache/lucene/util/LuceneTestCase.java b/lucene/test-framework/src/java/org/apache/lucene/util/LuceneTestCase.java
index c0083b9..46000ff 100644
--- a/lucene/test-framework/src/java/org/apache/lucene/util/LuceneTestCase.java
+++ b/lucene/test-framework/src/java/org/apache/lucene/util/LuceneTestCase.java
@@ -115,6 +115,7 @@ import org.junit.Test;
 import org.junit.rules.RuleChain;
 import org.junit.rules.TestRule;
 import org.junit.runner.RunWith;
+import org.junit.internal.AssumptionViolatedException;
 
 import com.carrotsearch.randomizedtesting.JUnit4MethodProvider;
 import com.carrotsearch.randomizedtesting.LifecycleScope;
@@ -2720,17 +2721,16 @@ public abstract class LuceneTestCase extends Assert {
 
   /** Checks a specific exception class is thrown by the given runnable, and returns it. */
   public static <T extends Throwable> T expectThrows(Class<T> expectedType, String noExceptionMessage, ThrowingRunnable runnable) {
-    try {
-      runnable.run();
-    } catch (Throwable e) {
-      if (expectedType.isInstance(e)) {
-        return expectedType.cast(e);
-      }
-      AssertionFailedError assertion = new AssertionFailedError("Unexpected exception type, expected " + expectedType.getSimpleName() + " but got " + e);
-      assertion.initCause(e);
-      throw assertion;
+    final Throwable thrown = _expectThrows(Collections.singletonList(expectedType), runnable);
+    if (expectedType.isInstance(thrown)) {
+      return expectedType.cast(thrown);
     }
-    throw new AssertionFailedError(noExceptionMessage);
+    if (null == thrown) {
+      throw new AssertionFailedError(noExceptionMessage);
+    }
+    AssertionFailedError assertion = new AssertionFailedError("Unexpected exception type, expected " + expectedType.getSimpleName() + " but got " + thrown);
+    assertion.initCause(thrown);
+    throw assertion;
   }
 
   /** Checks a specific exception class is thrown by the given runnable, and returns it. */
@@ -2739,16 +2739,13 @@ public abstract class LuceneTestCase extends Assert {
       throw new AssertionError("At least one expected exception type is required?");
     }
 
-    Throwable thrown = null;
-    try {
-      runnable.run();
-    } catch (Throwable e) {
+    final Throwable thrown = _expectThrows(expectedTypes, runnable);
+    if (null != thrown) {
       for (Class<? extends T> expectedType : expectedTypes) {
-        if (expectedType.isInstance(e)) {
-          return expectedType.cast(e);
+        if (expectedType.isInstance(thrown)) {
+          return expectedType.cast(thrown);
         }
       }
-      thrown = e;
     }
 
     List<String> exceptionTypes = expectedTypes.stream().map(c -> c.getSimpleName()).collect(Collectors.toList());
@@ -2771,29 +2768,28 @@ public abstract class LuceneTestCase extends Assert {
    */
   public static <TO extends Throwable, TW extends Throwable> TW expectThrows
   (Class<TO> expectedOuterType, Class<TW> expectedWrappedType, ThrowingRunnable runnable) {
-    try {
-      runnable.run();
-    } catch (Throwable e) {
-      if (expectedOuterType.isInstance(e)) {
-        Throwable cause = e.getCause();
-        if (expectedWrappedType.isInstance(cause)) {
-          return expectedWrappedType.cast(cause);
-        } else {
-          AssertionFailedError assertion = new AssertionFailedError
-              ("Unexpected wrapped exception type, expected " + expectedWrappedType.getSimpleName() 
-                  + " but got: " + cause);
-          assertion.initCause(e);
-          throw assertion;
-        }
+    final Throwable thrown = _expectThrows(Collections.singletonList(expectedOuterType), runnable);
+    if (null == thrown) {
+      throw new AssertionFailedError("Expected outer exception " + expectedOuterType.getSimpleName()
+                                     + " but no exception was thrown.");
+    }
+    if (expectedOuterType.isInstance(thrown)) {
+      Throwable cause = thrown.getCause();
+      if (expectedWrappedType.isInstance(cause)) {
+        return expectedWrappedType.cast(cause);
+      } else {
+        AssertionFailedError assertion = new AssertionFailedError
+          ("Unexpected wrapped exception type, expected " + expectedWrappedType.getSimpleName() 
+           + " but got: " + cause);
+        assertion.initCause(thrown);
+        throw assertion;
       }
-      AssertionFailedError assertion = new AssertionFailedError
-          ("Unexpected outer exception type, expected " + expectedOuterType.getSimpleName()
-           + " but got: " + e);
-      assertion.initCause(e);
-      throw assertion;
     }
-    throw new AssertionFailedError("Expected outer exception " + expectedOuterType.getSimpleName()
-        + " but no exception was thrown.");
+    AssertionFailedError assertion = new AssertionFailedError
+      ("Unexpected outer exception type, expected " + expectedOuterType.getSimpleName()
+       + " but got: " + thrown);
+    assertion.initCause(thrown);
+    throw assertion;
   }
 
   /**
@@ -2805,41 +2801,65 @@ public abstract class LuceneTestCase extends Assert {
    */
   public static <TO extends Throwable, TW extends Throwable> TO expectThrowsAnyOf
   (LinkedHashMap<Class<? extends TO>,List<Class<? extends TW>>> expectedOuterToWrappedTypes, ThrowingRunnable runnable) {
-    try {
-      runnable.run();
-    } catch (Throwable e) {
-      for (Map.Entry<Class<? extends TO>, List<Class<? extends TW>>> entry : expectedOuterToWrappedTypes.entrySet()) {
-        Class<? extends TO> expectedOuterType = entry.getKey();
-        List<Class<? extends TW>> expectedWrappedTypes = entry.getValue();
-        Throwable cause = e.getCause();
-        if (expectedOuterType.isInstance(e)) {
-          if (expectedWrappedTypes.isEmpty()) {
-            return null; // no wrapped exception
-          } else {
-            for (Class<? extends TW> expectedWrappedType : expectedWrappedTypes) {
-              if (expectedWrappedType.isInstance(cause)) {
-                return expectedOuterType.cast(e);
-              }
+    final List<Class<? extends TO>> outerClasses = expectedOuterToWrappedTypes.keySet().stream().collect(Collectors.toList());
+    final Throwable thrown = _expectThrows(outerClasses, runnable);
+    
+    if (null == thrown) {
+      List<String> outerTypes = outerClasses.stream().map(Class::getSimpleName).collect(Collectors.toList());
+      throw new AssertionFailedError("Expected any of the following outer exception types: " + outerTypes
+                                     + " but no exception was thrown.");
+    }
+    for (Map.Entry<Class<? extends TO>, List<Class<? extends TW>>> entry : expectedOuterToWrappedTypes.entrySet()) {
+      Class<? extends TO> expectedOuterType = entry.getKey();
+      List<Class<? extends TW>> expectedWrappedTypes = entry.getValue();
+      Throwable cause = thrown.getCause();
+      if (expectedOuterType.isInstance(thrown)) {
+        if (expectedWrappedTypes.isEmpty()) {
+          return null; // no wrapped exception
+        } else {
+          for (Class<? extends TW> expectedWrappedType : expectedWrappedTypes) {
+            if (expectedWrappedType.isInstance(cause)) {
+              return expectedOuterType.cast(thrown);
             }
-            List<String> wrappedTypes = expectedWrappedTypes.stream().map(Class::getSimpleName).collect(Collectors.toList());
-            AssertionFailedError assertion = new AssertionFailedError
-                ("Unexpected wrapped exception type, expected one of " + wrappedTypes + " but got: " + cause);
-            assertion.initCause(e);
-            throw assertion;
           }
+          List<String> wrappedTypes = expectedWrappedTypes.stream().map(Class::getSimpleName).collect(Collectors.toList());
+          AssertionFailedError assertion = new AssertionFailedError
+            ("Unexpected wrapped exception type, expected one of " + wrappedTypes + " but got: " + cause);
+          assertion.initCause(thrown);
+          throw assertion;
         }
       }
-      List<String> outerTypes = expectedOuterToWrappedTypes.keySet().stream().map(Class::getSimpleName).collect(Collectors.toList());
-      AssertionFailedError assertion = new AssertionFailedError
-          ("Unexpected outer exception type, expected one of " + outerTypes + " but got: " + e);
-      assertion.initCause(e);
-      throw assertion;
     }
-    List<String> outerTypes = expectedOuterToWrappedTypes.keySet().stream().map(Class::getSimpleName).collect(Collectors.toList());
-    throw new AssertionFailedError("Expected any of the following outer exception types: " + outerTypes
-        + " but no exception was thrown.");
+    List<String> outerTypes = outerClasses.stream().map(Class::getSimpleName).collect(Collectors.toList());
+    AssertionFailedError assertion = new AssertionFailedError
+      ("Unexpected outer exception type, expected one of " + outerTypes + " but got: " + thrown);
+    assertion.initCause(thrown);
+    throw assertion;
   }
 
+  /**
+   * Helper method for {@link #expectThrows} and {@link #expectThrowsAnyOf} that takes care of propagating
+   * any {@link AssertionError} or {@link AssumptionViolatedException} instances thrown if and only if they 
+   * are super classes of the <code>expectedTypes</code>.  Otherwise simply returns any {@link Throwable} 
+   * thrown, regardless of type, or null if the <code>runnable</code> completed w/o error.
+   */
+  private static Throwable _expectThrows(List<? extends Class<?>> expectedTypes, ThrowingRunnable runnable) {
+                                         
+    try {
+      runnable.run();
+    } catch (AssertionError | AssumptionViolatedException ae) {
+      for (Class<?> expectedType : expectedTypes) {
+        if (expectedType.isInstance(ae)) { // user is expecting this type explicitly
+          return ae;
+        }
+      }
+      throw ae;
+    } catch (Throwable e) {
+      return e;
+    }
+    return null;
+  }
+  
   /** Returns true if the file exists (can be opened), false
    *  if it cannot be opened, and (unlike Java's
    *  File.exists) throws IOException if there's some
diff --git a/lucene/test-framework/src/test/org/apache/lucene/util/TestExpectThrows.java b/lucene/test-framework/src/test/org/apache/lucene/util/TestExpectThrows.java
new file mode 100644
index 0000000..cfc70be
--- /dev/null
+++ b/lucene/test-framework/src/test/org/apache/lucene/util/TestExpectThrows.java
@@ -0,0 +1,155 @@
+/*
+ * 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.lucene.util;
+
+import java.util.concurrent.atomic.AtomicBoolean;
+import java.io.IOException;
+
+import org.junit.internal.AssumptionViolatedException;
+  
+public class TestExpectThrows extends LuceneTestCase {
+
+  private static class HuperDuperException extends IOException {
+    public HuperDuperException() {
+      /* No-Op */
+    }
+  }
+  
+  /** 
+   * Tests that {@link #expectThrows} behaves correctly when the Runnable throws (an 
+   * instance of a subclass of) the expected Exception type: by returning that Exception.
+   */
+  public void testPass() {
+    final AtomicBoolean ran = new AtomicBoolean(false);
+    final IOException returned = expectThrows(IOException.class, () -> {
+        ran.getAndSet(true);
+        throw new HuperDuperException();
+      });
+    assertTrue(ran.get());
+    assertNotNull(returned);
+    assertEquals(HuperDuperException.class, returned.getClass());
+  }
+  
+  /** 
+   * Tests that {@link #expectThrows} behaves correctly when the Runnable does not throw (an 
+   * instance of a subclass of) the expected Exception type: by throwing an assertion to 
+   * <code>FAIL</code> the test.
+   */
+  public void testFail() {
+    final AtomicBoolean ran = new AtomicBoolean(false);
+    AssertionError caught = null;
+    try {
+      final IOException returned = expectThrows(IOException.class, () -> {
+          ran.getAndSet(true);
+        });
+      fail("must not complete"); // NOTE: we don't use expectThrows to test expectThrows
+    } catch (AssertionError ae) {
+      caught = ae;
+    }
+    assertTrue(ran.get());
+    assertNotNull(caught);
+    assertEquals("Expected exception IOException but no exception was thrown", caught.getMessage());
+                 
+  }
+
+  /** 
+   * Tests that {@link #expectThrows} behaves correctly when the Runnable contains an  
+   * assertion that does not pass: by allowing that assertion to propogate and 
+   * <code>FAIL</code> the test.
+   */
+  public void testNestedFail() {
+    final AtomicBoolean ran = new AtomicBoolean(false);
+    AssertionError caught = null;
+    try {
+      final IOException returned = expectThrows(IOException.class, () -> {
+          ran.getAndSet(true);
+          fail("this failure should propogate");
+        });
+      fail("must not complete"); // NOTE: we don't use expectThrows to test expectThrows
+    } catch (AssertionError ae) {
+      caught = ae;
+    }
+    assertTrue(ran.get());
+    assertNotNull(caught);
+    assertEquals("this failure should propogate", caught.getMessage());
+  }
+  
+  /** 
+   * Tests that {@link #expectThrows} behaves correctly when the Runnable contains an 
+   * assumption that does not pass: by allowing that assumption to propogate and cause 
+   * the test to <code>SKIP</code>.
+   */
+  public void testNestedAssume() {
+    final AtomicBoolean ran = new AtomicBoolean(false);
+    AssumptionViolatedException caught = null;
+    try {
+      final IOException returned = expectThrows(IOException.class, () -> {
+          ran.getAndSet(true);
+          assumeTrue("this assumption should propogate", false);
+        });
+      fail("must not complete"); // NOTE: we don't use expectThrows to test expectThrows
+    } catch (AssumptionViolatedException ave) {
+      caught = ave;
+    }
+    assertTrue(ran.get());
+    assertNotNull(caught);
+    assertEquals("this assumption should propogate", caught.getMessage());
+  }
+
+  /** 
+   * Tests that {@link #expectThrows} behaves correctly when the Runnable contains an  
+   * assertion that does not pass but the caller has explicitly said they expect an Exception of that type:
+   * by returning that assertion failure Exception.
+   */
+  public void testExpectingNestedFail() {
+    final AtomicBoolean ran = new AtomicBoolean(false);
+    AssertionError returned = null;
+    try {
+      returned = expectThrows(AssertionError.class, () -> {
+          ran.getAndSet(true);
+          fail("this failure should be returned, not propogated");
+        });
+    } catch (AssertionError caught) { // NOTE: we don't use expectThrows to test expectThrows
+      assertNull("An exception should not have been thrown", caught);
+    }
+    assertTrue(ran.get());
+    assertNotNull(returned);
+    assertEquals("this failure should be returned, not propogated", returned.getMessage());
+  }
+  
+  /** 
+   * Tests that {@link #expectThrows} behaves correctly when the Runnable contains an 
+   * assumption that does not pass but the caller has explicitly said they expect an Exception of that type: 
+   * by returning that assumption failure Exception.
+   */
+  public void testExpectingNestedAssume() {
+    final AtomicBoolean ran = new AtomicBoolean(false);
+    AssumptionViolatedException returned = null;
+    try {
+      returned = expectThrows(AssumptionViolatedException.class, () -> {
+          ran.getAndSet(true);
+          assumeTrue("this assumption should be returned, not propogated", false);
+        });
+    } catch (AssumptionViolatedException caught) { // NOTE: we don't use expectThrows to test expectThrows
+      assertNull("An exception should not have been thrown", caught);
+    }
+    assertTrue(ran.get());
+    assertNotNull(returned);
+    assertEquals("this assumption should be returned, not propogated", returned.getMessage());
+  }
+  
+}