On Fri, 4 Apr 2025 18:31:33 GMT, Andy Goryachev <ango...@openjdk.org> wrote:
>> This change rewrites `Popup_parentWindow_Test` after a-long-time-ago-done
>> changes to Popup public API.
>>
>> Popup used to have an `owner` and `parentWindow` properties. I couldn't
>> track how far back that change happened so I can't fully say what these
>> Properties actually were, but to my knowledge both properties were replaced
>> by `ownerNode` and `ownerWindow` Properties respectively. New Properties are
>> read-only and are set while calling respective `Popup.show()` -
>> `Popup.show(Node, ...)` sets both Properties, while `Popup.show(Window,
>> ...)` sets only `ownerWindow` Property.
>>
>> A lot of original test scenarios were cut out because with current Popup API
>> they make little to no sense. Original `Popup_parentWindow_Test` extended
>> `PropertiesTestBase` and ran the regular Property access checks. Those are
>> parametrized and split into four test methods:
>> - `testGetBean` - confidence check for whether we can get the Property bean,
>> unnecessary since new tests access the Properties directly
>> - `testGetName` - Properties in `PropertiesTestBase` are referred to by
>> String-provided name and Reflection, so this was a confidence check whether
>> those Properties actually exist in the object - unnecessary, since we access
>> the Properties directly
>> - `testBinding` - I deemed those not doable, since the Properties we try to
>> access are read-only and cannot be bound to
>> - `testBasicAccess` - This is the only test I found that was workable into a
>> reimplementation, and so it is manually reimplemented as `Popup_owner_Test`
>>
>> Moreover, because of lack of `Popup.setParent()` API I also deemed more
>> complex test scenarios (cases referring to TestScene's `_window` Property)
>> unnecessary. Since there is no `Popup.show(Scene, ...)` those cases were
>> also eliminated. This leaves current test pool into three separate
>> parameters for one, similar test case.
>
> modules/javafx.graphics/src/test/java/test/javafx/stage/Popup_owner_Test.java
> line 55:
>
>> 53: * Instead we rely on checking this manually.
>> 54: */
>> 55: public final class Popup_owner_Test {
>
> minor: maybe `Popup_Owner_Test`?
We do seem to have this funny-looking `PascalCase_camelCase_Test` convention in
several places.
-------------
PR Review Comment: https://git.openjdk.org/jfx/pull/1763#discussion_r2029799082
