Hi Krishna,
Please find my inputs:
Test case needs to be updated with minor changes:
1) Copyright information needs to be added at the start of test file.
2) Jtreg test summary needs to be updated to mention what the test is
trying to achieve instead of JBS bug title.
3) Also since this test is specific to Windows we can remove the OS test
code present in test case with Jtreg @requires flag:
Instead of using the below code we can use @requires (os.family == "windows")
if (OSInfo.getOSType() != OSInfo.OSType.WINDOWS) {
System.out.println("The test is
suitable only for Windows OS. Skipped.");
return;
}
4) For multiline comments at Line no 1 & 26 in test case please update the
comment syntax to use :
/*
*
*
*/
Thanks,
Jay
From: Krishna Addepalli
Sent: Tuesday, January 09, 2018 2:30 PM
To: Prasanta Sadhukhan; [email protected]
Subject: Re: <Swing Dev> [10][11][JDK-8194044] Regression manual Test
javax/swing/JFileChooser/8067660/FileChooserTest.java fails
Hi Prasanta,Sergey,
I have added a testcase along with the changes and created a new webrev :
http://cr.openjdk.java.net/~kaddepalli/8194044/webrev01/
Please review this and provide your comments.
Thanks,
Krishna
From: Prasanta Sadhukhan
Sent: Thursday, January 4, 2018 10:54 AM
To: Krishna Addepalli <HYPERLINK
"mailto:[email protected]"[email protected]>; HYPERLINK
"mailto:[email protected]"[email protected]
Subject: Re: <Swing Dev> [10][11][JDK-8194044] Regression manual Test
javax/swing/JFileChooser/8067660/FileChooserTest.java fails
A regression test is added to prove that before your fix, java was failing and
after your fix is applied, it is passing so it is not related to older/newer
java versions. I saw we already have
test/jdk/javax/swing/JFileChooser/6945316/bug6945316.java which tests
isFileSystemRoot. Maybe, we can do something similar if it's not too much of a
task.
Regards
Prasanta
On 1/3/2018 5:13 PM, Krishna Addepalli wrote:
Thanks for the review Prasanta. However, I don't see a point to write a test
case for isFileSystemRoot(), since, it is not going to fail on any
(older/newer) java versions, and it was only introduced because of the fix for
JDK-8175015.
Let me know if you think otherwise.
Regards,
Krishna
From: Prasanta Sadhukhan
Sent: Wednesday, January 3, 2018 11:27 AM
To: Krishna Addepalli HYPERLINK
"mailto:[email protected]";<[email protected]>; HYPERLINK
"mailto:[email protected]"[email protected]
Subject: Re: <Swing Dev> [10][11][JDK-8194044] Regression manual Test
javax/swing/JFileChooser/8067660/FileChooserTest.java fails
Fix looks fine. But I guess, it is possible to add a automated regression test
to it utilising isFileSystemRoot().
Regards
Prasanta
On 1/2/2018 4:31 PM, Krishna Addepalli wrote:
Hi All,
Please review a fix for bug:
Bug: JDK-8194044 : https://bugs.openjdk.java.net/browse/JDK-8194044
Webrev: HYPERLINK
"http://cr.openjdk.java.net/%7Ekaddepalli/8194044/webrev00/"http://cr.openjdk.java.net/~kaddepalli/8194044/webrev00/
This was caused due to the fix for JDK-8175015, in which the line 446 in
Win32ShellFolderManager2.java was changed from "getDrives()" to
Win32ShellFolder2.listRoots().
While the earlier function returns an object of Win32ShellFolder2, the latter
returns an array of Files.
The condition on line 450: "return (sf.isFileSystem()&&sf.parent != null &&
sf.parent.equals(Win32ShellFolder2.listRoots())" was returning false because of
the wrong object being passed. Earlier it was a Win32ShellFolder2 object, and
the comparision was done properly, but with the changes, the equals fucnction
was receiving a file array object, and hence it was immediately returning
false, leading to the problem of empty strings being shown for Root drives.
The fix is to replace "Win32ShellFolder2.listRoots()" with "getDrives()"
function. With this fix, the regression is addressed, as well as the original
JDK-8175015 which was a memory leak issue.
Thanks,
Krishna
