You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@lucene.apache.org by "Chris M. Hostetter (Jira)" <ji...@apache.org> on 2019/10/04 00:39:00 UTC

[jira] [Updated] (LUCENE-8999) expectThrows doesn't play nicely with "assume" failures

     [ https://issues.apache.org/jira/browse/LUCENE-8999?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Chris M. Hostetter updated LUCENE-8999:
---------------------------------------
    Attachment: LUCENE-8999.patch
        Status: Open  (was: Open)

Attaching a patch where i started pursuing this idea – not just for AssumptionViolatedException but also for AssertionError, so that if someone has an {{assertEquals(...)}} that fails somewhere down the stack inside their {{expectThrows(Foo.class, ...)}} call, the test will fail with _that_ message, not one that says {{"Unexpected exception type, expected Foo but got AssertionError"}}

The patch also handles the possibility that the caller may explicitly pass {{AssumptionViolatedException.class}} or {{AssertionError.class}} to {{expectThrows(...)}} (something our tests do a surprisingly non-zero number of times) and it still does what the caller would expect: returning the caught AssumptionViolatedException/AssertionError instead of re-throwing it

I haven't run the full test suite yet, but so far it seems to work well ... what do folks think?

/cc [~dawid.weiss], [~rjernst], [~munendrasn], [~gerlowskija]

> expectThrows doesn't play nicely with "assume" failures
> -------------------------------------------------------
>
>                 Key: LUCENE-8999
>                 URL: https://issues.apache.org/jira/browse/LUCENE-8999
>             Project: Lucene - Core
>          Issue Type: Test
>            Reporter: Chris M. Hostetter
>            Priority: Major
>         Attachments: LUCENE-8999.patch
>
>
> Once upon a time, {{TestRunWithRestrictedPermissions}} use to have test methods that looked like this...
> {code:java}
> try {
>   runWithRestrictedPermissions(this::doSomeForbiddenStuff);
>   fail("this should not pass!");
> } catch (SecurityException se) {
>   // pass
> }
> {code}
> LUCENE-8938 changed this code to look like this...
> {code:java}
> expectThrows(SecurityException.class, () -> runWithRestrictedPermissions(this::doSomeForbiddenStuff));
> {code}
> 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...
> {noformat}
>    [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
> {noformat}
> ----
> 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?



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org