RFR: 8348423: [TestBug] stress test Nodes initialization from a background thread [v2]

[Kevin Rushforth]

On Fri, 31 Jan 2025 20:07:38 GMT, Andy Goryachev <angorya at openjdk.org> 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