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 <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Caleb Spare <cespare@gmail.com>

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)

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)

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

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.
 

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
 		}
 	}

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.

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")
 	}
 }

src/cmd/go/testdata/script/test_init.txt

@@ -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")
-	}
-}

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())
+}

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,