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

Reply via email to