[debug dump] Refactoring Modules and Unit Tests (sonic-net#1943)

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:

dump/helper.py

@@ -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 """
@@ -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)

dump/plugins/copp.py

@@ -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 = []
@@ -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",
@@ -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
@@ -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
 
@@ -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):

dump/plugins/evpn.py

@@ -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 = ''
@@ -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)

dump/plugins/executor.py

@@ -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):
@@ -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 []

dump/plugins/port.py

@@ -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)

dump/plugins/portchannel.py

@@ -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()
 
@@ -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)

dump/plugins/portchannel_member.py

@@ -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 = ''
@@ -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)

dump/plugins/route.py

@@ -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 = ''
@@ -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)

dump/plugins/vlan.py

@@ -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():
@@ -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"])

dump/plugins/vlan_member.py

@@ -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):
 
@@ -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")
-
-

dump/plugins/vxlan_tunnel.py

@@ -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 = ''
@@ -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)

dump/plugins/vxlan_tunnel_map.py

@@ -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 = ''
@@ -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"])

tests/dump_tests/module_tests/copp_test.py

@@ -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
@@ -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():
 
@@ -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)