On Thu, 19 Oct 2023 15:31:38 GMT, Andy Goryachev <ango...@openjdk.org> wrote:
>> Sai Pradeep Dandem has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> 8185831: Fixed code review comments
>
> modules/javafx.graphics/src/test/java/test/javafx/scene/Node_lookup_Test.java
> line 136:
>
>> 134: assertEquals(2, nodes.size());
>> 135: assertTrue(nodes.contains(g));
>> 136: assertTrue(nodes.contains(hg));
>
> I'd add one more test case to verify lack of regression when no pseudo class
> is specified. May be something like this (feel free to expand, add negative
> cases etc.):
>
>
> /**
> * Verifies that the lookup ignores pseudo classes when selector contains
> no explicit pseudo class.
> */
> @Test
> public void lookupPseudoTest2() {
> Group root = new Group();
>
> Group ab = new Group();
> ab.getStyleClass().addAll("a", "b");
>
> Group a = new Group();
> a.getStyleClass().addAll("a");
>
> Group a1 = new Group();
> a1.getStyleClass().addAll("a");
> a1.pseudoClassStateChanged(PseudoClass.getPseudoClass("P1"), true);
>
> ParentShim.getChildren(root).addAll(a, a1, ab);
>
> Set<Node> rv;
>
> rv = root.lookupAll(".a");
> assertTrue(rv.contains(a));
> assertTrue(rv.contains(a1));
>
> rv = root.lookupAll(".a:P1");
> assertTrue(rv.contains(a1));
> }
Added another test case to cover the scenario you mentioned. I used the same
nodes (have pseudo classes set) which are used in previous test case. Tried to
cover all possible scenarios to check for regression.
-------------
PR Review Comment: https://git.openjdk.org/jfx/pull/1245#discussion_r1366237785
