RFR: 8185831: Pseudo selectors do not appear to work in Node.lookupAll() [v6]

[Andy Goryachev]

On Fri, 20 Oct 2023 00:16:30 GMT, Sai Pradeep Dandem <duke at openjdk.org> wrote:

>> modules/javafx.graphics/src/test/java/test/javafx/scene/Node_lookup_Test.java line 154:
>> 
>>> 152:      */
>>> 153:     @Test
>>> 154:     public void lookupPseudoTest2() {
>> 
>> the reason I've used different nodes is because the situation you have with existing nodes might give false positive.
>> 
>> I wanted to make sure that selector "a." will collect nodes with ".a" and ".a:pseudo", but in the standard set of nodes with g and hg nodes - but they have the same :testPseudo1, so it's not exactly equivalent.
>> 
>> The other option is to add new nodes, one with ".x" and another with ".x:pseudo"
>> If you do that, it might be easier to change the javadoc format to do something like this:
>> 
>> 
>>  └─ Pane
>>      ├─ VFlow (Pane) .flow
>>      │   ├─ ClippedPane (Pane) .content
>> 
>> (but it's just a very minor suggestion)
>
> Thanks for the clarification. Now I got the point what you are saying about.
> Added a new test case to cover this scenario. Used x, y.. naming to make a clear separation from previous scenarios.
> 
> However do you still prefer to keep the lookupPseudoTest2() scenarios? This is almost similar to quickTest() and lookupAllTest() except that all the nodes used in this method have pseudo classes set.

we could keep it - the more the merrier.

ideally, we could use a more thorough approach: let's say, use two or three layers, and create as many nodes as there are combinations, to test every combination of the selector and search result.

but the main result we got is that there should be no regression w.r.t. the old implementation (I think).
two more people should definitely take a look at this PR still.

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

PR Review Comment: https://git.openjdk.org/jfx/pull/1245#discussion_r1366273861