On Fri, 20 Oct 2023 11:42:03 GMT, Alexey Ivanov <[email protected]> wrote:
>> test/jdk/java/awt/event/MouseWheelEvent/WheelModifier/WheelModifier.java
>> line 173:
>>
>>> 171: test.run();
>>> 172: } finally {
>>> 173: SwingUtilities.invokeAndWait(test.f::dispose);
>>
>> It would be good to ensure that f is not NULL before dispose.
>
>> It would be good to ensure that f is not NULL before dispose.
>
> I know we're doing it all the time… But is it really relevant?
>
> This has been bothering me for a while now… Let's clear things up.
>
> If the frame is `null`, something has gone awry. If the test code itself
> didn't throw the exception, we'll get:
>
>
> Exception in thread "main" java.lang.NullPointerException
> at WheelModifier.main(WheelModifier.java:173)
>
>
> The bad thing is that any exception thrown from the test itself — in the
> `try` block — gets overridden by the exception thrown from the `finally`
> block. Thus, we get the same exception as above.
>
> The overall result is that the test fails, yet it may hide the real reason,
> which makes it harder to analyse the root cause of the failure.
>
> Taking into account the above, *`null`-check is good even though it makes the
> code longer.*
Yes, if we have a null frame, I'd like to think that there's already an
exception thrown by a well written test,
and a finally block isn't normally the place you'd want to have an accidental
exception generated.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/16281#discussion_r1367502914
