RFR: 8288882: JFileChooser - empty (0 bytes) file is displayed as 1 KB [v14]

Alexey Ivanov aivanov at openjdk.org
Fri Aug 5 19:03:23 UTC 2022


On Thu, 28 Jul 2022 10:08:24 GMT, Abhishek Kumar <duke at openjdk.org> wrote:

>> JFileChooser - empty file size issue fixed. 
>> For empty file, now the size 0 bytes.
>> Manual Test Case "ZeroFileSizeCheck.java" created.
>
> Abhishek Kumar has updated the pull request incrementally with one additional commit since the last revision:
> 
>   creating and deleting test files dynamically

There are many comments. Could you, @kumarabhi006, summarise the approach taken, please?

src/java.desktop/share/classes/sun/swing/FilePane.java line 1235:

> 1233:             DecimalFormat df = new DecimalFormat("0.0");
> 1234:             double val = len/baseFileSize;
> 1235:             return  Double.valueOf(df.format(val));

Can we achieve the same effect without converting the value from double to string and back to double?

The returned value is `(len / 100L) / 10.0d`.

test/jdk/javax/swing/JFileChooser/FileSizeCheck.java line 62:

> 60:         Path dir = Paths.get(System.getProperty("test.src"));
> 61:         String [] tempFilesName = {"2.5-KB-File","2.8-MB-File","999-KB-File","1000-KB-File","2047-Byte-File","Empty-File"};
> 62:         int [] tempFilesSize = {2500, 2800000,999000,1000000,2047,0};

Does it make sense to sort the files by size by prefixing them? "1-Empty-File", "2-File-2047-Byte"?

Add a space after the commas.

test/jdk/javax/swing/JFileChooser/FileSizeCheck.java line 66:

> 64:         for (int i = 0 ; i < tempFilesName.length ; i++) {
> 65:             tempFilePaths[i] = dir.resolve(tempFilesName[i]);
> 66:         }

Can't the body of this for-loop be part of the next one?

There should be no space before the semi-colon.

test/jdk/javax/swing/JFileChooser/FileSizeCheck.java line 72:

> 70:             for (int i = 0 ; i < tempFilePaths.length ; i++) {
> 71:                 if (!Files.exists(tempFilePaths[i])){
> 72:                     RandomAccessFile f = new RandomAccessFile(tempFilePaths[i].toString(), "rw");

Suggestion:

                    RandomAccessFile f = new RandomAccessFile(tempFilePaths[i].toFile(), "rw");

would be more effective. The string version will create a `File` object internally.

test/jdk/javax/swing/JFileChooser/FileSizeCheck.java line 84:

> 82:         fc.showOpenDialog(null);
> 83: 
> 84:         // delete temp files

For delete to be run, the JFileChooser should be dismissed before the tester presses Pass or Fail.

test/jdk/javax/swing/JFileChooser/FileSizeCheck.java line 87:

> 85:         try {
> 86:             for (int i = 0 ; i < tempFilePaths.length ; ++i) {
> 87:                 Files.deleteIfExists(Paths.get(tempFilePaths[i].toString()));

Suggestion:

                Files.deleteIfExists(tempFilePaths[i]);


You already have `Path` objects, there's no need to convert them to a `String` and then back to `Path`.

test/jdk/javax/swing/JFileChooser/FileSizeCheck.java line 91:

> 89:         } catch (IOException ex) {
> 90:             throw new RuntimeException(ex);
> 91:         }

Probably, errors during test clean-up are not as important to fail the test?

test/jdk/javax/swing/JFileChooser/FileSizeCheck.java line 99:

> 97:         SwingUtilities.invokeAndWait(() -> {
> 98:             frame = new JFrame();
> 99:             PassFailJFrame.addTestWindow(frame);

You don't use a secondary frame. Maybe skip creating it?

Alternatively, you can add the `JFileChooser` as the content of the frame as it's done in #9597.

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

PR: https://git.openjdk.org/jdk/pull/9327



More information about the client-libs-dev mailing list