Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

8320303: Allow PassFailJFrame to accept single window creator #16717

Closed

Conversation

aivanov-jdk
Copy link
Member

@aivanov-jdk aivanov-jdk commented Nov 17, 2023

Enhance PassFailJFrame for single-window test UI which is the most common case so that there's no need to return List.of.

With this change, the lambda or method reference passed to Builder.testUI will return Window instead of List<Window> (or List<? extends Window>.

This change adds new functional interface WindowCreator which returns a single Window; the existing interface is renamed to WindowListCreator.

It's backwards compatible change; the FileChooserSymLinkTest.java test which uses Builder.testUI works without change.

It was my main concern that it will be impossible to have two interfaces where one creates a Windows and another — List<Windows>. The Java compiler automatically selects the correct method based on the return type of the lambda expression, so that both cases are supported. The requirement to return a List where only one window is needed bothered me, and I finally solved it.

In addition to that, I added description to all the overloads of the Builder.testUI method and updated the sample in the class documentation.

Since the PassFailJFrame class isn't supposed to be extended, I marked it final to prevent anyone from doing it.


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-8320303: Allow PassFailJFrame to accept single window creator (Enhancement - P4)

Reviewers

Reviewing

Using git

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

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

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 16717

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

Using diff file

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

Webrev

Link to Webrev Comment

@bridgekeeper
Copy link

bridgekeeper bot commented Nov 17, 2023

👋 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 openjdk bot added the rfr Pull request is ready for review label Nov 17, 2023
@openjdk
Copy link

openjdk bot commented Nov 17, 2023

@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.

@openjdk openjdk bot added the client client-libs-dev@openjdk.org label Nov 17, 2023
@mlbridge
Copy link

mlbridge bot commented Nov 17, 2023

Webrevs

@aivanov-jdk
Copy link
Member Author

The simplified usage is available in #16718.

@openjdk
Copy link

openjdk bot commented Nov 30, 2023

@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:

8320303: Allow PassFailJFrame to accept single window creator

Reviewed-by: serb

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 187 new commits pushed to the master branch:

  • e96e191: 8318586: Explicitly handle upcall stub allocation failure
  • 630bafd: 8320826: call allocate_shared_strings_array after all strings are interned
  • 0a60b0f: 8302233: HSS/LMS: keytool and jarsigner changes
  • 7ad7005: 8315034: File.mkdirs() occasionally fails to create folders on Windows shared folder
  • 41daa3b: 8320239: add dynamic switch for JvmtiVTMSTransitionDisabler sync protocol
  • 7c135c3: 8321066: Multiple JFR tests have started failing
  • 8bedb28: 8321119: Disable java/foreign/TestHandshake.java on Zero VMs
  • b1cbf55: 8321018: Parallel: Make some methods in ParCompactionManager private
  • c6a8278: 8321127: ProblemList java/util/stream/GatherersTest.java
  • a3eb664: 8315701: [macos] Regression: KeyEvent has different keycode on different keyboard layouts
  • ... and 177 more: https://git.openjdk.org/jdk/compare/9902d2eb177072c108933056cba544cc5a34bb54...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.

@openjdk openjdk bot added the ready Pull request is ready to be integrated label Nov 30, 2023
@aivanov-jdk
Copy link
Member Author

/integrate

@openjdk
Copy link

openjdk bot commented Dec 5, 2023

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

  • fd31f6a: 8321183: Incorrect warning from cds about the modules file
  • 027b5db: 8321215: Incorrect x86 instruction encoding for VSIB addressing mode
  • 61d0db3: 8318468: compiler/tiered/LevelTransitionTest.java fails with -XX:CompileThreshold=100 -XX:TieredStopAtLevel=1
  • 87516e2: 8320943: Files/probeContentType/Basic.java fails on latest Windows 11 - content type mismatch
  • 800f347: 8321216: SerialGC attempts to access the card table beyond the end of the heap during card table scan
  • a1fe16b: 8321300: Cleanup TestHFA
  • 5b02188: 8321105: Enable UseCryptoPmullForCRC32 for Neoverse V2
  • 4fbf22b: 8320652: ThreadInfo.isInNative needs to be updated to say what executing native code means
  • 672f373: 8321163: [test] OutputAnalyzer.getExitValue() unnecessarily logs even when process has already completed
  • 30817b7: 8317809: Insertion of free code blobs into code cache can be very slow during class unloading
  • ... and 236 more: https://git.openjdk.org/jdk/compare/9902d2eb177072c108933056cba544cc5a34bb54...master

Your commit was automatically rebased without conflicts.

@openjdk openjdk bot added the integrated Pull request has been integrated label Dec 5, 2023
@openjdk openjdk bot closed this Dec 5, 2023
@openjdk openjdk bot removed ready Pull request is ready to be integrated rfr Pull request is ready for review labels Dec 5, 2023
@openjdk
Copy link

openjdk bot commented Dec 5, 2023

@aivanov-jdk Pushed as commit 83ffc1a.

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

@aivanov-jdk aivanov-jdk deleted the 8320303-testUI-single branch December 5, 2023 17:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
client client-libs-dev@openjdk.org integrated Pull request has been integrated
Development

Successfully merging this pull request may close these issues.

2 participants