8294148: Support JSplitPane for instructions and test UI #17845

aivanov-jdk · 2024-02-14T13:22:48Z

This enhancement adds three methods to PassFailJFrame.Builder:

splitUIRight to display test UI to the right of the instructions;
splitUIBottom to display test UI to the bottom of the instructions;
splitUI: a convenience method for a default split orientation, currently defaults to splitUIRight.

If UI of a test is simple, it's reasonable to embed the test UI into the instruction UI. In this case, there's only one frame the tester will be interacting with.

The implementation is similar to that of testUI provided under JDK-8294156. The splitUI- method accept a reference to PanelCreator interface. Instead of creating an entire frame, you create only a component, usually a JPanel or Box, which contains all the test UI.

For example,

        PassFailJFrame.builder()
                      .instructions("Instructions for the test")
                      .splitUI(() -> {
                          JPanel panel = new JPanel();
                          panel.add(new JButton("Click Me"));
                          return panel;
                      })
                      .build()
                      .awaitAndCheck();

In addition to it, there's an advanced testUI method which accepts PanelCreator; in this case the test UI is hosted in JDialog with the instruction frame being its owner. Because the test UI is in a modeless owned dialog, it always displays above the instruction frame, switch to the test app brings up both the instructions and test UI. I don't expect it'd be used often, yet it's easy to implement.

Progress

Change must be properly reviewed (1 review required, with at least 1 Reviewer)
Change must not contain extraneous whitespace
Commit message must refer to an issue

Issue

JDK-8294148: Support JSplitPane for instructions and test UI (Enhancement - P4)

Reviewers

Harshitha Onkar (@honkar-jdk - Reviewer)
Dmitry Markov (@dmarkov20 - Reviewer)

Reviewing

Using git

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

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

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 17845

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

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/17845.diff

Webrev

Link to Webrev Comment

The test to be updated separately.

bridgekeeper · 2024-02-14T13:23:27Z

👋 Welcome back aivanov! 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 · 2024-02-14T13:24:45Z

@aivanov-jdk The following label will be automatically applied to this pull request:

client

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing list. If you would like to change these labels, use the /label pull request command.

mlbridge · 2024-02-14T13:27:09Z

Webrevs

00: Full (63bca8cf)

aivanov-jdk · 2024-02-14T14:11:32Z

See PR #17847 for a real-life usage example of splitUI.

honkar-jdk · 2024-02-14T18:47:55Z

test/jdk/java/awt/regtesthelpers/PassFailJFrame.java

