RFR: 8314906: [testbug] Create behavior tests for text input controls

[Kevin Rushforth]

On Wed, 23 Aug 2023 23:30:50 GMT, Andy Goryachev <angorya at openjdk.org> wrote:

> Creating the first batch of tests and testing framework that enables writing behavior tests for javafx controls, focusing on key bindings.  The idea is to make writing such tests a simple process.  
> 
> This PR deals with the descendants of TextInputControl (TextField, PasswordField, TextArea).  Most of the tests are headless, but in some cases (TextArea) a headful test is required because the behavior needs rendered text to function (example: page up / page down / line start / line end and the like).
> 
> The tests exercise the key bindings registered by the Skin (or, rather the associated Behavior) at least once, and sometimes more than once.
> 
> Some mappings cannot be tested due to Robot not supporting keypad events (created [JDK-8316307](https://bugs.openjdk.org/browse/JDK-8316307)).
> 
> In addition, the key bindings are documented in /doc-files/behavior markdown documents.

A couple quick comments I spotted while skimming this PR.

modules/javafx.graphics/src/test/java/test/com/sun/javafx/pgstub/StubToolkit.java line 971:

> 969:     public static StubToolkit ensure() {
> 970:         return (StubToolkit)Toolkit.getToolkit();
> 971:     }

This is unused (and may not be needed). I think you can revert the changes to this file.

tests/system/src/test/java/test/javafx/scene/control/behavior/Mod.java line 33:

> 31:  * Key Modifiers for use in behavior tests.
> 32:  */
> 33: public enum Mod {

If this needs to be public (meaning visible to other tests outside this test package), I might recommend a more descriptive name.

tests/system/src/test/java/test/util/Util.java line 67:

> 65:     private static final boolean MAC = OS.startsWith("Mac");
> 66:     private static final boolean LINUX = OS.startsWith("Linux");
> 67: 

This duplicates the logic in PlatformUtil. We just fixed a few other places to get rid of this sort of duplication, so I recommend removing this.

tests/system/src/test/java/test/util/Util.java line 480:

> 478:      */
> 479:     public static boolean isWindows(){
> 480:         return WINDOWS;

Are these really needed or can the tests call PlatformUtil directly? If there is a good reason to add them, then they should delegate to PlatformUtil rather than duplicate the functionality.

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

PR Review: https://git.openjdk.org/jfx/pull/1221#pullrequestreview-1627554862
PR Review Comment: https://git.openjdk.org/jfx/pull/1221#discussion_r1326387332
PR Review Comment: https://git.openjdk.org/jfx/pull/1221#discussion_r1326388978
PR Review Comment: https://git.openjdk.org/jfx/pull/1221#discussion_r1326391012
PR Review Comment: https://git.openjdk.org/jfx/pull/1221#discussion_r1326392044