Chris Y, > On 11 Oct 2018, at 09:45, Chris Yin <xu.y....@oracle.com> wrote: > > Please review below small change to fix test > sun/net/ftp/FtpURLConnectionLeak.Java, thanks > > Besides the original timeout issue, looks like the test not working as > expected even the results is pass. Per test description, we expect > FileNotFoundException and then verify connection closed, but look through > recent test run log, it’s actually got “sun.net.ftp.FtpLoginException: > Invalid username/password” and this exception been caught in IOException > block coincidentally, so the test result is pass, but it never hit > FileNotFoundException. The fix change will remove IOException catch block and > close server socket in close() of try-with-resouce to avoid possible invalid > exception catch and connection issue, also ftp 220 response message was > modified to allow FtpClient working correctly (I leave the corrupted message > style as it is since looks like it will be used to test JDK-8151586) > > bug: https://bugs.openjdk.java.net/browse/JDK-8187522 > <https://bugs.openjdk.java.net/browse/JDK-8187522> > webrev: http://cr.openjdk.java.net/~xyin/8187522/webrev.00/
I think this is good. The `dash` character in FtpCommandHandler is subtle. I kinda regret allowing that change, rather than creating a separate standalone test for 8151586. It just confusing and overloads the test infra. It might ok at this point, but a comment would be good. -Chris.
