On Thursday 17 March 2016 02:17 PM, Chris Hegarty wrote:
On 17 Mar 2016, at 06:57, vyom <vyom.tew...@oracle.com> wrote:
Hi Chris,

thanks for review, please find the updated webrev. I updated the existing test 
case to cover this issue.

http://cr.openjdk.java.net/~vtewari/8151586/webrev0.1/index.html 
<http://cr.openjdk.java.net/%7Evtewari/8151586/webrev0.1/index.html>
The source changes look fine.

The test: you have just updated the output of an existing test.
Do this test cover your changes already? Or have you forgotten
to include the test updates?
yes, any existing ftp tests like(B6427768 etc) which create a ftp connection and call getInputStream()() will cover my code change as well i already tested at my box.

I feel it is not worth to write a new test case which just create a URLConnection and call conn.getInputStream() as most of existing test cases are already doing this..

Vyom

-Chris.

Thanks,
Vyom

On Tuesday 15 March 2016 05:08 PM, Chris Hegarty wrote:
Vyom,

On 15/03/16 09:51, vyom wrote:
Hi,
Please review the below fix.

Bug: JDK-8151586 : Wrong exception catch for FTPClient in JDK-8055032
Webrev :http://cr.openjdk.java.net/~rgoel/~vyom/8151586/webrev0.0/
<http://cr.openjdk.java.net/%7Ergoel/%7Evyom/8151586/webrev0.0/>
The source change looks ok.

The test attempts to connect to an external ftp site. It
just fails on my machine with java.net.NoRouteToHostException.

Does the test need to run in othervm mode. I think not.

To simplify the test, just add 'throws IOException' to
main, so you don't have to deal with wrapping these in
RuntimeException. If a test fails with an IOException it
will be reported as failed by the test harness. Try it.

-Chris.

Reply via email to