8327838: Convert java/awt/FileDialog/MultipleMode/MultipleMode.html applet test to main #18205

azvegint · 2024-03-11T17:53:48Z

The test is converted to main.
Tested on Linux, Macos and Windows.

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-8327838: Convert java/awt/FileDialog/MultipleMode/MultipleMode.html applet test to main (Bug - P4)

Reviewers

Alexey Ivanov (@aivanov-jdk - Reviewer)
Tejesh R (@TejeshR13 - Committer)

Reviewing

Using git

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

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

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 18205

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

Using diff file

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

Webrev

Link to Webrev Comment

…plet test to main

bridgekeeper · 2024-03-11T17:54:35Z

👋 Welcome back azvegint! 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-03-11T17:56:03Z

@azvegint 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-03-11T17:59:19Z

Webrevs

test/jdk/java/awt/FileDialog/MultipleMode.java

test/jdk/java/awt/FileDialog/MultipleMode/MultipleMode.java

TejeshR13 · 2024-03-12T05:17:31Z

test/jdk/java/awt/FileDialog/MultipleMode.java

+        frame.add(panel, BorderLayout.NORTH);
+        frame.add(sysout, BorderLayout.CENTER);
+
+        frame.pack();


Frame size is too big and going out of the screen, might fix a size here.

It doesn't fit on the screen of my VM either.

One way to resolve this problem is to add

.position(PassFailJFrame.Position.TOP_LEFT_CORNER)

to PassFailJFrame builder as there's no easy way to move the instruction frame to the left so that both frames fit on the screen.

Updated.

What resolution and scale are you using?
I've tested at 1920x1080 and default scale 1, and it has plenty of room:

Now its fine on my screen. But its not in center, but top. Does it differ for windows and linux?

It is not in the center, because in the latest commit I added .position(PassFailJFrame.Position.TOP_LEFT_CORNER), as Alexey suggested.

I have 1920×1200 with 150%. This is why it doesn't fit. The second frame overlapped instructions a bit.

That would be 1280x800 user pixels ?

These days tests should be able to assume that there are 1920x1080 user-space pixels on the screen.

test/jdk/java/awt/FileDialog/MultipleMode.java

aivanov-jdk

Looks good to me.

Yet I left a couple of suggestions.

aivanov-jdk · 2024-03-12T09:54:48Z

test/jdk/java/awt/FileDialog/MultipleMode.java

+            sysout.append("FILES:\n");
+            for (File f : d.getFiles()) {
+                sysout.append("  %s\n".formatted(f));
+            }


If you like you can replace the for-loop with functional-style stream:

Suggested change

sysout.append("FILES:\n");

for (File f : d.getFiles()) {

sysout.append(" %s\n".formatted(f));

}

sysout.append("FILES:\n");

sysout.append(Arrays.stream(d.getFiles())

.map(" %s\n"::formatted)

.collect(Collectors.joining()));

test/jdk/java/awt/FileDialog/MultipleMode.java

openjdk · 2024-03-12T09:57:16Z

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

8327838: Convert java/awt/FileDialog/MultipleMode/MultipleMode.html applet test to main

Reviewed-by: aivanov, tr

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

78beb03: 8327750: Convert javax/swing/JFileChooser/FileFilterDescription/FileFilterDescription.java applet test to main
1f43fa0: 8326661: sun/java2d/cmm/ColorConvertOp/ColConvTest.java assumes profiles were generated by LCMS
013aff8: 8326606: Test javax/swing/text/BoxView/6494356/bug6494356.java performs a synchronization on a value based class
b92440f: 8247972: incorrect implementation of JVM TI GetObjectMonitorUsage
139681a: 8326497: Window.toFront() fails for iconified windows on Linux
9f7aff4: 8327788: G1: Improve concurrent reference processing documentation
251347b: 8326139: C2 SuperWord: split packs (match use/def packs, implemented, mutual independence)
e21da4c: 8327486: java/util/Properties/PropertiesStoreTest.java fails "Text 'xxx' could not be parsed at index 20" after 8174269
4d6235e: 8327716: RISC-V: Change type of vector_length param of several assembler functions from int to uint
586396c: 8327705: Remove mention of "applet" from java.text package description
... and 7 more: https://git.openjdk.org/jdk/compare/680ac2cebecf93e5924a441a5de6918cd7adf118...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.

azvegint · 2024-03-12T18:21:59Z

/integrate

openjdk · 2024-03-12T18:23:17Z

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

