On Thu, 17 Apr 2025 19:29:28 GMT, Harshitha Onkar <hon...@openjdk.org> wrote:
>> Open-sourced the following Swing tests:
>>
>> 1. javax/swing/JFileChooser/bug4464774.java
>> 2. javax/swing/JFileChooser/bug4522756.java
>> 3. javax/swing/JFileChooser/bug4759934.java
>> 4. javax/swing/JFileChooser/bug4943900.java
>> 5. javax/swing/JOptionPane/bug4194862.java
>
> Harshitha Onkar has updated the pull request incrementally with one
> additional commit since the last revision:
>
> remove redudant frame
test/jdk/javax/swing/JFileChooser/bug4522756.java line 29:
> 27: * @requires (os.family == "windows")
> 28: * @summary To verify that if for the first time JFileChooser is opened,
> 29: the icon for Desktop is not missing.
Suggestion:
* @summary Verifies that the Desktop icon is not missing when
JFileChooser is opened for the first time.
I presume it refers to the location bar on the left, does it?
test/jdk/javax/swing/JFileChooser/bug4759934.java line 58:
> 56: robot.setAutoWaitForIdle(true);
> 57: robot.setAutoDelay(50);
> 58: SwingUtilities.invokeAndWait(bug4759934::createTestUI);
Suggestion:
robot.setAutoDelay(50);
SwingUtilities.invokeAndWait(bug4759934::createTestUI);
I think a blank line here will separate the initial `Robot` configuration from
creating UI for the test.
test/jdk/javax/swing/JFileChooser/bug4943900.java line 57:
> 55: is updated properly.
> 56:
> 57: If the FileFilter works correctly, press PASS else press FAIL.
I wonder why so many people use capitals for *Pass* and *Fail*…
test/jdk/javax/swing/JFileChooser/bug4943900.java line 65:
> 63: } catch (Exception e) {
> 64: throw new SkippedException("LaF not supported", e);
> 65: }
Is the intention to run in system L&F only rather than in all L&Fs?
test/jdk/javax/swing/JFileChooser/bug4943900.java line 85:
> 83:
> 84: private static final class TextFileFilter extends FileFilter {
> 85: public boolean accept(File f) {
I'd rather add `@Override` annotation to both `accept` and `getDescription`.
test/jdk/javax/swing/JFileChooser/bug4943900.java line 100:
> 98: }
> 99:
> 100: public String getExtension(File f) {
Suggestion:
private static String getExtension(File f) {
This is an internal helper method for the class.
test/jdk/javax/swing/JOptionPane/bug4194862.java line 43:
> 41: private static final String INSTRUCTIONS = """
> 42: In the internal frame titled "Main",
> 43: click the "Show JOption dialog" button.
Suggestion:
click the "Show JOptionPane dialog" button.
And update the button text accordingly?
test/jdk/javax/swing/JOptionPane/bug4194862.java line 44:
> 42: In the internal frame titled "Main",
> 43: click the "Show JOption dialog" button.
> 44: A dialog will appear. It should be centered w.r.t
I guess “w.r.t.” is a common abbreviation, however, it always helps to spell
out words… in instructions.
test/jdk/javax/swing/JOptionPane/bug4194862.java line 57:
> 55: .columns(40)
> 56: .testUI(bug4194862::createAndShowUI)
> 57: .build()
Enable screenshot?
I've been thinking about a feature which could the screenshot automatically… as
a failure handler. There's a tracking bug for it,
[JDK-8317114](https://bugs.openjdk.org/browse/JDK-8317114): *Provide failure
handlers for PassFailJFrame*. Many UI manual tests could benefit from it,
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/24456#discussion_r2050947524
PR Review Comment: https://git.openjdk.org/jdk/pull/24456#discussion_r2050961582
PR Review Comment: https://git.openjdk.org/jdk/pull/24456#discussion_r2050953427
PR Review Comment: https://git.openjdk.org/jdk/pull/24456#discussion_r2050950163
PR Review Comment: https://git.openjdk.org/jdk/pull/24456#discussion_r2050952347
PR Review Comment: https://git.openjdk.org/jdk/pull/24456#discussion_r2050951867
PR Review Comment: https://git.openjdk.org/jdk/pull/24456#discussion_r2050954047
PR Review Comment: https://git.openjdk.org/jdk/pull/24456#discussion_r2050958581
PR Review Comment: https://git.openjdk.org/jdk/pull/24456#discussion_r2050957186
