Skip to content

Commit

Permalink
[debug dump] Refactoring Modules and Unit Tests (sonic-net#1943)
Browse files Browse the repository at this point in the history
What I did
Moved the "populate_mock" method to dump/helper.py i.e. common to all the tests
Moved the "add_to_ret_template" to Executor class i.e. common across all the modules.
How I did it
How to verify it
Unit Tests's:
  • Loading branch information
vivekrnv authored Nov 30, 2021
1 parent b550c44 commit 4e132c1
Show file tree
Hide file tree
Showing 21 changed files with 57 additions and 233 deletions.
14 changes: 13 additions & 1 deletion dump/helper.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import os, sys
import os, sys, json

def create_template_dict(dbs):
""" Generate a Template which will be returned by Executor Classes """
Expand Down Expand Up @@ -33,3 +33,15 @@ def sort_lists(ret_template):
if isinstance(ret_template[db][key], list):
ret_template[db][key].sort()
return ret_template


def populate_mock(db, db_names, dedicated_dbs):
for db_name in db_names:
db.connect(db_name)
# Delete any default data
db.delete_all_by_pattern(db_name, "*")
with open(dedicated_dbs[db_name]) as f:
mock_json = json.load(f)
for key in mock_json:
for field, value in mock_json[key].items():
db.set(db_name, key, field, value)
22 changes: 6 additions & 16 deletions dump/plugins/copp.py
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,6 @@ def __init__(self, match_engine=None):
self.trap_group = ""
self.trap_id = ""
self.ns = ""
self.ret_temp = {}

def fetch_all_trap_ids(self, ret):
traps = []
Expand Down Expand Up @@ -108,17 +107,11 @@ def execute(self, params):
def handle_state_db(self):
req = MatchRequest(db="STATE_DB", table="COPP_TRAP_TABLE", key_pattern=self.copp_trap_cfg_key)
ret = self.match_engine.fetch(req)
if not ret["error"] and len(ret["keys"]) > 0:
self.ret_temp["STATE_DB"]["keys"].append(ret["keys"][0])
else:
self.ret_temp["STATE_DB"]["tables_not_found"].append("COPP_TRAP_TABLE")
self.add_to_ret_template(req.table, req.db, ret["keys"], ret["error"])

req = MatchRequest(db="STATE_DB", table="COPP_GROUP_TABLE", key_pattern=self.trap_group)
ret = self.match_engine.fetch(req)
if not ret["error"] and len(ret["keys"]) > 0:
self.ret_temp["STATE_DB"]["keys"].append(ret["keys"][0])
else:
self.ret_temp["STATE_DB"]["tables_not_found"].append("COPP_GROUP_TABLE")
self.add_to_ret_template(req.table, req.db, ret["keys"], ret["error"])

def handle_appl_db(self):
req = MatchRequest(db="APPL_DB", table=APP_COPP_TABLE_NAME, key_pattern="*", field="trap_ids",
Expand Down Expand Up @@ -207,17 +200,15 @@ def __get_asic_policer_obj(self, policer_sai_obj):
return
req = MatchRequest(db="ASIC_DB", table=ASIC_POLICER_OBJ, key_pattern=policer_sai_obj, ns=self.ns)
ret = self.match_engine.fetch(req)
if not ret["error"] and len(ret["keys"]) > 0:
self.ret_temp["ASIC_DB"]["keys"].append(ret["keys"][0])
self.add_to_ret_template(req.table, req.db, ret["keys"], ret["error"], False)

def __get_asic_queue_obj(self, queue_sai_obj):
# Not adding tp tables_not_found because of the type of reason specified for policer obj
if not queue_sai_obj:
return
req = MatchRequest(db="ASIC_DB", table=ASIC_QUEUE_OBJ, field="SAI_QUEUE_ATTR_INDEX", value=queue_sai_obj, ns=self.ns)
ret = self.match_engine.fetch(req)
if not ret["error"] and len(ret["keys"]) > 0:
self.ret_temp["ASIC_DB"]["keys"].append(ret["keys"][0])
self.add_to_ret_template(req.table, req.db, ret["keys"], ret["error"], False)

def __get_asic_hostif_entry_obj(self, sai_trap_key):
# Not adding tp tables_not_found because of the type of reason specified for policer obj
Expand All @@ -231,9 +222,9 @@ def __get_asic_hostif_entry_obj(self, sai_trap_key):
req = MatchRequest(db="ASIC_DB", table=ASIC_HOSTIF_TABLE_ENTRY, field="SAI_HOSTIF_TABLE_ENTRY_ATTR_TRAP_ID",
value=sai_trap_vid, return_fields=["SAI_HOSTIF_TABLE_ENTRY_ATTR_HOST_IF"], ns=self.ns)
ret = self.match_engine.fetch(req)
self.add_to_ret_template(req.table, req.db, ret["keys"], ret["error"], False)
if not ret["error"] and len(ret["keys"]) > 0:
sai_hostif_table_entry_key = ret["keys"][0]
self.ret_temp["ASIC_DB"]["keys"].append(sai_hostif_table_entry_key)
sai_hostif_vid = ret["return_values"][sai_hostif_table_entry_key]["SAI_HOSTIF_TABLE_ENTRY_ATTR_HOST_IF"]
return sai_hostif_vid

Expand All @@ -243,8 +234,7 @@ def __get_asic_hostif_obj(self, sai_hostif_vid):
return
req = MatchRequest(db="ASIC_DB", table=ASIC_HOSTIF, key_pattern=sai_hostif_vid, ns=self.ns)
ret = self.match_engine.fetch(req)
if not ret["error"] and len(ret["keys"]) > 0:
self.ret_temp["ASIC_DB"]["keys"].append(ret["keys"][0])
self.add_to_ret_template(req.table, req.db, ret["keys"], ret["error"], False)

# When the user writes config to CONFIG_DB, that takes precedence over default config
def handle_user_and_default_config(self):
Expand Down
9 changes: 0 additions & 9 deletions dump/plugins/evpn.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ class Evpn(Executor):

def __init__(self, match_engine=None):
super().__init__(match_engine)
self.ret_temp = {}
self.ns = ''
self.remote_ip = ''
self.vlan = ''
Expand Down Expand Up @@ -41,14 +40,6 @@ def execute(self, params):
self.init_state_evpn_info(evpn_name)
return self.ret_temp

def add_to_ret_template(self, table, db, keys, err):
if not err and keys:
self.ret_temp[db]["keys"].extend(keys)
return True
else:
self.ret_temp[db]["tables_not_found"].extend([table])
return False

def init_evpn_appl_info(self, evpn_name):
req = MatchRequest(db="APPL_DB", table="VXLAN_REMOTE_VNI_TABLE", key_pattern=evpn_name, ns=self.ns)
ret = self.match_engine.fetch(req)
Expand Down
12 changes: 12 additions & 0 deletions dump/plugins/executor.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ def __init__(self, match_engine=None):
self.match_engine = MatchEngine(None)
else:
self.match_engine = match_engine
self.ret_temp = {}

@abstractmethod
def execute(self, params):
Expand All @@ -24,3 +25,14 @@ def execute(self, params):
@abstractmethod
def get_all_args(self, ns):
pass

def add_to_ret_template(self, table, db, keys, err, add_to_tables_not_found=True):
if db not in self.ret_temp:
return []

if not err and keys:
self.ret_temp[db]["keys"].extend(keys)
return keys
elif add_to_tables_not_found:
self.ret_temp[db]["tables_not_found"].extend([table])
return []
8 changes: 0 additions & 8 deletions dump/plugins/port.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,14 +29,6 @@ def execute(self, params):
self.init_state_port_info(port_name)
return self.ret_temp

def add_to_ret_template(self, table, db, keys, err):
if not err and keys:
self.ret_temp[db]["keys"].extend(keys)
return True
else:
self.ret_temp[db]["tables_not_found"].extend([table])
return False

def init_port_config_info(self, port_name):
req = MatchRequest(db="CONFIG_DB", table="PORT", key_pattern=port_name, ns=self.ns)
ret = self.match_engine.fetch(req)
Expand Down
9 changes: 0 additions & 9 deletions dump/plugins/portchannel.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ class Portchannel(Executor):

def __init__(self, match_engine=None):
super().__init__(match_engine)
self.ret_temp = {}
self.ns = ''
self.lag_members = set()

Expand All @@ -38,14 +37,6 @@ def execute(self, params_dict):
self.init_lag_asic_info(lag_type_objs_asic)
return self.ret_temp

def add_to_ret_template(self, table, db, keys, err):
if not err and keys:
self.ret_temp[db]["keys"].extend(keys)
return True
else:
self.ret_temp[db]["tables_not_found"].extend([table])
return False

def init_lag_config_info(self):
req = MatchRequest(db="CONFIG_DB", table="PORTCHANNEL", key_pattern=self.lag_name, ns=self.ns)
ret = self.match_engine.fetch(req)
Expand Down
9 changes: 0 additions & 9 deletions dump/plugins/portchannel_member.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ class Portchannel_Member(Executor):

def __init__(self, match_engine=None):
super().__init__(match_engine)
self.ret_temp = {}
self.ns = ''
self.lag_member_key = ''
self.lag = ''
Expand Down Expand Up @@ -39,14 +38,6 @@ def execute(self, params_dict):
self.init_lag_member_type_obj_asic_info()
return self.ret_temp

def add_to_ret_template(self, table, db, keys, err):
if not err and keys:
self.ret_temp[db]["keys"].extend(keys)
return True
else:
self.ret_temp[db]["tables_not_found"].extend([table])
return False

def init_lag_member_config_info(self):
req = MatchRequest(db="CONFIG_DB", table="PORTCHANNEL_MEMBER", key_pattern=self.lag_member_key, ns=self.ns)
ret = self.match_engine.fetch(req)
Expand Down
9 changes: 0 additions & 9 deletions dump/plugins/route.py
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,6 @@ def __init__(self, match_engine=None):
4) CLASS_BASED_NEXT_HOP_GROUP_TABLE
5) NEXTHOP_GROUP_TABLE
"""
self.ret_temp = {}
self.ns = ''
self.dest_net = ''
self.nh_id = ''
Expand Down Expand Up @@ -83,14 +82,6 @@ def execute(self, params):
self.init_asic_nh()
return self.ret_temp

def add_to_ret_template(self, table, db, keys, err, add_to_tables_not_found=True):
if not err and keys:
self.ret_temp[db]["keys"].extend(keys)
return keys
elif add_to_tables_not_found:
self.ret_temp[db]["tables_not_found"].extend([table])
return []

def init_route_config_info(self):
req = MatchRequest(db="CONFIG_DB", table="STATIC_ROUTE", key_pattern=self.dest_net, ns=self.ns)
ret = self.match_engine.fetch(req)
Expand Down
23 changes: 5 additions & 18 deletions dump/plugins/vlan.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,27 +30,18 @@ def execute(self, params_dict):
def init_vlan_config_info(self, vlan_name):
req = MatchRequest(db="CONFIG_DB", table="VLAN", key_pattern=vlan_name, ns=self.ns)
ret = self.match_engine.fetch(req)
if not ret["error"] and len(ret["keys"]) != 0:
self.ret_temp[req.db]["keys"] = ret["keys"]
else:
self.ret_temp[req.db]["tables_not_found"] = [req.table]
self.add_to_ret_template(req.table, req.db, ret["keys"], ret["error"])

def init_vlan_appl_info(self, vlan_name):
req = MatchRequest(db="APPL_DB", table="VLAN_TABLE", key_pattern=vlan_name, ns=self.ns)
ret = self.match_engine.fetch(req)
if not ret["error"] and len(ret["keys"]) != 0:
self.ret_temp[req.db]["keys"] = ret["keys"]
else:
self.ret_temp[req.db]["tables_not_found"] = [req.table]
self.add_to_ret_template(req.table, req.db, ret["keys"], ret["error"])

def init_state_vlan_info(self, vlan_name):
req = MatchRequest(db="STATE_DB", table="VLAN_TABLE", key_pattern=vlan_name, ns=self.ns)
ret = self.match_engine.fetch(req)
if not ret["error"] and len(ret["keys"]) != 0:
self.ret_temp[req.db]["keys"] = ret["keys"]
else:
self.ret_temp[req.db]["tables_not_found"] = [req.table]

self.add_to_ret_template(req.table, req.db, ret["keys"], ret["error"])

def init_asic_vlan_info(self, vlan_name):
# Convert 'Vlanxxx' to 'xxx'
if vlan_name[0:4] != "Vlan" or not vlan_name[4:].isnumeric():
Expand All @@ -62,8 +53,4 @@ def init_asic_vlan_info(self, vlan_name):
req = MatchRequest(db="ASIC_DB", table="ASIC_STATE:SAI_OBJECT_TYPE_VLAN", key_pattern="*", field="SAI_VLAN_ATTR_VLAN_ID",
value=str(vlan_num), ns=self.ns)
ret = self.match_engine.fetch(req)
if not ret["error"] and len(ret["keys"]) != 0:
self.ret_temp[req.db]["keys"] = ret["keys"]
else:
self.ret_temp[req.db]["tables_not_found"] = [req.table]

self.add_to_ret_template(req.table, req.db, ret["keys"], ret["error"])
14 changes: 2 additions & 12 deletions dump/plugins/vlan_member.py
Original file line number Diff line number Diff line change
Expand Up @@ -49,20 +49,12 @@ def init_vlan_member_config_info(self, vlan_name, member_name):
def init_vlan_member_appl_info(self, vlan_name, member_name):
req = MatchRequest(db="APPL_DB", table="VLAN_MEMBER_TABLE", key_pattern=vlan_name+':'+member_name+"*", ns=self.ns)
ret = self.match_engine.fetch(req)
if not ret["error"] and len(ret["keys"]) != 0:
for mem in ret["keys"]:
self.ret_temp[req.db]["keys"].append(mem)
else:
self.ret_temp[req.db]["tables_not_found"].append(req.table)
self.add_to_ret_template(req.table, req.db, ret["keys"], ret["error"])

def init_state_vlan_member_info(self, vlan_name, member_name):
req = MatchRequest(db="STATE_DB", table="VLAN_MEMBER_TABLE", key_pattern=vlan_name+'|'+member_name+"*", ns=self.ns)
ret = self.match_engine.fetch(req)
if not ret["error"] and len(ret["keys"]) != 0:
for mem in ret["keys"]:
self.ret_temp[req.db]["keys"].append(mem)
else:
self.ret_temp[req.db]["tables_not_found"].append(req.table)
self.add_to_ret_template(req.table, req.db, ret["keys"], ret["error"])

def init_asic_vlan_member_info(self, vlan_name, member_name):

Expand Down Expand Up @@ -140,5 +132,3 @@ def init_asic_vlan_member_info(self, vlan_name, member_name):
else:
self.ret_temp["ASIC_DB"]["tables_not_found"].append("ASIC_STATE:SAI_OBJECT_TYPE_VLAN_MEMBER")
self.ret_temp["ASIC_DB"]["tables_not_found"].append("ASIC_STATE:SAI_OBJECT_TYPE_BRIDGE_PORT")


9 changes: 0 additions & 9 deletions dump/plugins/vxlan_tunnel.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ class Vxlan_tunnel(Executor):

def __init__(self, match_engine=None):
super().__init__(match_engine)
self.ret_temp = {}
self.ns = ''
self.src_ip = ''
self.dst_ip = ''
Expand All @@ -37,14 +36,6 @@ def execute(self, params):
self.init_state_vxlan_tunnel_info(vxlan_tunnel_name)
return self.ret_temp

def add_to_ret_template(self, table, db, keys, err):
if not err and keys:
self.ret_temp[db]["keys"].extend(keys)
return True
else:
self.ret_temp[db]["tables_not_found"].extend([table])
return False

def init_vxlan_tunnel_config_info(self, vxlan_tunnel_name):
req = MatchRequest(db="CONFIG_DB", table="VXLAN_TUNNEL", key_pattern=vxlan_tunnel_name, ns=self.ns)
ret = self.match_engine.fetch(req)
Expand Down
9 changes: 0 additions & 9 deletions dump/plugins/vxlan_tunnel_map.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ class Vxlan_tunnel_map(Executor):

def __init__(self, match_engine=None):
super().__init__(match_engine)
self.ret_temp = {}
self.ns = ''
self.vlan = ''
self.vni = ''
Expand All @@ -35,14 +34,6 @@ def execute(self, params):
self.init_asic_vxlan_tunnel_map_info()
return self.ret_temp

def add_to_ret_template(self, table, db, keys, err):
if not err and keys:
self.ret_temp[db]["keys"].extend(keys)
return True
else:
self.ret_temp[db]["tables_not_found"].extend([table])
return False

def init_vxlan_tunnel_map_config_info(self, vxlan_tunnel_map_name):
req = MatchRequest(db="CONFIG_DB", table="VXLAN_TUNNEL_MAP", key_pattern=vxlan_tunnel_map_name, ns=self.ns,
return_fields=["vlan", "vni"])
Expand Down
16 changes: 2 additions & 14 deletions tests/dump_tests/module_tests/copp_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
import pytest
from deepdiff import DeepDiff
from mock import patch
from dump.helper import create_template_dict, sort_lists
from dump.helper import create_template_dict, sort_lists, populate_mock
from dump.plugins.copp import Copp
from dump.match_infra import MatchEngine, ConnectionPool
from swsscommon.swsscommon import SonicV2Connector
Expand All @@ -26,18 +26,6 @@
dedicated_dbs['STATE_DB'] = os.path.join(copp_files_path, "state_db.json")


def populate_mock(db, db_names):
for db_name in db_names:
db.connect(db_name)
# Delete any default data
db.delete_all_by_pattern(db_name, "*")
with open(dedicated_dbs[db_name]) as f:
mock_json = json.load(f)
for key in mock_json:
for field, value in mock_json[key].items():
db.set(db_name, key, field, value)


@pytest.fixture(scope="class", autouse=True)
def match_engine():

Expand All @@ -51,7 +39,7 @@ def match_engine():
# popualate the db with mock data
db_names = list(dedicated_dbs.keys())
try:
populate_mock(db, db_names)
populate_mock(db, db_names, dedicated_dbs)
except Exception as e:
assert False, "Mock initialization failed: " + str(e)

Expand Down
Loading

0 comments on commit 4e132c1

Please sign in to comment.