Usage of Java assertions in JTreg tests?

[Jonathan Gibbons]

On 11/16/22 5:48 AM, Volker Simonis wrote:
> On Tue, Nov 15, 2022 at 6:41 PM Alan Bateman <Alan.Bateman at oracle.com> wrote:
>> On 15/11/2022 17:19, Volker Simonis wrote:
>>> Hi,
>>>
>>> I recently stumbled upon
>>> test/jdk/java/lang/invoke/JavaDocExamplesTest.java which uses a lot of
>>> Java assertions like this:
>>>
>>> for (int n = 0; n <= 10; n++) {
>>>     Object[] badArityArgs = (n == 2 ? new Object[0] : new Object[n]);
>>>     try { assert((boolean) eq2.invokeExact(badArityArgs) && false); }
>>>     catch (IllegalArgumentException ex) { } // OK
>>> }
>>>
>>> The intent of this test is to verify that invokeExact() always throws
>>> an exception because otherwise the assertion will fire. Unfortunately,
>>> this test only does something at all if JVM is running with "-ea"
>>> which is not the default. In the standard configuration, all the code
>>> inside the "assert(..)" will just be skipped and the test always
>>> passes.
>>>
>>> This example (and a lot more under test/jdk/java/lang/invoke/) is
>>> there since JDK 7. The tests also use testNG assertions a lot (e.g.
>>> "assertEqual()", "assertTrue()", etc.) so I wonder if the plain java
>>> asserts shown in the example above are just typos and should be
>>> replaced by application level testNG assertions? What are these tests
>>> good for otherwise if they won't be executed most of the time?
>>>
>> I don't know the history of this test to know if it really meant to rely
>> on running with -ea or not.  Given that it has assertXXX methods then it
>> may have been partly converted to TestNG at some point. However, I don't
>> think they are as useless as it might seem. RunTests.gmk (the make file
>> that is used when use you `make run-test ...`) adds -ea -esa to the
>> command line when running non-hotspot tests so the assertions are
>> enabled, which is useful when running tests in areas that do make use of
>> assert.
> Thanks for the info. I usually don't run the tests through the make target.
>
> I think testing would be much more reliable if all the tests which
> depend on Java assertion would explicitly run with assertions  enabled
> (i.e. "@run main/othervm -ea"). Would you agree to such a change?

We should not change tests to use `/othervm` just to enable assertions.
That would have a negative impact on performance, if the tests are
normally run in agent mode.

It may be worth auditing the tests to see how many tests use `assert`
and which may be run without assertions enabled. Depending on that
info, we can decide a better course of action.

-- Jon


>
> A lot of tests already do this (e.g. test/jdk/jdk/incubator/vector/)
> but I also found completely useless hotspot tests which rely on
> assertions but by default run without "-ea" even if started from the
> "make test" target (e.g.
> test/hotspot/jtreg/compiler/c2/TestSignExtract.java). They should
> definitely be fixed. If you don't mind, I'll create JBS issues for
> these tasks.
>
> Best regards,
> Volker
>
>> -Alan