8319555: [TestBug] Utility for creating instruction window for manual tests

[andy-goryachev-oracle]

trafficstars

ManualTestWindow

This facility provides the base class for manual tests which displays the test instructions, the UI under test, and the Pass/Fail buttons.

Example:

public class ManualTestExample extends ManualTestWindow {
    public ManualTestExample() {
        super(
            "Manual Test Example",
            """
            Instructions:
            1. you will see a button named "Test"
            2. press the button
            3. verify that the button can be pressed""",
            400, 250
        );
    }

    public static void main(String[] args) throws Exception {
        launch(args);
    }

    @Override
    protected Node createContent() {
        return new Button("Test");
    }
}

Resulting application window:

Readme:

https://github.com/andy-goryachev-oracle/jfx/blob/8319555.manual/tests/manual/util/README.md

@prrace 's test EmojiTest has been converted to use the new test window as a demonstration (also fixed the Eclipse project so it works now).

Q: What other features can be added to the test window?

Edit: the sources are left in their original place at the root of the project.

---

Progress

• [x] Change must not contain extraneous whitespace
• [x] Commit message must refer to an issue
• [ ] Change must be properly reviewed (2 reviews required, with at least 1 Reviewer, 1 Author)

Warning

 ⚠️ Patch contains a binary file (tests/manual/util/doc/ManualTestWindow.png)

Issue

• JDK-8319555: [TestBug] Utility for creating instruction window for manual tests (Enhancement - P4)

Reviewers

• Alexander Matveev (@sashamatveev - Reviewer)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jfx.git pull/1413/head:pull/1413
$ git checkout pull/1413

Update a local copy of the PR:
$ git checkout pull/1413
$ git pull https://git.openjdk.org/jfx.git pull/1413/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 1413

View PR using the GUI difftool:
$ git pr show -t 1413

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jfx/pull/1413.diff

Webrev

Link to Webrev Comment

[bridgekeeper[bot]]

:wave: Welcome back angorya! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

[openjdk[bot]]

❗ This change is not yet ready to be integrated. See the Progress checklist in the description for automated requirements.

[mlbridge[bot]]

Webrevs

• 08: Full - Incremental (e4c8d29a)
• 07: Full - Incremental (45e53102)
• 06: Full - Incremental (a8fc661b)
• 05: Full - Incremental (76a4dc8c)
• 04: Full - Incremental (dbcdb542)
• 03: Full - Incremental (a106587a)
• 02: Full - Incremental (66d7d0d0)
• 01: Full - Incremental (22da890b)
• 00: Full (1cc09504)

[kevinrushforth]

Not a review, but just a general comment or two.

EmojiTest has been converted to use the new test window as a demonstration

That seems reasonable, but...

(also fixed the Eclipse project and moved sources to src/, still using the default package).

I'm not sure how much value there is in moving the files under a "src" directory without also providing a build script (ant or gradle) to be able to build it. We don't currently have the manual tests wired up to the build, so there is no good way to depend on anything outside of the directory in question (at least not without using an IDE, which is a developer convenience, not a build system). That should probably be solved first.

Q: Do we want to avoid using the default package?

For all manual tests? Probably not.

[andy-goryachev-oracle]

I'm not sure how much value there is in moving the files under a "src" directory

Thank you Kevin for the feedback! I had problems configuring the project with sources being in the root, but will take another look.

[openjdk[bot]]

@andy-goryachev-oracle Please do not rebase or force-push to an active PR as it invalidates existing review comments. Note for future reference, the bots always squash all changes into a single commit automatically as part of the integration. See OpenJDK Developers’ Guide for more information.

[andy-goryachev-oracle]

Moved the sources back to the root.

Eclipse has a weird behavior when it comes to modular projects or mixing modular and classpath sources: the project which has been modified (or converted to modular or back) needs to be closed and re-opened, otherwise Eclipse loses its marbles. Both 2023-06 and 2024-03 versions seem to exhibit this behavior.

[andy-goryachev-oracle]

@mstr2 @jperedadnr @sashamatveev @aghaisas would you gentlemen be interested in reviewing this?

[kevinrushforth]

This could use a second pair of eyes. I don't have time right now, but I'll add a couple comments.

/reviewers 2

[openjdk[bot]]

@kevinrushforth The total number of required reviews for this PR (including the jcheck configuration and the last /reviewers command) is now set to 2 (with at least 1 Reviewer, 1 Author).

[andy-goryachev-oracle]

Thank you all for comments and suggestions! Changed to extend Application. See if you like this better.

[openjdk[bot]]

⚠️ @andy-goryachev-oracle This pull request contains merges that bring in commits not present in the target repository. Since this is not a "merge style" pull request, these changes will be squashed when this pull request in integrated. If this is your intention, then please ignore this message. If you want to preserve the commit structure, you must change the title of this pull request to Merge <project>:<branch> where <project> is the name of another project in the OpenJDK organization (for example Merge jdk:master).

