Show VPPApiError value always and remove RegisterBinAPITypes for mock adapter

Change-Id: I3b216748df1a372f25cc94e3df5d7b1b2b7a8a40
Signed-off-by: Ondrej Fabry <ofabry@cisco.com>

diff --git a/adapter/mock/mock_adapter.go b/adapter/mock/mock_adapter.go
index 3f5686f..5ca190f 100644 (file)
--- a/adapter/mock/mock_adapter.go
+++ b/adapter/mock/mock_adapter.go
@@ -90,8 +90,15 @@ const (
 )
 
 // NewVppAdapter returns a new mock adapter.
-func NewVppAdapter() adapter.VppAdapter {
-       return &VppAdapter{}
+func NewVppAdapter() *VppAdapter {
+       a := &VppAdapter{
+               msgIDsToName: make(map[uint16]string),
+               msgNameToIds: make(map[string]uint16),
+               msgIDSeq:     1000,
+               binAPITypes:  make(map[string]reflect.Type),
+       }
+       a.registerBinAPITypes()
+       return a
 }
 
 // Connect emulates connecting the process to VPP.
@@ -119,21 +126,16 @@ func (a *VppAdapter) GetMsgNameByID(msgID uint16) (string, bool) {
 
        a.access.Lock()
        defer a.access.Unlock()
-       a.initMaps()
        msgName, found := a.msgIDsToName[msgID]
 
        return msgName, found
 }
 
-// RegisterBinAPITypes registers binary API message types in the mock adapter.
-func (a *VppAdapter) RegisterBinAPITypes(binAPITypes map[string]reflect.Type) {
+func (a *VppAdapter) registerBinAPITypes() {
        a.access.Lock()
        defer a.access.Unlock()
-       a.initMaps()
-       for _, v := range binAPITypes {
-               if msg, ok := reflect.New(v).Interface().(api.Message); ok {
-                       a.binAPITypes[msg.GetMessageName()] = v
-               }
+       for _, msg := range api.GetAllMessages() {
+               a.binAPITypes[msg.GetMessageName()] = reflect.TypeOf(msg).Elem()
        }
 }
 
@@ -177,8 +179,17 @@ func (a *VppAdapter) ReplyBytes(request MessageDTO, reply api.Message) ([]byte,
        log.Println("ReplyBytes ", replyMsgID, " ", reply.GetMessageName(), " clientId: ", request.ClientID)
 
        buf := new(bytes.Buffer)
-       struc.Pack(buf, &codec.VppReplyHeader{VlMsgID: replyMsgID, Context: request.ClientID})
-       struc.Pack(buf, reply)
+       err = struc.Pack(buf, &codec.VppReplyHeader{
+               VlMsgID: replyMsgID,
+               Context: request.ClientID,
+       })
+       if err != nil {
+               return nil, err
+       }
+       err = struc.Pack(buf, reply)
+       if err != nil {
+               return nil, err
+       }
 
        return buf.Bytes(), nil
 }
@@ -198,7 +209,6 @@ func (a *VppAdapter) GetMsgID(msgName string, msgCrc string) (uint16, error) {
 
        a.access.Lock()
        defer a.access.Unlock()
-       a.initMaps()
 
        msgID, found := a.msgNameToIds[msgName]
        if found {
@@ -213,24 +223,10 @@ func (a *VppAdapter) GetMsgID(msgName string, msgCrc string) (uint16, error) {
        return msgID, nil
 }
 
-// initMaps initializes internal maps (if not already initialized).
-func (a *VppAdapter) initMaps() {
-       if a.msgIDsToName == nil {
-               a.msgIDsToName = map[uint16]string{}
-               a.msgNameToIds = map[string]uint16{}
-               a.msgIDSeq = 1000
-       }
-
-       if a.binAPITypes == nil {
-               a.binAPITypes = map[string]reflect.Type{}
-       }
-}
-
 // SendMsg emulates sending a binary-encoded message to VPP.
 func (a *VppAdapter) SendMsg(clientID uint32, data []byte) error {
        switch a.mode {
        case useReplyHandlers:
-               a.initMaps()
                for i := len(a.replyHandlers) - 1; i >= 0; i-- {
                        replyHandler := a.replyHandlers[i]
 

diff --git a/api/vppapi_errors.go b/api/vppapi_errors.go
index c921e14..18ab87f 100644 (file)
--- a/api/vppapi_errors.go
+++ b/api/vppapi_errors.go
@@ -5,16 +5,26 @@ import (
        "strconv"
 )
 
+// RetvalToVPPApiError returns error for retval value.
+// Retval 0 returns nil error.
+func RetvalToVPPApiError(retval int32) error {
+       if retval == 0 {
+               return nil
+       }
+       return VPPApiError(retval)
+}
+
 // VPPApiError represents VPP's vnet API error that is usually
 // returned as Retval field in replies from VPP binary API.
 type VPPApiError int32
 
 func (e VPPApiError) Error() string {
+       errid := int64(e)
        var errstr string
        if s, ok := vppApiErrors[e]; ok {
-               errstr = s
+               errstr = fmt.Sprintf("%s (%d)", s, errid)
        } else {
-               errstr = strconv.Itoa(int(e))
+               errstr = strconv.FormatInt(errid, 10)
        }
        return fmt.Sprintf("VPPApiError: %s", errstr)
 }

diff --git a/api/vppapi_errors_test.go b/api/vppapi_errors_test.go
new file mode 100644 (file)
index 0000000..78e1fbf
--- /dev/null
+++ b/api/vppapi_errors_test.go
@@ -0,0 +1,23 @@
+package api
+
+import (
+       "testing"
+
+       . "github.com/onsi/gomega"
+)
+
+func TestUnspecified(t *testing.T) {
+       RegisterTestingT(t)
+
+       var err error = VPPApiError(-1)
+       errstr := err.Error()
+       Expect(errstr).Should(BeEquivalentTo("VPPApiError: Unspecified Error (-1)"))
+}
+
+func TestUnknown(t *testing.T) {
+       RegisterTestingT(t)
+
+       var err error = VPPApiError(-999)
+       errstr := err.Error()
+       Expect(errstr).Should(BeEquivalentTo("VPPApiError: -999"))
+}

diff --git a/cmd/binapi-generator/generate.go b/cmd/binapi-generator/generate.go
index e29c84d..33ab614 100644 (file)
--- a/cmd/binapi-generator/generate.go
+++ b/cmd/binapi-generator/generate.go
@@ -181,6 +181,7 @@ func generateHeader(ctx *context, w io.Writer) {
        fmt.Fprintf(w, "\t%s\n", filepath.Base(ctx.inputFile))
        fmt.Fprintln(w)
        fmt.Fprintln(w, "It contains these VPP binary API objects:")
+
        var printObjNum = func(obj string, num int) {
                if num > 0 {
                        if num > 1 {
@@ -194,6 +195,7 @@ func generateHeader(ctx *context, w io.Writer) {
        printObjNum("enum", len(ctx.packageData.Enums))
        printObjNum("union", len(ctx.packageData.Unions))
        printObjNum("service", len(ctx.packageData.Services))
+
        fmt.Fprintln(w, "*/")
        fmt.Fprintf(w, "package %s\n", ctx.packageName)
        fmt.Fprintln(w)

diff --git a/cmd/binapi-generator/generate_test.go b/cmd/binapi-generator/generate_test.go
index c1181f0..4b06733 100644 (file)
--- a/cmd/binapi-generator/generate_test.go
+++ b/cmd/binapi-generator/generate_test.go
@@ -15,6 +15,8 @@
 package main
 
 import (
+       "bufio"
+       "bytes"
        "os"
        "testing"
 
@@ -25,7 +27,7 @@ func TestGetInputFiles(t *testing.T) {
        RegisterTestingT(t)
        result, err := getInputFiles("testdata")
        Expect(err).ShouldNot(HaveOccurred())
-       Expect(result).To(HaveLen(5))
+       Expect(result).To(HaveLen(3))
        for _, file := range result {
                Expect(file).To(BeAnExistingFile())
        }
@@ -45,10 +47,10 @@ func TestGenerateFromFile(t *testing.T) {
        defer os.RemoveAll(outDir)
        err := generateFromFile("testdata/acl.api.json", outDir)
        Expect(err).ShouldNot(HaveOccurred())
-       fileInfo, err := os.Stat(outDir + "/acl/acl.go")
+       fileInfo, err := os.Stat(outDir + "/acl/acl.ba.go")
        Expect(err).ShouldNot(HaveOccurred())
        Expect(fileInfo.IsDir()).To(BeFalse())
-       Expect(fileInfo.Name()).To(BeEquivalentTo("acl.go"))
+       Expect(fileInfo.Name()).To(BeEquivalentTo("acl.ba.go"))
 }
 
 func TestGenerateFromFileInputError(t *testing.T) {
@@ -56,7 +58,7 @@ func TestGenerateFromFileInputError(t *testing.T) {
        outDir := "test_output_directory"
        err := generateFromFile("testdata/nonexisting.json", outDir)
        Expect(err).Should(HaveOccurred())
-       Expect(err.Error()).To(ContainSubstring("reading data from file failed"))
+       Expect(err.Error()).To(ContainSubstring("invalid input file name"))
 }
 
 func TestGenerateFromFileReadJsonError(t *testing.T) {
@@ -64,7 +66,7 @@ func TestGenerateFromFileReadJsonError(t *testing.T) {
        outDir := "test_output_directory"
        err := generateFromFile("testdata/input-read-json-error.json", outDir)
        Expect(err).Should(HaveOccurred())
-       Expect(err.Error()).To(ContainSubstring("JSON unmarshall failed"))
+       Expect(err.Error()).To(ContainSubstring("invalid input file name"))
 }
 
 func TestGenerateFromFileGeneratePackageError(t *testing.T) {
@@ -87,7 +89,7 @@ func TestGetContext(t *testing.T) {
        result, err := getContext("testdata/af_packet.api.json", outDir)
        Expect(err).ShouldNot(HaveOccurred())
        Expect(result).ToNot(BeNil())
-       Expect(result.outputFile).To(BeEquivalentTo(outDir + "/af_packet/af_packet.go"))
+       Expect(result.outputFile).To(BeEquivalentTo(outDir + "/af_packet/af_packet.ba.go"))
 }
 
 func TestGetContextNoJsonFile(t *testing.T) {
@@ -102,12 +104,11 @@ func TestGetContextNoJsonFile(t *testing.T) {
 func TestGetContextInterfaceJson(t *testing.T) {
        RegisterTestingT(t)
        outDir := "test_output_directory"
-       result, err := getContext("testdata/interface.json", outDir)
+       result, err := getContext("testdata/ip.api.json", outDir)
        Expect(err).ShouldNot(HaveOccurred())
        Expect(result).ToNot(BeNil())
        Expect(result.outputFile)
-       Expect(result.outputFile).To(BeEquivalentTo(outDir + "/interfaces/interfaces.go"))
-
+       Expect(result.outputFile).To(BeEquivalentTo(outDir + "/ip/ip.ba.go"))
 }
 
 func TestReadJson(t *testing.T) {
@@ -129,7 +130,6 @@ func TestReadJsonError(t *testing.T) {
        Expect(result).To(BeNil())
 }
 
-/*
 func TestGeneratePackage(t *testing.T) {
        RegisterTestingT(t)
        // prepare context
@@ -140,9 +140,13 @@ func TestGeneratePackage(t *testing.T) {
        inputData, err := readFile("testdata/ip.api.json")
        Expect(err).ShouldNot(HaveOccurred())
        testCtx.inputBuff = bytes.NewBuffer(inputData)
-       inFile, _ := parseJSON(inputData)
+       jsonRoot, err := parseJSON(inputData)
+       Expect(err).ShouldNot(HaveOccurred())
+       testCtx.packageData, err = parsePackage(testCtx, jsonRoot)
+       Expect(err).ShouldNot(HaveOccurred())
        outDir := "test_output_directory"
-       outFile, _ := os.Create(outDir)
+       outFile, err := os.Create(outDir)
+       Expect(err).ShouldNot(HaveOccurred())
        defer os.RemoveAll(outDir)
 
        // prepare writer
@@ -152,7 +156,6 @@ func TestGeneratePackage(t *testing.T) {
        Expect(err).ShouldNot(HaveOccurred())
 }
 
-
 func TestGenerateMessageType(t *testing.T) {
        RegisterTestingT(t)
        // prepare context
@@ -163,31 +166,25 @@ func TestGenerateMessageType(t *testing.T) {
        inputData, err := readFile("testdata/ip.api.json")
        Expect(err).ShouldNot(HaveOccurred())
        testCtx.inputBuff = bytes.NewBuffer(inputData)
-       inFile, _ := parseJSON(inputData)
+       jsonRoot, err := parseJSON(inputData)
+       Expect(err).ShouldNot(HaveOccurred())
        outDir := "test_output_directory"
-       outFile, _ := os.Create(outDir)
+       outFile, err := os.Create(outDir)
+       Expect(err).ShouldNot(HaveOccurred())
+       testCtx.packageData, err = parsePackage(testCtx, jsonRoot)
+       Expect(err).ShouldNot(HaveOccurred())
        defer os.RemoveAll(outDir)
 
        // prepare writer
        writer := bufio.NewWriter(outFile)
 
-       types := inFile.Map("types")
-       testCtx.types = map[string]string{
-               "u32": "sw_if_index",
-               "u8":  "weight",
-       }
-       Expect(types.Len()).To(BeEquivalentTo(1))
-       for i := 0; i < types.Len(); i++ {
-               typ := types.At(i)
-               Expect(writer.Buffered()).To(BeZero())
-               err := generateMessage(testCtx, writer, typ, true)
-               Expect(err).ShouldNot(HaveOccurred())
+       for _, msg := range testCtx.packageData.Messages {
+               generateMessage(testCtx, writer, &msg)
                Expect(writer.Buffered()).ToNot(BeZero())
-
        }
 }
 
-func TestGenerateMessageName(t *testing.T) {
+/*func TestGenerateMessageName(t *testing.T) {
        RegisterTestingT(t)
        // prepare context
        testCtx := new(context)

diff --git a/core/channel.go b/core/channel.go
index 5f7763e..718f89c 100644 (file)
--- a/core/channel.go
+++ b/core/channel.go
@@ -261,9 +261,8 @@ func (ch *channel) processReply(reply *vppReply, expSeqNum uint16, msg api.Messa
        if strings.HasSuffix(msg.GetMessageName(), "_reply") {
                // TODO: use categories for messages to avoid checking message name
                if f := reflect.Indirect(reflect.ValueOf(msg)).FieldByName("Retval"); f.IsValid() {
-                       if retval := f.Int(); retval != 0 {
-                               err = api.VPPApiError(retval)
-                       }
+                       retval := int32(f.Int())
+                       err = api.RetvalToVPPApiError(retval)
                }
        }
 

diff --git a/core/channel_test.go b/core/channel_test.go
index 197dda4..4a9ab2b 100644 (file)
--- a/core/channel_test.go
+++ b/core/channel_test.go
@@ -37,7 +37,7 @@ func setupTest(t *testing.T) *testCtx {
        RegisterTestingT(t)
 
        ctx := &testCtx{
-               mockVpp: &mock.VppAdapter{},
+               mockVpp: mock.NewVppAdapter(),
        }
 
        var err error