On Mon, 20 Nov 2023 19:51:05 GMT, Alexey Ivanov <[email protected]> wrote:
>> 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.
For some reasons when I used the `current directory i.e. scratch`, test failed
in my local machine.
`String tmpDir = System.getProperty(".");`
> 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 updated.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/16674#discussion_r1401527591
PR Review Comment: https://git.openjdk.org/jdk/pull/16674#discussion_r1401526335