Merge pull request #19370 from shashwat010/visualize

Remove passing of anonymous visualize function

Author: serathius

bill-of-materials.json

@@ -13,7 +13,7 @@
 		"licenses": [
 			{
 				"type": "MIT License",
-				"confidence": 1
+				"confidence": 0.96875
 			}
 		]
 	},

tests/go.mod

@@ -16,7 +16,7 @@ replace (
 )
 
 require (
-	github.com/anishathalye/porcupine v0.1.4
+	github.com/anishathalye/porcupine v1.0.0
 	github.com/coreos/go-semver v0.3.1
 	github.com/golang/protobuf v1.5.4
 	github.com/google/go-cmp v0.6.0

tests/go.sum

@@ -2,8 +2,8 @@ cloud.google.com/go v0.26.0/go.mod h1:aQUYkXzVsufM+DwF1aE+0xfcU+56JwCaLick0ClmMT
 github.com/BurntSushi/toml v0.3.1/go.mod h1:xHWCNGjB5oqiDr8zfno3MHue2Ht5sIBksp03qcyfWMU=
 github.com/VividCortex/ewma v1.2.0 h1:f58SaIzcDXrSy3kWaHNvuJgJ3Nmz59Zji6XoJR/q1ow=
 github.com/VividCortex/ewma v1.2.0/go.mod h1:nz4BbCtbLyFDeC9SUHbtcT5644juEuWfUAUnGx7j5l4=
-github.com/anishathalye/porcupine v0.1.4 h1:rRekB2jH1mbtLPEzuqyMHp4scU52Bcc1jgkPi1kWFQA=
-github.com/anishathalye/porcupine v0.1.4/go.mod h1:/X9OQYnVb7DzfKCQVO4tI1Aq+o56UJW+RvN/5U4EuZA=
+github.com/anishathalye/porcupine v1.0.0 h1:93eF6d26IMDky+G4h8FcLuYp1oO+no8a//I7asq/oKI=
+github.com/anishathalye/porcupine v1.0.0/go.mod h1:WM0SsFjWNl2Y4BqHr/E/ll2yY1GY1jqn+W7Z/84Zoog=
 github.com/beorn7/perks v1.0.1 h1:VlbKKnNfV8bJzeqoa4cOKqO6bYr3WgKZxO8Z16+hsOM=
 github.com/beorn7/perks v1.0.1/go.mod h1:G2ZrVWU2WbWT9wwq4/hrbKbnv/1ERSJQ0ibhJ6rlkpw=
 github.com/bgentry/speakeasy v0.2.0 h1:tgObeVOf8WAvtuAX6DhJ4xks4CFNwPDZiqzGqIHE51E=

tests/robustness/main_test.go

@@ -102,7 +102,7 @@ func testRobustness(ctx context.Context, t *testing.T, lg *zap.Logger, s scenari
 		client.ValidateGotAtLeastOneProgressNotify(t, r.Client, s.Watch.RequestProgress || watchProgressNotifyEnabled)
 	}
 	validateConfig := validate.Config{ExpectRevisionUnique: s.Traffic.ExpectUniqueRevision()}
-	r.Visualize = validate.ValidateAndReturnVisualize(t, lg, validateConfig, r.Client, persistedRequests, 5*time.Minute)
+	r.Visualize = validate.ValidateAndReturnVisualize(t, lg, validateConfig, r.Client, persistedRequests, 5*time.Minute).Visualize
 
 	panicked = false
 }

tests/robustness/validate/operations.go

@@ -32,10 +32,31 @@ var (
 	errFutureRevRespRequested = errors.New("request about a future rev with response")
 )
 
