On Tue, 22 Nov 2022 18:41:16 GMT, John Hendrikx <jhendr...@openjdk.org> wrote:
>> modules/javafx.controls/src/test/java/test/javafx/scene/control/AccordionTest.java
>> line 57:
>>
>>> 55:
>>> 56: @Before public void setup() {
>>> 57: tk = Toolkit.getToolkit();//This step is not needed (Just to
>>> make sure StubToolkit is loaded into VM)
>>
>> not equivalent.
>> I wonder if we need an assertTrue here
>
> Not needed in the setup, if it is `null` later or throws an exception the
> test failure is clear enough.
>
> Also a bit out of scope of this PR.
Yes, adding an assert does seem out of scope of this PR, although Andy comment
points out that this is not guaranteed to be behavior neutral. If for some
reason the returned type cannot be cast to StubToolkit, the former code would
throw CCE while the modified code will not. The purpose of that call might have
been merely to ensure that the toolkit was loaded, but it's also possible that
it was intended to ensure that the toolkit was an instance of StubToolkit
(which it will be absent a bug in how the tests are launched...many things will
break if it isn't).
-------------
PR: https://git.openjdk.org/jfx/pull/959
