From f0ecad00e3fafc6aeeb4bd8db3209987fcc058c8 Mon Sep 17 00:00:00 2001 From: Piotr Tabor Date: Sun, 10 Jan 2021 22:51:57 +0100 Subject: [PATCH 1/3] Use temp-directory that is covered by framework level cleanup Prior to this PR, the e2e tests where creating dirs like: ``` /tmp/testname1.etcd030299846 /tmp/testname0.etcd039445123 /tmp/testname0.etcd206372065 ``` and not cleaning them, that led to disk-space-exceeded flakes. After the PR, the testing.TB tempdir mechanism is used and the names are being cleaned and are more miningful: ``` ../../bin/etcd --name test-TestCtlV3EndpointHashKV-2 --listen-client-urls http://localhost:20010 --advertise-client-urls http://localhost:20010 --listen-peer-urls https://localhost:20011 --initial-advertise-peer-urls https://localhost:20011 --initial-cluster-token new --data-dir /tmp/TestCtlV3EndpointHashKV429176179/003 --snapshot-count 100000 --experimental-initial-corrupt-check --peer-auto-tls --initial-cluster test-TestCtlV3EndpointHashKV-0=https://localhost:20001,test-TestCtlV3EndpointHashKV-1=https://localhost:20006,test-TestCtlV3EndpointHashKV-2=https://localhost:20011 ``` --- test.sh | 2 +- tests/e2e/cluster_test.go | 13 ++++--------- tests/e2e/ctl_v3_snapshot_test.go | 24 ++++++++---------------- 3 files changed, 13 insertions(+), 26 deletions(-) diff --git a/test.sh b/test.sh index 06137b9eb..7ffd7c58b 100755 --- a/test.sh +++ b/test.sh @@ -67,7 +67,7 @@ fi # This options make sense for cases where SUT (System Under Test) is compiled by test. COMMON_TEST_FLAGS=("${RACE}") -if [[ ! -z "${CPU}" ]]; then +if [[ -n "${CPU}" ]]; then COMMON_TEST_FLAGS+=("--cpu=${CPU}") fi diff --git a/tests/e2e/cluster_test.go b/tests/e2e/cluster_test.go index c3d8227b1..3ea04ca67 100644 --- a/tests/e2e/cluster_test.go +++ b/tests/e2e/cluster_test.go @@ -16,7 +16,6 @@ package e2e import ( "fmt" - "io/ioutil" "net/url" "os" "strings" @@ -171,7 +170,7 @@ type etcdProcessClusterConfig struct { func newEtcdProcessCluster(t testing.TB, cfg *etcdProcessClusterConfig) (*etcdProcessCluster, error) { skipInShortMode(t) - etcdCfgs := cfg.etcdServerProcessConfigs() + etcdCfgs := cfg.etcdServerProcessConfigs(t) epc := &etcdProcessCluster{ cfg: cfg, procs: make([]etcdProcess, cfg.clusterSize), @@ -217,7 +216,7 @@ func (cfg *etcdProcessClusterConfig) peerScheme() string { return peerScheme } -func (cfg *etcdProcessClusterConfig) etcdServerProcessConfigs() []*etcdServerProcessConfig { +func (cfg *etcdProcessClusterConfig) etcdServerProcessConfigs(tb testing.TB) []*etcdServerProcessConfig { if cfg.basePort == 0 { cfg.basePort = etcdProcessBasePort } @@ -247,14 +246,10 @@ func (cfg *etcdProcessClusterConfig) etcdServerProcessConfigs() []*etcdServerPro } purl := url.URL{Scheme: cfg.peerScheme(), Host: fmt.Sprintf("localhost:%d", port+1)} - name := fmt.Sprintf("testname%d", i) + name := fmt.Sprintf("test-%s-%d", tb.Name(), i) dataDirPath := cfg.dataDirPath if cfg.dataDirPath == "" { - var derr error - dataDirPath, derr = ioutil.TempDir("", name+".etcd") - if derr != nil { - panic(fmt.Sprintf("could not get tempdir for datadir: %s", derr)) - } + dataDirPath = tb.TempDir() } initialCluster[i] = fmt.Sprintf("%s=%s", name, purl.String()) diff --git a/tests/e2e/ctl_v3_snapshot_test.go b/tests/e2e/ctl_v3_snapshot_test.go index 4bcb3123d..c95a10bda 100644 --- a/tests/e2e/ctl_v3_snapshot_test.go +++ b/tests/e2e/ctl_v3_snapshot_test.go @@ -18,7 +18,6 @@ import ( "encoding/json" "fmt" "io" - "math/rand" "os" "path/filepath" "strings" @@ -32,13 +31,6 @@ import ( func TestCtlV3Snapshot(t *testing.T) { testCtl(t, snapshotTest) } -// TODO: Replace with testing.T.TestDir() in golang-1.15. -func tempDir(tb testing.TB) string { - dir := filepath.Join(os.TempDir(), tb.Name(), fmt.Sprint(rand.Int())) - os.MkdirAll(dir, 0700) - return dir -} - func snapshotTest(cx ctlCtx) { maintenanceInitKeys(cx) @@ -50,7 +42,7 @@ func snapshotTest(cx ctlCtx) { cx.t.Fatalf("snapshot: ctlV3Put error (%v)", err) } - fpath := filepath.Join(tempDir(cx.t), "snapshot") + fpath := filepath.Join(cx.t.TempDir(), "snapshot") defer os.RemoveAll(fpath) if err = ctlV3SnapshotSave(cx, fpath); err != nil { @@ -72,7 +64,7 @@ func snapshotTest(cx ctlCtx) { func TestCtlV3SnapshotCorrupt(t *testing.T) { testCtl(t, snapshotCorruptTest) } func snapshotCorruptTest(cx ctlCtx) { - fpath := filepath.Join(tempDir(cx.t), "snapshot") + fpath := filepath.Join(cx.t.TempDir(), "snapshot") defer os.RemoveAll(fpath) if err := ctlV3SnapshotSave(cx, fpath); err != nil { @@ -89,7 +81,7 @@ func snapshotCorruptTest(cx ctlCtx) { } f.Close() - datadir := filepath.Join(tempDir(cx.t), "data") + datadir := cx.t.TempDir() defer os.RemoveAll(datadir) serr := spawnWithExpect( @@ -107,7 +99,7 @@ func snapshotCorruptTest(cx ctlCtx) { func TestCtlV3SnapshotStatusBeforeRestore(t *testing.T) { testCtl(t, snapshotStatusBeforeRestoreTest) } func snapshotStatusBeforeRestoreTest(cx ctlCtx) { - fpath := filepath.Join(tempDir(cx.t), "snapshot") + fpath := filepath.Join(cx.t.TempDir(), "snapshot") defer os.RemoveAll(fpath) if err := ctlV3SnapshotSave(cx, fpath); err != nil { @@ -120,7 +112,7 @@ func snapshotStatusBeforeRestoreTest(cx ctlCtx) { cx.t.Fatalf("snapshotTest getSnapshotStatus error (%v)", err) } - dataDir := filepath.Join(tempDir(cx.t), "data") + dataDir := cx.t.TempDir() defer os.RemoveAll(dataDir) serr := spawnWithExpect( append(cx.PrefixArgs(), "snapshot", "restore", @@ -201,7 +193,7 @@ func TestIssue6361(t *testing.T) { } } - fpath := filepath.Join(tempDir(t), "snapshot") + fpath := filepath.Join(t.TempDir(), "test.snapshot") defer os.RemoveAll(fpath) t.Log("etcdctl saving snapshot...") @@ -214,7 +206,7 @@ func TestIssue6361(t *testing.T) { t.Fatal(err) } - newDataDir := tempDir(t) + newDataDir := filepath.Join(t.TempDir(), "test.data") defer os.RemoveAll(newDataDir) t.Log("etcdctl restoring the snapshot...") @@ -249,7 +241,7 @@ func TestIssue6361(t *testing.T) { t.Fatal(err) } - newDataDir2 := filepath.Join(tempDir(t), "newdata") + newDataDir2 := t.TempDir() defer os.RemoveAll(newDataDir2) name2 := "infra2" From 73ef0cde013f9fe93abe1cba0107fa3d291e288e Mon Sep 17 00:00:00 2001 From: Piotr Tabor Date: Sat, 30 Jan 2021 13:30:10 +0100 Subject: [PATCH 2/3] Update travis to use 1.15.7 go version. --- .travis.yml | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/.travis.yml b/.travis.yml index ecfffe14e..18149c3c2 100644 --- a/.travis.yml +++ b/.travis.yml @@ -6,7 +6,7 @@ sudo: required services: docker go: - - 1.14.3 + - 1.15.7 - tip notifications: @@ -30,13 +30,13 @@ env: matrix: fast_finish: true allow_failures: - - go: 1.14.3 + - go: 1.15.7 env: TARGET=linux-amd64-grpcproxy - - go: 1.14.3 + - go: 1.15.7 env: TARGET=linux-amd64-coverage - go: tip env: TARGET=linux-amd64-fmt-unit-go-tip-2-cpu - - go: 1.14.3 + - go: 1.15.7 env: TARGET=linux-386-unit-1-cpu exclude: - go: tip From 0d7a671c75fa378420090578c8dec95675a8b0ae Mon Sep 17 00:00:00 2001 From: Piotr Tabor Date: Sun, 31 Jan 2021 00:00:26 +0100 Subject: [PATCH 3/3] Tests: Fix vet warnings about Fatal in sub-goroutines. % (cd tests && go vet ./...) stderr: # go.etcd.io/etcd/tests/v3/integration/clientv3/concurrency_test stderr: integration/clientv3/concurrency/election_test.go:74:6: call to (*T).Fatal from a non-test goroutine stderr: integration/clientv3/concurrency/mutex_test.go:57:4: call to (*T).Fatal from a non-test goroutine --- tests/integration/clientv3/concurrency/election_test.go | 3 ++- tests/integration/clientv3/concurrency/mutex_test.go | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/integration/clientv3/concurrency/election_test.go b/tests/integration/clientv3/concurrency/election_test.go index fd518e3b5..ad9ba7aa2 100644 --- a/tests/integration/clientv3/concurrency/election_test.go +++ b/tests/integration/clientv3/concurrency/election_test.go @@ -65,13 +65,14 @@ func TestResumeElection(t *testing.T) { respChan := make(chan *clientv3.GetResponse) go func() { + defer close(respChan) o := e.Observe(ctx) respChan <- nil for { select { case resp, ok := <-o: if !ok { - t.Fatal("Observe() channel closed prematurely") + t.Error("Observe() channel closed prematurely") } // Ignore any observations that candidate1 was elected if string(resp.Kvs[0].Value) == "candidate1" { diff --git a/tests/integration/clientv3/concurrency/mutex_test.go b/tests/integration/clientv3/concurrency/mutex_test.go index 7180b9ab8..35b3a697f 100644 --- a/tests/integration/clientv3/concurrency/mutex_test.go +++ b/tests/integration/clientv3/concurrency/mutex_test.go @@ -54,7 +54,7 @@ func TestMutexLockSessionExpired(t *testing.T) { defer close(m2Locked) // m2 blocks since m1 already acquired lock /my-lock/ if err2 = m2.Lock(context.TODO()); err2 == nil { - t.Fatal("expect session expired error") + t.Error("expect session expired error") } }()