owner: migrate test-infra to testify

owner/fail_test.go

@@ -10,6 +10,7 @@
 // distributed under the License is distributed on an "AS IS" BASIS,
 // See the License for the specific language governing permissions and
 // limitations under the License.
+
 // +build !windows
 
 package owner
@@ -26,32 +27,34 @@ import (
 
 	"github.com/pingcap/failpoint"
 	"github.com/pingcap/parser/terror"
-
+	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	"go.etcd.io/etcd/clientv3"
 	"google.golang.org/grpc"
 )
 
 var (
-	dialTimeout = 5 * time.Second
+	dialTimeout = 3 * time.Second
 	retryCnt    = math.MaxInt32
 )
 
 func TestFailNewSession(t *testing.T) {
-	t.Parallel()
-	os.Remove("new_session:0")
+	_ = os.Remove("new_session:0")
 	ln, err := net.Listen("unix", "new_session:0")
 	require.NoError(t, err)
+
 	addr := ln.Addr()
 	endpoints := []string{fmt.Sprintf("%s://%s", addr.Network(), addr.String())}
-	require.Nil(t, err)
+	require.NoError(t, err)
+
 	srv := grpc.NewServer(grpc.ConnectionTimeout(time.Minute))
 	var stop sync.WaitGroup
 	stop.Add(1)
+
 	go func() {
+		defer stop.Done()
 		err = srv.Serve(ln)
-		require.Errorf(t, err, "can't serve gRPC requests")
-		stop.Done()
+		assert.NoError(t, err)
 	}()
 
 	defer func() {
@@ -67,21 +70,21 @@ func TestFailNewSession(t *testing.T) {
 		require.NoError(t, err)
 		defer func() {
 			if cli != nil {
-				cli.Close()
+				_ = cli.Close()
 			}
-			require.Nil(t, failpoint.Disable("github.com/pingcap/tidb/owner/closeClient"))
+			require.NoError(t, failpoint.Disable("github.com/pingcap/tidb/owner/closeClient"))
 		}()
-		require.NoError(t, err, failpoint.Enable("github.com/pingcap/tidb/owner/closeClient", `return(true)`))
+		require.NoError(t, failpoint.Enable("github.com/pingcap/tidb/owner/closeClient", `return(true)`))
 
 		// TODO: It takes more than 2s here in etcd client, the CI takes 5s to run this test.
 		// The config is hard coded, not way to control it outside.
 		// Call stack:
 		// https://github.com/etcd-io/etcd/blob/ae9734e/clientv3/concurrency/session.go#L38
 		// https://github.com/etcd-io/etcd/blob/ae9734ed278b7a1a7dfc82e800471ebbf9fce56f/clientv3/client.go#L253
 		// https://github.com/etcd-io/etcd/blob/ae9734ed278b7a1a7dfc82e800471ebbf9fce56f/clientv3/retry_interceptor.go#L63
-		_, err = NewSession(context.Background(), "fail_new_serssion", cli, retryCnt, ManagerSessionTTL)
+		_, err = NewSession(context.Background(), "fail_new_session", cli, retryCnt, ManagerSessionTTL)
 		isContextDone := terror.ErrorEqual(grpc.ErrClientConnClosing, err) || terror.ErrorEqual(context.Canceled, err)
-		require.True(t, isContextDone, "err %v", err)
+		require.Truef(t, isContextDone, "err %v", err)
 	}()
 
 	func() {
@@ -92,16 +95,16 @@ func TestFailNewSession(t *testing.T) {
 		require.NoError(t, err)
 		defer func() {
 			if cli != nil {
-				cli.Close()
+				_ = cli.Close()
 			}
-			require.Nil(t, failpoint.Disable("github.com/pingcap/tidb/owner/closeGrpc"))
+			require.NoError(t, failpoint.Disable("github.com/pingcap/tidb/owner/closeGrpc"))
 		}()
-		require.Nil(t, failpoint.Enable("github.com/pingcap/tidb/owner/closeGrpc", `return(false)`))
+		require.NoError(t, failpoint.Enable("github.com/pingcap/tidb/owner/closeGrpc", `return(true)`))
 
 		// TODO: It takes more than 2s here in etcd client, the CI takes 5s to run this test.
 		// The config is hard coded, not way to control it outside.
-		_, err = NewSession(context.Background(), "fail_new_serssion", cli, retryCnt, ManagerSessionTTL)
+		_, err = NewSession(context.Background(), "fail_new_session", cli, retryCnt, ManagerSessionTTL)
 		isContextDone := terror.ErrorEqual(grpc.ErrClientConnClosing, err) || terror.ErrorEqual(context.Canceled, err)
-		require.True(t, isContextDone, "err %v", err)
+		require.Truef(t, isContextDone, "err %v", err)
 	}()
 }

owner/main_test.go

@@ -14,26 +14,16 @@
 package owner
 
 import (
-	"os"
 	"testing"
 
-	"github.com/pingcap/log"
-	"github.com/pingcap/tidb/util/logutil"
 	"github.com/pingcap/tidb/util/testbridge"
 	"go.uber.org/goleak"
-	"go.uber.org/zap"
 )
 
 func TestMain(m *testing.M) {
 	testbridge.WorkaroundGoCheckFlags()
-
-	// Ignore this test on the windows platform, because calling unix socket with address in
-	// host:port format fails on windows.
-	logLevel := os.Getenv("log_level")
-	err := logutil.InitLogger(logutil.NewLogConfig(logLevel, "", "", logutil.EmptyFileLogConfig, false))
-	if err != nil {
-		log.Fatal("put failed", zap.Error(err))
+	opts := []goleak.Option{
+		goleak.IgnoreTopFunction("go.etcd.io/etcd/pkg/logutil.(*MergeLogger).outputLoop"),
 	}
-
-	goleak.VerifyTestMain(m)
+	goleak.VerifyTestMain(m, opts...)
 }

owner/manager_test.go

@@ -11,12 +11,13 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
+// +build !windows
+
 package owner_test
 
 import (
 	"context"
 	"fmt"
-	"runtime"
 	"testing"
 	"time"
 
@@ -36,94 +37,79 @@ import (
 
 const testLease = 5 * time.Millisecond
 
-func checkOwner(d DDL, fbVal bool) (isOwner bool) {
-	manager := d.OwnerManager()
-	// The longest to wait for 30 seconds to
-	// make sure that campaigning owners is completed.
-	for i := 0; i < 6000; i++ {
-		time.Sleep(5 * time.Millisecond)
-		isOwner = manager.IsOwner()
-		if isOwner == fbVal {
-			break
-		}
-	}
-	return
-}
-
 func TestSingle(t *testing.T) {
-	if runtime.GOOS == "windows" {
-		t.Skip("integration.NewClusterV3 will create file contains a colon which is not allowed on Windows")
-	}
 	store, err := mockstore.NewMockStore()
 	require.NoError(t, err)
 	defer func() {
 		err := store.Close()
 		require.NoError(t, err)
 	}()
 
-	clus := integration.NewClusterV3(t, &integration.ClusterConfig{Size: 1})
-	defer clus.Terminate(t)
-	cli := clus.RandClient()
+	cluster := integration.NewClusterV3(t, &integration.ClusterConfig{Size: 1})
+	defer cluster.Terminate(t)
+
+	client := cluster.RandClient()
 	ctx := goctx.Background()
 	ic := infoschema.NewCache(2)
 	ic.Insert(infoschema.MockInfoSchemaWithSchemaVer(nil, 0), 0)
 	d := NewDDL(
 		ctx,
-		WithEtcdClient(cli),
+		WithEtcdClient(client),
 		WithStore(store),
 		WithLease(testLease),
 		WithInfoCache(ic),
 	)
 	err = d.Start(nil)
-	require.NoErrorf(t, err, "DDL start failed")
+	require.NoError(t, err)
 	defer func() {
 		_ = d.Stop()
 	}()
 
 	isOwner := checkOwner(d, true)
-	require.Truef(t, isOwner, "expet true, got isOowner false")
+	require.True(t, isOwner)
 
 	// test for newSession failed
 	ctx, cancel := goctx.WithCancel(ctx)
-	manager := owner.NewOwnerManager(ctx, cli, "ddl", "ddl_id", DDLOwnerKey)
+	manager := owner.NewOwnerManager(ctx, client, "ddl", "ddl_id", DDLOwnerKey)
 	cancel()
+
 	err = manager.CampaignOwner()
-	if !terror.ErrorEqual(err, goctx.Canceled) &&
-		!terror.ErrorEqual(err, goctx.DeadlineExceeded) {
-		require.NoErrorf(t, err, "campaigned result don't match, err %v")
-	}
+	comment := fmt.Sprintf("campaigned result don't match, err %v", err)
+	require.True(t, terror.ErrorEqual(err, goctx.Canceled) || terror.ErrorEqual(err, goctx.DeadlineExceeded), comment)
+
 	isOwner = checkOwner(d, true)
-	require.Truef(t, isOwner, "expected true, got isOwner false")
+	require.True(t, isOwner)
+
 	// The test is used to exit campaign loop.
 	d.OwnerManager().Cancel()
 	isOwner = checkOwner(d, false)
-	require.Falsef(t, isOwner, "expected false, got isOwner:%v")
+	require.False(t, isOwner)
+
 	time.Sleep(200 * time.Millisecond)
+
+	// err is ok to be not nil since we canceled the manager.
 	ownerID, _ := manager.GetOwnerID(goctx.Background())
-	// The error is ok to be not nil since we canceled the manager.
-	require.Emptyf(t, ownerID, "owner %s is not empty", ownerID)
+	require.Equal(t, "", ownerID)
 }
 
 func TestCluster(t *testing.T) {
-	if runtime.GOOS == "windows" {
-		t.Skip("integration.NewClusterV3 will create file contains a colon which is not allowed on Windows")
-	}
-	tmpTTL := 3
-	orignalTTL := owner.ManagerSessionTTL
-	owner.ManagerSessionTTL = tmpTTL
+	originalTTL := owner.ManagerSessionTTL
+	owner.ManagerSessionTTL = 3
 	defer func() {
-		owner.ManagerSessionTTL = orignalTTL
+		owner.ManagerSessionTTL = originalTTL
 	}()
+
 	store, err := mockstore.NewMockStore()
 	require.NoError(t, err)
 	defer func() {
 		err := store.Close()
 		require.NoError(t, err)
 	}()
-	clus := integration.NewClusterV3(t, &integration.ClusterConfig{Size: 4})
-	defer clus.Terminate(t)
 
-	cli := clus.Client(0)
+	cluster := integration.NewClusterV3(t, &integration.ClusterConfig{Size: 4})
+	defer cluster.Terminate(t)
+
+	cli := cluster.Client(0)
 	ic := infoschema.NewCache(2)
 	ic.Insert(infoschema.MockInfoSchemaWithSchemaVer(nil, 0), 0)
 	d := NewDDL(
@@ -133,11 +119,14 @@ func TestCluster(t *testing.T) {
 		WithLease(testLease),
 		WithInfoCache(ic),
 	)
+
 	err = d.Start(nil)
-	require.NoErrorf(t, err, "DDL start failed")
+	require.NoError(t, err)
+
 	isOwner := checkOwner(d, true)
-	require.Truef(t, isOwner, "expect true, got isOwner false")
-	cli1 := clus.Client(1)
+	require.True(t, isOwner)
+
+	cli1 := cluster.Client(1)
 	ic2 := infoschema.NewCache(2)
 	ic2.Insert(infoschema.MockInfoSchemaWithSchemaVer(nil, 0), 0)
 	d1 := NewDDL(
@@ -148,20 +137,24 @@ func TestCluster(t *testing.T) {
 		WithInfoCache(ic2),
 	)
 	err = d1.Start(nil)
-	require.NoErrorf(t, err, "DDL start failed")
+	require.NoError(t, err)
+
 	isOwner = checkOwner(d1, false)
-	require.Falsef(t, isOwner, "expect false, got isOwner true")
+	require.False(t, isOwner)
 
 	// Delete the leader key, the d1 become the owner.
-	cliRW := clus.Client(2)
+	cliRW := cluster.Client(2)
 	err = deleteLeader(cliRW, DDLOwnerKey)
 	require.NoError(t, err)
+
 	isOwner = checkOwner(d, false)
-	require.Falsef(t, isOwner, "expect false, got isOwner true")
+	require.False(t, isOwner)
+
 	err = d.Stop()
-	require.NoErrorf(t, err, "DDL stop failed")
+	require.NoError(t, err)
+
 	// d3 (not owner) stop
-	cli3 := clus.Client(3)
+	cli3 := cluster.Client(3)
 	ic3 := infoschema.NewCache(2)
 	ic3.Insert(infoschema.MockInfoSchemaWithSchemaVer(nil, 0), 0)
 	d3 := NewDDL(
@@ -172,36 +165,56 @@ func TestCluster(t *testing.T) {
 		WithInfoCache(ic3),
 	)
 	err = d3.Start(nil)
-	require.NoErrorf(t, err, "DDL start failed")
+	require.NoError(t, err)
 	defer func() {
 		err = d3.Stop()
-		require.NoErrorf(t, err, "DDL stop failed")
+		require.NoError(t, err)
 	}()
+
 	isOwner = checkOwner(d3, false)
-	require.Falsef(t, isOwner, "expect false, got isOwner true")
+	require.False(t, isOwner)
+
 	err = d3.Stop()
-	require.NoErrorf(t, err, "DDL stop failed")
+	require.NoError(t, err)
 
 	// Cancel the owner context, there is no owner.
 	err = d1.Stop()
-	require.NoErrorf(t, err, "DDL stop failed")
+	require.NoError(t, err)
+
 	session, err := concurrency.NewSession(cliRW)
-	require.NoErrorf(t, err, "new session failed")
-	elec := concurrency.NewElection(session, DDLOwnerKey)
+	require.NoError(t, err)
+
+	election := concurrency.NewElection(session, DDLOwnerKey)
 	logPrefix := fmt.Sprintf("[ddl] %s ownerManager %s", DDLOwnerKey, "useless id")
 	logCtx := logutil.WithKeyValue(context.Background(), "owner info", logPrefix)
-	_, err = owner.GetOwnerInfo(goctx.Background(), logCtx, elec, "useless id")
-	require.Truef(t, terror.ErrorEqual(err, concurrency.ErrElectionNoLeader), "get owner info result don't match, err %v")
+	_, err = owner.GetOwnerInfo(goctx.Background(), logCtx, election, "useless id")
+	require.Truef(t, terror.ErrorEqual(err, concurrency.ErrElectionNoLeader), "get owner info result don't match, err %v", err)
+}
+
+func checkOwner(d DDL, fbVal bool) (isOwner bool) {
+	manager := d.OwnerManager()
+	// The longest to wait for 30 seconds to
+	// make sure that campaigning owners is completed.
+	for i := 0; i < 6000; i++ {
+		time.Sleep(5 * time.Millisecond)
+		isOwner = manager.IsOwner()
+		if isOwner == fbVal {
+			break
+		}
+	}
+	return
 }
 
 func deleteLeader(cli *clientv3.Client, prefixKey string) error {
 	session, err := concurrency.NewSession(cli)
 	if err != nil {
 		return errors.Trace(err)
 	}
-	defer session.Close()
-	elec := concurrency.NewElection(session, prefixKey)
-	resp, err := elec.Leader(goctx.Background())
+	defer func() {
+		_ = session.Close()
+	}()
+	election := concurrency.NewElection(session, prefixKey)
+	resp, err := election.Leader(goctx.Background())
 	if err != nil {
 		return errors.Trace(err)
 	}