zip: in TestVCS, skip instead of failing after VCS error

The underlying package doesn't interact with VCS. The test just does
this to simulate what the go command does. A VCS failure indicates a
problem with the test environment, not with the code being tested.

Fixes golang/go#38466

Change-Id: I1c36f7c9c644b22e01fa073a5d8a58c9a3893145
Reviewed-on: https://go-review.googlesource.com/c/mod/+/228397
Run-TryBot: Jay Conrod <jayconrod@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>

zip/zip_test.go

@@ -18,6 +18,7 @@ import (
 	"path/filepath"
 	"runtime"
 	"strings"
+	"sync/atomic"
 	"testing"
 	"time"
 
@@ -726,6 +727,9 @@ func TestVCS(t *testing.T) {
 		t.Skip()
 	}
 
+	var downloadErrorCount int32
+	const downloadErrorLimit = 3
+
 	haveVCS := make(map[string]bool)
 	for _, vcs := range []string{"git", "hg"} {
 		_, err := exec.LookPath(vcs)
@@ -941,7 +945,18 @@ func TestVCS(t *testing.T) {
 			repo, dl, cleanup, err := downloadVCSZip(test.vcs, test.url, test.rev, test.subdir)
 			defer cleanup()
 			if err != nil {
-				t.Fatal(err)
+				// This may fail if there's a problem with the network or upstream
+				// repository. The package being tested doesn't directly interact with
+				// VCS tools; the test just does this to simulate what the go command
+				// does. So an error should cause a skip instead of a failure. But we
+				// should fail after too many errors so we don't lose test coverage
+				// when something changes permanently.
+				n := atomic.AddInt32(&downloadErrorCount, 1)
+				if n < downloadErrorLimit {
+					t.Skipf("failed to download zip from repository: %v", err)
+				} else {
+					t.Fatalf("failed to download zip from repository (repeated failure): %v", err)
+				}
 			}
 
 			// Create a module zip from that archive.