Thanks, Max.
Please see inline.
On 01/21/2015 11:29 AM, Wang Weijun wrote:
Thanks for adding so many tests. Some suggestions:
- JarUtils.java
You can use the new InputStream.transferTo() method.
Sure, I didn't know about that, thanks.
I am not sure if I understand updateJar correctly. It looks like srcJarFile is
opened multiple times so its entries are duplicated a lot in the destination.
Or is there a secret break?
There is no any secret, just a bug. It is not necessary to open
srcJarFile multiple times.
I have updated the webrev, updateJar() method does the following:
- creates a new jar file (destJarFilename)
- puts files which are specified in files parameter to destJarFilename
- copies files from srcJarFilename to destJarFilename if they are no
files with the same names in destJarFilename
- Utils.java
The mixed using of File and Files is strange, but you are free to keep it.
Agree. I removed this method since tests don't need it anymore after I
removed cleanup() methods (please see below).
- TimestampCheck.java
You can make Handler Autocloseable to use try-with-resources in tests.
Good idea.
- Various tests
I am not a fan of calling Utils.cleanup() in final block. Unless you have
created huge garbages, those files are precious when the test fails (given you
provide -retain in jtreg, which I always do).
Agree, those files may be helpful, they are not huge. I have removed a
cleanup() methods.
Here is an updated webrev with your suggested changes and a couple of
others:
- added @ignore tag for BadKeyUsageTest since it fails due to a bug in JDK
- updated MultipleWarningsTest test to check ExtendedKeyUsage case
instead of KeyUsage
http://cr.openjdk.java.net/~asmotrak/8049171/webrev.01/
Artem
Thanks
Max
On Jan 21, 2015, at 14:35, Artem Smotrakov <artem.smotra...@oracle.com> wrote:
Hello,
Please review a couple of new tests for jarsigner's warnings. Basically tests
run jarsigner and check warning/error messages and exit codes according to [1].
https://bugs.openjdk.java.net/browse/JDK-8049171
http://cr.openjdk.java.net/~asmotrak/8049171/webrev.00
[1] http://docs.oracle.com/javase/7/docs/technotes/tools/windows/jarsigner.html
Artem
