Support mocking reply for more multi requests at once

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

diff --git a/adapter/mock/mock_adapter.go b/adapter/mock/mock_adapter.go
index 0b2e8d5..dab51a6 100644 (file)
--- a/adapter/mock/mock_adapter.go
+++ b/adapter/mock/mock_adapter.go
@@ -30,10 +30,10 @@ import (
        "github.com/lunixbochs/struc"
 )
 
-type ReplyMode int
+type replyMode int
 
 const (
-       _                ReplyMode = 0
+       _                replyMode = 0
        useRepliesQueue            = 1 // use replies in the queue
        useReplyHandlers           = 2 // use reply handler
 )
@@ -51,7 +51,7 @@ type VppAdapter struct {
        replies       []api.Message  // FIFO queue of messages
        replyHandlers []ReplyHandler // callbacks that are able to calculate mock responses
        repliesLock   sync.Mutex     // mutex for the queue
-       mode          ReplyMode      // mode in which the mock operates
+       mode          replyMode      // mode in which the mock operates
 }
 
 // defaultReply is a default reply message that mock adapter returns for a request.
@@ -250,8 +250,7 @@ func (a *VppAdapter) SendMsg(clientID uint32, data []byte) error {
                for i, reply := range a.replies {
                        if i > 0 && reply.GetMessageName() == "control_ping_reply" {
                                // hack - do not send control_ping_reply immediately, leave it for the the next callback
-                               a.replies = []api.Message{}
-                               a.replies = append(a.replies, reply)
+                               a.replies = a.replies[i:]
                                return nil
                        }
                        msgID, _ := a.GetMsgID(reply.GetMessageName(), reply.GetCrcString())

diff --git a/api/api.go b/api/api.go
index c247417..98932fa 100644 (file)
--- a/api/api.go
+++ b/api/api.go
@@ -233,15 +233,15 @@ func (ch *Channel) receiveReplyInternal(msg Message) (LastReplyReceived bool, er
                        return false, err
                }
                if vppReply.MessageID != expMsgID {
-                       err = fmt.Errorf("invalid message ID %d, expected %d "+
-                               "(also check if multiple goroutines are not sharing one GoVPP channel)", vppReply.MessageID, expMsgID)
+                       err = fmt.Errorf("received invalid message ID, expected %d (%s), but got %d (check if multiple goroutines are not sharing single GoVPP channel)",
+                               expMsgID, msg.GetMessageName(), vppReply.MessageID)
                        return false, err
                }
                // decode the message
                err = ch.MsgDecoder.DecodeMsg(vppReply.Data, msg)
 
        case <-time.After(ch.replyTimeout):
-               err = fmt.Errorf("no reply received within the timeout period %ds", ch.replyTimeout/time.Second)
+               err = fmt.Errorf("no reply received within the timeout period %s", ch.replyTimeout)
        }
        return
 }

diff --git a/api/api_test.go b/api/api_test.go
index 3e77f48..9af6e71 100644 (file)
--- a/api/api_test.go
+++ b/api/api_test.go
@@ -49,6 +49,8 @@ func setupTest(t *testing.T) *testCtx {
        ctx.ch, err = ctx.conn.NewAPIChannel()
        Expect(err).ShouldNot(HaveOccurred())
 
+       ctx.ch.SetReplyTimeout(time.Millisecond)
+
        return ctx
 }
 
