On Thu, 25 Mar 2021 14:15:47 GMT, Kevin Rushforth <k...@openjdk.org> wrote:
>> modules/javafx.graphics/src/test/java/test/javafx/scene/input/MouseEventTest.java
>> line 139:
>>
>>> 137: case ']':
>>> 138: --bracketCount;
>>> 139: assertTrue("Too many closing brackets: " + str,
>>> bracketCount >= 0);
>>
>> This test can fail due to a malformed `toString` result in the node
>> (`getIntersectedNode()`), which I would think is outside the scope of this
>> test. In practice, this test's result is dependent on what node I choose to
>> test.
>>
>> Shouldn't we be testing the structure of the string and not its contents?
>
> On the one hand, I can see your point that it's somewhat tangential that
> Rectangle has matched brackets, but you could say the same about the Point3D.
> Here is the result of MouseEvent.toString for the test:
>
> MouseEvent [source =
> javafx.event.Event$$Lambda$110/0x0000000800c52fa8@461111c0,
> target = javafx.event.Event$$Lambda$110/0x0000000800c52fa8@461111c0,
> eventType = MOUSE_PRESSED,
> consumed = false, x = 18.0, y = 27.0, z = 150.0, button = PRIMARY,
> primaryButtonDown,
> pickResult = PickResult [node = Rectangle[x=0.0, y=0.0, width=0.0,
> height=0.0, fill=0x000000ff],
> point = Point3D [x = 15.0, y = 25.0, z = 100.0], distance = 33.0]]
>
> The test would be more complicated if it were changed to ignore the results
> of the `toString()` of both the intersected node and the point. If I were to
> go down this path, I would likely just change it to do match a regex of
> `"^MouseEvent [.*PickResult [.*]]$"`, which would be simple. Do you think
> it's worth it?
>
> Maybe instead I could add a comment that this test checks for matching
> brackets in the entire composed string returned by `MouseEvent::toString`,
> including `PickResult::toString`, which in turn includes `Node::toString` for
> the intersected node?
I don't think it matters too much, but a change in any of the classes used here
could break this test, which is a bit off. I will be satisfied with a comment
warning about this case so if it breaks it will be easy to see where the
problem is.
-------------
PR: https://git.openjdk.java.net/jfx/pull/443
