Thanks for review. I’ve update patch http://cr.openjdk.java.net/~anazarov/8071566/webrev.01/
—Andrey > On 3 Jan 2017, at 14:25, Chris Hegarty <chris.hega...@oracle.com> wrote: > > Andrey, > >> On 30 Dec 2016, at 14:11, Andrey Nazarov <andrey.x.naza...@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.