[andy-goryachev-oracle]

it's in the test code, but ok - removed commented out code. thanks!

[bridgekeeper[bot]]

@andy-goryachev-oracle This pull request has been inactive for more than 4 weeks and will be automatically closed if another 4 weeks passes without any activity. To avoid this, simply add a new comment to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration!

[andy-goryachev-oracle]

keep-alive

[mstr2]

I run manual tests on the command line like so:

java --module-path=$env:JFX_SDK --add-modules=javafx.controls ./tests/manual/events/PlatformPreferencesChangedTest.java

This doesn't work with the new utility class, since it is outside of the class file hierarchy. Short of using a build system, how would I run a manual test with this new class?

[andy-goryachev-oracle]

Short of using a build system, how would I run a manual test with this new class?

I am running from within Eclipse. Here is the command line (remove newlines) it launches the EmojiTest. I think the key is to add the location of compiled classes via -classpath:

/Library/Java/JavaVirtualMachines/jdk-21.jdk/Contents/Home/bin/java
-XX:+ShowCodeDetailsInExceptionMessages
-Dfile.encoding=UTF-8
-Dstdout.encoding=UTF-8
-Dstderr.encoding=UTF-8
-p /Users/angorya/Projects/jfx-1/jfx/rt/modules/javafx.base/bin:/Users/angorya/Projects/jfx-1/jfx/rt/modules/javafx.graphics/bin:/Users/angorya/Projects/jfx-1/jfx/rt/modules/javafx.controls/bin
-classpath /Users/angorya/Projects/jfx-1/jfx/rt/tests/manual/util/bin:/Users/angorya/Projects/jfx-1/jfx/rt/tests/manual/text/bin
--add-reads javafx.base=java.management
--add-reads javafx.base=jdk.management
--add-exports javafx.base/com.sun.javafx.property=javafx.graphics
--add-exports javafx.base/test.javafx.collections=javafx.graphics
--add-exports javafx.base/test.util.memory=javafx.graphics
--add-exports java.base/sun.security.util=javafx.graphics
--add-reads javafx.base=java.management
--add-reads javafx.base=jdk.management
--add-exports javafx.graphics/test.com.sun.javafx.pgstub=javafx.controls
--add-exports javafx.base/test.com.sun.javafx.binding=javafx.controls
--add-exports javafx.base/test.util.memory=javafx.controls
--add-exports javafx.base/test.javafx.collections=javafx.controls
--add-exports javafx.base/com.sun.javafx.property=javafx.graphics
--add-exports javafx.base/test.javafx.collections=javafx.graphics
--add-exports javafx.base/test.util.memory=javafx.graphics
--add-exports java.base/sun.security.util=javafx.graphics
--add-reads javafx.base=java.management
--add-reads javafx.base=jdk.management
--add-modules=javafx.base,javafx.graphics,javafx.controls
--add-opens javafx.controls/test.com.sun.javafx.scene.control.test=javafx.base
--add-exports javafx.base/com.sun.javafx=ALL-UNNAMED
-Djava.library.path=../../../build/sdk/lib EmojiTest

(also, there might be some unnecessary -add-exports here as well)

[andy-goryachev-oracle]

But this is a good question: is there a better way to organize the tests to make it easier to use common code?

[mstr2]

Short of using a build system, how would I run a manual test with this new class?

I am running from within Eclipse. Here is the command line (remove newlines) it launches the EmojiTest. I think the key is to add the location of compiled classes via -classpath:

Yes this will work, but it requires me to compile ManualTestWindow.java before. That's quite a lot of heavy-lifting without tooling support (like you get from Eclipse). Maybe we should integrate the manual tests into the Gradle build, so that running a manual test might be as easy as invoking the application plugin's :run task for the manual test.

[kevinrushforth]

We need to provide a build script for any manual test that isn't self-contained in a single directory (see my earlier comment). For example, we have a build script for MonkeyTester and FXMediaPlayer.

As a separate step, we could make it available as part of the gradle build, as long as we can do it without adding individual tests to build.gradle (sort of like we do for apps), but the previous is the minimum.

This should probably be put on the back burner until the next test sprint.

[andy-goryachev-oracle]

or possibly build the common test code into a jar as a part of the standard build and include it along with -Djava.library.path?

[andy-goryachev-oracle]

moving to DRAFT until the next test sprint (after jfx23 ships).

[bridgekeeper[bot]]

@andy-goryachev-oracle This pull request has been inactive for more than 8 weeks and will be automatically closed if another 8 weeks passes without any activity. To avoid this, simply add a new comment to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration!

[andy-goryachev-oracle]

keep-alive

[bridgekeeper[bot]]

@andy-goryachev-oracle This pull request has been inactive for more than 8 weeks and will be automatically closed if another 8 weeks passes without any activity. To avoid this, simply add a new comment to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration!

[andy-goryachev-oracle]

this PR will have to wait until the next test sprint.