archive/zip: possibly useless test

[ainar-g]

While playing around with the new vet and its nilness check, I have found an interesting bug. In src/archive/zip/reader_test.go:985 we have a test for #10956, but the test doesn't actually check anything:

_, err := NewReader(bytes.NewReader(data), int64(len(data)))
// …
if err == nil && !strings.Contains(err.Error(), want) {
	t.Errorf("error = %v; want %q", err, want)
}

It should obviously be if err != nil && … here. But when I changed it, the test failed, because it returned ErrFormat instead of the custom Errorf'd error that was expected. I ran all of the package zip's tests with -cover and the coverage profile shows that the branch in (*Reader).init is never taken.

[adg]

It seems that fix applied for #10956 (that I reviewed and approved) wasn't actually a fix; maybe in theory, but it was never tested in practice.

As you say, the example that @dvykov provided doesn't seem to get past the readDirectoryEnd function, so the additional check added to the init method is never reached. I sent a change.

[gopherbot]

Change https://golang.org/cl/179757 mentions this issue: archive/zip: remove unused special case

[odeke-em]

Thank you @ainar-g for the report and @adg for jumping on this and sending the fixing CL!
As per the wait-release tag added to the CL, it seems like this is punted to Go1.14. I shall update here accordingly and add the Early-In-Cycle tag too.