[
https://issues.apache.org/jira/browse/LUCENE-8999?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Chris M. Hostetter reassigned LUCENE-8999:
------------------------------------------
Fix Version/s: 8.3
master (9.0)
Affects Version/s: 8.2
Assignee: Chris M. Hostetter
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...
{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?
was:
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?
> 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
> Affects Versions: 8.2
> Reporter: Chris M. Hostetter
> Assignee: Chris M. Hostetter
> Priority: Major
> Fix For: master (9.0), 8.3
>
> Attachments: LUCENE-8999.patch
>
>
> 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...
> {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-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org
