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
aivanov at openjdk.org
Mon Nov 20 20:04:15 UTC 2023
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
More information about the client-libs-dev
mailing list