On 21/03/16 08:11, vyom wrote:
Hi,

i modified the existing ftp server that we are using it for our testing
so that when server starts it will return a response which contains some
empty new lines. This change will ensures that readServerResponse()
throws the "IndexOutOfBoundsException" which we are catching.

As existing test cases are covering my code changes that's why i decided
not to write new test case.

Thanks for the explanation Vyom, that's fine. I can sponsor
this change for you.

-Chris.

Thanks,
Vyom


On Thursday 17 March 2016 03:01 PM, vyom wrote:


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