code review 4815048: zip: add a test for the previous >65k files fix

zip: add a test for the previous >65k files fix

This surprisingly takes 30 seconds on my fast machine
so disabling by default. Need to optimize the Writer
at some point.

[bradfitz, 2011-07-22 14:23:36]

Hello golang-dev@googlegroups.com (cc: golang-dev@googlegroups.com),

I'd like you to review this change to
https://go.googlecode.com/hg/

[bradfitz, 2011-07-22 14:28:29]

Hello golang-dev@googlegroups.com (cc: golang-dev@googlegroups.com),

Please take another look.

[r, 2011-07-22 22:06:31]

LGTM

http://codereview.appspot.com/4815048/diff/8001/src/pkg/archive/zip/zip_test.go
File src/pkg/archive/zip/zip_test.go (right):

http://codereview.appspot.com/4815048/diff/8001/src/pkg/archive/zip/zip_test....
src/pkg/archive/zip/zip_test.go:5: // Tests that involved both reading &
writing.
s/involved/involve/

http://codereview.appspot.com/4815048/diff/8001/src/pkg/archive/zip/zip_test....
src/pkg/archive/zip/zip_test.go:38: }
maybe put some content into the file?
"this is %d.dat"?

http://codereview.appspot.com/4815048/diff/8001/src/pkg/archive/zip/zip_test....
src/pkg/archive/zip/zip_test.go:48: if got := len(zr.File); got != nFiles {
can you then easily the content is right?

[r2, 2011-07-22 22:07:33]

i didn't mean LGTM

On 23/07/2011, at 8:06 AM, r@golang.org wrote:

> LGTM
> 
> 
>
http://codereview.appspot.com/4815048/diff/8001/src/pkg/archive/zip/zip_test.go
> File src/pkg/archive/zip/zip_test.go (right):
> 
>
http://codereview.appspot.com/4815048/diff/8001/src/pkg/archive/zip/zip_test....
> src/pkg/archive/zip/zip_test.go:5: // Tests that involved both reading &
> writing.
> s/involved/involve/
> 
>
http://codereview.appspot.com/4815048/diff/8001/src/pkg/archive/zip/zip_test....
> src/pkg/archive/zip/zip_test.go:38: }
> maybe put some content into the file?
> "this is %d.dat"?
> 
>
http://codereview.appspot.com/4815048/diff/8001/src/pkg/archive/zip/zip_test....
> src/pkg/archive/zip/zip_test.go:48: if got := len(zr.File); got !=
> nFiles {
> can you then easily the content is right?
> 
> http://codereview.appspot.com/4815048/

[bradfitz, 2011-07-22 23:59:28]

http://codereview.appspot.com/4815048/diff/8001/src/pkg/archive/zip/zip_test.go
File src/pkg/archive/zip/zip_test.go (right):

http://codereview.appspot.com/4815048/diff/8001/src/pkg/archive/zip/zip_test....
src/pkg/archive/zip/zip_test.go:5: // Tests that involved both reading &
writing.
On 2011/07/22 22:06:31, r wrote:
> s/involved/involve/

Done.

http://codereview.appspot.com/4815048/diff/8001/src/pkg/archive/zip/zip_test....
src/pkg/archive/zip/zip_test.go:38: }
On 2011/07/22 22:06:31, r wrote:
> maybe put some content into the file?
> "this is %d.dat"?

I did in v1 of this CL.  It didn't add anything (test anything extra) except
much more slowness.  All I really care about in this test is the TOC is
readable.

http://codereview.appspot.com/4815048/diff/8001/src/pkg/archive/zip/zip_test....
src/pkg/archive/zip/zip_test.go:48: if got := len(zr.File); got != nFiles {
On 2011/07/22 22:06:31, r wrote:
> can you then easily the content is right?

Done.

[bradfitz, 2011-07-23 00:00:55]

Hello golang-dev@googlegroups.com, r@golang.org, r@google.com (cc:
golang-dev@googlegroups.com),

Please take another look.

[r, 2011-07-23 00:02:36]

LGTM

http://codereview.appspot.com/4815048/diff/10002/src/pkg/archive/zip/zip_test.go
File src/pkg/archive/zip/zip_test.go (right):

http://codereview.appspot.com/4815048/diff/10002/src/pkg/archive/zip/zip_test...
src/pkg/archive/zip/zip_test.go:5: // Tests that involve both reading & writing.
s/&/&/ (!)
i mean
s/&/and/

[bradfitz, 2011-07-23 01:31:56]

*** Submitted as http://code.google.com/p/go/source/detail?r=1a4e153f70fd ***

zip: add a test for the previous >65k files fix

This surprisingly takes 30 seconds on my fast machine
so disabling by default. Need to optimize the Writer
at some point.

R=golang-dev, r, r
CC=golang-dev
http://codereview.appspot.com/4815048

[adg, 2011-07-23 06:14:29]

Thanks, Brad.