On Fri, 31 Jan 2025 20:07:38 GMT, Andy Goryachev <[email protected]> wrote:
>> tests/system/src/test/java/test/robot/javafx/scene/NodeInitializationBackgroundThreadTest.java
>> line 305:
>>
>>> 303: return c;
>>> 304: }, (c) -> {
>>> 305: c.show(); // fails here
>>
>> As mentioned in the JBS issue, I think we should consider filing an issue to
>> update the spec of `ComboBoxPopupControl::show` to require calling it on the
>> JavaFX app thread (throwing an exception if called off thread). Either way,
>> we should ensure that the skins themselves never call show unless the
>> control is showing.
>
> yes, we'll deal with later.
> I think there might be a solution which allows show() of an unconnected
> popup, and if that we'll use the ticket to add a javadoc comment and modify
> the test.
Yes, whatever we end up doing will be done as a follow-up.
>> tests/system/src/test/java/test/robot/javafx/scene/NodeInitializationBackgroundThreadTest.java
>> line 779:
>>
>>> 777:
>>> 778: private static boolean nextBoolean() {
>>> 779: return new Random().nextBoolean();
>>
>> It is a best practice when using Random to share a single instance and not
>> allocate a new one each time. It is more performant and more random.
>
> while true, the shared instance will add synchronization and this is
> something I would like to avoid. The degree of randomness is not that
> important in this case.
>
> the only thing that we lose is reproducibility, but in the case of a headful
> test it's probably impossible anyway, due to many other factors.
>
> In any case, I think the main purpose of this test is a sort of sanity check
> of the requirement, not an exhaustive test of all the properties and all the
> methods that the application can call from a background thread - at least
> that what I thought at the time of writing. We can always add more stress to
> it.
OK, that sounds reasonable. Maybe one other thing to consider would be
`ThreadLocalRandom`, but it's probably OK ei ther way.
-------------
PR Review Comment: https://git.openjdk.org/jfx/pull/1690#discussion_r1937984839
PR Review Comment: https://git.openjdk.org/jfx/pull/1690#discussion_r1937987472
