Alex,
Thanks for adding the test, few comments:
PackTestZip64.java:
1. compareTwoFile, I would read the entire file into
ByteArrayInputStream, compare
the total first ie. fast fail, but what you have is also ok.
2. generateLargeJar: I would replace lines 126,127 and 128 using
for-each loop,
by using Collections.list<Enumeration> e); will make it more compact.
Though you are usign Try-With-Resources on the input jar, how about
3. Though the test might not take too long on your system, I am
concerned it may
take too long on our Jprt system.
Otherwise looks good.
Kumar
On 1/16/2014 8:21 AM, Alexander Zuev wrote:
Sherman, Kumar,
i have fixed the glitches you have found and changed the test so it
creates a new jar
based on the golden.jar content (to have a reasonable set of various
data to start with),
then adding to it 65536 empty files to test how we cope with such a
huge jars.
The testing time is less than a 30 seconds on my machine (not the
top-of-the-line) so i
decided not to bother with conditional testing, lets test our behavior
every time in full.
The updated webrev can be found at
http://cr.openjdk.java.net/~kizune/8029646/webrev.02
/Alex
On 1/15/14 21:34, Xueming Shen wrote:
On 1/15/14 7:01 AM, Alexander Zuev wrote:
Hello,
the new webrev with all the typos and comments fixed can be found
at http://cr.openjdk.java.net/~kizune/8029646/webrev.01/
/Alex
(1) jarmagic can be just a static constant somewhere or a stack
variable. not big deal though.
(2) the test only "tests" EOF for s. there is possibility that the
newly created has more extra
bytes at the end...though in theory this should not happen, it
might be better just add an
extra line to check the sizes of two first first?
(3) the rest of the change looks good, but I agreed with Kumar that
you may need to add a
regression test for a jar file with > 64k entries. otherwise
the code for zip64 end is not
being tested. the code looks fine, but I would trust a
regression test more than my eyes:-)
Thanks!
-Sherman
