RFR: 8071566 Improve testing for multi-version JAR file maker tool
Andrey Nazarov
andrey.x.nazarov at oracle.com
Wed Jan 18 16:18:24 UTC 2017
> On 18 Jan 2017, at 17:59, Chris Hegarty <chris.hegarty at oracle.com> wrote:
>
>
>> 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.
Thanks
>
> -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