94b4ed5: 8327096: (fc) java/nio/channels/FileChannel/Size.java fails on partition incapable of creating large files
b9c3dc3: 8327738: Remove unused internal method sun.n.w.p.h.HttpURLConnection.setDefaultAuthenticator
5b41466: 8327729: Remove deprecated xxxObject methods from jdk.internal.misc.Unsafe
313e814: 8324682: Remove redefinition of NULL for XLC compiler
8a3bdd5: 8327995: Remove unused Unused_Variable
201042f: 8327487: Further augment WorstCaseTests with more cases
379ad1f: 8312444: Delete unused parameters and variables in SocketPermission
782206b: 8327748: Convert javax/swing/JFileChooser/6798062/bug6798062.java applet test to main
0776fff: 8327794: RISC-V: enable extension features based on impid (Rivos specific change)
cfd9209: 8327751: Convert javax/swing/JInternalFrame/6726866/bug6726866.java applet test to main
... and 19 more: https://git.openjdk.org/jdk/compare/680ac2cebecf93e5924a441a5de6918cd7adf118...master

Your commit was automatically rebased without conflicts.

openjdk · 2024-03-12T18:23:23Z

@azvegint Pushed as commit 30249c4.

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

prrace · 2024-03-12T20:55:48Z

test/jdk/java/awt/FileDialog/MultipleMode.java

+        frame.add(panel, BorderLayout.NORTH);
+        frame.add(sysout, BorderLayout.CENTER);
+
+        frame.pack();


That would be 1280x800 user pixels ?

These days tests should be able to assume that there are 1920x1080 user-space pixels on the screen.

prrace · 2024-03-12T20:56:02Z

test/jdk/java/awt/FileDialog/MultipleMode.java

+            .awaitAndCheck();
+    }
+
+    private static Frame init() {


init() -> createMultiModeFrame() ?

8327838 Convert java/awt/FileDialog/MultipleMode/MultipleMode.html ap…

d33c317

…plet test to main

openjdk bot added the rfr Pull request is ready for review label Mar 11, 2024

openjdk bot added the client client-libs-dev@openjdk.org label Mar 11, 2024

aivanov-jdk suggested changes Mar 11, 2024

View reviewed changes

test/jdk/java/awt/FileDialog/MultipleMode.java Outdated Show resolved Hide resolved

test/jdk/java/awt/FileDialog/MultipleMode.java Outdated Show resolved Hide resolved

test/jdk/java/awt/FileDialog/MultipleMode/MultipleMode.java Outdated Show resolved Hide resolved

review comments

a9f8e21

TejeshR13 reviewed Mar 12, 2024

View reviewed changes

test/jdk/java/awt/FileDialog/MultipleMode.java Show resolved Hide resolved

aivanov-jdk approved these changes Mar 12, 2024

View reviewed changes

openjdk bot added the ready Pull request is ready to be integrated label Mar 12, 2024

review comments

58e3d73

TejeshR13 approved these changes Mar 12, 2024

View reviewed changes

aivanov-jdk approved these changes Mar 12, 2024

View reviewed changes

openjdk bot added the integrated Pull request has been integrated label Mar 12, 2024

openjdk bot closed this Mar 12, 2024

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

prrace reviewed Mar 12, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

8327838: Convert java/awt/FileDialog/MultipleMode/MultipleMode.html applet test to main #18205

8327838: Convert java/awt/FileDialog/MultipleMode/MultipleMode.html applet test to main #18205

azvegint commented Mar 11, 2024 •

edited by openjdk bot

Loading

bridgekeeper bot commented Mar 11, 2024

openjdk bot commented Mar 11, 2024

mlbridge bot commented Mar 11, 2024 •

edited

Loading

TejeshR13 Mar 12, 2024

aivanov-jdk Mar 12, 2024

azvegint Mar 12, 2024

TejeshR13 Mar 12, 2024

azvegint Mar 12, 2024

aivanov-jdk Mar 12, 2024

prrace Mar 12, 2024

aivanov-jdk left a comment

aivanov-jdk Mar 12, 2024

openjdk bot commented Mar 12, 2024 •

edited

Loading

azvegint commented Mar 12, 2024

openjdk bot commented Mar 12, 2024

openjdk bot commented Mar 12, 2024

prrace Mar 12, 2024

prrace Mar 12, 2024

8327838: Convert java/awt/FileDialog/MultipleMode/MultipleMode.html applet test to main #18205

8327838: Convert java/awt/FileDialog/MultipleMode/MultipleMode.html applet test to main #18205

Conversation

azvegint commented Mar 11, 2024 • edited by openjdk bot Loading

Progress

Issue

Reviewers

Reviewing

Webrev

bridgekeeper bot commented Mar 11, 2024

openjdk bot commented Mar 11, 2024

mlbridge bot commented Mar 11, 2024 • edited Loading

Webrevs

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aivanov-jdk left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

openjdk bot commented Mar 12, 2024 • edited Loading

azvegint commented Mar 12, 2024

openjdk bot commented Mar 12, 2024

openjdk bot commented Mar 12, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

azvegint commented Mar 11, 2024 •

edited by openjdk bot

Loading

mlbridge bot commented Mar 11, 2024 •

edited

Loading

openjdk bot commented Mar 12, 2024 •

edited

Loading