From 892683bef86cacc2ccda2b4df2b079171bd92164 Mon Sep 17 00:00:00 2001 From: Ondrej Fabry Date: Wed, 22 Aug 2018 06:07:04 +0200 Subject: Show VPPApiError value always and remove RegisterBinAPITypes for mock adapter Change-Id: I3b216748df1a372f25cc94e3df5d7b1b2b7a8a40 Signed-off-by: Ondrej Fabry --- adapter/mock/mock_adapter.go | 50 +++++++++++++++------------------ api/vppapi_errors.go | 14 +++++++-- api/vppapi_errors_test.go | 23 +++++++++++++++ cmd/binapi-generator/generate.go | 2 ++ cmd/binapi-generator/generate_test.go | 53 +++++++++++++++++------------------ core/channel.go | 5 ++-- core/channel_test.go | 2 +- 7 files changed, 88 insertions(+), 61 deletions(-) create mode 100644 api/vppapi_errors_test.go diff --git a/adapter/mock/mock_adapter.go b/adapter/mock/mock_adapter.go index 3f5686f..5ca190f 100644 --- 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 --- 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 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 --- 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 --- 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 --- 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 --- 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 -- cgit 1.2.3-korg