[VOL-2370] - Fix the add-delete-add flow sequence

This commit fixes the issue where a flow is first added
and then deleted and readded again.

Change-Id: I1ec1d931037c3086c65299195c87875f6cb96717
diff --git a/rw_core/core/device_agent.go b/rw_core/core/device_agent.go
index 06ba307..ca3ffbb 100755
--- a/rw_core/core/device_agent.go
+++ b/rw_core/core/device_agent.go
@@ -251,6 +251,65 @@
 	response.Done()
 }
 
+//deleteFlowWithoutPreservingOrder removes a flow specified by index from the flows slice.  This function will
+//panic if the index is out of range.
+func deleteFlowWithoutPreservingOrder(flows []*ofp.OfpFlowStats, index int) []*ofp.OfpFlowStats {
+	flows[index] = flows[len(flows)-1]
+	flows[len(flows)-1] = nil
+	return flows[:len(flows)-1]
+}
+
+//deleteGroupWithoutPreservingOrder removes a group specified by index from the groups slice.  This function will
+//panic if the index is out of range.
+func deleteGroupWithoutPreservingOrder(groups []*ofp.OfpGroupEntry, index int) []*ofp.OfpGroupEntry {
+	groups[index] = groups[len(groups)-1]
+	groups[len(groups)-1] = nil
+	return groups[:len(groups)-1]
+}
+
+func flowsToUpdateToDelete(newFlows, existingFlows []*ofp.OfpFlowStats) (updatedNewFlows, flowsToDelete, updatedAllFlows []*ofp.OfpFlowStats) {
+	// Process flows
+	for _, flow := range existingFlows {
+		if idx := fu.FindFlows(newFlows, flow); idx == -1 {
+			updatedAllFlows = append(updatedAllFlows, flow)
+		} else {
+			// We have a matching flow (i.e. the following field matches: "TableId", "Priority", "Flags", "Cookie",
+			// "Match".  If this is an exact match (i.e. all other fields matches as well) then this flow will be
+			// ignored.  Otherwise, the previous flow will be deleted and the new one added
+			if proto.Equal(newFlows[idx], flow) {
+				// Flow already exist, remove it from the new flows but keep it in the updated flows slice
+				newFlows = deleteFlowWithoutPreservingOrder(newFlows, idx)
+				updatedAllFlows = append(updatedAllFlows, flow)
+			} else {
+				// Minor change to flow, delete old and add new one
+				flowsToDelete = append(flowsToDelete, flow)
+			}
+		}
+	}
+	updatedAllFlows = append(updatedAllFlows, newFlows...)
+	return newFlows, flowsToDelete, updatedAllFlows
+}
+
+func groupsToUpdateToDelete(newGroups, existingGroups []*ofp.OfpGroupEntry) (updatedNewGroups, groupsToDelete, updatedAllGroups []*ofp.OfpGroupEntry) {
+	for _, group := range existingGroups {
+		if idx := fu.FindGroup(newGroups, group.Desc.GroupId); idx == -1 { // does not exist now
+			updatedAllGroups = append(updatedAllGroups, group)
+		} else {
+			// Follow same logic as flows
+			if proto.Equal(newGroups[idx], group) {
+				// Group already exist, remove it from the new groups
+				newGroups = deleteGroupWithoutPreservingOrder(newGroups, idx)
+				updatedAllGroups = append(updatedAllGroups, group)
+			} else {
+				// Minor change to group, delete old and add new one
+				groupsToDelete = append(groupsToDelete, group)
+			}
+		}
+	}
+	updatedAllGroups = append(updatedAllGroups, newGroups...)
+	return newGroups, groupsToDelete, updatedAllGroups
+}
+
 func (agent *DeviceAgent) addFlowsAndGroupsToAdapter(newFlows []*ofp.OfpFlowStats, newGroups []*ofp.OfpGroupEntry, flowMetadata *voltha.FlowMetadata) (coreutils.Response, error) {
 	log.Debugw("addFlowsAndGroups", log.Fields{"deviceId": agent.deviceID, "flows": newFlows, "groups": newGroups, "flowMetadata": flowMetadata})
 
@@ -266,33 +325,14 @@
 	existingFlows := proto.Clone(device.Flows).(*voltha.Flows)
 	existingGroups := proto.Clone(device.FlowGroups).(*ofp.FlowGroups)
 
-	var updatedFlows []*ofp.OfpFlowStats
-	var flowsToDelete []*ofp.OfpFlowStats
-	var groupsToDelete []*ofp.OfpGroupEntry
-	var updatedGroups []*ofp.OfpGroupEntry
-
 	// Process flows
-	updatedFlows = append(updatedFlows, newFlows...)
-	for _, flow := range existingFlows.Items {
-		if idx := fu.FindFlows(newFlows, flow); idx == -1 {
-			updatedFlows = append(updatedFlows, flow)
-		} else {
-			flowsToDelete = append(flowsToDelete, flow)
-		}
-	}
+	newFlows, flowsToDelete, updatedAllFlows := flowsToUpdateToDelete(newFlows, existingFlows.Items)
 
 	// Process groups
-	updatedGroups = append(updatedGroups, newGroups...)
-	for _, group := range existingGroups.Items {
-		if fu.FindGroup(newGroups, group.Desc.GroupId) == -1 { // does not exist now
-			updatedGroups = append(updatedGroups, group)
-		} else {
-			groupsToDelete = append(groupsToDelete, group)
-		}
-	}
+	newGroups, groupsToDelete, updatedAllGroups := groupsToUpdateToDelete(newGroups, existingGroups.Items)
 
 	// Sanity check
-	if (len(updatedFlows) | len(flowsToDelete) | len(updatedGroups) | len(groupsToDelete)) == 0 {
+	if (len(updatedAllFlows) | len(flowsToDelete) | len(updatedAllGroups) | len(groupsToDelete)) == 0 {
 		log.Debugw("nothing-to-update", log.Fields{"deviceId": agent.deviceID, "flows": newFlows, "groups": newGroups})
 		return coreutils.DoneResponse(), nil
 	}
@@ -306,11 +346,11 @@
 	dType := agent.adapterMgr.getDeviceType(device.Type)
 	if !dType.AcceptsAddRemoveFlowUpdates {
 
-		if len(updatedGroups) != 0 && reflect.DeepEqual(existingGroups.Items, updatedGroups) && len(updatedFlows) != 0 && reflect.DeepEqual(existingFlows.Items, updatedFlows) {
+		if len(updatedAllGroups) != 0 && reflect.DeepEqual(existingGroups.Items, updatedAllGroups) && len(updatedAllFlows) != 0 && reflect.DeepEqual(existingFlows.Items, updatedAllFlows) {
 			log.Debugw("nothing-to-update", log.Fields{"deviceId": agent.deviceID, "flows": newFlows, "groups": newGroups})
 			return coreutils.DoneResponse(), nil
 		}
-		go agent.sendBulkFlowsToAdapters(device, &voltha.Flows{Items: updatedFlows}, &voltha.FlowGroups{Items: updatedGroups}, flowMetadata, response)
+		go agent.sendBulkFlowsToAdapters(device, &voltha.Flows{Items: updatedAllFlows}, &voltha.FlowGroups{Items: updatedAllGroups}, flowMetadata, response)
 
 	} else {
 		flowChanges := &ofp.FlowChanges{
@@ -326,8 +366,8 @@
 	}
 
 	// store the changed data
-	device.Flows = &voltha.Flows{Items: updatedFlows}
-	device.FlowGroups = &voltha.FlowGroups{Items: updatedGroups}
+	device.Flows = &voltha.Flows{Items: updatedAllFlows}
+	device.FlowGroups = &voltha.FlowGroups{Items: updatedAllGroups}
 	if err := agent.updateDeviceWithoutLock(device); err != nil {
 		return coreutils.DoneResponse(), status.Errorf(codes.Internal, "failure-updating-%s", agent.deviceID)
 	}
diff --git a/rw_core/core/device_agent_test.go b/rw_core/core/device_agent_test.go
index 3f22002..8200ae7 100755
--- a/rw_core/core/device_agent_test.go
+++ b/rw_core/core/device_agent_test.go
@@ -23,10 +23,12 @@
 	"github.com/opencord/voltha-lib-go/v2/pkg/kafka"
 	"github.com/opencord/voltha-lib-go/v2/pkg/log"
 	lm "github.com/opencord/voltha-lib-go/v2/pkg/mocks"
+	ofp "github.com/opencord/voltha-protos/v2/go/openflow_13"
 	"github.com/opencord/voltha-protos/v2/go/voltha"
 	"github.com/phayes/freeport"
 	"github.com/stretchr/testify/assert"
 	"math/rand"
+	"sort"
 	"strings"
 	"sync"
 	"testing"
@@ -225,3 +227,232 @@
 
 	wg.Wait()
 }
+
+func isFlowSliceEqual(a, b []*ofp.OfpFlowStats) bool {
+	if len(a) != len(b) {
+		return false
+	}
+	sort.Slice(a, func(i, j int) bool {
+		return a[i].Id < a[j].Id
+	})
+	sort.Slice(b, func(i, j int) bool {
+		return b[i].Id < b[j].Id
+	})
+	for idx := range a {
+		if !proto.Equal(a[idx], b[idx]) {
+			return false
+		}
+	}
+	return true
+}
+
+func isGroupSliceEqual(a, b []*ofp.OfpGroupEntry) bool {
+	if len(a) != len(b) {
+		return false
+	}
+	sort.Slice(a, func(i, j int) bool {
+		return a[i].Desc.GroupId < a[j].Desc.GroupId
+	})
+	sort.Slice(b, func(i, j int) bool {
+		return b[i].Desc.GroupId < b[j].Desc.GroupId
+	})
+	for idx := range a {
+		if !proto.Equal(a[idx], b[idx]) {
+			return false
+		}
+	}
+	return true
+}
+
+func TestFlowsToUpdateToDelete_EmptySlices(t *testing.T) {
+	newFlows := []*ofp.OfpFlowStats{}
+	existingFlows := []*ofp.OfpFlowStats{}
+	expectedNewFlows := []*ofp.OfpFlowStats{}
+	expectedFlowsToDelete := []*ofp.OfpFlowStats{}
+	expectedUpdatedAllFlows := []*ofp.OfpFlowStats{}
+	uNF, fD, uAF := flowsToUpdateToDelete(newFlows, existingFlows)
+	assert.True(t, isFlowSliceEqual(uNF, expectedNewFlows))
+	assert.True(t, isFlowSliceEqual(fD, expectedFlowsToDelete))
+	assert.True(t, isFlowSliceEqual(uAF, expectedUpdatedAllFlows))
+}
+
+func TestFlowsToUpdateToDelete_NoExistingFlows(t *testing.T) {
+	newFlows := []*ofp.OfpFlowStats{
+		{Id: 123, TableId: 1230, Priority: 100, IdleTimeout: 0, Flags: 0, Cookie: 1230000, PacketCount: 0},
+		{Id: 124, TableId: 1240, Priority: 1000, IdleTimeout: 0, Flags: 0, Cookie: 1240000, PacketCount: 0},
+		{Id: 125, TableId: 1250, Priority: 1000, IdleTimeout: 0, Flags: 0, Cookie: 1250000, PacketCount: 0},
+	}
+	existingFlows := []*ofp.OfpFlowStats{}
+	expectedNewFlows := []*ofp.OfpFlowStats{
+		{Id: 123, TableId: 1230, Priority: 100, IdleTimeout: 0, Flags: 0, Cookie: 1230000, PacketCount: 0},
+		{Id: 124, TableId: 1240, Priority: 1000, IdleTimeout: 0, Flags: 0, Cookie: 1240000, PacketCount: 0},
+		{Id: 125, TableId: 1250, Priority: 1000, IdleTimeout: 0, Flags: 0, Cookie: 1250000, PacketCount: 0},
+	}
+	expectedFlowsToDelete := []*ofp.OfpFlowStats{}
+	expectedUpdatedAllFlows := []*ofp.OfpFlowStats{
+		{Id: 123, TableId: 1230, Priority: 100, IdleTimeout: 0, Flags: 0, Cookie: 1230000, PacketCount: 0},
+		{Id: 124, TableId: 1240, Priority: 1000, IdleTimeout: 0, Flags: 0, Cookie: 1240000, PacketCount: 0},
+		{Id: 125, TableId: 1250, Priority: 1000, IdleTimeout: 0, Flags: 0, Cookie: 1250000, PacketCount: 0},
+	}
+	uNF, fD, uAF := flowsToUpdateToDelete(newFlows, existingFlows)
+	assert.True(t, isFlowSliceEqual(uNF, expectedNewFlows))
+	assert.True(t, isFlowSliceEqual(fD, expectedFlowsToDelete))
+	assert.True(t, isFlowSliceEqual(uAF, expectedUpdatedAllFlows))
+}
+
+func TestFlowsToUpdateToDelete_UpdateNoDelete(t *testing.T) {
+	newFlows := []*ofp.OfpFlowStats{
+		{Id: 123, TableId: 1230, Priority: 100, IdleTimeout: 0, Flags: 0, Cookie: 1230000, PacketCount: 0},
+		{Id: 124, TableId: 1240, Priority: 1000, IdleTimeout: 0, Flags: 0, Cookie: 1240000, PacketCount: 0},
+		{Id: 125, TableId: 1250, Priority: 1000, IdleTimeout: 0, Flags: 0, Cookie: 1250000, PacketCount: 0},
+	}
+	existingFlows := []*ofp.OfpFlowStats{
+		{Id: 121, TableId: 1210, Priority: 100, IdleTimeout: 0, Flags: 0, Cookie: 1210000, PacketCount: 0},
+		{Id: 124, TableId: 1240, Priority: 1000, IdleTimeout: 0, Flags: 0, Cookie: 1240000, PacketCount: 0},
+		{Id: 122, TableId: 1220, Priority: 1000, IdleTimeout: 0, Flags: 0, Cookie: 1220000, PacketCount: 0},
+	}
+	expectedNewFlows := []*ofp.OfpFlowStats{
+		{Id: 123, TableId: 1230, Priority: 100, IdleTimeout: 0, Flags: 0, Cookie: 1230000, PacketCount: 0},
+		{Id: 125, TableId: 1250, Priority: 1000, IdleTimeout: 0, Flags: 0, Cookie: 1250000, PacketCount: 0},
+	}
+	expectedFlowsToDelete := []*ofp.OfpFlowStats{}
+	expectedUpdatedAllFlows := []*ofp.OfpFlowStats{
+		{Id: 123, TableId: 1230, Priority: 100, IdleTimeout: 0, Flags: 0, Cookie: 1230000, PacketCount: 0},
+		{Id: 124, TableId: 1240, Priority: 1000, IdleTimeout: 0, Flags: 0, Cookie: 1240000, PacketCount: 0},
+		{Id: 125, TableId: 1250, Priority: 1000, IdleTimeout: 0, Flags: 0, Cookie: 1250000, PacketCount: 0},
+		{Id: 121, TableId: 1210, Priority: 100, IdleTimeout: 0, Flags: 0, Cookie: 1210000, PacketCount: 0},
+		{Id: 122, TableId: 1220, Priority: 1000, IdleTimeout: 0, Flags: 0, Cookie: 1220000, PacketCount: 0},
+	}
+	uNF, fD, uAF := flowsToUpdateToDelete(newFlows, existingFlows)
+	assert.True(t, isFlowSliceEqual(uNF, expectedNewFlows))
+	assert.True(t, isFlowSliceEqual(fD, expectedFlowsToDelete))
+	assert.True(t, isFlowSliceEqual(uAF, expectedUpdatedAllFlows))
+}
+
+func TestFlowsToUpdateToDelete_UpdateAndDelete(t *testing.T) {
+	newFlows := []*ofp.OfpFlowStats{
+		{Id: 123, TableId: 1230, Priority: 100, IdleTimeout: 0, Flags: 0, Cookie: 1230000, PacketCount: 20},
+		{Id: 124, TableId: 1240, Priority: 1000, IdleTimeout: 0, Flags: 0, Cookie: 1240000, PacketCount: 0},
+		{Id: 125, TableId: 1250, Priority: 1000, IdleTimeout: 10, Flags: 0, Cookie: 1250000, PacketCount: 0},
+		{Id: 126, TableId: 1260, Priority: 1000, IdleTimeout: 0, Flags: 0, Cookie: 1260000, PacketCount: 0},
+		{Id: 127, TableId: 1270, Priority: 1000, IdleTimeout: 0, Flags: 0, Cookie: 1270000, PacketCount: 0},
+	}
+	existingFlows := []*ofp.OfpFlowStats{
+		{Id: 121, TableId: 1210, Priority: 100, IdleTimeout: 0, Flags: 0, Cookie: 1210000, PacketCount: 0},
+		{Id: 122, TableId: 1220, Priority: 1000, IdleTimeout: 0, Flags: 0, Cookie: 1220000, PacketCount: 0},
+		{Id: 123, TableId: 1230, Priority: 100, IdleTimeout: 0, Flags: 0, Cookie: 1230000, PacketCount: 0},
+		{Id: 124, TableId: 1240, Priority: 1000, IdleTimeout: 0, Flags: 0, Cookie: 1240000, PacketCount: 0},
+		{Id: 125, TableId: 1250, Priority: 1000, IdleTimeout: 0, Flags: 0, Cookie: 1250000, PacketCount: 0},
+	}
+	expectedNewFlows := []*ofp.OfpFlowStats{
+		{Id: 123, TableId: 1230, Priority: 100, IdleTimeout: 0, Flags: 0, Cookie: 1230000, PacketCount: 20},
+		{Id: 125, TableId: 1250, Priority: 1000, IdleTimeout: 10, Flags: 0, Cookie: 1250000, PacketCount: 0},
+		{Id: 126, TableId: 1260, Priority: 1000, IdleTimeout: 0, Flags: 0, Cookie: 1260000, PacketCount: 0},
+		{Id: 127, TableId: 1270, Priority: 1000, IdleTimeout: 0, Flags: 0, Cookie: 1270000, PacketCount: 0},
+	}
+	expectedFlowsToDelete := []*ofp.OfpFlowStats{
+		{Id: 123, TableId: 1230, Priority: 100, IdleTimeout: 0, Flags: 0, Cookie: 1230000, PacketCount: 0},
+		{Id: 125, TableId: 1250, Priority: 1000, IdleTimeout: 0, Flags: 0, Cookie: 1250000, PacketCount: 0},
+	}
+	expectedUpdatedAllFlows := []*ofp.OfpFlowStats{
+		{Id: 121, TableId: 1210, Priority: 100, IdleTimeout: 0, Flags: 0, Cookie: 1210000, PacketCount: 0},
+		{Id: 122, TableId: 1220, Priority: 1000, IdleTimeout: 0, Flags: 0, Cookie: 1220000, PacketCount: 0},
+		{Id: 123, TableId: 1230, Priority: 100, IdleTimeout: 0, Flags: 0, Cookie: 1230000, PacketCount: 20},
+		{Id: 124, TableId: 1240, Priority: 1000, IdleTimeout: 0, Flags: 0, Cookie: 1240000, PacketCount: 0},
+		{Id: 125, TableId: 1250, Priority: 1000, IdleTimeout: 10, Flags: 0, Cookie: 1250000, PacketCount: 0},
+		{Id: 126, TableId: 1260, Priority: 1000, IdleTimeout: 0, Flags: 0, Cookie: 1260000, PacketCount: 0},
+		{Id: 127, TableId: 1270, Priority: 1000, IdleTimeout: 0, Flags: 0, Cookie: 1270000, PacketCount: 0},
+	}
+	uNF, fD, uAF := flowsToUpdateToDelete(newFlows, existingFlows)
+	assert.True(t, isFlowSliceEqual(uNF, expectedNewFlows))
+	assert.True(t, isFlowSliceEqual(fD, expectedFlowsToDelete))
+	assert.True(t, isFlowSliceEqual(uAF, expectedUpdatedAllFlows))
+}
+
+func TestGroupsToUpdateToDelete_EmptySlices(t *testing.T) {
+	newGroups := []*ofp.OfpGroupEntry{}
+	existingGroups := []*ofp.OfpGroupEntry{}
+	expectedNewGroups := []*ofp.OfpGroupEntry{}
+	expectedGroupsToDelete := []*ofp.OfpGroupEntry{}
+	expectedUpdatedAllGroups := []*ofp.OfpGroupEntry{}
+	uNG, gD, uAG := groupsToUpdateToDelete(newGroups, existingGroups)
+	assert.True(t, isGroupSliceEqual(uNG, expectedNewGroups))
+	assert.True(t, isGroupSliceEqual(gD, expectedGroupsToDelete))
+	assert.True(t, isGroupSliceEqual(uAG, expectedUpdatedAllGroups))
+}
+
+func TestGroupsToUpdateToDelete_NoExistingGroups(t *testing.T) {
+	newGroups := []*ofp.OfpGroupEntry{
+		{Desc: &ofp.OfpGroupDesc{Type: 1, GroupId: 10, Buckets: nil}},
+		{Desc: &ofp.OfpGroupDesc{Type: 2, GroupId: 20, Buckets: nil}},
+	}
+	existingGroups := []*ofp.OfpGroupEntry{}
+	expectedNewGroups := []*ofp.OfpGroupEntry{
+		{Desc: &ofp.OfpGroupDesc{Type: 1, GroupId: 10, Buckets: nil}},
+		{Desc: &ofp.OfpGroupDesc{Type: 2, GroupId: 20, Buckets: nil}},
+	}
+	expectedGroupsToDelete := []*ofp.OfpGroupEntry{}
+	expectedUpdatedAllGroups := []*ofp.OfpGroupEntry{
+		{Desc: &ofp.OfpGroupDesc{Type: 1, GroupId: 10, Buckets: nil}},
+		{Desc: &ofp.OfpGroupDesc{Type: 2, GroupId: 20, Buckets: nil}},
+	}
+	uNG, gD, uAG := groupsToUpdateToDelete(newGroups, existingGroups)
+	assert.True(t, isGroupSliceEqual(uNG, expectedNewGroups))
+	assert.True(t, isGroupSliceEqual(gD, expectedGroupsToDelete))
+	assert.True(t, isGroupSliceEqual(uAG, expectedUpdatedAllGroups))
+}
+
+func TestGroupsToUpdateToDelete_UpdateNoDelete(t *testing.T) {
+	newGroups := []*ofp.OfpGroupEntry{
+		{Desc: &ofp.OfpGroupDesc{Type: 1, GroupId: 10, Buckets: nil}},
+		{Desc: &ofp.OfpGroupDesc{Type: 2, GroupId: 20, Buckets: nil}},
+	}
+	existingGroups := []*ofp.OfpGroupEntry{
+		{Desc: &ofp.OfpGroupDesc{Type: 2, GroupId: 20, Buckets: nil}},
+		{Desc: &ofp.OfpGroupDesc{Type: 3, GroupId: 30, Buckets: nil}},
+		{Desc: &ofp.OfpGroupDesc{Type: 4, GroupId: 40, Buckets: nil}},
+	}
+	expectedNewGroups := []*ofp.OfpGroupEntry{
+		{Desc: &ofp.OfpGroupDesc{Type: 1, GroupId: 10, Buckets: nil}},
+	}
+	expectedGroupsToDelete := []*ofp.OfpGroupEntry{}
+	expectedUpdatedAllGroups := []*ofp.OfpGroupEntry{
+		{Desc: &ofp.OfpGroupDesc{Type: 1, GroupId: 10, Buckets: nil}},
+		{Desc: &ofp.OfpGroupDesc{Type: 2, GroupId: 20, Buckets: nil}},
+		{Desc: &ofp.OfpGroupDesc{Type: 3, GroupId: 30, Buckets: nil}},
+		{Desc: &ofp.OfpGroupDesc{Type: 4, GroupId: 40, Buckets: nil}},
+	}
+	uNG, gD, uAG := groupsToUpdateToDelete(newGroups, existingGroups)
+	assert.True(t, isGroupSliceEqual(uNG, expectedNewGroups))
+	assert.True(t, isGroupSliceEqual(gD, expectedGroupsToDelete))
+	assert.True(t, isGroupSliceEqual(uAG, expectedUpdatedAllGroups))
+}
+
+func TestGroupsToUpdateToDelete_UpdateWithDelete(t *testing.T) {
+	newGroups := []*ofp.OfpGroupEntry{
+		{Desc: &ofp.OfpGroupDesc{Type: 1, GroupId: 10, Buckets: nil}},
+		{Desc: &ofp.OfpGroupDesc{Type: 2, GroupId: 20, Buckets: []*ofp.OfpBucket{{WatchPort: 10}}}},
+	}
+	existingGroups := []*ofp.OfpGroupEntry{
+		{Desc: &ofp.OfpGroupDesc{Type: 2, GroupId: 20, Buckets: nil}},
+		{Desc: &ofp.OfpGroupDesc{Type: 3, GroupId: 30, Buckets: nil}},
+		{Desc: &ofp.OfpGroupDesc{Type: 4, GroupId: 40, Buckets: nil}},
+	}
+	expectedNewGroups := []*ofp.OfpGroupEntry{
+		{Desc: &ofp.OfpGroupDesc{Type: 1, GroupId: 10, Buckets: nil}},
+		{Desc: &ofp.OfpGroupDesc{Type: 2, GroupId: 20, Buckets: []*ofp.OfpBucket{{WatchPort: 10}}}},
+	}
+	expectedGroupsToDelete := []*ofp.OfpGroupEntry{
+		{Desc: &ofp.OfpGroupDesc{Type: 2, GroupId: 20, Buckets: nil}},
+	}
+	expectedUpdatedAllGroups := []*ofp.OfpGroupEntry{
+		{Desc: &ofp.OfpGroupDesc{Type: 1, GroupId: 10, Buckets: nil}},
+		{Desc: &ofp.OfpGroupDesc{Type: 2, GroupId: 20, Buckets: []*ofp.OfpBucket{{WatchPort: 10}}}},
+		{Desc: &ofp.OfpGroupDesc{Type: 3, GroupId: 30, Buckets: nil}},
+		{Desc: &ofp.OfpGroupDesc{Type: 4, GroupId: 40, Buckets: nil}},
+	}
+	uNG, gD, uAG := groupsToUpdateToDelete(newGroups, existingGroups)
+	assert.True(t, isGroupSliceEqual(uNG, expectedNewGroups))
+	assert.True(t, isGroupSliceEqual(gD, expectedGroupsToDelete))
+	assert.True(t, isGroupSliceEqual(uAG, expectedUpdatedAllGroups))
+}