On Wed, 23 Nov 2022 23:27:21 GMT, Kevin Rushforth <k...@openjdk.org> wrote:

>> could we just create a method in Util instead of dragging multi-line 
>> construct across the tests?
>> 
>> e.g. `Util.loadStubToolkit()` ?
>
> Yes, but this is now getting to be a bit out of scope for this PR. Even 
> adding the `assertTrue` requires running all the tests (which thankfully, GHA 
> will do).
> 
> Perhaps a better approach is to not make any more changes here, and file a 
> follow-on bug to A) add a utility method, and B) call it from a 
> `@BeforeClass` method (no need to call it more than once per class).

> I wouldn't recommend that, since it will then obscure the main reason for 
> this call -- to load the toolkit (honestly, the cast was probably an 
> afterthought). So this would be OK:
> 
> ```
>         tk = Toolkit.getToolkit();//This step is not needed (Just to make 
> sure StubToolkit is loaded into VM)
>         assertTrue(tk instanceof StubToolkit);
> ```

Sorry, I should be more clear, I meant to replace it with what you wrote 
(including the comment, although perhaps rephrase that to "Ensure StubToolkit 
is loading into the VM", as the "not needed" bit is clearly false.

As for further optimization with BeforeClass, I think we may be overthinking 
this -- it is just a getter (after the first call), and moving it to 
BeforeClass will require even more changes (the field would need to become 
static).

-------------

PR: https://git.openjdk.org/jfx/pull/959

Reply via email to