From 5e21032b3d179a8bc0d9e70e4725629defad9d8b Mon Sep 17 00:00:00 2001 From: "Bryan C. Mills" Date: Thu, 18 Jul 2019 17:34:59 -0400 Subject: [PATCH] Revert "cmd/go: move automatic testing.Init call into generated test code" This reverts CL 176098. Reason for revert: added complexity, but did not completely fix the underlying problem. A complete solution would not be worth the complexity, and as a partial solution this is probably not worth the complexity either. Updates #31859 Change-Id: Ifd34c292fd1b811c60afe3c339e5edd3f37190c8 Reviewed-on: https://go-review.googlesource.com/c/go/+/186817 Run-TryBot: Bryan C. Mills TryBot-Result: Gobot Gobot Reviewed-by: Ian Lance Taylor Reviewed-by: Caleb Spare --- src/cmd/go/go_test.go | 6 ++ src/cmd/go/internal/list/list.go | 2 +- src/cmd/go/internal/load/pkg.go | 3 +- src/cmd/go/internal/load/test.go | 43 +--------- src/cmd/go/internal/test/test.go | 2 +- src/cmd/go/internal/work/exec.go | 9 -- src/cmd/go/testdata/flag_test.go | 9 +- src/cmd/go/testdata/script/test_init.txt | 86 ------------------- .../testdata/standalone_testmain_flag_test.go | 29 +++++++ src/testing/testing.go | 5 -- 10 files changed, 44 insertions(+), 150 deletions(-) delete mode 100644 src/cmd/go/testdata/script/test_init.txt create mode 100644 src/cmd/go/testdata/standalone_testmain_flag_test.go diff --git a/src/cmd/go/go_test.go b/src/cmd/go/go_test.go index e07f97d068..f6caa01fd2 100644 --- a/src/cmd/go/go_test.go +++ b/src/cmd/go/go_test.go @@ -3176,6 +3176,12 @@ func TestGoTestFooTestWorks(t *testing.T) { tg.run("test", "testdata/standalone_test.go") } +func TestGoTestTestMainSeesTestingFlags(t *testing.T) { + tg := testgo(t) + defer tg.cleanup() + tg.run("test", "testdata/standalone_testmain_flag_test.go") +} + // Issue 22388 func TestGoTestMainWithWrongSignature(t *testing.T) { tg := testgo(t) diff --git a/src/cmd/go/internal/list/list.go b/src/cmd/go/internal/list/list.go index e7e78e7c59..4a6633d9a1 100644 --- a/src/cmd/go/internal/list/list.go +++ b/src/cmd/go/internal/list/list.go @@ -459,7 +459,7 @@ func runList(cmd *base.Command, args []string) { } if pmain != nil { pkgs = append(pkgs, pmain) - data := pmain.Internal.TestmainGo + data := *pmain.Internal.TestmainGo h := cache.NewHash("testmain") h.Write([]byte("testmain\n")) h.Write(data) diff --git a/src/cmd/go/internal/load/pkg.go b/src/cmd/go/internal/load/pkg.go index 4eb4ba690f..d52df046ff 100644 --- a/src/cmd/go/internal/load/pkg.go +++ b/src/cmd/go/internal/load/pkg.go @@ -177,8 +177,7 @@ type PackageInternal struct { OmitDebug bool // tell linker not to write debug information GobinSubdir bool // install target would be subdir of GOBIN BuildInfo string // add this info to package main - TestinginitGo []byte // content for _testinginit.go - TestmainGo []byte // content for _testmain.go + TestmainGo *[]byte // content for _testmain.go Asmflags []string // -asmflags for this package Gcflags []string // -gcflags for this package diff --git a/src/cmd/go/internal/load/test.go b/src/cmd/go/internal/load/test.go index c247d56c81..afff5deaaa 100644 --- a/src/cmd/go/internal/load/test.go +++ b/src/cmd/go/internal/load/test.go @@ -102,7 +102,6 @@ func TestPackagesAndErrors(p *Package, cover *TestCover) (pmain, ptest, pxtest * var stk ImportStack stk.Push(p.ImportPath + " (test)") rawTestImports := str.StringList(p.TestImports) - var ptestImportsTesting, pxtestImportsTesting bool for i, path := range p.TestImports { p1 := loadImport(pre, path, p.Dir, p, &stk, p.Internal.Build.TestImportPos[path], ResolveImport) if str.Contains(p1.Deps, p.ImportPath) || p1.ImportPath == p.ImportPath { @@ -117,9 +116,6 @@ func TestPackagesAndErrors(p *Package, cover *TestCover) (pmain, ptest, pxtest * } p.TestImports[i] = p1.ImportPath imports = append(imports, p1) - if path == "testing" { - ptestImportsTesting = true - } } stk.Pop() stk.Push(p.ImportPath + "_test") @@ -133,9 +129,6 @@ func TestPackagesAndErrors(p *Package, cover *TestCover) (pmain, ptest, pxtest * ximports = append(ximports, p1) } p.XTestImports[i] = p1.ImportPath - if path == "testing" { - pxtestImportsTesting = true - } } stk.Pop() @@ -145,9 +138,6 @@ func TestPackagesAndErrors(p *Package, cover *TestCover) (pmain, ptest, pxtest * *ptest = *p ptest.Error = ptestErr ptest.ForTest = p.ImportPath - if ptestImportsTesting { - ptest.Internal.TestinginitGo = formatTestinginit(p) - } ptest.GoFiles = nil ptest.GoFiles = append(ptest.GoFiles, p.GoFiles...) ptest.GoFiles = append(ptest.GoFiles, p.TestGoFiles...) @@ -212,9 +202,6 @@ func TestPackagesAndErrors(p *Package, cover *TestCover) (pmain, ptest, pxtest * Gccgoflags: p.Internal.Gccgoflags, }, } - if pxtestImportsTesting { - pxtest.Internal.TestinginitGo = formatTestinginit(pxtest) - } if pxtestNeedsPtest { pxtest.Internal.Imports = append(pxtest.Internal.Imports, ptest) } @@ -337,7 +324,9 @@ func TestPackagesAndErrors(p *Package, cover *TestCover) (pmain, ptest, pxtest * if err != nil && pmain.Error == nil { pmain.Error = &PackageError{Err: err.Error()} } - pmain.Internal.TestmainGo = data + if data != nil { + pmain.Internal.TestmainGo = &data + } return pmain, ptest, pxtest } @@ -485,15 +474,6 @@ func loadTestFuncs(ptest *Package) (*testFuncs, error) { return t, err } -// formatTestinginit returns the content of the _testinginit.go file for p. -func formatTestinginit(p *Package) []byte { - var buf bytes.Buffer - if err := testinginitTmpl.Execute(&buf, p); err != nil { - panic("testinginit template execution failed") // shouldn't be possible - } - return buf.Bytes() -} - // formatTestmain returns the content of the _testmain.go file for t. func formatTestmain(t *testFuncs) ([]byte, error) { var buf bytes.Buffer @@ -623,23 +603,6 @@ func checkTestFunc(fn *ast.FuncDecl, arg string) error { return nil } -var testinginitTmpl = lazytemplate.New("init", ` -package {{.Name}} - -import _go_testing "testing" - -{{/* -Call testing.Init before any other user initialization code runs. -(This file is passed to the compiler first.) -This provides the illusion of the old behavior where testing flags -were registered as part of the testing package's initialization. -*/}} -var _ = func() bool { - _go_testing.Init() - return true -}() -`) - var testmainTmpl = lazytemplate.New("main", ` // Code generated by 'go test'. DO NOT EDIT. diff --git a/src/cmd/go/internal/test/test.go b/src/cmd/go/internal/test/test.go index cc7c4564e5..95000011d8 100644 --- a/src/cmd/go/internal/test/test.go +++ b/src/cmd/go/internal/test/test.go @@ -843,7 +843,7 @@ func builderTest(b *work.Builder, p *load.Package) (buildAction, runAction, prin if !cfg.BuildN { // writeTestmain writes _testmain.go, // using the test description gathered in t. - if err := ioutil.WriteFile(testDir+"_testmain.go", pmain.Internal.TestmainGo, 0666); err != nil { + if err := ioutil.WriteFile(testDir+"_testmain.go", *pmain.Internal.TestmainGo, 0666); err != nil { return nil, nil, nil, err } } diff --git a/src/cmd/go/internal/work/exec.go b/src/cmd/go/internal/work/exec.go index c1bb9416cb..944b23f1d8 100644 --- a/src/cmd/go/internal/work/exec.go +++ b/src/cmd/go/internal/work/exec.go @@ -542,15 +542,6 @@ func (b *Builder) build(a *Action) (err error) { } } - // Write out the _testinginit.go file for any test packages that import "testing". - if a.Package.Internal.TestinginitGo != nil { - initfile := objdir + "_testinginit.go" - if err := b.writeFile(initfile, a.Package.Internal.TestinginitGo); err != nil { - return err - } - gofiles = append([]string{initfile}, gofiles...) - } - // Run cgo. if a.Package.UsesCgo() || a.Package.UsesSwig() { // In a package using cgo, cgo compiles the C, C++ and assembly files with gcc. diff --git a/src/cmd/go/testdata/flag_test.go b/src/cmd/go/testdata/flag_test.go index a4e5507f2c..ddf613d870 100644 --- a/src/cmd/go/testdata/flag_test.go +++ b/src/cmd/go/testdata/flag_test.go @@ -1,19 +1,16 @@ -// Copyright 2019 The Go Authors. All rights reserved. -// Use of this source code is governed by a BSD-style -// license that can be found in the LICENSE file. - package flag_test import ( "flag" + "log" "testing" ) var v = flag.Int("v", 0, "v flag") -// Run this as go test pkg -args -v=7 +// Run this as go test pkg -v=7 func TestVFlagIsSet(t *testing.T) { if *v != 7 { - t.Fatal("v flag not set") + log.Fatal("v flag not set") } } diff --git a/src/cmd/go/testdata/script/test_init.txt b/src/cmd/go/testdata/script/test_init.txt deleted file mode 100644 index 73b4f3c768..0000000000 --- a/src/cmd/go/testdata/script/test_init.txt +++ /dev/null @@ -1,86 +0,0 @@ -# Tests for automatic testing.Init calls when using 'go test'. - -env GO111MODULE=on - -# A TestMain should be able to access testing flags if it calls flag.Parse -# without needing to use testing.Init. -# Test code can use the name 'testing' without colliding with generated -# testinginit code. -# Tests running under 'go test' should observe that testing.Init is called -# before any user package initialization code runs. -go test -stdout TestMain -stdout TestInit -stdout TestExt - --- go.mod -- -module m - --- init_test.go -- -package testinitflag - -import ( - "flag" - "fmt" - "os" - Testing "testing" -) - -func testFlagsInitialized() bool { - found := false - flag.VisitAll(func(f *flag.Flag) { - if f.Name == "test.count" { - found = true - } - }) - return found -} - -var testing int -var testingInitAtInitialization = testFlagsInitialized() - -func TestInit(t *Testing.T) { - if !testingInitAtInitialization { - t.Fatal("testing.Init not called before package initialization") - } - fmt.Printf("TestInit\n") -} - -func TestMain(m *Testing.M) { - fmt.Printf("TestMain\n") - flag.Parse() - if !testFlagsInitialized() { - fmt.Println("testing flags not registered") - os.Exit(1) - } - os.Exit(m.Run()) -} - --- external_test.go -- -package testinitflag_test - -import ( - "flag" - "fmt" - Testing "testing" -) - -func testFlagsInitialized() bool { - found := false - flag.VisitAll(func(f *flag.Flag) { - if f.Name == "test.count" { - found = true - } - }) - return found -} - -var testing int -var testingInitAtInitialization = testFlagsInitialized() - -func TestExt(t *Testing.T) { - fmt.Printf("TestExt\n") - if !testingInitAtInitialization { - t.Fatal("testing.Init not called before package initialization") - } -} diff --git a/src/cmd/go/testdata/standalone_testmain_flag_test.go b/src/cmd/go/testdata/standalone_testmain_flag_test.go new file mode 100644 index 0000000000..a59555bb61 --- /dev/null +++ b/src/cmd/go/testdata/standalone_testmain_flag_test.go @@ -0,0 +1,29 @@ +// Copyright 2019 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package standalone_testmain_flag_test + +import ( + "flag" + "fmt" + "os" + "testing" +) + +func TestMain(m *testing.M) { + // A TestMain should be able to access testing flags if it calls + // flag.Parse without needing to use testing.Init. + flag.Parse() + found := false + flag.VisitAll(func(f *flag.Flag) { + if f.Name == "test.count" { + found = true + } + }) + if !found { + fmt.Println("testing flags not registered") + os.Exit(1) + } + os.Exit(m.Run()) +} diff --git a/src/testing/testing.go b/src/testing/testing.go index 339df13f43..6ab9b79196 100644 --- a/src/testing/testing.go +++ b/src/testing/testing.go @@ -1075,11 +1075,6 @@ type testDeps interface { // It is not meant to be called directly and is not subject to the Go 1 compatibility document. // It may change signature from release to release. func MainStart(deps testDeps, tests []InternalTest, benchmarks []InternalBenchmark, examples []InternalExample) *M { - // In most cases, Init has already been called by the testinginit code - // that 'go test' injects into test packages. - // Call it again here to handle cases such as: - // - test packages that don't import "testing" (such as example-only packages) - // - direct use of MainStart (though that isn't well-supported) Init() return &M{ deps: deps,