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
