RFR 8212807: tools/jar/multiRelease/Basic.java times out

[Brent Christian]

Thank you for elaborating.  The new version looks good.

-Brent

On 5/30/19 1:29 PM, Lance Andersen wrote:
> Hi Brent,
>> On May 30, 2019, at 4:02 PM, Brent Christian 
>> <brent.christian at oracle.com <mailto:brent.christian at oracle.com>> wrote:
>>
>> Hi, Lance
> 
> Thank you for the review.
> 
>>
>> This change is to collect more information in case this happens again, 
>> yes?
> 
> This changes reduces the use of ProcessBuilder resulting in much 
> improved test runs similar to what I did for:
> https://mail.openjdk.java.net/pipermail/core-libs-dev/2019-April/059471.html
> 
> I took out the timing output from the tests after verifying the 
> reduction in the test runs (which I ran 100 on the failing systems via 
> mach 5 comparing before/after times)
> 
> Using ToolProvider is much more efficient than ProcessBuilder as I found 
> out from the previous issue.  The reduction in time was in line with the 
> previous issue :-)
>>
>> Looks pretty good - just a couple comments:
>>
>>
>> test/jdk/tools/jar/multiRelease/Basic.java
>>
>> 536         jar("ufm", jarfile, manifest.toString(),
>>
>> Is there a reason not to convert this to call jarTool() ?
> 
> Yes,  java.util.jar.Attributes uses java.util.Logging to emit a warning 
> for some reason which makes it a bit more difficult to deal with in this 
> specific test.  So I left this one test for now.  At some point I want 
> to go back through the other tests which use MRTestBase  and convert the 
> tests to also use ToolProvider and I can look to  revisit the issue then.
> 
> Right now I am trying to cut down on the noise of some of the random 
> timeouts :-)
>>
>> --
>>
>> test/jdk/tools/jar/multiRelease/MRTestBase.java
>>
>>
>> L146-L152
>>
>> indentation looks off-by-one
>>
> 
> Thank you for catching this,  I updated the webrev : 
> http://cr.openjdk.java.net/~lancea/8212807/webrev.01/
> 
> Best
> Lance
>>
>> Thanks,
>> -Brent
>>
>> On 5/30/19 9:21 AM, Lance Andersen wrote:
>>> Hi all,
>>> The following fix addresses an issue with an occasional timeout for 
>>> tools/jar/multiRelease/Basic.java.
>>> The webrev can be found at: 
>>> http://cr.openjdk.java.net/~lancea/8212807/webrev.00/index.html 
>>> <http://cr.openjdk.java.net/~lancea/8212807/webrev.00/index.html>
>>> Best,
>>> Lance
>>>  <http://oracle.com/us/design/oracle-email-sig-198324.gif>
>>>  <http://oracle.com/us/design/oracle-email-sig-198324.gif> 
>>> <http://oracle.com/us/design/oracle-email-sig-198324.gif>
>>>  <http://oracle.com/us/design/oracle-email-sig-198324.gif>Lance 
>>> Andersen| Principal Member of Technical Staff | +1.781.442.2037
>>> Oracle Java Engineering
>>> 1 Network Drive
>>> Burlington, MA 01803
>>> Lance.Andersen at oracle.com <mailto:Lance.Andersen at oracle.com> 
>>> <mailto:Lance.Andersen at oracle.com>
> 
> <http://oracle.com/us/design/oracle-email-sig-198324.gif>
> <http://oracle.com/us/design/oracle-email-sig-198324.gif><http://oracle.com/us/design/oracle-email-sig-198324.gif>
> <http://oracle.com/us/design/oracle-email-sig-198324.gif>Lance Andersen| 
> Principal Member of Technical Staff | +1.781.442.2037
> Oracle Java Engineering
> 1 Network Drive
> Burlington, MA 01803
> Lance.Andersen at oracle.com <mailto:Lance.Andersen at oracle.com>
> 
> 
>