On Tue, 9 Feb 2021 13:46:15 GMT, Kevin Rushforth <k...@openjdk.org> wrote:
>> Nir Lisker has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Removed printing and narrowed the exception
>
> modules/javafx.base/src/test/java/test/javafx/util/converter/LocalDateTimeStringConverterTest.java
> line 167:
>
>> 165: // force a chronology change with an invalid Japanese date
>> 166: try {
>> 167: System.out.println(converter.toString(LocalDateTime.of(1,
>> 1, 1, 1, 1, 1)));
>
> Since you are expecting an exception, it seems a good idea to fail if you
> didn't get one. Something like:
> fail("Did not get the expected exception");
> Also, you might consider narrowing the `catch` to just the exception that you
> expect, so that an unexpected exception will also fail the test.
The test checks that the chronology didn't change as a result of catching the
exception internally. The thrown exception is from `formatter.parse` later on
and will happen regardless of this fix. The only reason I catch and ignore an
exception here is so that we can reach the assertion line after that.
If we want to test that an exception is thrown, it should be as part of an
input-output test (and we can do the same for other converters).
-------------
PR: https://git.openjdk.java.net/jfx/pull/393
