Skip to content

Commit

Permalink
Integration tests: Use zaptest.Logger based testing.TB
Browse files Browse the repository at this point in the history
Thanks to this the logs:
  - are automatically printed if the test fails.
  - are in pretty consistent format.
  - are annotated by 'member' information of the cluster emitting them.

Side changes:
  - Set propert default got DefaultWarningApplyDuration (used to be '0')
  - Name the members based on their 'place' on the list (as opposed to
'random')
  • Loading branch information
ptabor committed Mar 9, 2021
1 parent efb584c commit 87258ef
Show file tree
Hide file tree
Showing 7 changed files with 50 additions and 22 deletions.
10 changes: 10 additions & 0 deletions raft/logger.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,16 @@ func SetLogger(l Logger) {
raftLoggerMu.Unlock()
}

func ResetDefaultLogger() {
SetLogger(defaultLogger)
}

func getLogger() Logger {
raftLoggerMu.Lock()
defer raftLoggerMu.Unlock()
return raftLogger
}

var (
defaultLogger = &DefaultLogger{Logger: log.New(os.Stderr, "raft", log.LstdFlags)}
discardLogger = &DefaultLogger{Logger: log.New(ioutil.Discard, "", 0)}
Expand Down
2 changes: 1 addition & 1 deletion raft/raft.go
Original file line number Diff line number Diff line change
Expand Up @@ -230,7 +230,7 @@ func (c *Config) validate() error {
}

if c.Logger == nil {
c.Logger = raftLogger
c.Logger = getLogger()
}

if c.ReadOnlyOption == ReadOnlyLeaseBased && !c.CheckQuorum {
Expand Down
2 changes: 1 addition & 1 deletion raft/status.go
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ func (s Status) MarshalJSON() ([]byte, error) {
func (s Status) String() string {
b, err := s.MarshalJSON()
if err != nil {
raftLogger.Panicf("unexpected error: %v", err)
getLogger().Panicf("unexpected error: %v", err)
}
return string(b)
}
8 changes: 4 additions & 4 deletions raft/storage.go
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ func (ms *MemoryStorage) Entries(lo, hi, maxSize uint64) ([]pb.Entry, error) {
return nil, ErrCompacted
}
if hi > ms.lastIndex()+1 {
raftLogger.Panicf("entries' hi(%d) is out of bound lastindex(%d)", hi, ms.lastIndex())
getLogger().Panicf("entries' hi(%d) is out of bound lastindex(%d)", hi, ms.lastIndex())
}
// only contains dummy entries.
if len(ms.ents) == 1 {
Expand Down Expand Up @@ -200,7 +200,7 @@ func (ms *MemoryStorage) CreateSnapshot(i uint64, cs *pb.ConfState, data []byte)

offset := ms.ents[0].Index
if i > ms.lastIndex() {
raftLogger.Panicf("snapshot %d is out of bound lastindex(%d)", i, ms.lastIndex())
getLogger().Panicf("snapshot %d is out of bound lastindex(%d)", i, ms.lastIndex())
}

ms.snapshot.Metadata.Index = i
Expand All @@ -223,7 +223,7 @@ func (ms *MemoryStorage) Compact(compactIndex uint64) error {
return ErrCompacted
}
if compactIndex > ms.lastIndex() {
raftLogger.Panicf("compact %d is out of bound lastindex(%d)", compactIndex, ms.lastIndex())
getLogger().Panicf("compact %d is out of bound lastindex(%d)", compactIndex, ms.lastIndex())
}

i := compactIndex - offset
Expand Down Expand Up @@ -266,7 +266,7 @@ func (ms *MemoryStorage) Append(entries []pb.Entry) error {
case uint64(len(ms.ents)) == offset:
ms.ents = append(ms.ents, entries...)
default:
raftLogger.Panicf("missing log entry [last: %d, append at: %d]",
getLogger().Panicf("missing log entry [last: %d, append at: %d]",
ms.lastIndex(), entries[0].Index)
}
return nil
Expand Down
2 changes: 1 addition & 1 deletion test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ function integration_extra {

function integration_pass {
local pkgs=${USERPKG:-"./integration/..."}
run_for_module "tests" go_test "${pkgs}" "parallel" : -timeout="${TIMEOUT:-15m}" "-v" "${COMMON_TEST_FLAGS[@]}" "${RUN_ARG[@]}" "$@" || return $?
run_for_module "tests" go_test "${pkgs}" "parallel" : -timeout="${TIMEOUT:-15m}" "${COMMON_TEST_FLAGS[@]}" "${RUN_ARG[@]}" "$@" || return $?
integration_extra "$@"
}

Expand Down
2 changes: 1 addition & 1 deletion tests/integration/clientv3/cluster_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -265,7 +265,7 @@ func TestMemberPromote(t *testing.T) {
// create and launch learner member based on the response of V3 Member Add API.
// (the response has information on peer urls of the existing members in cluster)
learnerMember := clus.MustNewMember(t, memberAddResp)
clus.Members = append(clus.Members, learnerMember)

if err := learnerMember.Launch(); err != nil {
t.Fatal(err)
}
Expand Down
46 changes: 32 additions & 14 deletions tests/integration/cluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,11 +36,11 @@ import (
pb "go.etcd.io/etcd/api/v3/etcdserverpb"
"go.etcd.io/etcd/client/v2"
"go.etcd.io/etcd/client/v3"
"go.etcd.io/etcd/pkg/v3/logutil"
"go.etcd.io/etcd/pkg/v3/testutil"
"go.etcd.io/etcd/pkg/v3/tlsutil"
"go.etcd.io/etcd/pkg/v3/transport"
"go.etcd.io/etcd/pkg/v3/types"
"go.etcd.io/etcd/raft/v3"
"go.etcd.io/etcd/server/v3/embed"
"go.etcd.io/etcd/server/v3/etcdserver"
"go.etcd.io/etcd/server/v3/etcdserver/api/etcdhttp"
Expand All @@ -52,6 +52,8 @@ import (
"go.etcd.io/etcd/server/v3/etcdserver/api/v3lock"
lockpb "go.etcd.io/etcd/server/v3/etcdserver/api/v3lock/v3lockpb"
"go.etcd.io/etcd/server/v3/etcdserver/api/v3rpc"
"go.uber.org/zap/zapcore"
"go.uber.org/zap/zaptest"

"github.com/soheilhy/cmux"
"go.uber.org/zap"
Expand Down Expand Up @@ -167,8 +169,14 @@ type ClusterConfig struct {
}

type cluster struct {
cfg *ClusterConfig
Members []*member
cfg *ClusterConfig
Members []*member
lastMemberNum int
}

func (c *cluster) generateMemberName() string {
c.lastMemberNum++
return fmt.Sprintf("m%v", c.lastMemberNum)
}

func schemeFromTLSInfo(tls *transport.TLSInfo) string {
Expand Down Expand Up @@ -294,7 +302,7 @@ func (c *cluster) HTTPMembers() []client.Member {
func (c *cluster) mustNewMember(t testing.TB) *member {
m := mustNewMember(t,
memberConfig{
name: c.name(rand.Int()),
name: c.generateMemberName(),
authToken: c.cfg.AuthToken,
peerTLS: c.cfg.PeerTLS,
clientTLS: c.cfg.ClientTLS,
Expand Down Expand Up @@ -707,18 +715,28 @@ func mustNewMember(t testing.TB, mcfg memberConfig) *member {
m.WatchProgressNotifyInterval = mcfg.WatchProgressNotifyInterval

m.InitialCorruptCheck = true
m.WarningApplyDuration = embed.DefaultWarningApplyDuration

lcfg := logutil.DefaultZapLoggerConfig
m.LoggerConfig = &lcfg
m.LoggerConfig.OutputPaths = []string{"/dev/null"}
m.LoggerConfig.ErrorOutputPaths = []string{"/dev/null"}
level := zapcore.InfoLevel
if os.Getenv("CLUSTER_DEBUG") != "" {
m.LoggerConfig.OutputPaths = []string{"stderr"}
m.LoggerConfig.ErrorOutputPaths = []string{"stderr"}
level = zapcore.DebugLevel
}
m.Logger, err = m.LoggerConfig.Build()
if err != nil {
t.Fatal(err)

if t != nil {
options := zaptest.WrapOptions(zap.Fields(zap.String("member", mcfg.name)))
m.Logger = zaptest.NewLogger(t, zaptest.Level(level), options)
if t != nil {
t.Cleanup(func() {
// if we didn't cleanup the logger, the consecutive test
// might reuse this (t).
raft.ResetDefaultLogger()
})
}
} else {
m.Logger, err = zap.NewDevelopment(zap.IncreaseLevel(level))
if err != nil {
log.Panic(err)
}
}
return m
}
Expand Down Expand Up @@ -1518,6 +1536,6 @@ func (c *ClusterV3) MustNewMember(t testing.TB, resp *clientv3.MemberAddResponse
m.InitialPeerURLsMap[mm.Name] = mm.PeerURLs
}
m.InitialPeerURLsMap[m.Name] = types.MustNewURLs(resp.Member.PeerURLs)

c.Members = append(c.Members, m)
return m
}

0 comments on commit 87258ef

Please sign in to comment.