On Mon, 10 Oct 2022 16:14:56 GMT, Alexey Ivanov <aiva...@openjdk.org> wrote:
>> Tejesh R has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Updated based on review comments
>
> test/jdk/javax/swing/JFileChooser/FileViewNPETest.java line 35:
>
>> 33:
>> 34: import javax.swing.filechooser.FileView;
>> 35: import jdk.test.lib.Platform;
>
> It's not used any more, please remove.
>
> Leave one blank line after the last import and the following comment with the
> jtreg tags.
Updated.
> test/jdk/javax/swing/JFileChooser/FileViewNPETest.java line 65:
>
>> 63: JFileChooser jfc;
>> 64:
>> 65: //Initialize the components
>
> I believe the comment is redundant. Being placed before the instruction text,
> it's confusing. The method name `initialize` is self-explanatory.
Updated.
> test/jdk/javax/swing/JFileChooser/FileViewNPETest.java line 77:
>
>> 75: 3. If NPE occurs on second selection then test FAILS,
>> else test
>> 76: is PASS.
>> 77: """;
>
> I propose the following text:
>
>
> Instructions to Test:
> 1. The traversable folder is set to the Documents folder,
> if it exists, in the user's home folder, otherwise
> it's the user's home. Other folders are non-traversable.
> 2. When the file chooser appears on the screen, select any
> non-traversable folder from "Look-In" combo box,
> for example the user's folder or a folder above it.
> (The folder will not be opened since it's non-traversable).
> 3. Select the Documents folder again.
> 4. If NullPointerException occurs in the step 3, click Fail,
> otherwise click Pass.
>
> It consistently uses _“folder”_, no mention of “directory”. It explains that
> the user's home could be the root of the traversable tree. The instructions
> use imperative to select either _Fail_ or _Pass_.
4th point has been modified, since user can only select Pass, otherwise the
test itself fails.
> test/jdk/javax/swing/JFileChooser/FileViewNPETest.java line 81:
>
>> 79: passFailJFrame = new PassFailJFrame("Test Instructions",
>> INSTRUCTIONS, 5L, 13, 40);
>> 80: jfc = new JFileChooser();
>> 81: String path = System.getProperty("user.home") + File.separator +
>> "Documents";
>
> Suggestion:
>
>
> String userHome = System.getProperty("user.home");
> String docs = userHome + File.separator + "Documents";
> String path = (new File(docs).exists()) ? docs : userHome;
>
> This will make the test more robust, it can continue to run even if
> `Documents` folder does not exist.
Updated.
> test/jdk/javax/swing/JFileChooser/FileViewNPETest.java line 84:
>
>> 82:
>> 83: jfc.setCurrentDirectory(new File(path));
>> 84: jfc.setFileView(new CustomFileView(path));
>
> Suggestion:
>
> jfc.setFileView(new CustomFileView(path));
> jfc.setControlButtonsAreShown(false);
>
> Hide _Open_ and _Cancel_ buttons in the file chooser.
Updated.
-------------
PR: https://git.openjdk.org/jdk/pull/10485
