RFR: 8319938: com/sun/java/swing/plaf/gtk/TestFileChooserSingleDirectorySelection.java fails with "getSelectedFiles returned empty array for LAF: javax.swing.plaf.metal.MetalLookAndFeel" [v2]

[Alexey Ivanov]

On Thu, 16 Nov 2023 05:19:43 GMT, Abhishek Kumar <abhiscxk at openjdk.org> wrote:

>> 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.
>
> Abhishek Kumar has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Review comment fix

test/jdk/com/sun/java/swing/plaf/gtk/TestFileChooserSingleDirectorySelection.java line 66:

> 64:         try {
> 65:             // create test directory
> 66:             String tmpDir = System.getProperty("java.io.tmpdir");

This is the purpose of the [*scratch* directory in jtreg](https://openjdk.org/jtreg/faq.html#scratch-directory), and it's automatically [cleaned up after the test](https://openjdk.org/jtreg/writetests.html#cleanFiles): <q cite="https://openjdk.org/jtreg/writetests.html#cleanFiles">jtreg will automatically clean up any files written in the scratch directory.</q>

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.

test/jdk/com/sun/java/swing/plaf/gtk/TestFileChooserSingleDirectorySelection.java line 88:

> 86:                 testFile.mkdir();
> 87:             }
> 88:             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/jdk/com/sun/java/swing/plaf/gtk/TestFileChooserSingleDirectorySelection.java line 98:

> 96:             }
> 97:         } catch (Exception e) {
> 98:             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.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/16674#discussion_r1399678519
PR Review Comment: https://git.openjdk.org/jdk/pull/16674#discussion_r1399677679
PR Review Comment: https://git.openjdk.org/jdk/pull/16674#discussion_r1399680175