Merge pull request #13055 from ptabor/20210528-beckport-integration-fixes

tests/e2e/testing.go

@@ -25,7 +25,7 @@ import (
 
 func BeforeTest(t testing.TB) {
 	skipInShortMode(t)
-	testutil.BeforeTest(t)
+	testutil.RegisterLeakDetection(t)
 	os.Setenv(verify.ENV_VERIFY, verify.ENV_VERIFY_ALL_VALUE)
 
 	path, err := os.Getwd()

tests/integration/clientv3/connectivity/server_shutdown_test.go

@@ -17,6 +17,7 @@ package connectivity_test
 import (
 	"bytes"
 	"context"
+	"fmt"
 	"testing"
 	"time"
 
@@ -243,8 +244,10 @@ func TestBalancerUnderServerStopInflightLinearizableGetOnRestart(t *testing.T) {
 		{pinLeader: false, stopPinFirst: true},
 		{pinLeader: false, stopPinFirst: false},
 	}
-	for i := range tt {
-		testBalancerUnderServerStopInflightRangeOnRestart(t, true, tt[i])
+	for _, w := range tt {
+		t.Run(fmt.Sprintf("%#v", w), func(t *testing.T) {
+			testBalancerUnderServerStopInflightRangeOnRestart(t, true, w)
+		})
 	}
 }
 
@@ -255,8 +258,10 @@ func TestBalancerUnderServerStopInflightSerializableGetOnRestart(t *testing.T) {
 		{pinLeader: false, stopPinFirst: true},
 		{pinLeader: false, stopPinFirst: false},
 	}
-	for i := range tt {
-		testBalancerUnderServerStopInflightRangeOnRestart(t, false, tt[i])
+	for _, w := range tt {
+		t.Run(fmt.Sprintf("%#v", w), func(t *testing.T) {
+			testBalancerUnderServerStopInflightRangeOnRestart(t, false, w)
+		})
 	}
 }
 

tests/integration/clientv3/watch_test.go

@@ -611,8 +611,6 @@ func TestConfigurableWatchProgressNotifyInterval(t *testing.T) {
 }
 
 func TestWatchRequestProgress(t *testing.T) {
-	integration.BeforeTest(t)
-
 	if integration.ThroughProxy {
 		t.Skipf("grpc-proxy does not support WatchProgress yet")
 	}

tests/integration/cluster.go

@@ -1288,16 +1288,8 @@ type ClusterV3 struct {
 // for each cluster member.
 func NewClusterV3(t testutil.TB, cfg *ClusterConfig) *ClusterV3 {
 	t.Helper()
-	testutil.SkipTestIfShortMode(t, "Cannot create clusters in --short tests")
 
-	wd, err := os.Getwd()
-	if err != nil {
-		t.Fatal(err)
-	}
-	if !strings.HasPrefix(wd, os.TempDir()) {
-		t.Errorf("Working directory '%s' expected to be in temp-dir ('%s')."+
-			"Have you executed integration.BeforeTest(t) ?", wd, os.TempDir())
-	}
+	assertInTestContext(t)
 
 	cfg.UseGRPC = true
 

tests/integration/testing.go

@@ -30,29 +30,85 @@ import (
 )
 
 var grpc_logger grpc_logsettable.SettableLoggerV2
+var insideTestContext bool
 
 func init() {
 	grpc_logger = grpc_logsettable.ReplaceGrpcLoggerV2()
 }
 
-func BeforeTest(t testutil.TB) {
-	testutil.BeforeTest(t)
+type testOptions struct {
+	goLeakDetection bool
+	skipInShort     bool
+}
 
-	grpc_logger.Set(zapgrpc.NewLogger(zaptest.NewLogger(t).Named("grpc")))
+func newTestOptions(opts ...TestOption) *testOptions {
+	o := &testOptions{goLeakDetection: true, skipInShort: true}
+	for _, opt := range opts {
+		opt(o)
+	}
+	return o
+}
 
-	// Integration tests should verify written state as much as possible.
-	os.Setenv(verify.ENV_VERIFY, verify.ENV_VERIFY_ALL_VALUE)
+type TestOption func(opt *testOptions)
+
+// WithoutGoLeakDetection disables checking whether a testcase leaked a goroutine.
+func WithoutGoLeakDetection() TestOption {
+	return func(opt *testOptions) { opt.goLeakDetection = false }
+}
+
+func WithoutSkipInShort() TestOption {
+	return func(opt *testOptions) { opt.skipInShort = false }
+}
+
+// BeforeTestExternal initializes test context and is targeted for external APIs.
+// In general the `integration` package is not targeted to be used outside of
+// etcd project, but till the dedicated package is developed, this is
+// the best entry point so far (without backward compatibility promise).
+func BeforeTestExternal(t testutil.TB) {
+	BeforeTest(t, WithoutSkipInShort(), WithoutGoLeakDetection())
+}
+
+func BeforeTest(t testutil.TB, opts ...TestOption) {
+	t.Helper()
+	options := newTestOptions(opts...)
+
+	if options.skipInShort {
+		testutil.SkipTestIfShortMode(t, "Cannot create clusters in --short tests")
+	}
+
+	if options.goLeakDetection {
+		testutil.RegisterLeakDetection(t)
+	}
 
 	previousWD, err := os.Getwd()
 	if err != nil {
 		t.Fatal(err)
 	}
-	os.Chdir(t.TempDir())
+	previousInsideTestContext := insideTestContext
+
+	// Registering cleanup early, such it will get executed even if the helper fails.
 	t.Cleanup(func() {
 		grpc_logger.Reset()
+		insideTestContext = previousInsideTestContext
 		os.Chdir(previousWD)
 	})
 
+	if insideTestContext {
+		t.Fatal("already in test context. BeforeTest was likely already called")
+	}
+
+	grpc_logger.Set(zapgrpc.NewLogger(zaptest.NewLogger(t).Named("grpc")))
+	insideTestContext = true
+
+	// Integration tests should verify written state as much as possible.
+	os.Setenv(verify.ENV_VERIFY, verify.ENV_VERIFY_ALL_VALUE)
+	os.Chdir(t.TempDir())
+}
+
+func assertInTestContext(t testutil.TB) {
+	if !insideTestContext {
+		t.Errorf("the function can be called only in the test context. Was integration.BeforeTest() called ?")
+	}
 }
 
 func MustAbsPath(path string) string {

tests/integration/testing_test.go

@@ -0,0 +1,28 @@
+// Copyright 2021 The etcd Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package integration_test
+
+import (
+	"testing"
+	"time"
+
+	"go.etcd.io/etcd/tests/v3/integration"
+)
+
+func TestBeforeTestWithoutLeakDetection(t *testing.T) {
+	integration.BeforeTest(t, integration.WithoutGoLeakDetection(), integration.WithoutSkipInShort())
+	// Intentional leak that should get ignored
+	go time.Sleep(2 * time.Second)
+}

tests/integration/v2store/store_tag_not_v2v3_test.go

@@ -32,7 +32,6 @@ type v2TestStore struct {
 func (s *v2TestStore) Close() {}
 
 func newTestStore(t *testing.T, ns ...string) StoreCloser {
-	integration.BeforeTest(t)
 	if len(ns) == 0 {
 		t.Logf("new v2 store with no namespace")
 	}
@@ -41,6 +40,7 @@ func newTestStore(t *testing.T, ns ...string) StoreCloser {
 
 // Ensure that the store can recover from a previously saved state.
 func TestStoreRecover(t *testing.T) {
+	integration.BeforeTest(t)
 	s := newTestStore(t)
 	defer s.Close()
 	var eidx uint64 = 4

tests/integration/v3_lease_test.go

@@ -233,7 +233,6 @@ func TestV3LeaseCheckpoint(t *testing.T) {
 
 	var ttl int64 = 300
 	leaseInterval := 2 * time.Second
-	BeforeTest(t)
 	clus := NewClusterV3(t, &ClusterConfig{
 		Size:                    3,
 		EnableLeaseCheckpoint:   true,