cmd/go: fix coverage rebuild corner case

If you have a package p1 with an xtest (package p1_test)
that imports p2, where p2 itself imports p1, then when
trying to do coverage for p1 we need to make sure to
recompile p2. The problem was that the overall package
import graph looked like:

    main -> p1_test -> p2 -> p1

Since we were recompiling p1 with coverage, we correctly
figured out that because p2 depends on a package being
recompiled due to coverage, p2 also needs to be split (forked) to
insert the dependency on the modified p1. But then we used
the same logic to split p1_test and main, with the effect that
the changes to p2 and p1_test and main were lost, since the
caller was still holding on to the original main, not the split version.

Change the code to treat main and p1_test as "already split"
and just update them in place.

Fixes #23314.

Change-Id: If7edeca6e39cdaeb5b9380d00b0c7d8c5891f086
Reviewed-on: https://go-review.googlesource.com/86237
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>

diff --git a/src/cmd/go/go_test.go b/src/cmd/go/go_test.go
index ddf097d240fd648287b578e59a04769be2261775..8cf9cfbb109da1830e5d396fd2d2480d984de541 100644 (file)
--- a/src/cmd/go/go_test.go
+++ b/src/cmd/go/go_test.go
@@ -2471,6 +2471,17 @@ func TestCoverageSyncAtomicImport(t *testing.T) {
        tg.run("test", "-short", "-cover", "-covermode=atomic", "-coverpkg=coverdep/p1", "coverdep")
 }
 
+func TestCoverageDepLoop(t *testing.T) {
+       tg := testgo(t)
+       defer tg.cleanup()
+       tg.parallel()
+       tg.setenv("GOPATH", filepath.Join(tg.pwd(), "testdata"))
+       // coverdep2/p1's xtest imports coverdep2/p2 which imports coverdep2/p1.
+       // Make sure that coverage on coverdep2/p2 recompiles coverdep2/p2.
+       tg.run("test", "-short", "-cover", "coverdep2/p1")
+       tg.grepStdout("coverage: 100.0% of statements", "expected 100.0% coverage")
+}
+
 func TestCoverageImportMainLoop(t *testing.T) {
        tg := testgo(t)
        defer tg.cleanup()

diff --git a/src/cmd/go/internal/test/test.go b/src/cmd/go/internal/test/test.go
index 94844b5c68c36fb43c94dd2d632a7f2773fe456d..7f8954a7d92ce94fc97683b57751c5aff4dc0a9f 100644 (file)
--- a/src/cmd/go/internal/test/test.go
+++ b/src/cmd/go/internal/test/test.go
@@ -1003,7 +1003,7 @@ func builderTest(b *work.Builder, p *load.Package) (buildAction, runAction, prin
                // This will cause extra compilation, so for now we only do it
                // when testCover is set. The conditions are more general, though,
                // and we may find that we need to do it always in the future.
-               recompileForTest(pmain, p, ptest)
+               recompileForTest(pmain, p, ptest, pxtest)
        }
 
        for _, cp := range pmain.Internal.Imports {
@@ -1159,14 +1159,17 @@ Search:
        return stk
 }
 
-func recompileForTest(pmain, preal, ptest *load.Package) {
+func recompileForTest(pmain, preal, ptest, pxtest *load.Package) {
        // The "test copy" of preal is ptest.
        // For each package that depends on preal, make a "test copy"
        // that depends on ptest. And so on, up the dependency tree.
        testCopy := map[*load.Package]*load.Package{preal: ptest}
        for _, p := range load.PackageList([]*load.Package{pmain}) {
+               if p == preal {
+                       continue
+               }
                // Copy on write.
-               didSplit := false
+               didSplit := p == pmain || p == pxtest
                split := func() {
                        if didSplit {
                                return

diff --git a/src/cmd/go/testdata/src/coverdep2/p1/p.go b/src/cmd/go/testdata/src/coverdep2/p1/p.go
new file mode 100644 (file)
index 0000000..fd31527
--- /dev/null
+++ b/src/cmd/go/testdata/src/coverdep2/p1/p.go
@@ -0,0 +1,3 @@
+package p1
+
+func F() int { return 1 }

diff --git a/src/cmd/go/testdata/src/coverdep2/p1/p_test.go b/src/cmd/go/testdata/src/coverdep2/p1/p_test.go
new file mode 100644 (file)
index 0000000..c402568
--- /dev/null
+++ b/src/cmd/go/testdata/src/coverdep2/p1/p_test.go
@@ -0,0 +1,10 @@
+package p1_test
+
+import (
+       "coverdep2/p2"
+       "testing"
+)
+
+func Test(t *testing.T) {
+       p2.F()
+}

diff --git a/src/cmd/go/testdata/src/coverdep2/p2/p2.go b/src/cmd/go/testdata/src/coverdep2/p2/p2.go
new file mode 100644 (file)
index 0000000..33561bb
--- /dev/null
+++ b/src/cmd/go/testdata/src/coverdep2/p2/p2.go
@@ -0,0 +1,7 @@
+package p2
+
+import "coverdep2/p1"
+
+func F() {
+       p1.F()
+}