AssertThrows should probably propogate instances of AssertionError or AssumptionViolatedException unless they are also instances of user expected Type

[hossman]

Deep Background: Over in the lucene-solr project we still use junit4, and have our own version of expectThrows that was originally inspired by the corresponding method in some junit4 betas (which -- IIUC -- was renamed assertThrows before being ultimately being refactored out of junit4, in favor of ExpectedException, but does now exist in junit5 as part of AssertThrows ...) and has since evolved and expanded slightly over time and developed variants for supporting multiple expected types (primarily for dealing with various filesystem related exceptions that annoyingly differ based on JVM impl/version, etc...).

Recent Background: We ran into some trouble with test code that looked like this...

  // in a test
  expectThrows(SecurityException.class, () ->
    runWithRestrictedPermissions(this::doSomeForbiddenStuff, /* Some Permissions */));
...
// in our test framework
public static <T> T runWithRestrictedPermissions(PrivilegedExceptionAction<T> action, Permission... permissions) throws Exception {
  assumeTrue("runWithRestrictedPermissions requires a SecurityManager ...",
             System.getSecurityManager() != null);

The problem being that on platforms where the test should have been SKIPed due to the failed assumption, we were actually getting FAILures from expectThrows, because instead of catching the (expected) SecurityExceptionit was catchingAssumptionViolatedException`

Current: We've decided to modify the core bits of code in our various expectThrows impls to use a helper method to better handle this type of situation, with the meant of the code essentially looking like this...

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 Throwable if it matches expectedTypes
  // else Fail w/appropriate message
  //
}
//
// Fail w/appropriate message since no Exception was thrown
//

Future Suggestion: In the interest of being good OSS neighbors, I'm opening this junit issue to suggest that as a project you may want to consider making a similar change to the logic in AssertThrows

[marcphilipp]

Tentatively slated for 5.6 M2 for team discussion.

[marcphilipp]

Team decision: Closed with the same rationale as in #1689.