cmd/go: rebuild as needed for tests of packages that add methods
If A's external test package imports B, which imports A,
and A's (internal) test code also adds something to A that
invalidates anything in the export data from a build of A
without its test code, then strictly speaking we need to
rebuild B against the test-augmented version of A before
using it to build A's external test package.
We've been skating by without doing this for a very long time,
but I knew we'd need to handle it better eventually,
I planned for it in the new build cache simplifications,
and the code was ready. Now that we have a real-world
test case that needs it, turn on the "proper rebuilding" code.
It doesn't really matter how much things slow down, since
a real-world test cases that caused an internal compiler error
before is now handled correctly, but it appears to be small:
I wasn't able to measure an effect on "go test -a -c fmt".
And of course most builds won't use -a and will be cached well.
Fixes #6204.
Fixes #23701.
Change-Id: I2cd60cf400d1928428979ab05831f48ff7cee6ca
Reviewed-on: https://go-review.googlesource.com/92215
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@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 7db62da..92600b6 100644
--- a/src/cmd/go/go_test.go
+++ b/src/cmd/go/go_test.go
@@ -5439,6 +5439,44 @@
tg.grepStdout(`ok\s+vetfail/p2`, "did not run vetfail/p2")
}
+func TestTestRebuild(t *testing.T) {
+ tg := testgo(t)
+ defer tg.cleanup()
+ tg.parallel()
+
+ // golang.org/issue/23701.
+ // b_test imports b with augmented method from export_test.go.
+ // b_test also imports a, which imports b.
+ // Must not accidentally see un-augmented b propagate through a to b_test.
+ tg.tempFile("src/a/a.go", `package a
+ import "b"
+ type Type struct{}
+ func (*Type) M() b.T {return 0}
+ `)
+ tg.tempFile("src/b/b.go", `package b
+ type T int
+ type I interface {M() T}
+ `)
+ tg.tempFile("src/b/export_test.go", `package b
+ func (*T) Method() *T { return nil }
+ `)
+ tg.tempFile("src/b/b_test.go", `package b_test
+ import (
+ "testing"
+ "a"
+ . "b"
+ )
+ func TestBroken(t *testing.T) {
+ x := new(T)
+ x.Method()
+ _ = new(a.Type)
+ }
+ `)
+
+ tg.setenv("GOPATH", tg.path("."))
+ tg.run("test", "b")
+}
+
func TestInstallDeps(t *testing.T) {
tooSlow(t)
tg := testgo(t)
diff --git a/src/cmd/go/internal/test/test.go b/src/cmd/go/internal/test/test.go
index bf68480..a99c6a5 100644
--- a/src/cmd/go/internal/test/test.go
+++ b/src/cmd/go/internal/test/test.go
@@ -897,7 +897,7 @@
t.ImportXtest = true
}
- if ptest != p && localCover {
+ if ptest != p {
// We have made modifications to the package p being tested
// and are rebuilding p (as ptest).
// Arrange to rebuild all packages q such that
@@ -906,13 +906,6 @@
// Strictly speaking, the rebuild is only necessary if the
// modifications to p change its export metadata, but
// determining that is a bit tricky, so we rebuild always.
- // TODO(rsc): Once we get export metadata changes
- // handled properly, look into the expense of dropping
- // "&& localCover" above.
- //
- // 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, pxtest)
}
