Request for review: 7201156 : jar tool fails to convert file separation characters for list and extract

[Chris Hegarty]

Sean,

Looks good to me. Thanks for updating the test.

-Chris.

On 11/13/2012 03:17 AM, Sean Chou wrote:
> Hi Alan,
>
> Here is the updated webrev:
> http://cr.openjdk.java.net/~zhouyx/7201156/webrev.03/  .
>
>
> On Mon, Nov 12, 2012 at 6:00 PM, Alan Bateman <Alan.Bateman at oracle.com>wrote:
>
>> On 12/11/2012 09:08, Sean Chou wrote:
>>
>>> Hi ,
>>>
>>> I didn't realize that rt.jar would miss before. The testcase is updated
>>> to create a temp file as well as test the extract option. However, extract
>>> testing will create a directory if it passes, I just deleted it in
>>> testcase.  Please take a look.
>>>
>>> webrev: http://cr.openjdk.java.net/~**zhouyx/7201156/webrev.02/<http://cr.openjdk.java.net/~zhouyx/7201156/webrev.02/><
>>> http://cr.openjdk.java.net/%**7Ezhouyx/7201156/webrev.02/<http://cr.openjdk.java.net/%7Ezhouyx/7201156/webrev.02/>>
>>>   .
>>>
>>>   Thanks for removing the dependency on rt.jar.
>>
>> A couple of comments on the updated test:
>>
>> - in main then it still has "pathRtJar" so I assume you just forgot to
>> rename that.
>>
> I forgot the name had its meaning.
>
>>
>> - it would be good to change createJarFile to use try-with-resources so
>> that an error doesn't cause it to terminate with an open file.
>>
> Changed.
>
>>
>> - testJarExact, I assume you intended to name this testJarExtract.
>>
> Yes!
>
>>
>> - L114-117, the "./" is not needed. It would be okay to leave those files
>> there if you want, jtreg will clean them up too.
>
> Thanks for the hint. I removed these lines, so the testcase looks tidy.
>
>>
>>
>> -Alan.
>>
>>
>>
>
>