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

[Roger Riggs]

Hi Amy,

Ok, fine as is.


My point was that there are now 2 exceptions in the log, one of which is 
an artifact of
the test only to provide the message and useful information is spread 
across those two exceptions.
There is no information lost but there is redundant information that has 
to be evaluated.

Roger


On 5/3/2017 12:18 AM, Amy Lu wrote:
> Thank you Pavel, Paul, Roger and Daniel for your reviewing.
>
> On 5/3/17 5:02 AM, Roger Riggs wrote:
>> But ": Must throw ClassCastException for parameter which is not 
>> Comparable."
>> in Empty navigableMap is much more useful.
>>
>> And wrapping exceptions in exceptions makes debugging much harder, 
>> because
>> it requires unwrapping the context and figuring out what the test was 
>> trying to
>> do and which was the relevant source line.
>
> As the message and exception trace are all kept, it won't make 
> debugging harder.
>
> For example for EmptyNavigableMap.java:
>
> No exception case:
>         assertThrowsCCE(
>             () -> {},
>             description + ": Must throw ClassCastException for 
> parameter which is not Comparable.");
>
> The output:
>
> test 
> EmptyNavigableMap.testSubMap("emptyNavigableMap().descendingMap().descendingMap()", 
> {}): failure
> java.lang.AssertionError: 
> emptyNavigableMap().descendingMap().descendingMap(): Must throw 
> ClassCastException for parameter which is not Comparable.
> Expected ClassCastException to be thrown, but nothing was thrown
>     at EmptyNavigableMap.assertThrows(EmptyNavigableMap.java:76)
>     at EmptyNavigableMap.assertThrowsCCE(EmptyNavigableMap.java:82)
>     at EmptyNavigableMap.testSubMap(EmptyNavigableMap.java:227)
> ...
> Caused by: java.lang.AssertionError: Expected ClassCastException to be 
> thrown, but nothing was thrown
>     at org.testng.Assert.expectThrows(Assert.java:1018)
>     at org.testng.Assert.assertThrows(Assert.java:989)
>     at EmptyNavigableMap.assertThrows(EmptyNavigableMap.java:74)
>     ... 17 more
>
> Wrong exception case:
>         assertThrowsCCE(
>             () -> {
>                 throw new NullPointerException();
>             },
>             description + ": Must throw ClassCastException for 
> parameter which is not Comparable.");
>
> The output:
>
> test 
> EmptyNavigableMap.testSubMap("emptyNavigableMap().descendingMap().descendingMap()", 
> {}): failure
> java.lang.AssertionError: 
> emptyNavigableMap().descendingMap().descendingMap(): Must throw 
> ClassCastException for parameter which is not Comparable.
> Expected ClassCastException to be thrown, but NullPointerException was 
> thrown
>     at EmptyNavigableMap.assertThrows(EmptyNavigableMap.java:76)
>     at EmptyNavigableMap.assertThrowsCCE(EmptyNavigableMap.java:82)
>     at EmptyNavigableMap.testSubMap(EmptyNavigableMap.java:227)
> ...
> Caused by: java.lang.AssertionError: Expected ClassCastException to be 
> thrown, but NullPointerException was thrown
>     at org.testng.Assert.expectThrows(Assert.java:1013)
>     at org.testng.Assert.assertThrows(Assert.java:989)
>     at EmptyNavigableMap.assertThrows(EmptyNavigableMap.java:74)
>     ... 17 more
>
>
>>
>> I should probably try to file a bug against TestNG.assertThrows to 
>> add a variant with
>> a message. Many/most of the TestNG asserts have variants with messages. 
>
> Good point!
>
>> - java/util/Map/Defaults.java:167  The {}'s around the lambda value 
>> are not necessary (like 166).
> I'll remove it when push.
>
> Thanks,
> Amy