Skip to content

Commit

Permalink
Merge pull request #18782 from mmorel-35/tests/testifier/require-error
Browse files Browse the repository at this point in the history
fix: use require instead of t.Fatal(err) in tests/robustness package
  • Loading branch information
ahrtr authored Oct 30, 2024
2 parents 8ec90c6 + 4017eba commit c2bfbe8
Show file tree
Hide file tree
Showing 7 changed files with 25 additions and 45 deletions.
6 changes: 3 additions & 3 deletions tests/robustness/client/watch.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ import (
"testing"
"time"

"github.com/stretchr/testify/require"

"go.etcd.io/etcd/tests/v3/framework/e2e"
"go.etcd.io/etcd/tests/v3/robustness/identity"
"go.etcd.io/etcd/tests/v3/robustness/report"
Expand All @@ -32,9 +34,7 @@ func CollectClusterWatchEvents(ctx context.Context, t *testing.T, clus *e2e.Etcd
memberMaxRevisionChans := make([]chan int64, len(clus.Procs))
for i, member := range clus.Procs {
c, err := NewRecordingClient(member.EndpointsGRPC(), ids, baseTime)
if err != nil {
t.Fatal(err)
}
require.NoError(t, err)
memberMaxRevisionChan := make(chan int64, 1)
memberMaxRevisionChans[i] = memberMaxRevisionChan
wg.Add(1)
Expand Down
10 changes: 4 additions & 6 deletions tests/robustness/failpoint/cluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import (
"testing"
"time"

"github.com/stretchr/testify/require"
"go.uber.org/zap"

clientv3 "go.etcd.io/etcd/client/v3"
Expand Down Expand Up @@ -56,9 +57,8 @@ func (f memberReplace) Inject(ctx context.Context, t *testing.T, lg *zap.Logger,
if err != nil {
return nil, err
}
if !found {
t.Fatal("Member not found")
}
require.Truef(t, found, "Member not found")

// Need to wait health interval for cluster to accept member changes
time.Sleep(etcdserver.HealthInterval)
lg.Info("Removing member", zap.String("member", member.Config().Name))
Expand All @@ -70,9 +70,7 @@ func (f memberReplace) Inject(ctx context.Context, t *testing.T, lg *zap.Logger,
if err != nil {
return nil, err
}
if found {
t.Fatal("Expected member to be removed")
}
require.Falsef(t, found, "Expected member to be removed")

for member.IsRunning() {
err = member.Kill()
Expand Down
5 changes: 2 additions & 3 deletions tests/robustness/report/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import (
"testing"

"github.com/anishathalye/porcupine"
"github.com/stretchr/testify/require"
"go.uber.org/zap"

"go.etcd.io/etcd/tests/v3/robustness/model"
Expand Down Expand Up @@ -53,9 +54,7 @@ func persistClientReports(t *testing.T, lg *zap.Logger, path string, reports []C
for _, r := range reports {
clientDir := filepath.Join(path, fmt.Sprintf("client-%d", r.ClientID))
err := os.MkdirAll(clientDir, 0700)
if err != nil {
t.Fatal(err)
}
require.NoError(t, err)
if len(r.Watch) != 0 {
persistWatchOperations(t, lg, filepath.Join(clientDir, "watch.json"), r.Watch)
} else {
Expand Down
17 changes: 5 additions & 12 deletions tests/robustness/report/report.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"testing"
"time"

"github.com/stretchr/testify/require"
"go.uber.org/zap"

"go.etcd.io/etcd/tests/v3/framework/e2e"
Expand All @@ -45,17 +46,11 @@ func testResultsDirectory(t *testing.T) string {
}
path, err := filepath.Abs(filepath.Join(
resultsDirectory, strings.ReplaceAll(t.Name(), "/", "_"), fmt.Sprintf("%v", time.Now().UnixNano())))
if err != nil {
t.Fatal(err)
}
require.NoError(t, err)
err = os.RemoveAll(path)
if err != nil {
t.Fatal(err)
}
require.NoError(t, err)
err = os.MkdirAll(path, 0700)
if err != nil {
t.Fatal(err)
}
require.NoError(t, err)
return path
}

Expand Down Expand Up @@ -84,9 +79,7 @@ func (r *TestReport) Report(t *testing.T, force bool) {
func persistMemberDataDir(t *testing.T, lg *zap.Logger, member e2e.EtcdProcess, path string) {
lg.Info("Saving member data dir", zap.String("member", member.Config().Name), zap.String("path", path))
err := os.Rename(memberDataDir(member), path)
if err != nil {
t.Fatal(err)
}
require.NoError(t, err)
}

func memberDataDir(member e2e.EtcdProcess) string {
Expand Down
6 changes: 3 additions & 3 deletions tests/robustness/scenarios/scenarios.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ import (
"testing"
"time"

"github.com/stretchr/testify/require"

"go.etcd.io/etcd/api/v3/version"
"go.etcd.io/etcd/client/pkg/v3/fileutil"
"go.etcd.io/etcd/tests/v3/framework/e2e"
Expand Down Expand Up @@ -153,9 +155,7 @@ func Exploratory(_ *testing.T) []TestScenario {

func Regression(t *testing.T) []TestScenario {
v, err := e2e.GetVersionFromBinary(e2e.BinPath.Etcd)
if err != nil {
t.Fatalf("Failed checking etcd version binary, binary: %q, err: %v", e2e.BinPath.Etcd, err)
}
require.NoErrorf(t, err, "Failed checking etcd version binary, binary: %q", e2e.BinPath.Etcd)

scenarios := []TestScenario{}
scenarios = append(scenarios, TestScenario{
Expand Down
21 changes: 6 additions & 15 deletions tests/robustness/traffic/traffic.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"testing"
"time"

"github.com/stretchr/testify/require"
"go.uber.org/zap"
"golang.org/x/time/rate"

Expand Down Expand Up @@ -62,15 +63,11 @@ func SimulateTraffic(ctx context.Context, t *testing.T, lg *zap.Logger, clus *e2
limiter := rate.NewLimiter(rate.Limit(profile.MaximalQPS), 200)

cc, err := client.NewRecordingClient(endpoints, ids, baseTime)
if err != nil {
t.Fatal(err)
}
require.NoError(t, err)
defer cc.Close()
// Ensure that first operation succeeds
_, err = cc.Put(ctx, "start", "true")
if err != nil {
t.Fatalf("First operation failed, validation requires first operation to succeed, err: %s", err)
}
require.NoErrorf(t, err, "First operation failed, validation requires first operation to succeed")
wg := sync.WaitGroup{}
nonUniqueWriteLimiter := NewConcurrencyLimiter(profile.MaxNonUniqueRequestConcurrency)
finish := make(chan struct{})
Expand All @@ -79,9 +76,7 @@ func SimulateTraffic(ctx context.Context, t *testing.T, lg *zap.Logger, clus *e2
for i := 0; i < profile.ClientCount; i++ {
wg.Add(1)
c, nerr := client.NewRecordingClient([]string{endpoints[i%len(endpoints)]}, ids, baseTime)
if nerr != nil {
t.Fatal(nerr)
}
require.NoError(t, nerr)
go func(c *client.RecordingClient) {
defer wg.Done()
defer c.Close()
Expand Down Expand Up @@ -111,9 +106,7 @@ func SimulateTraffic(ctx context.Context, t *testing.T, lg *zap.Logger, clus *e2
var fr *report.FailpointInjection
select {
case frp, ok := <-failpointInjected:
if !ok {
t.Fatalf("Failed to collect failpoint report")
}
require.Truef(t, ok, "Failed to collect failpoint report")
fr = &frp
case <-ctx.Done():
t.Fatalf("Traffic finished before failure was injected: %s", ctx.Err())
Expand All @@ -126,9 +119,7 @@ func SimulateTraffic(ctx context.Context, t *testing.T, lg *zap.Logger, clus *e2
time.Sleep(time.Second)
// Ensure that last operation succeeds
_, err = cc.Put(ctx, "tombstone", "true")
if err != nil {
t.Fatalf("Last operation failed, validation requires last operation to succeed, err: %s", err)
}
require.NoErrorf(t, err, "Last operation failed, validation requires last operation to succeed")
reports = append(reports, cc.Report())

totalStats := calculateStats(reports, startTime, endTime)
Expand Down
5 changes: 2 additions & 3 deletions tests/robustness/validate/validate.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"time"

"github.com/anishathalye/porcupine"
"github.com/stretchr/testify/require"
"go.uber.org/zap"

"go.etcd.io/etcd/tests/v3/robustness/model"
Expand All @@ -30,9 +31,7 @@ import (
// ValidateAndReturnVisualize returns visualize as porcupine.linearizationInfo used to generate visualization is private.
func ValidateAndReturnVisualize(t *testing.T, lg *zap.Logger, cfg Config, reports []report.ClientReport, persistedRequests []model.EtcdRequest, timeout time.Duration) (visualize func(basepath string) error) {
err := checkValidationAssumptions(reports, persistedRequests)
if err != nil {
t.Fatalf("Broken validation assumptions: %s", err)
}
require.NoErrorf(t, err, "Broken validation assumptions")
linearizableOperations := patchLinearizableOperations(reports, persistedRequests)
serializableOperations := filterSerializableOperations(reports)

Expand Down

0 comments on commit c2bfbe8

Please sign in to comment.