integration.BeforeTest can be run without leak-detection.

This commit is contained in:
Piotr Tabor 2021-05-27 18:44:14 +02:00
parent b3f16d6691
commit 3f13d3a2d5
13 changed files with 116 additions and 37 deletions

View File

@ -16,6 +16,8 @@ import (
"time" "time"
) )
// TODO: Replace with https://github.com/uber-go/goleak.
/* /*
CheckLeakedGoroutine verifies tests do not leave any leaky CheckLeakedGoroutine verifies tests do not leave any leaky
goroutines. It returns true when there are goroutines still goroutines. It returns true when there are goroutines still
@ -28,10 +30,9 @@ running(leaking) after all tests.
} }
func TestSample(t *testing.T) { func TestSample(t *testing.T) {
BeforeTest(t) RegisterLeakDetection(t)
... ...
} }
*/ */
func CheckLeakedGoroutine() bool { func CheckLeakedGoroutine() bool {
gs := interestingGoroutines() gs := interestingGoroutines()
@ -94,22 +95,22 @@ func CheckAfterTest(d time.Duration) error {
return fmt.Errorf("appears to have leaked %s:\n%s", bad, stacks) return fmt.Errorf("appears to have leaked %s:\n%s", bad, stacks)
} }
// BeforeTest is a convenient way to register before-and-after code to a test. // RegisterLeakDetection is a convenient way to register before-and-after code to a test.
// If you execute BeforeTest, you don't need to explicitly register AfterTest. // If you execute RegisterLeakDetection, you don't need to explicitly register AfterTest.
func BeforeTest(t TB) { func RegisterLeakDetection(t TB) {
if err := CheckAfterTest(10 * time.Millisecond); err != nil { if err := CheckAfterTest(10 * time.Millisecond); err != nil {
t.Skip("Found leaked goroutined BEFORE test", err) t.Skip("Found leaked goroutined BEFORE test", err)
return return
} }
t.Cleanup(func() { t.Cleanup(func() {
AfterTest(t) afterTest(t)
}) })
} }
// AfterTest is meant to run in a defer that executes after a test completes. // afterTest is meant to run in a defer that executes after a test completes.
// It will detect common goroutine leaks, retrying in case there are goroutines // It will detect common goroutine leaks, retrying in case there are goroutines
// not synchronously torn down, and fail the test if any goroutines are stuck. // not synchronously torn down, and fail the test if any goroutines are stuck.
func AfterTest(t TB) { func afterTest(t TB) {
// If test-failed the leaked goroutines list is hidding the real // If test-failed the leaked goroutines list is hidding the real
// source of problem. // source of problem.
if !t.Failed() { if !t.Failed() {

View File

@ -35,7 +35,7 @@ func TestMain(m *testing.M) {
func TestSample(t *testing.T) { func TestSample(t *testing.T) {
SkipTestIfShortMode(t, "Counting leaked routines is disabled in --short tests") SkipTestIfShortMode(t, "Counting leaked routines is disabled in --short tests")
defer AfterTest(t) defer afterTest(t)
ranSample = true ranSample = true
for range make([]struct{}, 100) { for range make([]struct{}, 100) {
go func() { go func() {

View File

@ -35,7 +35,7 @@ func NewClient(t *testing.T, cfg Config) (*Client, error) {
} }
func TestDialCancel(t *testing.T) { func TestDialCancel(t *testing.T) {
testutil.BeforeTest(t) testutil.RegisterLeakDetection(t)
// accept first connection so client is created with dial timeout // accept first connection so client is created with dial timeout
ln, err := net.Listen("unix", "dialcancel:12345") ln, err := net.Listen("unix", "dialcancel:12345")
@ -87,7 +87,7 @@ func TestDialCancel(t *testing.T) {
} }
func TestDialTimeout(t *testing.T) { func TestDialTimeout(t *testing.T) {
testutil.BeforeTest(t) testutil.RegisterLeakDetection(t)
wantError := context.DeadlineExceeded wantError := context.DeadlineExceeded

View File

@ -23,7 +23,7 @@ import (
) )
func TestTxnPanics(t *testing.T) { func TestTxnPanics(t *testing.T) {
testutil.BeforeTest(t) testutil.RegisterLeakDetection(t)
kv := &kv{} kv := &kv{}

View File

@ -187,7 +187,7 @@ func TestStreamReaderDialResult(t *testing.T) {
// TestStreamReaderStopOnDial tests a stream reader closes the connection on stop. // TestStreamReaderStopOnDial tests a stream reader closes the connection on stop.
func TestStreamReaderStopOnDial(t *testing.T) { func TestStreamReaderStopOnDial(t *testing.T) {
testutil.BeforeTest(t) testutil.RegisterLeakDetection(t)
h := http.Header{} h := http.Header{}
h.Add("X-Server-Version", version.Version) h.Add("X-Server-Version", version.Version)
tr := &respWaitRoundTripper{rrt: &respRoundTripper{code: http.StatusOK, header: h}} tr := &respWaitRoundTripper{rrt: &respRoundTripper{code: http.StatusOK, header: h}}

View File

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

View File

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

View File

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

View File

@ -1288,16 +1288,8 @@ type ClusterV3 struct {
// for each cluster member. // for each cluster member.
func NewClusterV3(t testutil.TB, cfg *ClusterConfig) *ClusterV3 { func NewClusterV3(t testutil.TB, cfg *ClusterConfig) *ClusterV3 {
t.Helper() t.Helper()
testutil.SkipTestIfShortMode(t, "Cannot create clusters in --short tests")
wd, err := os.Getwd() assertInTestContext(t)
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())
}
cfg.UseGRPC = true cfg.UseGRPC = true

View File

@ -30,29 +30,85 @@ import (
) )
var grpc_logger grpc_logsettable.SettableLoggerV2 var grpc_logger grpc_logsettable.SettableLoggerV2
var insideTestContext bool
func init() { func init() {
grpc_logger = grpc_logsettable.ReplaceGrpcLoggerV2() grpc_logger = grpc_logsettable.ReplaceGrpcLoggerV2()
} }
func BeforeTest(t testutil.TB) { type testOptions struct {
testutil.BeforeTest(t) 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. type TestOption func(opt *testOptions)
os.Setenv(verify.ENV_VERIFY, verify.ENV_VERIFY_ALL_VALUE)
// 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() previousWD, err := os.Getwd()
if err != nil { if err != nil {
t.Fatal(err) 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() { t.Cleanup(func() {
grpc_logger.Reset() grpc_logger.Reset()
insideTestContext = previousInsideTestContext
os.Chdir(previousWD) 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 { func MustAbsPath(path string) string {

View File

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

View File

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

View File

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