Minor changes

Changes: - add log message when reader closes in socketclient - tweak colors in govpp diff command - code cleanup in codec package - enable debug profiling in tests - fix integration test README - detect VPP crashes in integration tests Signed-off-by: Ondrej Fabry <[email protected]>
FDio · Jun 27, 2024 · 222186e · 222186e
1 parent 5818b80
commit 222186e
Show file tree

Hide file tree

Showing 16 changed files with 362 additions and 318 deletions.
diff --git a/adapter/socketclient/socketclient.go b/adapter/socketclient/socketclient.go
@@ -268,7 +268,7 @@ func (c *Client) Connect() error {
 		return err
 	}
 
-	if err := c.open(); err != nil {
+	if err := c.open(c.clientName); err != nil {
 		_ = c.disconnect()
 		return err
 	}
@@ -353,12 +353,12 @@ const (
 	deleteMsgContext = byte(124)
 )
 
-func (c *Client) open() error {
+func (c *Client) open(clientName string) error {
 	var msgCodec = codec.DefaultCodec
 
 	// Request socket client create
 	req := &memclnt.SockclntCreate{
-		Name: c.clientName,
+		Name: clientName,
 	}
 	msg, err := msgCodec.EncodeMsg(req, sockCreateMsgId)
 	if err != nil {
@@ -540,6 +540,7 @@ func (c *Client) readerLoop() {
 		msg, err := c.readMsg(buf[:])
 		if err != nil {
 			if isClosedError(err) {
+				log.Debugf("reader closed: %v", err)
 				return
 			}
 			log.Debugf("readMsg error: %v", err)

diff --git a/binapigen/vppapi/input.go b/binapigen/vppapi/input.go
@@ -133,7 +133,7 @@ func cloneRepoLocally(repo string, commit string, branch string, depth int) (str
 	} else if err != nil {
 		return "", fmt.Errorf("failed to check if cache exists: %w", err)
 	}
-	logrus.Debugf("local repo dir: %q, fetching %q", cachePath, commit)
+	logrus.Debugf("using local repo dir: %q, fetching %q", cachePath, commit)
 
 	cmd := exec.Command("git", "fetch", "--tags", "origin")
 	cmd.Dir = cachePath

diff --git a/cmd/govpp/cmd_vppapi_diff.go b/cmd/govpp/cmd_vppapi_diff.go
@@ -22,8 +22,6 @@ import (
 	"github.com/olekukonko/tablewriter"
 	"github.com/sirupsen/logrus"
 	"github.com/spf13/cobra"
-
-	"go.fd.io/govpp/binapigen/vppapi"
 )
 
 // TODO:
@@ -103,7 +101,7 @@ func runVppApiDiffCmd(out io.Writer, opts VppApiDiffCmdOptions) error {
 		return err
 	}
 
-	vppAgainst, err := vppapi.ResolveVppInput(opts.Against)
+	vppAgainst, err := resolveVppInput(opts.Against)
 	if err != nil {
 		return fmt.Errorf("resolving --against failed: %w", err)
 	}

diff --git a/cmd/govpp/compare.go b/cmd/govpp/compare.go
@@ -115,7 +115,7 @@ func CompareSchemas(schema1, schema2 *vppapi.Schema) []Difference {
 		differences = append(differences, Difference{
 			Type: VersionDifference,
 			Description: color.Sprintf("Schema version is different: %s vs %s",
-				clrWhite.Sprint(schema1.Version), clrWhite.Sprint(schema2.Version)),
+				clrDiffVersion.Sprint(schema1.Version), clrDiffVersion.Sprint(schema2.Version)),
 			Value1: schema1.Version,
 			Value2: schema2.Version,
 		})
@@ -126,7 +126,7 @@ func CompareSchemas(schema1, schema2 *vppapi.Schema) []Difference {
 			Type: TotalFilesDifference,
 			Description: color.Sprintf("Total file count %s from %v to %v",
 				clrWhite.Sprint(numberChangeString(len(schema1.Files), len(schema2.Files))),
-				clrWhite.Sprint(len(schema1.Files)), clrWhite.Sprint(len(schema2.Files))),
+				clrDiffNumber.Sprint(len(schema1.Files)), clrDiffNumber.Sprint(len(schema2.Files))),
 			Value1: len(schema1.Files),
 			Value2: len(schema2.Files),
 		})
@@ -169,7 +169,7 @@ func compareSchemaFiles(files1 []vppapi.File, files2 []vppapi.File) []Difference
 		} else {
 			differences = append(differences, Difference{
 				Type:        FileRemovedDifference,
-				Description: color.Sprintf("File removed: %s", clrWhite.Sprint(fileName)),
+				Description: color.Sprintf("File removed: %s", clrDiffFile.Sprint(fileName)),
 				Value1:      file1,
 				Value2:      nil,
 			})
@@ -181,7 +181,7 @@ func compareSchemaFiles(files1 []vppapi.File, files2 []vppapi.File) []Difference
 		if _, ok := fileMap1[fileName]; !ok {
 			differences = append(differences, Difference{
 				Type:        FileAddedDifference,
-				Description: color.Sprintf("File added: %s", clrWhite.Sprint(fileName)),
+				Description: color.Sprintf("File added: %s", clrDiffFile.Sprint(fileName)),
 				Value1:      nil,
 				Value2:      file2,
 			})
@@ -228,7 +228,7 @@ func compareFiles(file1, file2 vppapi.File) []Difference {
 		differences = append(differences, Difference{
 			Type: FileVersionDifference,
 			Description: color.Sprintf("File version changed from %s to %s",
-				clrWhite.Sprint(fileVer1), clrWhite.Sprint(fileVer2)),
+				clrDiffVersion.Sprint(fileVer1), clrDiffVersion.Sprint(fileVer2)),
 			Value1: fileVer1,
 			Value2: fileVer2,
 		})
@@ -237,7 +237,7 @@ func compareFiles(file1, file2 vppapi.File) []Difference {
 		differences = append(differences, Difference{
 			Type: FileCrcDifference,
 			Description: color.Sprintf("File CRC changed from %s to %s",
-				clrWhite.Sprint(file1.CRC), clrWhite.Sprint(file2.CRC)),
+				clrDiffVersion.Sprint(file1.CRC), clrDiffVersion.Sprint(file2.CRC)),
 			Value1: file1.CRC,
 			Value2: file2.CRC,
 		})
@@ -289,7 +289,7 @@ func compareFiles(file1, file2 vppapi.File) []Difference {
 		} else {
 			differences = append(differences, Difference{
 				Type:        MessageRemovedDifference,
-				Description: color.Sprintf("Message removed: %s", clrCyan.Sprint(msgName)),
+				Description: color.Sprintf("Message removed: %s", clrDiffMessage.Sprint(msgName)),
 				Value1:      msg1,
 				Value2:      nil,
 			})
@@ -300,7 +300,7 @@ func compareFiles(file1, file2 vppapi.File) []Difference {
 		if _, ok := msgMap1[msgName]; !ok {
 			differences = append(differences, Difference{
 				Type:        MessageAddedDifference,
-				Description: color.Sprintf("Message added: %s", clrCyan.Sprint(msgName)),
+				Description: color.Sprintf("Message added: %s", clrDiffMessage.Sprint(msgName)),
 				Value1:      nil,
 				Value2:      msg,
 			})
@@ -324,15 +324,15 @@ func compareMessages(msg1 vppapi.Message, msg2 vppapi.Message) []Difference {
 		differences = append(differences, Difference{
 			Type: MessageCrcDifference,
 			Description: color.Sprintf("Message %s changed CRC from %s to %s",
-				clrCyan.Sprint(msg1.Name), clrWhite.Sprint(msg1.CRC), clrWhite.Sprint(msg2.CRC)),
+				clrDiffMessage.Sprint(msg1.Name), clrDiffVersion.Sprint(msg1.CRC), clrDiffVersion.Sprint(msg2.CRC)),
 			Value1: msg1.CRC,
 			Value2: msg2.CRC,
 		})
 	}
 
 	// Compare message comments
 	if msg1.Comment != msg2.Comment {
-		desc := color.Sprintf("Message %s comment ", clrCyan.Sprint(msg1.Name))
+		desc := color.Sprintf("Message %s comment ", clrDiffMessage.Sprint(msg1.Name))
 		if msg1.Comment == "" {
 			desc += "added"
 		} else if msg2.Comment == "" {
@@ -355,7 +355,7 @@ func compareMessages(msg1 vppapi.Message, msg2 vppapi.Message) []Difference {
 				differences = append(differences, Difference{
 					Type: MsgOptionChangedDifference,
 					Description: color.Sprintf("Message %s changed option %s from %q to %q",
-						clrCyan.Sprint(msg1.Name), clrWhite.Sprint(option), clrWhite.Sprint(val1), clrWhite.Sprint(val2)),
+						clrDiffMessage.Sprint(msg1.Name), clrDiffOption.Sprint(option), clrDiffOption.Sprint(val1), clrDiffOption.Sprint(val2)),
 					Value1: keyValString(option, val1),
 					Value2: keyValString(option, val2),
 				})
@@ -364,7 +364,7 @@ func compareMessages(msg1 vppapi.Message, msg2 vppapi.Message) []Difference {
 			differences = append(differences, Difference{
 				Type: MsgOptionRemovedDifference,
 				Description: color.Sprintf("Message %s removed option: %s",
-					clrCyan.Sprint(msg1.Name), clrWhite.Sprint(option)),
+					clrDiffMessage.Sprint(msg1.Name), clrDiffOption.Sprint(keyValString(option, val1))),
 				Value1: keyValString(option, val1),
 				Value2: "",
 			})
@@ -376,7 +376,7 @@ func compareMessages(msg1 vppapi.Message, msg2 vppapi.Message) []Difference {
 			differences = append(differences, Difference{
 				Type: MsgOptionAddedDifference,
 				Description: color.Sprintf("Message %s added option: %s",
-					clrCyan.Sprint(msg2.Name), clrWhite.Sprint(keyValString(option, val2))),
+					clrDiffMessage.Sprint(msg2.Name), clrDiffOption.Sprint(keyValString(option, val2))),
 				Value1: "",
 				Value2: keyValString(option, val2),
 			})
@@ -390,7 +390,7 @@ func numberOfContentChangedDifference(typ string, c1, c2 int) Difference {
 	return Difference{
 		Type: FileContentsChangedDifference,
 		Description: color.Sprintf("Number of %s has %s from %v to %v",
-			clrWhite.Sprint(typ), clrWhite.Sprint(numberChangeString(c1, c2)), clrWhite.Sprint(c1), clrWhite.Sprint(c2)),
+			clrWhite.Sprint(typ), clrWhite.Sprint(numberChangeString(c1, c2)), clrDiffNumber.Sprint(c1), clrDiffNumber.Sprint(c2)),
 		Value1: c1,
 		Value2: c2,
 	}
@@ -411,5 +411,5 @@ func keyValString(k, v string) string {
 	if v == "" {
 		return k
 	}
-	return color.Sprintf("%s=%s", k, v)
+	return color.Sprintf("%s='%s'", k, v)
 }
diff --git a/cmd/govpp/input.go b/cmd/govpp/input.go
@@ -15,6 +15,7 @@
 package main
 
 import (
+	"fmt"
 	"os"
 	"path"
 	"path/filepath"
@@ -42,8 +43,13 @@ func resolveVppInput(input string) (*vppapi.VppInput, error) {
 
 	tookSec := time.Since(t).Seconds()
 
-	logrus.Tracef("resolved VPP input %q in %.3fs\n%s\n - API dir: %s\n - VPP Version: %s\n - Files: %v",
-		input, tookSec, strings.Repeat("-", 100), vppInput.ApiDirectory, vppInput.Schema.Version, len(vppInput.Schema.Files))
+	logrus.WithFields(map[string]interface{}{
+		"took":    fmt.Sprintf("%.3fs", tookSec),
+		"version": vppInput.Schema.Version,
+		"files":   len(vppInput.Schema.Files),
+		"apiDir":  len(vppInput.Schema.Files),
+	}).Tracef("resolved VPP input %q\n%s\n - API dir: %s\n - VPP Version: %s\n - Files: %v",
+		input, strings.Repeat("-", 100), vppInput.ApiDirectory, vppInput.Schema.Version, len(vppInput.Schema.Files))
 
 	return vppInput, nil
 }

diff --git a/cmd/govpp/util.go b/cmd/govpp/util.go
@@ -25,9 +25,12 @@ import (
 )
 
 var (
-	clrWhite    = color.Style{color.White}
-	clrCyan     = color.Style{color.Cyan}
-	clrDiffFile = color.Style{color.Yellow}
+	clrWhite       = color.Style{color.White}
+	clrDiffMessage = color.Style{color.Cyan}
+	clrDiffOption  = color.Style{color.Blue}
+	clrDiffFile    = color.Style{color.Yellow}
+	clrDiffVersion = color.Style{color.LightMagenta}
+	clrDiffNumber  = color.Style{color.LightBlue}
 )
 
 const (