-func validateLinearizableOperationsAndVisualize(lg *zap.Logger, operations []porcupine.Operation, timeout time.Duration) (result porcupine.CheckResult, visualize func(basepath string) error) {
+type Results struct {
+	Info         porcupine.LinearizationInfo
+	Model        porcupine.Model
+	Linearizable porcupine.CheckResult
+	Lg           *zap.Logger // TODO: Remove logger from struct and instead of making it an argument for Visualize
+}
+
+func (r Results) Visualize(path string) error {
+	r.Lg.Info("Saving visualization", zap.String("path", path))
+	err := porcupine.VisualizePath(r.Model, r.Info, path)
+	if err != nil {
+		return fmt.Errorf("failed to visualize, err: %w", err)
+	}
+	return nil
+}
+
+func validateLinearizableOperationsAndVisualize(
+	lg *zap.Logger,
+	operations []porcupine.Operation,
+	timeout time.Duration,
+) (results Results) {
 	lg.Info("Validating linearizable operations", zap.Duration("timeout", timeout))
 	start := time.Now()
 	result, info := porcupine.CheckOperationsVerbose(model.NonDeterministicModel, operations, timeout)
+
 	switch result {
 	case porcupine.Illegal:
 		lg.Error("Linearization failed", zap.Duration("duration", time.Since(start)))
@@ -46,13 +67,11 @@ func validateLinearizableOperationsAndVisualize(lg *zap.Logger, operations []por
 	default:
 		panic(fmt.Sprintf("Unknown Linearization result %s", result))
 	}
-	return result, func(path string) error {
-		lg.Info("Saving visualization", zap.String("path", path))
-		err := porcupine.VisualizePath(model.NonDeterministicModel, info, path)
-		if err != nil {
-			return fmt.Errorf("failed to visualize, err: %w", err)
-		}
-		return nil
+	return Results{
+		Info:         info,
+		Model:        model.NonDeterministicModel,
+		Linearizable: result,
+		Lg:           lg,
 	}
 }
 

tests/robustness/validate/validate.go

@@ -28,18 +28,18 @@ import (
 	"go.etcd.io/etcd/tests/v3/robustness/report"
 )
 
-// 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) {
+func ValidateAndReturnVisualize(t *testing.T, lg *zap.Logger, cfg Config, reports []report.ClientReport, persistedRequests []model.EtcdRequest, timeout time.Duration) Results {
 	err := checkValidationAssumptions(reports, persistedRequests)
 	require.NoErrorf(t, err, "Broken validation assumptions")
 	linearizableOperations := patchLinearizableOperations(reports, persistedRequests)
 	serializableOperations := filterSerializableOperations(reports)
 
-	linearizable, visualize := validateLinearizableOperationsAndVisualize(lg, linearizableOperations, timeout)
-	if linearizable != porcupine.Ok {
+	results := validateLinearizableOperationsAndVisualize(lg, linearizableOperations, timeout)
+	if results.Linearizable != porcupine.Ok {
 		t.Error("Failed linearization, skipping further validation")
-		return visualize
+		return results
 	}
+
 	// TODO: Use requests from linearization for replay.
 	replay := model.NewReplay(persistedRequests)
 
@@ -51,7 +51,7 @@ func ValidateAndReturnVisualize(t *testing.T, lg *zap.Logger, cfg Config, report
 	if err != nil {
 		t.Errorf("Failed validating serializable operations, err: %s", err)
 	}
-	return visualize
+	return results
 }
 
 type Config struct {

tests/robustness/validate/validate_test.go

@@ -45,7 +45,7 @@ func TestDataReports(t *testing.T) {
 
 			persistedRequests, err := report.LoadClusterPersistedRequests(lg, path)
 			require.NoError(t, err)
-			visualize := ValidateAndReturnVisualize(t, zaptest.NewLogger(t), Config{}, reports, persistedRequests, 5*time.Minute)
+			visualize := ValidateAndReturnVisualize(t, zaptest.NewLogger(t), Config{}, reports, persistedRequests, 5*time.Minute).Visualize
 
 			err = visualize(filepath.Join(path, "history.html"))
 			require.NoError(t, err)