On Wed, 12 May 2021 23:36:01 GMT, Kevin Rushforth <k...@openjdk.org> wrote:
>> Ajit Ghaisas has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> fix apps and unit tests
>
> modules/javafx.controls/src/test/java/test/javafx/scene/control/cell/ParameterisedPrebuiltCellTest.java
> line 73:
>
>> 71: try {
>> 72: cell = cellClass.getDeclaredConstructor().newInstance();
>> 73: } catch (Exception e) {
>
> This will now catch and ignore runtime exceptions as well, where it
> previously would cause the test to fail. This could end up missing a real
> problem. The two additional checked exceptions, along with any runtime
> exceptions, should cause the test to fail. In fact, I don't know why the
> existing exceptions are ignored (unless there is a compelling reason to
> ignore them that I'm not aware of), so probably the best thing to do would be
> to eliminate the try / catch entirely, and instead add a `throws Exception`
> to the method declaration.
Removed try/catch block and marked the method with `throws Exception`
> modules/javafx.graphics/src/test/java/test/com/sun/javafx/test/binding/ReflectionHelper.java
> line 46:
>
>> 44: try {
>> 45: return cls.getDeclaredConstructor().newInstance();
>> 46: } catch (final Exception e) {
>
> This could re-wrap a `RuntimeException` needlessly, but is OK in this case
> (it's a test and it won't change whether or not it fails). The more
> equivalent fix would be to either list the checked exception individually, or
> to add a `catch (RuntimeException)` before the `catch (Exception)` and
> re-throw the rte without wrapping it.
Avoided the re-wrap of a `RuntimeException` as suggested.
-------------
PR: https://git.openjdk.java.net/jfx/pull/491
