8319938: TestFileChooserSingleDirectorySelection.java fails with "getSelectedFiles returned empty array" #16674

kumarabhi006 · 2023-11-15T09:50:19Z

The test fails for JFileChooser selection mode set to DIRECTORIES_ONLY. For DIRECTORIES_ONLY mode, there may not be any directories in home directory and due to that test failed. Added the code to create temporary directories and files for the test.
Tested the current change on the machine it failed for multiple times, no failure observed.
CI link attached in JBS.

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-8319938: TestFileChooserSingleDirectorySelection.java fails with "getSelectedFiles returned empty array" (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/16674/head:pull/16674
$ git checkout pull/16674

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

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 16674

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

Using diff file

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

Webrev

Link to Webrev Comment

bridgekeeper · 2023-11-15T09:52:07Z

👋 Welcome back abhiscxk! 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 · 2023-11-15T09:53:33Z

@kumarabhi006 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 · 2023-11-15T09:56:39Z

Webrevs

prrace · 2023-11-15T18:40:57Z

test/jdk/com/sun/java/swing/plaf/gtk/TestFileChooserSingleDirectorySelection.java

+ *          is enabled for JFileChooser.
+ * @run main TestFileChooserSingleDirectorySelection
+ */
+


I don't know why it was necessary to move all around all the above lines.
And whilst import java.io sorts after java.awt, it is long standing convention that the "core"
packages (easily distinguished these days as those in the java.base module) are listed before the desktop / AWT / Swing ones. This is true of product source as well as tests.
So you should just undo all of the above except for adding the new bug id.

And whilst import java.io sorts after java.awt, it is long standing convention that the "core"
packages (easily distinguished these days as those in the java.base module) are listed before the desktop / AWT / Swing ones. This is true of product source as well as tests.

@prrace This is the first time I hear this. In all the classes I've seen in AWT and Swing, the imports are sorted alphabetically with java.io after java.awt.

I just looked at JComponent, Component, Font, and

jdk/src/java.desktop/share/classes/sun/font/FontManager.java

Lines 28 to 30 in 6ce0ebb

import java.awt.Font;

import java.awt.FontFormatException;

import java.io.File;

jdk/src/java.desktop/share/classes/sun/font/Font2D.java

Lines 28 to 36 in 6ce0ebb

import java.awt.Font;

import java.awt.font.FontRenderContext;

import java.awt.geom.AffineTransform;

import java.lang.ref.Reference;

import java.lang.ref.SoftReference;

import java.lang.ref.WeakReference;

import java.util.concurrent.ConcurrentHashMap;

import java.util.Locale;

import java.util.Set;

Sun/Oracle Code Conventions for Java Programming Language say nothing about the order of imports. (The HTML version has absolutely broken formatting now.) Google Java Style Guide states, …the imported names appear in ASCII sort order.

Andreas Lundblad's draft for Java Style Guidelines, which is more or less followed by the OpenJDK project, defines the following order:

Import statements should be sorted…

…primarily by non-static / static with non-static imports first.

…secondarily by package origin according to the following order

java packages

javax packages

external packages (e.g. org.xml)

internal packages (e.g. com.sun)

…tertiary by package and class identifier in lexicographical order

Andreas opened PR 14 to incorporate it into OpenJDK Developers' Guide, but there are objections to making the Java Code Conventions part of the developers' guide.

I believe it's not even the first time such a discussion happens in the clientlibs. There are no guidelines…

Personally, I think putting java.awt classes above java.io or java.util makes perfect sense for clientlibs classes — it is what we care the most, it is what we're developing and maintaining, so java.awt is more relevant to us. And it automatically sorts this way in IDE; manually sorting imports is tedious and error-prone.

I don't know why it was necessary to move all around all the above lines.

@prrace I nearly always ask to move the jtreg tags to the class declaration, especially for new tests. When you open the test file in IDE, the copyright block above imports is collapsed, so is the jtreg tags if they're placed above the imports.

If the jtreg tags are placed in a comment that precedes the class declaration, after the imports, they're not collapsed — you can see them right away without scrolling or clicking. I consider the jtreg tags quite relevant to see them easily.

Most older tests place the jtreg tags above the imports; it could've been the requirement or limitation of jtreg at that time. It's not a limitation any more.

As for moving the jtreg tags in existing tests when the test is modified, I also do it occasionally for the reasons outlined above: the jtreg tags describe the test and belong to the class declaration, akin a javadoc comment. (Yet I am against using the javadoc syntax, starting with two asterisks, for the jtreg tasks because it is not a javadoc and because it removes the IDE warnings for the unknown javadoc tasks.)

And whilst import java.io sorts after java.awt, it is long standing convention that the "core"
packages (easily distinguished these days as those in the java.base module) are listed before the desktop / AWT / Swing ones. This is true of product source as well as tests.

@prrace This is the first time I hear this. In all the classes I've seen in AWT and Swing, the imports are sorted alphabetically with java.io after java.awt.

Today I've stumbled upon a few files where java.io and java.util come before java.awt, such as DialogOwnerTest.java, CustomFont.java, DeviceScale.java,

jdk/test/jdk/java/awt/print/PrinterJob/DrawImage.java

Lines 32 to 36 in 3789983

import java.util.*;

import java.text.*;

import java.io.*;

import java.net.*;

import java.awt.*;

It's a handful of files out of more than a hundred modified files in #16785; it doesn't look as a convention. Do wildcard imports even count?

prrace · 2023-11-15T18:46:27Z

test/jdk/com/sun/java/swing/plaf/gtk/TestFileChooserSingleDirectorySelection.java

-
-            // create temporary files inside testDir
-            tempFile = new File[5];
+


Does this test HAVE to use the home dir to create the temporary folders and files ?
Is there some reason some part of the test absolutely requires the home directory ?
It isn't obvious to me.
Why can't you instead use System.getProperty("java.io.tmpdir");

There is no need to have the home dir to create the temporary folders and files. Since I was testing in my local mahine to create temp folders in home directory, I kept it as it is.

Changed home dir to java.io.tmpdir for creating temporary folders and files.

aivanov-jdk · 2023-11-20T19:50:18Z

test/jdk/com/sun/java/swing/plaf/gtk/TestFileChooserSingleDirectorySelection.java

+            // Create a test directory that contains only files
+            testFile = new File(tmpDir, "testFile");
+            if (!testFile.exists()) {
+                testFile.mkdir();
+            }
+            testFile.deleteOnExit();


This is confusing: testFile is actually a directory.

You could improve the readability of the test by using methods: createFoldersOnlyDir, createFilesOnlyDir, populateDirs, populateFiles or something similar — it would make the comments in the code unnecessary.

Test updated.

aivanov-jdk · 2023-11-20T19:51:05Z

test/jdk/com/sun/java/swing/plaf/gtk/TestFileChooserSingleDirectorySelection.java

-
-            // create temporary files inside testDir
-            tempFile = new File[5];
+            String tmpDir = System.getProperty("java.io.tmpdir");


This is the purpose of the scratch directory in jtreg, and it's automatically cleaned up after the test: jtreg will automatically clean up any files written in the scratch directory.

The test is designed for jtreg, so the simplest solution is to use the current directory.

If someone runs the test directly, without using jtreg, then it's their task to set up the current directory or clean up after the test.

For some reasons when I used the current directory i.e. scratch, test failed in my local machine.
String tmpDir = System.getProperty(".");

For some reasons when I used the current directory i.e. scratch, test failed in my local machine. String tmpDir = System.getProperty(".");

How does it fail?
Is it because System.getProperty(".") returns null? The property "." doesn't exist, does it?

Use new File(".") which resolves to the current directory. Alternatively, remove references to paths, just create directories and files, they will be created in the current directory if you don't specify an absolute path.

How does it fail? Is it because System.getProperty(".") returns null? The property "." doesn't exist, does it?

No, temp files and directories are created in scratch directory. But the files gets deselected after selection which returns as no file selected and test failed.

But the files gets deselected after selection which returns as no file selected and test failed.

How is it? It sounds like magic… The working directory shouldn't matter; if it does, the test isn't as stable as it should be.

Surprise to me also. It failed every single time when temporary files are created in scratch.

The test isn't as stable as it should be.

Unable to trace what is missing in the test.

It took a long while for me to figure it out. What matters is whether the File object is created with an absolute path or not. By using System.getProperty("java.io.tmpdir") as the base path, you create an absolute path for the parent directory.

Yet if you use pass null as the first parameter (as I said, there's no system property named ".", so tmpDir is null) or use the constructor with one parameter new File("testDir"), the file has relative path. This causes some issues…

When you create JFileChooser object, you always pass testDir as its starting directory. Then, for each case, you set it once again. Here's what I see in a PropertyChangeListener:

>> setCurrentDirectory directoryChanged: /home/aivanov/jdk-dev/testDir/testDir -> testDir << setCurrentDirectory SelectedFileChangedProperty: /home/aivanov/jdk-dev/testDir/subDir_3 -> null SelectedFilesChangedProperty: [Ljava.io.File;@3fed803b -> null directoryChanged: testDir -> /home/aivanov/jdk-dev/testDir SelectedFileChangedProperty: null -> null SelectedFilesChangedProperty: null -> [Ljava.io.File;@1bf88ed6

This is why see a directory gets selected but then becomes unselected again.

Getting the absolute file — testDir = new File("testDir").getAbsoluteFile() — resolves this problem, and the test starts to work in the current directory. I tested it by running with jtreg, then the current directory is the scratch directory, and as a stand-alone app, in this case the current directory is whatever the current directory is.

So either way works as long as the starting File object is created with an absolute path.

aivanov-jdk · 2023-11-20T19:52:32Z

test/jdk/com/sun/java/swing/plaf/gtk/TestFileChooserSingleDirectorySelection.java

        } catch (Exception e) {
            e.printStackTrace();


The exception should've been propagated: if an exception is thrown, test setup code has failed to prepare the expected environment — the test would likely fail too, then let it fail quickly and report the real reason.

aivanov-jdk · 2023-12-04T20:41:36Z

test/jdk/com/sun/java/swing/plaf/gtk/TestFileChooserSingleDirectorySelection.java

@@ -23,7 +23,7 @@

 /*
 * @test
- * @bug 6972078
+ * @bug 6972078 8319938


It's a test bug, no code in JDK is changed as the result — the bugid shouldn't be added.

aivanov-jdk · 2023-12-04T20:42:45Z

test/jdk/com/sun/java/swing/plaf/gtk/TestFileChooserSingleDirectorySelection.java

+    private static File[] SubDirs;
+    private static File[] subFiles;


You don't use these arrays, they could be local variables; but even there they're not needed — a local variable for a current file or directory is enough.

aivanov-jdk · 2023-12-04T20:45:35Z

test/jdk/com/sun/java/swing/plaf/gtk/TestFileChooserSingleDirectorySelection.java

+        createFoldersOnlyDir();
+        createFilesOnlyDir();
+        populateDirs();
+        populateFiles();


Returning and passing the File object would make code flow clearer:

Suggested change

createFoldersOnlyDir();

createFilesOnlyDir();

populateDirs();

populateFiles();

testDir = createFoldersOnlyDir();

populateDirs(testDir);

testFile = createFilesOnlyDir();

populateFiles(testFile);

I still find testFile confusing because it represents a directory. What about testDirDirs, testDirFiles?

Updated the var names.

aivanov-jdk · 2023-12-04T20:47:09Z

test/jdk/com/sun/java/swing/plaf/gtk/TestFileChooserSingleDirectorySelection.java

+        SubDirs = new File[10];
+        for (int i = 0; i < 10; ++i) {
+            SubDirs[i] = new File(testDir, "subDir_" + (i+1));
+            SubDirs[i].mkdir();
+            SubDirs[i].deleteOnExit();
+        }


The array is redundant:

Suggested change

SubDirs = new File[10];

for (int i = 0; i < 10; ++i) {

SubDirs[i] = new File(testDir, "subDir_" + (i+1));

SubDirs[i].mkdir();

SubDirs[i].deleteOnExit();

}

for (int i = 0; i < 10; ++i) {

File subDir = new File(testDir, "subDir_" + (i+1));

subDir.mkdir();

subDir.deleteOnExit();

}

aivanov-jdk · 2023-12-04T20:51:05Z

test/jdk/com/sun/java/swing/plaf/gtk/TestFileChooserSingleDirectorySelection.java

+        subFiles = new File[10];
+        for (int i = 0; i < 10; ++i) {
+            subFiles[i] = File.createTempFile("subFiles_" + (i+1),
+                    ".txt", new File(testFile.getAbsolutePath()));
+            subFiles[i].deleteOnExit();
+        }


The array is redundant. I also suggest following the same pattern for creating files:

Suggested change

subFiles = new File[10];

for (int i = 0; i < 10; ++i) {

subFiles[i] = File.createTempFile("subFiles_" + (i+1),

".txt", new File(testFile.getAbsolutePath()));

subFiles[i].deleteOnExit();

}

for (int i = 0; i < 10; ++i) {

File subFile = new File(testFile, "subFiles_" + (i+1));

subFile.createNewFile();

subFile.deleteOnExit();

}

aivanov-jdk · 2023-12-04T20:52:28Z

test/jdk/com/sun/java/swing/plaf/gtk/TestFileChooserSingleDirectorySelection.java

+                fileChooser.setCurrentDirectory(testDir);
            });


fileChooser.setFileSelectionMode that you call below should be called on EDT, I suggest moving it into this block.

This applies to all check-* methods.

To eliminate duplicating code for disposing of the frame, create a helper method and call it from the finally block.

fileChooser.setFileSelectionMode that you call below should be called on EDT, I suggest moving it into this block.

This applies to all check-* methods.

Accessed on EDT.

To eliminate duplicating code for disposing of the frame, create a helper method and call it from the finally block.

Updated, looks cleaner code.

aivanov-jdk · 2023-12-04T21:00:58Z

test/jdk/com/sun/java/swing/plaf/gtk/TestFileChooserSingleDirectorySelection.java

Since GitHub doesn't allow adding comments to lines which aren't modified:

In doTesting, you should get the location and dimensions of the frame on EDT (and for the button too).

The passed field is to be declared volatile: you modify it on EDT in the button listener but you read its value on main thread.

Basically, you don't need the button, you can run the code in the button action listener directly. Be sure to run it on EDT though.

The listener code can be modified to perform only one volatile write:

public void actionPerformed(ActionEvent evt) { File files[] = fileChooser.getSelectedFiles(); passed = files.length != 0; }

Button and passed field are removed since it may not be required. The selected files are fetched in checkResult method.

aivanov-jdk · 2023-12-05T13:31:28Z

test/jdk/com/sun/java/swing/plaf/gtk/TestFileChooserSingleDirectorySelection.java

+        return dirsDir;
+    }
+
+    private static void populateDirs(File dirsDir) {


Suggested change

private static void populateDirs(File dirsDir) {

private static void populateDirs(File parent) {

It better conveys the meaning of the parameter.

aivanov-jdk · 2023-12-05T13:32:43Z

test/jdk/com/sun/java/swing/plaf/gtk/TestFileChooserSingleDirectorySelection.java

+            File SubDirs = new File(dirsDir, "subDir_" + (i+1));
+            SubDirs.mkdir();
+            SubDirs.deleteOnExit();


Suggested change

File SubDirs = new File(dirsDir, "subDir_" + (i+1));

SubDirs.mkdir();

SubDirs.deleteOnExit();

File subDir = new File(dirsDir, "subDir_" + (i+1));

subDir.mkdir();

subDir.deleteOnExit();

It's one subdirectory; local variable names start with lower-case letter in Java.

aivanov-jdk · 2023-12-05T13:33:17Z

test/jdk/com/sun/java/swing/plaf/gtk/TestFileChooserSingleDirectorySelection.java

+        return filesDir;
+    }
+
+    private static void populateFiles(File filesDir) throws Exception {


Suggested change

private static void populateFiles(File filesDir) throws Exception {

private static void populateFiles(File parent) throws Exception {

Or parentDir.

aivanov-jdk · 2023-12-05T13:36:13Z

test/jdk/com/sun/java/swing/plaf/gtk/TestFileChooserSingleDirectorySelection.java

+    }
+
+    private static void checkFileOnlyTest(UIManager.LookAndFeelInfo laf,
+                                          File crntDir) throws Exception {


Suggested change

File crntDir) throws Exception {

File dir) throws Exception {

Alternatively, spell out current; if you prefer to abbreviate, curr is rather standard abbreviation for ‘current’. However, I think dir is enough, its meaning is easily inferred as it's passed to fileChooser.setCurrentDirectory.

aivanov-jdk · 2023-12-05T13:38:01Z

test/jdk/com/sun/java/swing/plaf/gtk/TestFileChooserSingleDirectorySelection.java

+    private static void checkDirectoriesOnlyTest(UIManager.LookAndFeelInfo laf,
+                                             File crntDir) throws Exception {


Suggested change

private static void checkDirectoriesOnlyTest(UIManager.LookAndFeelInfo laf,

File crntDir) throws Exception {

private static void checkDirectoriesOnlyTest(UIManager.LookAndFeelInfo laf,

File crntDir) throws Exception {

Align the parameters, if you follow this style.

aivanov-jdk · 2023-12-05T13:41:09Z

test/jdk/com/sun/java/swing/plaf/gtk/TestFileChooserSingleDirectorySelection.java

+            frameLocation = fileChooser.getLocationOnScreen();
+            frameWidth = frame.getWidth();
+            frameHeight = frame.getHeight();


Suggested change

frameLocation = fileChooser.getLocationOnScreen();

frameWidth = frame.getWidth();

frameHeight = frame.getHeight();

bounds = new Rectangle(fileChooser.getLocationOnScreen(),

frame.getSize());

Isn't one variable enough? Moreover frameLocation stores the location of fileChooser.

Since now there's only one click location, this can be simplified further:

Suggested change

frameLocation = fileChooser.getLocationOnScreen();

frameWidth = frame.getWidth();

frameHeight = frame.getHeight();

Point fileChooserLocation = fileChooser.getLocationOnScreen();

fileChooserLocation.y += frame.getHeight() / 3;

clickLocation = new Point(fileChooserLocation);

Then you modify your clickMouse method to accept two parameters: Point and xOffset.

aivanov-jdk · 2023-12-05T13:49:44Z

test/jdk/com/sun/java/swing/plaf/gtk/TestFileChooserSingleDirectorySelection.java

    private static void checkResult(UIManager.LookAndFeelInfo laf) {
-        if (!passed)
+        File files[] = fileChooser.getSelectedFiles();
+        if (files.length == 0) {
            throw new RuntimeException("getSelectedFiles returned " +
-                    "empty array for LAF: "+laf.getClassName());
+                    "empty array for LAF: " + laf.getClassName());
+        }
+    }


And you ignored Be sure to run it on EDT: fileChooser.getSelectedFiles() is to be called on EDT.

I missed here to access fileChooser.getSelectedFiles() on EDT.

aivanov-jdk · 2023-12-05T14:00:33Z

test/jdk/com/sun/java/swing/plaf/gtk/TestFileChooserSingleDirectorySelection.java

-            checkFilesAndDirectoriesTest(laf);
+            checkFileOnlyTest(laf, testDirFiles);
+            checkDirectoriesOnlyTest(laf, testDirDirs);
+            checkFilesAndDirectoriesTest(laf, testDirDirs);


So, FILES_AND_DIRECTORIES mode is tested with directories only. Is it intentional? It's the problem in JDK-6972078… that directories can't be selected.

So, FILES_AND_DIRECTORIES mode is tested with directories only. Is it intentional? It's the problem in JDK-6972078… that directories can't be selected.

Yes, it is tested with directories only.

Is it intentional? It's the problem in JDK-6972078… that directories can't be selected.

Not intentional though. Thought of creating a single folder with the mix of files and directories initially but getting the location of files in FILES_ONLY mode was an issue in different LAF. So, created separate folders for directories and files and tested.

Is it intentional? It's the problem in JDK-6972078… that directories can't be selected.

Not intentional though. Thought of creating a single folder with the mix of files and directories initially but getting the location of files in FILES_ONLY mode was an issue in different LAF. So, created separate folders for directories and files and tested.

It's fine as long as the original issue is reproducible with directories only too.

aivanov-jdk · 2023-12-05T18:09:39Z

Is it possible to shorten the subject of the issue? It is too long and wordy, yet it doesn't add valuable context.

Ideally, the summary should be concise.

kumarabhi006 · 2023-12-05T18:31:54Z

@aivanov-jdk Updated the suggested changes,

kumarabhi006 · 2023-12-05T18:32:11Z

Is it possible to shorten the subject of the issue? It is too long and wordy, yet it doesn't add valuable context.

Ideally, the summary should be concise.

Yeah sure, it can be changed.

aivanov-jdk

ActionEvent and ActionListener aren't used any more, remove them, please.

Could you also update the copyright year in the file?

aivanov-jdk · 2023-12-05T18:58:47Z

test/jdk/com/sun/java/swing/plaf/gtk/TestFileChooserSingleDirectorySelection.java

-        }
+    private static void checkResult(UIManager.LookAndFeelInfo laf) throws Exception {
+        SwingUtilities.invokeAndWait(() -> {
+            File files[] = fileChooser.getSelectedFiles();


Suggested change

File files[] = fileChooser.getSelectedFiles();

File[] files = fileChooser.getSelectedFiles();

Let's use Java-style array declaration.

openjdk · 2023-12-05T19:01:46Z

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

8319938: TestFileChooserSingleDirectorySelection.java fails with "getSelectedFiles returned empty array"

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

cc25d8b: 8319662: ForkJoinPool trims worker threads too slowly
90e433d: 8320144: Compilation crashes when a custom annotation with invalid default value is used
50f3124: 8320892: AArch64: Restore FPU control state after JNI
0217b5a: 8321248: ClassFile API ClassModel::verify is inconsistent with the rest of the API
7fbfb3b: 8321369: Unproblemlist gc/cslocker/TestCSLocker.java
2678e4c: 8319111: Mismatched MemorySegment heap access is not consistently intrinsified
a0920aa: 8321159: SymbolLookup.libraryLookup(Path, Arena) Assumes default Filesystem
9d77677: 8321124: java/util/stream/GatherersTest.java times out
4c96aac: 8320935: Move CDS config initialization code to cdsConfig.cpp
78d0958: 8321406: Null IDs should be resolved as before catalogs are added
... and 716 more: https://git.openjdk.org/jdk/compare/4ea1b99c1a6efe144af381ea538f93718e9baf74...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 · 2023-12-05T19:29:17Z

test/jdk/com/sun/java/swing/plaf/gtk/TestFileChooserSingleDirectorySelection.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2022, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2023, Oracle and/or its affiliates. All rights reserved.


Suggested change

* Copyright (c) 2023, Oracle and/or its affiliates. All rights reserved.

* Copyright (c) 2022, 2023, Oracle and/or its affiliates. All rights reserved.

You always preserve the first year.

aivanov-jdk · 2023-12-05T19:51:07Z

Pay attention to the warning displayed by the bot: retarget the bug in JBS to 22 if you're going to integrate before the fork occurs.

TejeshR13 · 2023-12-06T05:17:34Z

test/jdk/com/sun/java/swing/plaf/gtk/TestFileChooserSingleDirectorySelection.java

+        robot.waitForIdle();
+    }
+
+    private static void checkResult(UIManager.LookAndFeelInfo laf) throws Exception {


Instead of passing the laf through all the methods, can't we create and use a class variable?

Instead of passing the laf through all the methods, can't we create and use a class variable?

Can be done but can't see any functional improvements. This seems ok to me.

Yeah, those are to reduce redundancy in code, not functional...... Otherwise the fix looks fine.

It's not redundancy: passing the laf explicitly makes the data flow clearer, even though laf isn't directly used.

I agree that making laf a class field doesn't provide much benefit.

In a way, it could be removed completely, however, reporting the L&F at which the test case failed gives more details, you don't need to look at the test log to find the L&F.

kumarabhi006 · 2023-12-06T16:40:44Z

/integrate

openjdk · 2023-12-06T16:41:27Z

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

cc25d8b: 8319662: ForkJoinPool trims worker threads too slowly
90e433d: 8320144: Compilation crashes when a custom annotation with invalid default value is used
50f3124: 8320892: AArch64: Restore FPU control state after JNI
0217b5a: 8321248: ClassFile API ClassModel::verify is inconsistent with the rest of the API
7fbfb3b: 8321369: Unproblemlist gc/cslocker/TestCSLocker.java
2678e4c: 8319111: Mismatched MemorySegment heap access is not consistently intrinsified
a0920aa: 8321159: SymbolLookup.libraryLookup(Path, Arena) Assumes default Filesystem
9d77677: 8321124: java/util/stream/GatherersTest.java times out
4c96aac: 8320935: Move CDS config initialization code to cdsConfig.cpp
78d0958: 8321406: Null IDs should be resolved as before catalogs are added
... and 716 more: https://git.openjdk.org/jdk/compare/4ea1b99c1a6efe144af381ea538f93718e9baf74...master

Your commit was automatically rebased without conflicts.

openjdk · 2023-12-06T16:41:33Z

@kumarabhi006 Pushed as commit 4ef24e2.

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

Test fix

846c5de

openjdk bot added the rfr Pull request is ready for review label Nov 15, 2023

openjdk bot added the client client-libs-dev@openjdk.org label Nov 15, 2023

prrace reviewed Nov 15, 2023

View reviewed changes

Review comment fix

f0f4c96

aivanov-jdk reviewed Nov 20, 2023

View reviewed changes

kumarabhi006 mentioned this pull request Nov 22, 2023

8224261: JProgressBar always with border painted around it #16467

Closed

3 tasks

Test update

9c74599

aivanov-jdk mentioned this pull request Nov 30, 2023

8320608: Many jtreg printing tests are missing the @printer keyword #16785

Closed

3 tasks

aivanov-jdk suggested changes Dec 4, 2023

View reviewed changes

Review comment fix

cd69d57

aivanov-jdk suggested changes Dec 5, 2023

View reviewed changes

Access on EDT and few changes

aa034e9

aivanov-jdk approved these changes Dec 5, 2023

View reviewed changes

openjdk bot added the ready Pull request is ready to be integrated label Dec 5, 2023

Remove unused imports

25965f3

aivanov-jdk suggested changes Dec 5, 2023

View reviewed changes

openjdk bot removed the ready Pull request is ready to be integrated label Dec 5, 2023

Copyright year update

ee6d003

aivanov-jdk approved these changes Dec 5, 2023

View reviewed changes

openjdk bot added the ready Pull request is ready to be integrated label Dec 5, 2023

TejeshR13 reviewed Dec 6, 2023

View reviewed changes

TejeshR13 approved these changes Dec 6, 2023

View reviewed changes

openjdk bot added the integrated Pull request has been integrated label Dec 6, 2023

openjdk bot closed this Dec 6, 2023

openjdk bot removed ready Pull request is ready to be integrated rfr Pull request is ready for review labels Dec 6, 2023

	import java.awt.Font;
	import java.awt.FontFormatException;
	import java.io.File;

	import java.awt.Font;
	import java.awt.font.FontRenderContext;
	import java.awt.geom.AffineTransform;
	import java.lang.ref.Reference;
	import java.lang.ref.SoftReference;
	import java.lang.ref.WeakReference;
	import java.util.concurrent.ConcurrentHashMap;
	import java.util.Locale;
	import java.util.Set;

	import java.util.*;
	import java.text.*;
	import java.io.*;
	import java.net.*;
	import java.awt.*;


		// create temporary files inside testDir
		tempFile = new File[5];

		private static File[] SubDirs;
		private static File[] subFiles;

	private static void populateDirs(File dirsDir) {
	private static void populateDirs(File parent) {

	private static void populateFiles(File filesDir) throws Exception {
	private static void populateFiles(File parent) throws Exception {

	File crntDir) throws Exception {
	File dir) throws Exception {

		private static void checkDirectoriesOnlyTest(UIManager.LookAndFeelInfo laf,
		File crntDir) throws Exception {

	File files[] = fileChooser.getSelectedFiles();
	File[] files = fileChooser.getSelectedFiles();

	* Copyright (c) 2023, Oracle and/or its affiliates. All rights reserved.
	* Copyright (c) 2022, 2023, Oracle and/or its affiliates. All rights reserved.

8319938: TestFileChooserSingleDirectorySelection.java fails with "getSelectedFiles returned empty array" #16674

8319938: TestFileChooserSingleDirectorySelection.java fails with "getSelectedFiles returned empty array" #16674

Conversation

kumarabhi006 commented Nov 15, 2023 • edited by openjdk bot Loading

Progress

Issue

Reviewers

Reviewing

Webrev

bridgekeeper bot commented Nov 15, 2023

openjdk bot commented Nov 15, 2023

mlbridge bot commented Nov 15, 2023 • 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aivanov-jdk Nov 20, 2023 • edited Loading

Choose a reason for hiding this comment

kumarabhi006 Nov 22, 2023 • edited Loading

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 Nov 20, 2023 • edited Loading

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

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

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

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 commented Dec 5, 2023

kumarabhi006 commented Dec 5, 2023

kumarabhi006 commented Dec 5, 2023

aivanov-jdk left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

openjdk bot commented Dec 5, 2023 • edited Loading

Choose a reason for hiding this comment

aivanov-jdk commented Dec 5, 2023

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

kumarabhi006 commented Dec 6, 2023

openjdk bot commented Dec 6, 2023

openjdk bot commented Dec 6, 2023

kumarabhi006 commented Nov 15, 2023 •

edited by openjdk bot

Loading

mlbridge bot commented Nov 15, 2023 •

edited

Loading

aivanov-jdk Nov 20, 2023 •

edited

Loading

kumarabhi006 Nov 22, 2023 •

edited

Loading

aivanov-jdk Nov 20, 2023 •

edited

Loading

openjdk bot commented Dec 5, 2023 •

edited

Loading