JDK 9 RFR of JDK-8023897: Replace/update/rename executeAndCatch in various tests to assertThrows

[Roger Riggs]

Hi Amy,

Yes, a good cleanup.

- EmptyNavigableMap.java in assertThrows(class, runnable, message) will be a
   bit awkward to debug since the stack trace of the real cause will be 
buried in
   the cause of the new AssertionError thrown.

Without re-implementing the TestNG.expectThrows[1] method, I would be 
more inclined
to print the message and then re-throw the exception.
Still not best, but doesn't require knowing to look in the exception 
cause to find the
location of the original throw.

Ditto EmptyNavigableSet.java:72

The messages are useful when debugging to know the reason for the 
exception without
having to reverse engineer the code.


- java/util/Map/Defaults.java:167  The  {}'s around the lambda value are 
not necessary (like 166).

Thanks, Roger

[1] 
https://github.com/cbeust/testng/blob/master/src/main/java/org/testng/Assert.java 
: 1119

On 5/2/2017 7:15 AM, Pavel Rappo wrote:
> Hi Amy,
>
> Thanks a lot for doing this! I personally think it's a beginning of a very
> valuable refactoring to our test base. It turns out this little piece of
> functionality, namely, checking that a particular exception is thrown from code
> is error-prone. I have seen many buggy implementations defective in one way or
> another. What's worse is that sometimes many of them coexist in the same code
> base. Even in the JDK this beast has many names:
>
>      tryCatch, verifyException, expectThrows, checkException, expectThrow,
>      assertThrows, THROWS, check, uncaughtException, testThrowException,
>      assertSecurityException, ...
>
> This list does not include countless ad-hoc bare try-catch constructs. After
> this has been done, I would go even further, and in some cases change tests that
> use @Test(expectedException=...) annotation to use this method instead.
>
> Looks good. Wait for a Reviewer and you are good to go.
>
> Thanks,
> -Pavel
>
>> On 2 May 2017, at 10:19, Amy Lu <amy.lu at oracle.com> wrote:
>>
>> Please review this test-only change.
>>
>> Some java/util tests use a function executeAndCatch which essentially asserts that an exception is thrown, while some other tests use assertThrows for doing the same work. For both cases, with jtreg upgraded to testng 6.9.5 (CODETOOLS-7901639), test can then leverage TestNG Assert.assertThrows
>>
>> Please review the patch to update executeAndCatch and assertThrows to Assert.assertThrows for java/util testng tests.
>>
>> bug: https://bugs.openjdk.java.net/browse/JDK-8023897
>> webrev: http://cr.openjdk.java.net/~amlu/8023897/webrev.00/
>>
>> Thanks,
>> Amy
>>