[LUCENE-8999] expectThrows doesn't play nicely with "assume" failures

• Type: Test
• Status: Closed
• Priority: Major
• Resolution: Fixed
• Affects Version/s: 8.2
• Fix Version/s: 9.0, 8.3
• Lucene Fields: New
• Assignee: Chris M. Hostetter
• Reporter: Chris M. Hostetter
• Created: 03/Oct/19 17:58
• Updated: 28/Aug/22 15:51
• Resolved: 07/Oct/19 18:54

Description

Up to Lucene 8.2, if expectThrows (or one of it's variants) was given a Runnable that contained an assert/assume call which failed somehwere down it's stack, it would catch these and re-wrap them in a new assertion failure. (unless it matched the "expected" exception type). This would mean that tests which should have been SKIPed due to a bad assumption about the local ENV would instead FAIL.

This issue tracks a change such that expectThrow now directly re-throws any instances of AssertionError or AssumptionViolatedException unless they are instances of the expected exception type specified by the user.

Original jira summary below...

---

Once upon a time, TestRunWithRestrictedPermissions use to have test methods that looked like this...

try {
  runWithRestrictedPermissions(this::doSomeForbiddenStuff);
  fail("this should not pass!");
} catch (SecurityException se) {
  // pass
}

LUCENE-8938 changed this code to look like this...

expectThrows(SecurityException.class, () -> runWithRestrictedPermissions(this::doSomeForbiddenStuff));

But a nuance of the existing code that isn't captured in the new code is that runWithRestrictedPermissions(...) explicitly uses assumeTrue(..., System.getSecurityManager() != null) to ensure that if a security manager is not in use, the test should be SKIPed and not considered a pass or a fail.

The key issue being that assumeTrue(...) (and other 'assume' related methods like it) throws an AssumptionViolatedException when the condition isn't met, expecting this to propagate up to the Test Runner.

With the old code this worked as expected - the AssumptionViolatedException would abort execution before the fail(...) but not be caught by the catch and bubble up all the way to the test runner so the test would be recorded as a SKIP.

With the new code, expectThrows() is catching the AssumptionViolatedException and since it doesn't match the expected SecurityException.class is generating a test failure instead...

   [junit4] Suite: org.apache.lucene.util.TestRunWithRestrictedPermissions
   [junit4]   2> NOTE: download the large Jenkins line-docs file by running 'ant get-jenkins-line-docs' in the lucene directory.
   [junit4]   2> NOTE: reproduce with: ant test  -Dtestcase=TestRunWithRestrictedPermissions -Dtests.method=testCompletelyForbidden2 -Dtests.seed=4181E5FE9E84DBC4 -Dtests.multiplier=2 -Dtests.nightly=true -Dtests.slow=true -Dtests.linedocsfile=/home/jenkins/lucene-data/enwiki.random.lines.txt -Dtests.locale=luy -Dtests.timezone=Etc/GMT-7 -Dtests.asserts=true -Dtests.file.encoding=US-ASCII
   [junit4] FAILURE 0.10s J7  | TestRunWithRestrictedPermissions.testCompletelyForbidden2 <<<
   [junit4]    > Throwable #1: junit.framework.AssertionFailedError: Unexpected exception type, expected SecurityException but got org.junit.AssumptionViolatedException: runWithRestrictedPermissions requires a SecurityManager enabled
   [junit4]    >        at __randomizedtesting.SeedInfo.seed([4181E5FE9E84DBC4:16509163A0E04B41]:0)
   [junit4]    >        at org.apache.lucene.util.LuceneTestCase.expectThrows(LuceneTestCase.java:2729)
   [junit4]    >        at org.apache.lucene.util.LuceneTestCase.expectThrows(LuceneTestCase.java:2718)
   [junit4]    >        at org.apache.lucene.util.TestRunWithRestrictedPermissions.testCompletelyForbidden2(TestRunWithRestrictedPermissions.java:39)
   [junit4]    >        at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
   [junit4]    >        at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
   [junit4]    >        at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
   [junit4]    >        at java.base/java.lang.reflect.Method.invoke(Method.java:566)
   [junit4]    >        at java.base/java.lang.Thread.run(Thread.java:834)
   [junit4]    > Caused by: org.junit.AssumptionViolatedException: runWithRestrictedPermissions requires a SecurityManager enabled
   [junit4]    >        at com.carrotsearch.randomizedtesting.RandomizedTest.assumeTrue(RandomizedTest.java:725)
   [junit4]    >        at org.apache.lucene.util.LuceneTestCase.assumeTrue(LuceneTestCase.java:873)
   [junit4]    >        at org.apache.lucene.util.LuceneTestCase.runWithRestrictedPermissions(LuceneTestCase.java:2917)
   [junit4]    >        at org.apache.lucene.util.TestRunWithRestrictedPermissions.lambda$testCompletelyForbidden2$2(TestRunWithRestrictedPermissions.java:40)
   [junit4]    >        at org.apache.lucene.util.LuceneTestCase.expectThrows(LuceneTestCase.java:2724)
   [junit4]    >        ... 37 more

---

While there might be easy fixes that could be made explicitly to TestRunWithRestrictedPermissions to deal with this particular problem, it seems like perhaps we should consider changes to better deal with this type of problem that might exist elsewhere or occur in the future?

Issue Links

• relates to: LUCENE-8938 Replace try-fail-catch test patterns
• links to: junit5#2050