RFR 8151586: Wrong exception catch for FTPClient in JDK-8055032

[vyom]

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,
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.tewari at 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.
>