+         *              already set
+         * @throws IllegalArgumentException if {panelCreator} is {@code null}
+         */
+        public Builder splitUI(PanelCreator panelCreator) {


Suggested change

public Builder splitUI(PanelCreator panelCreator) {

public Builder splitUI(PanelCreator panelCreator, int splitUIOrientation ) {

Would it be easier if we use splitUIOrientation or an enum similar to Position to determine the split than have three different methods - splitUI, splitUIRight & splitUIBottom? And eliminate the private splitUI() and have the orientation logic within public splitUI()

I don't think so… I find the Position cumbersome too.

Using JSplitPane.HORIZONTAL_SPLIT or JSplitPane.VERTICAL_SPLIT in a public method of Builder exposes the internal detail that JSplitPane is used. What if we decide to implement it differently? After all, the UI could be placed into a horizontal or vertical Box? Passing an int as the orientation is not type-safe, we'll need to verify the parameter or let JSplitPane constructor do it.

An enum is type-safe but it's cumbersome to type:

splitUI(() -> new JPanel(), PassFailJFrame.SplitOrientation.HORIZONTAL) // or splitUI(() -> new JPanel(), JSplitPane.HORIZONTAL_SPLIT)

does not look as good to me as

splitUIRight(() -> new JPanel())

-Right explicitly conveys the chosen position and is much shorter than the above samples with enum or constants from JSplitPane.

Initially, I wanted to re-use Position but decided not to … for flexibility. Split UI could be combined with additional test UI windows… Not that I see a use case for such a use…

The three methods make the Builder more crowded for the sake of brevity of its usage in the test code. I expect it would be splitUI in majority of cases.

In terms of flexibility & future extensibility (if any) your approach works better.

honkar-jdk

JSplitPane addition to PassFailJFame looks good.

openjdk · 2024-02-14T22:52:44Z

@aivanov-jdk This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

8294148: Support JSplitPane for instructions and test UI

Reviewed-by: honkar, dmarkov

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 67 new commits pushed to the master branch:

b366492: 8326152: Bad copyright header in test/jdk/java/util/zip/DeflaterDictionaryTests.java
9451677: 8326100: DeflaterDictionaryTests should use Deflater.getBytesWritten instead of Deflater.getTotalOut
d422811: 8324630: C1: Canonicalizer::do_LookupSwitch doesn't break the loop when the successor is found
3742bc6: 8323795: jcmd Compiler.codecache should print total size of code cache
099b744: 8326117: ProblemList serviceability/jvmti/vthread/SuspendWithInterruptLock/SuspendWithInterruptLock.java#default in Xcomp mode
39627bc: 6510914: JScrollBar.getMinimumSize() breaks the contract of JComponent.setMinimumSize()
7004c27: 8303972: (zipfs) Make test/jdk/jdk/nio/zipfs/TestLocOffsetFromZip64EF.java independent of the zip command line
c2d9fa2: 8326000: Remove obsolete comments for class sun.security.ssl.SunJSSE
f50df10: 8299023: TestPLABResize.java and TestPLABPromotion.java are failing intermittently
cf13086: 8317697: refactor-encapsulate x86 VM_Version::CpuidInfo
... and 57 more: https://git.openjdk.org/jdk/compare/13d9e8ff38536287b82c54bb63bd2d20f65615dc...master

As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

➡️ To integrate this PR with the above commit message to the master branch, type /integrate in a new comment.

aivanov-jdk · 2024-02-20T16:29:10Z

/integrate

openjdk · 2024-02-20T16:30:34Z

Going to push as commit b419e95.
Since your change was applied there have been 89 commits pushed to the master branch:

9538f5d: 8319651: Several network tests ignore vm flags when start java process
4957d6a: 8325441: IGV: update pom.xml such that IntelliJ can import as maven project
d60331a: 8326196: Serial: Remove SerialHeap::generation_iterate
beed414: 8326209: G1: Remove G1ConcurrentMark::_total_cleanup_time
56073cd: 8326171: Serial: Remove VerifyGCLevel
bb675af: 8325542: CTW: Runner can produce negative StressSeed
5bf859c: 8325610: CTW: Add StressIncrementalInlining to stress options
0d28531: 8318812: LauncherHelper.checkAndLoadMain closes jar file that's about to be re-opened
69a11c7: 8326222: Fix copyright year in src/hotspot/share/gc/shenandoah/shenandoahHeap.cpp
9fbcc00: 8326127: JFR: Add SafepointCleanupTask to hardToTestEvents of TestLookForUntestedEvents
... and 79 more: https://git.openjdk.org/jdk/compare/13d9e8ff38536287b82c54bb63bd2d20f65615dc...master

Your commit was automatically rebased without conflicts.

openjdk · 2024-02-20T16:30:39Z

@aivanov-jdk Pushed as commit b419e95.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.

aivanov-jdk added 7 commits December 22, 2023 20:25

8294148: Use JSplitPane to display test UI with the instructions

3206035

8294148: A test to demonstrate JSplitPane feature

17e576f

Update formatting in classic createUI method

5a5afaa

Merge master

5addc87

Add description to splitUI methods

a5e3933

Ensure synchronized access to windowList field

65920e7

Revert changes to PrintLatinCJKTest

63bca8c

The test to be updated separately.

openjdk bot added the rfr Pull request is ready for review label Feb 14, 2024

openjdk bot added the client client-libs-dev@openjdk.org label Feb 14, 2024

aivanov-jdk mentioned this pull request Feb 14, 2024

8325762: Use PassFailJFrame.Builder.splitUI() in PrintLatinCJKTest.java #17847

Closed

3 tasks

honkar-jdk reviewed Feb 14, 2024

View reviewed changes

honkar-jdk approved these changes Feb 14, 2024

View reviewed changes

openjdk bot added the ready Pull request is ready to be integrated label Feb 14, 2024

dmarkov20 approved these changes Feb 19, 2024

View reviewed changes

openjdk bot added the integrated Pull request has been integrated label Feb 20, 2024

openjdk bot closed this Feb 20, 2024

openjdk bot removed ready Pull request is ready to be integrated rfr Pull request is ready for review labels Feb 20, 2024

aivanov-jdk deleted the 8294148-splitUI branch February 20, 2024 16:51

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

8294148: Support JSplitPane for instructions and test UI #17845

8294148: Support JSplitPane for instructions and test UI #17845

aivanov-jdk commented Feb 14, 2024 •

edited by openjdk bot

Loading

bridgekeeper bot commented Feb 14, 2024

openjdk bot commented Feb 14, 2024

mlbridge bot commented Feb 14, 2024

aivanov-jdk commented Feb 14, 2024

honkar-jdk Feb 14, 2024 •

edited

Loading

aivanov-jdk Feb 14, 2024 •

edited

Loading

honkar-jdk Feb 14, 2024 •

edited

Loading

honkar-jdk left a comment

openjdk bot commented Feb 14, 2024 •

edited

Loading

aivanov-jdk commented Feb 20, 2024

openjdk bot commented Feb 20, 2024

openjdk bot commented Feb 20, 2024

	public Builder splitUI(PanelCreator panelCreator) {
	public Builder splitUI(PanelCreator panelCreator, int splitUIOrientation ) {

8294148: Support JSplitPane for instructions and test UI #17845

8294148: Support JSplitPane for instructions and test UI #17845

Conversation

aivanov-jdk commented Feb 14, 2024 • edited by openjdk bot Loading

Progress

Issue

Reviewers

Reviewing

Webrev

bridgekeeper bot commented Feb 14, 2024

openjdk bot commented Feb 14, 2024

mlbridge bot commented Feb 14, 2024

Webrevs

aivanov-jdk commented Feb 14, 2024

honkar-jdk Feb 14, 2024 • edited Loading

Choose a reason for hiding this comment

aivanov-jdk Feb 14, 2024 • edited Loading

Choose a reason for hiding this comment

honkar-jdk Feb 14, 2024 • edited Loading

Choose a reason for hiding this comment

honkar-jdk left a comment

Choose a reason for hiding this comment

openjdk bot commented Feb 14, 2024 • edited Loading

aivanov-jdk commented Feb 20, 2024

openjdk bot commented Feb 20, 2024

openjdk bot commented Feb 20, 2024

aivanov-jdk commented Feb 14, 2024 •

edited by openjdk bot

Loading

honkar-jdk Feb 14, 2024 •

edited

Loading

aivanov-jdk Feb 14, 2024 •

edited

Loading

honkar-jdk Feb 14, 2024 •

edited

Loading

openjdk bot commented Feb 14, 2024 •

edited

Loading