RFR: 8349750: [TestBug] NodeInitializationStressTest: remaining Nodes [v2]

[Kevin Rushforth]

On Tue, 25 Feb 2025 19:25:25 GMT, Andy Goryachev <angorya at openjdk.org> wrote:

>> tests/system/src/test/java/test/robot/javafx/scene/NodeInitializationStressTest.java line 227:
>> 
>>> 225: public class NodeInitializationStressTest extends RobotTestBase {
>>> 226:     /* debugging aid: set this flag to true and comment out assumeFalse(SKIP_TEST) to run specific test(s). */
>>> 227:     private static final boolean SKIP_TEST = false;
>> 
>> Maybe it's time to remove this flag? Does Eclipse not provide a way to run a single test?
>
> Yes, the problem in Eclipse is that it creates a launch configuration which is always incorrect (the dependencies must be modified for each new configuration).
> I'd like to keep it.  There is very little overhead.

OK. Given how long-running this test is I can see the value when debugging or adding / modifying tests.

>> tests/system/src/test/java/test/robot/javafx/scene/NodeInitializationStressTest.java line 656:
>> 
>>> 654: 
>>> 655:     @Test
>>> 656:     public void imageView() {
>> 
>> You mentioned that there was a problem with modifying a writable image on a background thread. Similarly, there might be a problem loading animated GIF images on a background thread. Is there a bug filed?
>
> I guess it works as designed, see ImageView:254
> 
> 
> Toolkit.getImageAccessor().getImageProperty(_image).addListener(platformImageChangeListener.getWeakListener());
> 
> so an animated GIF would most likely be off limits from a background thread as well.

I don't think it's intentional. See [JDK-8222211](https://bugs.openjdk.org/browse/JDK-8222211) where we fixed a problem with not being able to construct an image on a background thread. It's quite possible that the fix for JDK-8222211 is insufficient. So I think a follow-up bug is in order.

A possible fix would be to defer both the animation and setting up image ChangeListener until we know that it is on the FX app thread (similar to what you did for Pagination).

>> tests/system/src/test/java/test/robot/javafx/scene/NodeInitializationStressTest.java line 737:
>> 
>>> 735:             c.setFitHeight(nextDouble(200));
>>> 736:             c.setFitWidth(nextDouble(200));
>>> 737:             //c.setMediaPlayer(new MediaPlayer(new Media("no-data-url-support")));
>> 
>> Would it be useful to load a media file? If so, you could use the default media file that Ensemble uses.
>
> I don't think we want to download media from an URL as part of this test, and data URLs are not supported by Media (and it will be too long anyway).
> 
> I think we can classify this under rubric "one can only create a JavaFX object in a background thread, but not utilize it".

That seems reasonable.

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

PR Review Comment: https://git.openjdk.org/jfx/pull/1713#discussion_r1970558804
PR Review Comment: https://git.openjdk.org/jfx/pull/1713#discussion_r1970568328
PR Review Comment: https://git.openjdk.org/jfx/pull/1713#discussion_r1970576260