RFR: 8071566 Improve testing for multi-version JAR file maker tool

Chris Hegarty chris.hegarty at oracle.com
Wed Jan 18 14:59:32 UTC 2017


> On 17 Jan 2017, at 15:33, Andrey Nazarov <andrey.x.nazarov at oracle.com> wrote:
> 
> Thanks for review. I’ve update patch http://cr.openjdk.java.net/~anazarov/8071566/webrev.01/

I think this is ok.

-Chris.

> 
> —Andrey
>> On 3 Jan 2017, at 14:25, Chris Hegarty <chris.hegarty at oracle.com> wrote:
>> 
>> Andrey,
>> 
>>> On 30 Dec 2016, at 14:11, Andrey Nazarov <andrey.x.nazarov at oracle.com> wrote:
>>> 
>>> Hi,
>>> 
>>> Following tests for Jar tool were added:
>>> 
>>> Tests for API validator. Jar tool should detect API changes between releases. 
>>> Test for custom manifest file. 
>>> Test for version format in --release option 
>>> 
>>> Also refactoring of existing tests was made:
>>> 1. Common base class “MRTestBase.java” was extracted. 
>>> 2. Result and process builders were substituted by existing library classes jdk/test/lib/process/*
>>> 
>>> Please review.
>>> 
>>> Review: http://cr.openjdk.java.net/~anazarov/8071566/webrev.00/
>>> Issue: https://bugs.openjdk.java.net/browse/JDK-8071566 <https://bugs.openjdk.java.net/browse/JDK-8071566>
>> 
>> I think the changes are generally ok. A few comments:
>> 
>> 1) The webrev shows ApiValidatorTest.java  (was test/tools/jar/multiRelease/Basic.java).
>>    I assume ApiValidatorTest.java is just a new file, and not Basic.java copied.
>>    These files should have no relationship in mercurial.
>> 
>> 2) Style. The existing style is a little confused to me.
>>    Please check line length, <= 80 chars where possible
>>    Align dots ( fluid method invocations ) where possible
>>    Align method arguments where possible, and makes sense
>> 
>> 3) Where deleting files/directories, please use FileUtils from the test library.
>> 
>> -Chris.
> 



More information about the core-libs-dev mailing list