Andrey,
> On 30 Dec 2016, at 14:11, Andrey Nazarov <[email protected]> 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.