all: make unit tests work with Go 1.13 (#20053)

Most of these changes are related to the Go 1.13 changes to test binary
flag handling. 

* cmd/geth: make attach tests more reliable

This makes the test wait for the endpoint to come up by polling
it instead of waiting for two seconds.

* tests: fix test binary flags for Go 1.13

Calling flag.Parse during package initialization is prohibited
as of Go 1.13 and causes test failures. Call it in TestMain instead.

* crypto/ecies: remove useless -dump flag in tests

* p2p/simulations: fix test binary flags for Go 1.13

Calling flag.Parse during package initialization is prohibited
as of Go 1.13 and causes test failures. Call it in TestMain instead.

* build: remove workaround for ./... vendor matching

This workaround was necessary for Go 1.8. The Go 1.9 release changed
the expansion rules to exclude vendored packages.

* Makefile: use relative path for GOBIN

This makes the "Run ./build/bin/..." line look nicer.

* les: fix test binary flags for Go 1.13

Calling flag.Parse during package initialization is prohibited
as of Go 1.13 and causes test failures. Call it in TestMain instead.

Author: fjl

Makefile

@@ -8,7 +8,7 @@
 .PHONY: geth-darwin geth-darwin-386 geth-darwin-amd64
 .PHONY: geth-windows geth-windows-386 geth-windows-amd64
 
-GOBIN = $(shell pwd)/build/bin
+GOBIN = ./build/bin
 GO ?= latest
 
 geth:

build/ci.go

@@ -214,7 +214,6 @@ func doInstall(cmdline []string) {
 	if flag.NArg() > 0 {
 		packages = flag.Args()
 	}
-	packages = build.ExpandPackagesNoVendor(packages)
 
 	if *arch == "" || *arch == runtime.GOARCH {
 		goinstall := goTool("install", buildFlags(env)...)
@@ -311,7 +310,6 @@ func doTest(cmdline []string) {
 	if len(flag.CommandLine.Args()) > 0 {
 		packages = flag.CommandLine.Args()
 	}
-	packages = build.ExpandPackagesNoVendor(packages)
 
 	// Run the actual tests.
 	// Test a single package at a time. CI builders are slow

cmd/geth/consolecmd_test.go

@@ -87,7 +87,7 @@ func TestIPCAttachWelcome(t *testing.T) {
 		"--port", "0", "--maxpeers", "0", "--nodiscover", "--nat", "none",
 		"--etherbase", coinbase, "--shh", "--ipcpath", ipc)
 
-	time.Sleep(2 * time.Second) // Simple way to wait for the RPC endpoint to open
+	waitForEndpoint(t, ipc, 3*time.Second)
 	testAttachWelcome(t, geth, "ipc:"+ipc, ipcAPIs)
 
 	geth.Interrupt()
@@ -101,8 +101,9 @@ func TestHTTPAttachWelcome(t *testing.T) {
 		"--port", "0", "--maxpeers", "0", "--nodiscover", "--nat", "none",
 		"--etherbase", coinbase, "--rpc", "--rpcport", port)
 
-	time.Sleep(2 * time.Second) // Simple way to wait for the RPC endpoint to open
-	testAttachWelcome(t, geth, "http://localhost:"+port, httpAPIs)
+	endpoint := "http://127.0.0.1:" + port
+	waitForEndpoint(t, endpoint, 3*time.Second)
+	testAttachWelcome(t, geth, endpoint, httpAPIs)
 
 	geth.Interrupt()
 	geth.ExpectExit()
@@ -116,8 +117,9 @@ func TestWSAttachWelcome(t *testing.T) {
 		"--port", "0", "--maxpeers", "0", "--nodiscover", "--nat", "none",
 		"--etherbase", coinbase, "--ws", "--wsport", port)
 
-	time.Sleep(2 * time.Second) // Simple way to wait for the RPC endpoint to open
-	testAttachWelcome(t, geth, "ws://localhost:"+port, httpAPIs)
+	endpoint := "ws://127.0.0.1:" + port
+	waitForEndpoint(t, endpoint, 3*time.Second)
+	testAttachWelcome(t, geth, endpoint, httpAPIs)
 
 	geth.Interrupt()
 	geth.ExpectExit()

cmd/geth/run_test.go

@@ -17,13 +17,16 @@
 package main
 
 import (
+	"context"
 	"fmt"
 	"io/ioutil"
 	"os"
 	"testing"
+	"time"
 
 	"github.com/docker/docker/pkg/reexec"
 	"github.com/ethereum/go-ethereum/internal/cmdtest"
+	"github.com/ethereum/go-ethereum/rpc"
 )
 
 func tmpdir(t *testing.T) string {
@@ -96,3 +99,28 @@ func runGeth(t *testing.T, args ...string) *testgeth {
 
 	return tt
 }
+
+// waitForEndpoint attempts to connect to an RPC endpoint until it succeeds.
+func waitForEndpoint(t *testing.T, endpoint string, timeout time.Duration) {
+	probe := func() bool {
+		ctx, cancel := context.WithTimeout(context.Background(), timeout)
+		defer cancel()
+		c, err := rpc.DialContext(ctx, endpoint)
+		if c != nil {
+			_, err = c.SupportedModules()
+			c.Close()
+		}
+		return err == nil
+	}
+
+	start := time.Now()
+	for {
+		if probe() {
+			return
+		}
+		if time.Since(start) > timeout {
+			t.Fatal("endpoint", endpoint, "did not open within", timeout)
+		}
+		time.Sleep(200 * time.Millisecond)
+	}
+}

crypto/ecies/ecies_test.go

@@ -35,22 +35,13 @@ import (
 	"crypto/rand"
 	"crypto/sha256"
 	"encoding/hex"
-	"flag"
 	"fmt"
 	"math/big"
 	"testing"
 
 	"github.com/ethereum/go-ethereum/crypto"
 )
 
-var dumpEnc bool
-
-func init() {
-	flDump := flag.Bool("dump", false, "write encrypted test message to file")
-	flag.Parse()
-	dumpEnc = *flDump
-}
-
 // Ensure the KDF generates appropriately sized keys.
 func TestKDF(t *testing.T) {
 	msg := []byte("Hello, world")

internal/build/util.go

@@ -153,32 +153,6 @@ func GoTool(tool string, args ...string) *exec.Cmd {
 	return exec.Command(filepath.Join(runtime.GOROOT(), "bin", "go"), args...)
 }
 
-// ExpandPackagesNoVendor expands a cmd/go import path pattern, skipping
-// vendored packages.
-func ExpandPackagesNoVendor(patterns []string) []string {
-	expand := false
-	for _, pkg := range patterns {
-		if strings.Contains(pkg, "...") {
-			expand = true
-		}
-	}
-	if expand {
-		cmd := GoTool("list", patterns...)
-		out, err := cmd.CombinedOutput()
-		if err != nil {
-			log.Fatalf("package listing failed: %v\n%s", err, string(out))
-		}
-		var packages []string
-		for _, line := range strings.Split(string(out), "\n") {
-			if !strings.Contains(line, "/vendor/") {
-				packages = append(packages, strings.TrimSpace(line))
-			}
-		}
-		return packages
-	}
-	return patterns
-}
-
 // UploadSFTP uploads files to a remote host using the sftp command line tool.
 // The destination host may be specified either as [user@]host[: or as a URI in
 // the form sftp://[user@]host[:port].

les/api_test.go

@@ -43,11 +43,25 @@ import (
 	"github.com/mattn/go-colorable"
 )
 
-/*
-This test is not meant to be a part of the automatic testing process because it
-runs for a long time and also requires a large database in order to do a meaningful
-request performance test. When testServerDataDir is empty, the test is skipped.
-*/
+// Additional command line flags for the test binary.
+var (
+	loglevel   = flag.Int("loglevel", 0, "verbosity of logs")
+	simAdapter = flag.String("adapter", "exec", "type of simulation: sim|socket|exec|docker")
+)
+
+func TestMain(m *testing.M) {
+	flag.Parse()
+	log.PrintOrigins(true)
+	log.Root().SetHandler(log.LvlFilterHandler(log.Lvl(*loglevel), log.StreamHandler(colorable.NewColorableStderr(), log.TerminalFormat(true))))
+	// register the Delivery service which will run as a devp2p
+	// protocol when using the exec adapter
+	adapters.RegisterServices(services)
+	os.Exit(m.Run())
+}
+
+// This test is not meant to be a part of the automatic testing process because it
+// runs for a long time and also requires a large database in order to do a meaningful
+// request performance test. When testServerDataDir is empty, the test is skipped.
 
 const (
 	testServerDataDir  = "" // should always be empty on the master branch
@@ -377,29 +391,13 @@ func getCapacityInfo(ctx context.Context, t *testing.T, server *rpc.Client) (min
 	return
 }
 
-func init() {
-	flag.Parse()
-	// register the Delivery service which will run as a devp2p
-	// protocol when using the exec adapter
-	adapters.RegisterServices(services)
-
-	log.PrintOrigins(true)
-	log.Root().SetHandler(log.LvlFilterHandler(log.Lvl(*loglevel), log.StreamHandler(colorable.NewColorableStderr(), log.TerminalFormat(true))))
-}
-
-var (
-	adapter  = flag.String("adapter", "exec", "type of simulation: sim|socket|exec|docker")
-	loglevel = flag.Int("loglevel", 0, "verbosity of logs")
-	nodes    = flag.Int("nodes", 0, "number of nodes")
-)
-
 var services = adapters.Services{
 	"lesclient": newLesClientService,
 	"lesserver": newLesServerService,
 }
 
 func NewNetwork() (*simulations.Network, func(), error) {
-	adapter, adapterTeardown, err := NewAdapter(*adapter, services)
+	adapter, adapterTeardown, err := NewAdapter(*simAdapter, services)
 	if err != nil {
 		return nil, adapterTeardown, err
 	}

p2p/simulations/http_test.go

@@ -22,6 +22,7 @@ import (
 	"fmt"
 	"math/rand"
 	"net/http/httptest"
+	"os"
 	"reflect"
 	"sync"
 	"sync/atomic"
@@ -38,15 +39,13 @@ import (
 	"github.com/mattn/go-colorable"
 )
 
-var (
-	loglevel = flag.Int("loglevel", 2, "verbosity of logs")
-)
+func TestMain(m *testing.M) {
+	loglevel := flag.Int("loglevel", 2, "verbosity of logs")
 
-func init() {
 	flag.Parse()
-
 	log.PrintOrigins(true)
 	log.Root().SetHandler(log.LvlFilterHandler(log.Lvl(*loglevel), log.StreamHandler(colorable.NewColorableStderr(), log.TerminalFormat(true))))
+	os.Exit(m.Run())
 }
 
 // testService implements the node.Service interface and provides protocols

tests/init_test.go

@@ -18,6 +18,7 @@ package tests
 
 import (
 	"encoding/json"
+	"flag"
 	"fmt"
 	"io"
 	"io/ioutil"
@@ -33,6 +34,17 @@ import (
 	"github.com/ethereum/go-ethereum/params"
 )
 
+// Command line flags to configure the interpreters.
+var (
+	testEVM   = flag.String("vm.evm", "", "EVM configuration")
+	testEWASM = flag.String("vm.ewasm", "", "EWASM configuration")
+)
+
+func TestMain(m *testing.M) {
+	flag.Parse()
+	os.Exit(m.Run())
+}
+
 var (
 	baseDir            = filepath.Join(".", "testdata")
 	blockTestDir       = filepath.Join(baseDir, "BlockchainTests")

tests/state_test.go

@@ -19,12 +19,10 @@ package tests
 import (
 	"bufio"
 	"bytes"
-	"flag"
 	"fmt"
 	"reflect"
 	"testing"
 
-	"github.com/ethereum/go-ethereum/cmd/utils"
 	"github.com/ethereum/go-ethereum/core/vm"
 )
 
@@ -71,20 +69,15 @@ func TestState(t *testing.T) {
 // Transactions with gasLimit above this value will not get a VM trace on failure.
 const traceErrorLimit = 400000
 
-// The VM config for state tests that accepts --vm.* command line arguments.
-var testVMConfig = func() vm.Config {
-	vmconfig := vm.Config{}
-	flag.StringVar(&vmconfig.EVMInterpreter, utils.EVMInterpreterFlag.Name, utils.EVMInterpreterFlag.Value, utils.EVMInterpreterFlag.Usage)
-	flag.StringVar(&vmconfig.EWASMInterpreter, utils.EWASMInterpreterFlag.Name, utils.EWASMInterpreterFlag.Value, utils.EWASMInterpreterFlag.Usage)
-	flag.Parse()
-	return vmconfig
-}()
-
 func withTrace(t *testing.T, gasLimit uint64, test func(vm.Config) error) {
-	err := test(testVMConfig)
+	// Use config from command line arguments.
+	config := vm.Config{EVMInterpreter: *testEVM, EWASMInterpreter: *testEWASM}
+	err := test(config)
 	if err == nil {
 		return
 	}
+
+	// Test failed, re-run with tracing enabled.
 	t.Error(err)
 	if gasLimit > traceErrorLimit {
 		t.Log("gas limit too high for EVM trace")
@@ -93,7 +86,8 @@ func withTrace(t *testing.T, gasLimit uint64, test func(vm.Config) error) {
 	buf := new(bytes.Buffer)
 	w := bufio.NewWriter(buf)
 	tracer := vm.NewJSONLogger(&vm.LogConfig{DisableMemory: true}, w)
-	err2 := test(vm.Config{Debug: true, Tracer: tracer})
+	config.Debug, config.Tracer = true, tracer
+	err2 := test(config)
 	if !reflect.DeepEqual(err, err2) {
 		t.Errorf("different error for second run: %v", err2)
 	}