On Mon, 23 Mar 2026 21:09:10 GMT, Marius Hanl <[email protected]> wrote:
>> modules/javafx.controls/src/test/java/test/com/sun/javafx/scene/control/infrastructure/MouseEventFirer.java
>> line 138:
>>
>>> 136:
>>> 137: if (node.getScene() == null || node.getScene().getWindow() ==
>>> null) {
>>> 138: throw new IllegalArgumentException("Node: " + node + "
>>> must be in a Scene and Window " +
>>
>> maybe instruct to use StageLoader instead, addin an example in the comments?
>> something like this:
>>
>> // The target node must be in a Scene and Window, example:
>> // private StageLoader sl;
>> // ..
>> // sl = new StageLoader(node);
>> // ...
>> // sl.dispose()
>> if (node.getScene() == null || node.getScene().getWindow() == null) {
>> throw new IllegalArgumentException("Target node must be in a
>> Scene and Window (use StageLoader?)");
>
> I thought about this as well, but my future plan (sneakpeak :)) is to rather
> have an abstract class that provides all needed methods, including the
> `stageLoader` cleanup and exception handler setup. And all controls test
> extend it. Saves us a lot of duplicate code, and if we need to change
> something there, we can do it just once.
>
> But the biggest benefit why I want to push this is to make it easier to write
> tests for new contributors, as there is essentially only one way to spin up a
> stage, fire mouse events, all in one class and used in every test. No
> ambiguity.
>
> And in this case, we would not really need this comment (I want to document
> everything on the abstract class then). If that seems good for you too, I
> would like to move on with that one time and rather do not clutter this class
> (or other test classes, effectively spreading all information around
> different test classes).
(For reference, I followed the same message as in:
https://github.com/openjdk/jfx/pull/1829)
-------------
PR Review Comment: https://git.openjdk.org/jfx/pull/2124#discussion_r2977618456