@@ -197,10 +199,9 @@ func TestMultiRequestReplySwInterfaceTapDump(t *testing.T) {
 
        // mock reply
        for i := 1; i <= 10; i++ {
-               byteName := []byte("dev-name-test")
                ctx.mockVpp.MockReply(&tap.SwInterfaceTapDetails{
                        SwIfIndex: uint32(i),
-                       DevName:   byteName,
+                       DevName:   []byte("dev-name-test"),
                })
        }
        ctx.mockVpp.MockReply(&vpe.ControlPingReply{})
@@ -327,8 +328,6 @@ func TestSetReplyTimeout(t *testing.T) {
        ctx := setupTest(t)
        defer ctx.teardownTest()
 
-       ctx.ch.SetReplyTimeout(time.Millisecond)
-
        // first one request should work
        ctx.mockVpp.MockReply(&vpe.ControlPingReply{})
        err := ctx.ch.SendRequest(&vpe.ControlPing{}).ReceiveReply(&vpe.ControlPingReply{})
@@ -340,6 +339,47 @@ func TestSetReplyTimeout(t *testing.T) {
        Expect(err.Error()).To(ContainSubstring("timeout"))
 }
 
+func TestSetReplyTimeoutMultiRequest(t *testing.T) {
+       ctx := setupTest(t)
+       defer ctx.teardownTest()
+
+       for i := 1; i <= 3; i++ {
+               ctx.mockVpp.MockReply(&interfaces.SwInterfaceDetails{
+                       SwIfIndex:     uint32(i),
+                       InterfaceName: []byte("if-name-test"),
+               })
+       }
+       ctx.mockVpp.MockReply(&vpe.ControlPingReply{})
+
+       cnt := 0
+       sendMultiRequest := func() error {
+               reqCtx := ctx.ch.SendMultiRequest(&interfaces.SwInterfaceDump{})
+               for {
+                       msg := &interfaces.SwInterfaceDetails{}
+                       stop, err := reqCtx.ReceiveReply(msg)
+                       if stop {
+                               break // break out of the loop
+                       }
+                       if err != nil {
+                               return err
+                       }
+                       cnt++
+               }
+               return nil
+       }
+
+       // first one request should work
+       err := sendMultiRequest()
+       Expect(err).ShouldNot(HaveOccurred())
+
+       // no other reply ready - expect timeout
+       err = sendMultiRequest()
+       Expect(err).Should(HaveOccurred())
+       Expect(err.Error()).To(ContainSubstring("timeout"))
+
+       Expect(cnt).To(BeEquivalentTo(3))
+}
+
 func TestReceiveReplyNegative(t *testing.T) {
        ctx := setupTest(t)
        defer ctx.teardownTest()
@@ -362,3 +402,49 @@ func TestReceiveReplyNegative(t *testing.T) {
        Expect(err).Should(HaveOccurred())
        Expect(err.Error()).To(ContainSubstring("invalid request context"))
 }
+
+func TestMultiRequestDouble(t *testing.T) {
+       ctx := setupTest(t)
+       defer ctx.teardownTest()
+
+       // mock reply
+       for i := 1; i <= 3; i++ {
+               ctx.mockVpp.MockReply(&interfaces.SwInterfaceDetails{
+                       SwIfIndex:     uint32(i),
+                       InterfaceName: []byte("if-name-test"),
+               })
+       }
+       ctx.mockVpp.MockReply(&vpe.ControlPingReply{})
+       for i := 1; i <= 3; i++ {
+               ctx.mockVpp.MockReply(&interfaces.SwInterfaceDetails{
+                       SwIfIndex:     uint32(i),
+                       InterfaceName: []byte("if-name-test"),
+               })
+       }
+       ctx.mockVpp.MockReply(&vpe.ControlPingReply{})
+
+       cnt := 0
+       sendMultiRequest := func() error {
+               reqCtx := ctx.ch.SendMultiRequest(&interfaces.SwInterfaceDump{})
+               for {
+                       msg := &interfaces.SwInterfaceDetails{}
+                       stop, err := reqCtx.ReceiveReply(msg)
+                       if stop {
+                               break // break out of the loop
+                       }
+                       if err != nil {
+                               return err
+                       }
+                       cnt++
+               }
+               return nil
+       }
+
+       err := sendMultiRequest()
+       Expect(err).ShouldNot(HaveOccurred())
+
+       err = sendMultiRequest()
+       Expect(err).ShouldNot(HaveOccurred())
+
+       Expect(cnt).To(BeEquivalentTo(6))
+}

diff --git a/core/msg_codec.go b/core/msg_codec.go
index 77fb9a9..e32916b 100644 (file)
--- a/core/msg_codec.go
+++ b/core/msg_codec.go
@@ -20,8 +20,8 @@ import (
        "fmt"
        "reflect"
 
-       logger "github.com/sirupsen/logrus"
        "github.com/lunixbochs/struc"
+       logger "github.com/sirupsen/logrus"
 
        "git.fd.io/govpp.git/api"
 )

diff --git a/core/request_handler.go b/core/request_handler.go
index dc02ee7..4a62754 100644 (file)
--- a/core/request_handler.go
+++ b/core/request_handler.go
@@ -48,34 +48,34 @@ func (c *Connection) watchRequests(ch *api.Channel, chMeta *channelMetadata) {
 func (c *Connection) processRequest(ch *api.Channel, chMeta *channelMetadata, req *api.VppRequest) error {
        // check whether we are connected to VPP
        if atomic.LoadUint32(&c.connected) == 0 {
-               error := errors.New("not connected to VPP, ignoring the request")
-               log.Error(error)
-               sendReply(ch, &api.VppReply{Error: error})
-               return error
+               err := errors.New("not connected to VPP, ignoring the request")
+               log.Error(err)
+               sendReply(ch, &api.VppReply{Error: err})
+               return err
        }
 
        // retrieve message ID
        msgID, err := c.GetMessageID(req.Message)
        if err != nil {
-               error := fmt.Errorf("unable to retrieve message ID: %v", err)
+               err = fmt.Errorf("unable to retrieve message ID: %v", err)
                log.WithFields(logger.Fields{
                        "msg_name": req.Message.GetMessageName(),
                        "msg_crc":  req.Message.GetCrcString(),
-               }).Error(error)
-               sendReply(ch, &api.VppReply{Error: error})
-               return error
+               }).Error(err)
+               sendReply(ch, &api.VppReply{Error: err})
+               return err
        }
 
        // encode the message into binary
        data, err := c.codec.EncodeMsg(req.Message, msgID)
        if err != nil {
-               error := fmt.Errorf("unable to encode the messge: %v", err)
+               err = fmt.Errorf("unable to encode the messge: %v", err)
                log.WithFields(logger.Fields{
                        "context": chMeta.id,
                        "msg_id":  msgID,
-               }).Error(error)
-               sendReply(ch, &api.VppReply{Error: error})
-               return error
+               }).Error(err)
+               sendReply(ch, &api.VppReply{Error: err})
+               return err
        }
 
        if log.Level == logger.DebugLevel { // for performance reasons - logrus does some processing even if debugs are disabled
@@ -83,6 +83,7 @@ func (c *Connection) processRequest(ch *api.Channel, chMeta *channelMetadata, re
                        "context":  chMeta.id,
                        "msg_id":   msgID,
                        "msg_size": len(data),
+                       "msg_name": req.Message.GetMessageName(),
                }).Debug("Sending a message to VPP.")
        }
 
@@ -199,12 +200,12 @@ func (c *Connection) messageNameToID(msgName string, msgCrc string) (uint16, err
        // get the ID using VPP API
        id, err := c.vpp.GetMsgID(msgName, msgCrc)
        if err != nil {
-               error := fmt.Errorf("unable to retrieve message ID: %v", err)
+               err = fmt.Errorf("unable to retrieve message ID: %v", err)
                log.WithFields(logger.Fields{
                        "msg_name": msgName,
                        "msg_crc":  msgCrc,
-               }).Error(error)
-               return id, error
+               }).Error(err)
+               return id, err
        }
 
        c.msgIDsLock.Lock()