On Wed, 23 Nov 2022 23:38:59 GMT, John Hendrikx <jhendr...@openjdk.org> wrote:
>> 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). I've adjusted all of these now to either a check that it is a StubToolkit, or if the variable was actually used, a check if the variable is of the correct type. ------------- PR: https://git.openjdk.org/jfx/pull/959
