From 39b0b1a1a6506c8c25fcff56f4d70a85739dcb6a Mon Sep 17 00:00:00 2001 From: Felix Lange Date: Wed, 11 Sep 2019 14:41:22 +0200 Subject: [PATCH] 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. --- Makefile | 2 +- build/ci.go | 2 -- cmd/geth/consolecmd_test.go | 12 ++++++----- cmd/geth/run_test.go | 28 ++++++++++++++++++++++++ crypto/ecies/ecies_test.go | 9 -------- internal/build/util.go | 26 ---------------------- les/api_test.go | 42 +++++++++++++++++------------------- p2p/simulations/http_test.go | 9 ++++---- tests/init_test.go | 12 +++++++++++ tests/state_test.go | 20 ++++++----------- 10 files changed, 79 insertions(+), 83 deletions(-) diff --git a/Makefile b/Makefile index 4bf52f5c96..5d4a82de83 100644 --- a/Makefile +++ b/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: diff --git a/build/ci.go b/build/ci.go index d4e2814ec8..002f6bb152 100644 --- a/build/ci.go +++ b/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 diff --git a/cmd/geth/consolecmd_test.go b/cmd/geth/consolecmd_test.go index 4360451195..33c83b7ede 100644 --- a/cmd/geth/consolecmd_test.go +++ b/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() diff --git a/cmd/geth/run_test.go b/cmd/geth/run_test.go index da82facac3..f7b735b84c 100644 --- a/cmd/geth/run_test.go +++ b/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) + } +} diff --git a/crypto/ecies/ecies_test.go b/crypto/ecies/ecies_test.go index 2836b81260..2def505d0c 100644 --- a/crypto/ecies/ecies_test.go +++ b/crypto/ecies/ecies_test.go @@ -35,7 +35,6 @@ import ( "crypto/rand" "crypto/sha256" "encoding/hex" - "flag" "fmt" "math/big" "testing" @@ -43,14 +42,6 @@ import ( "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") diff --git a/internal/build/util.go b/internal/build/util.go index 971d948c44..a1f4567776 100644 --- a/internal/build/util.go +++ b/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]. diff --git a/les/api_test.go b/les/api_test.go index 7d3b4ce5da..660af8eeec 100644 --- a/les/api_test.go +++ b/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 } diff --git a/p2p/simulations/http_test.go b/p2p/simulations/http_test.go index ed43c0ed76..84f6ce2a51 100644 --- a/p2p/simulations/http_test.go +++ b/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 diff --git a/tests/init_test.go b/tests/init_test.go index 053cbd6fcd..d715de02fb 100644 --- a/tests/init_test.go +++ b/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") diff --git a/tests/state_test.go b/tests/state_test.go index 0cf124d727..d25b157275 100644 --- a/tests/state_test.go +++ b/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) }