On Tue, 22 Nov 2022 19:04:00 GMT, Kevin Rushforth <k...@openjdk.org> wrote:
>> 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). This looks like bad test design. If the toolkit needs to be of a specific type when running tests, then check that this is the case once before running all the tests. As for this PR, I don't mind leaving it as is or reverting these changes. I don't think that a CCE is realistic here. ------------- PR: https://git.openjdk.org/jfx/pull/959
