add missing loopback unit tests, etc.

config/main.py

@@ -2078,18 +2078,17 @@ def remove_portchannel(ctx, portchannel_name):
         if is_portchannel_present_in_db(db, portchannel_name) is False:
             ctx.fail("{} is not present.".format(portchannel_name))
 
-    # Dont let to remove port channel if vlan membership exists
-    for k,v in db.get_table('VLAN_MEMBER'):
-        if v == portchannel_name:
-            ctx.fail("{} has vlan {} configured, remove vlan membership to proceed".format(portchannel_name, str(k)))
+        # Dont let to remove port channel if vlan membership exists
+        for k,v in db.get_table('VLAN_MEMBER'): # TODO: MISSING CONSTRAINT IN YANG MODEL
+            if v == portchannel_name:
+                ctx.fail("{} has vlan {} configured, remove vlan membership to proceed".format(portchannel_name, str(k)))
 
-    if len([(k, v) for k, v in db.get_table('PORTCHANNEL_MEMBER') if k == portchannel_name]) != 0:
-        click.echo("Error: Portchannel {} contains members. Remove members before deleting Portchannel!".format(portchannel_name))
-    else:
-        try:
-            db.set_entry('PORTCHANNEL', portchannel_name, None)
-        except JsonPatchConflict:
-            ctx.fail("{} is not present.".format(portchannel_name))
+        if len([(k, v) for k, v in db.get_table('PORTCHANNEL_MEMBER') if k == portchannel_name]) != 0: # TODO: MISSING CONSTRAINT IN YANG MODEL
+            click.echo("Error: Portchannel {} contains members. Remove members before deleting Portchannel!".format(portchannel_name))
+    try:
+        db.set_entry('PORTCHANNEL', portchannel_name, None)
+    except JsonPatchConflict:
+        ctx.fail("{} is not present.".format(portchannel_name))
 
 @portchannel.group(cls=clicommon.AbbreviationGroup, name='member')
 @click.pass_context
@@ -6161,9 +6160,9 @@ def add_loopback(ctx, loopback_name):
             ctx.fail("{} is invalid, name should have prefix '{}' and suffix '{}' "
                     .format(loopback_name, CFG_LOOPBACK_PREFIX, CFG_LOOPBACK_NO))
 
-    lo_intfs = [k for k, v in config_db.get_table('LOOPBACK_INTERFACE').items() if type(k) != tuple]
-    if loopback_name in lo_intfs:
-        ctx.fail("{} already exists".format(loopback_name))
+        lo_intfs = [k for k, v in config_db.get_table('LOOPBACK_INTERFACE').items() if type(k) != tuple]
+        if loopback_name in lo_intfs:
+            ctx.fail("{} already exists".format(loopback_name)) # TODO: MISSING CONSTRAINT IN YANG VALIDATION
 
     try:
         config_db.set_entry('LOOPBACK_INTERFACE', loopback_name, {"NULL" : "NULL"})

config/validated_config_db_connector.py

@@ -36,10 +36,12 @@ def validated_set_entry(table, key, value):
     gcu_patch = jsonpatch.JsonPatch(gcu_json_input)
     format = ConfigFormat.CONFIGDB.name
     config_format = ConfigFormat[format.upper()]
+
+    config_db = ConfigDBConnector()
+    config_db.connect()
     try:
         GenericUpdater().apply_patch(patch=gcu_patch, config_format=config_format, verbose=False, dry_run=False, ignore_non_yang_tables=False, ignore_paths=None)
     except EmptyTableError:
-        config_db = ConfigDBConnector()
-        config_db.connect()
+        print(config_db)
         config_db.set_entry(table, key, None)
 

tests/config_test.py

@@ -15,6 +15,7 @@
 from sonic_py_common import device_info
 from utilities_common.db import Db
 from utilities_common.general import load_module_from_source
+from mock import patch
 
 from generic_config_updater.generic_updater import ConfigFormat
 
@@ -862,6 +863,7 @@ def test_apply_patch__only_required_params__default_values_used_for_optional_par
                 result = self.runner.invoke(config.config.commands["apply-patch"], [self.any_path], catch_exceptions=False)
 
         # Assert
+        print(result.output)
         self.assertEqual(expected_exit_code, result.exit_code)
         self.assertTrue(expected_output in result.output)
         mock_generic_updater.apply_patch.assert_called_once()
@@ -988,6 +990,7 @@ def test_replace__only_required_params__default_values_used_for_optional_params(
 
         # Assert
         self.assertEqual(expected_exit_code, result.exit_code)
+        print(result.output)
         self.assertTrue(expected_output in result.output)
         mock_generic_updater.replace.assert_called_once()
         mock_generic_updater.replace.assert_has_calls([expected_call_with_default_values])
@@ -1614,3 +1617,87 @@ def test_hostname_add(self, db_conn_patch, get_cmd_module):
     @classmethod
     def teardown_class(cls):
         print("TEARDOWN")
+
+
+class TestConfigLoopback(object):
+    @classmethod
+    def setup_class(cls):
+        print("SETUP")
+        import config.main
+        importlib.reload(config.main)
+
+    @patch("config.validated_config_db_connector.validated_set_entry", mock.Mock(side_effect=ValueError))
+    def test_add_loopback_with_invalid_name_yang_validation(self):
+        config.ADHOC_VALIDATION = False
+        runner = CliRunner()
+        db = Db()
+        obj = {'db':db.cfgdb}
+
+        result = runner.invoke(config.config.commands["loopback"].commands["add"], ["Loopbax1"], obj=obj)
+        print(result.exit_code)
+        print(result.output)
+        assert result.exit_code != 0
+        assert "Error: Loopbax1 is invalid, name should have prefix 'Loopback' and suffix '<0-999>'" in result.output
+
+    def test_add_loopback_with_invalid_name_adhoc_validation(self):
+        config.ADHOC_VALIDATION = True
+        runner = CliRunner()
+        db = Db()
+        obj = {'db':db.cfgdb}
+
+        result = runner.invoke(config.config.commands["loopback"].commands["add"], ["Loopbax1"], obj=obj)
+        print(result.exit_code)
+        print(result.output)
+        assert result.exit_code != 0
+        assert "Error: Loopbax1 is invalid, name should have prefix 'Loopback' and suffix'<0-999>'" in result.output
+
+    def test_del_nonexistent_loopback_adhoc_validation(self):
+        config.ADHOC_VALIDATION = True
+        runner = CliRunner()
+        db = Db()
+        obj = {'db':db.cfgdb}
+
+        result = runner.invoke(config.config.commands["loopback"].commands["del"], ["Loopback12"], obj=obj)
+        print(result.exit_code)
+        print(result.output)
+        assert result.exit_code != 0
+        assert "Loopback12 does not exist" in result.output
+
+    def test_del_nonexistent_loopback_adhoc_validation(self):
+        config.ADHOC_VALIDATION = True
+        runner = CliRunner()
+        db = Db()
+        obj = {'db':db.cfgdb}
+
+        result = runner.invoke(config.config.commands["loopback"].commands["del"], ["Loopbax1"], obj=obj)
+        print(result.exit_code)
+        print(result.output)
+        assert result.exit_code != 0
+        assert "Loopbax1 is invalid, name should have prefix 'Loopback' and suffix '<0-999>'" in result.output
+
+    @patch("config.validated_config_db_connector.validated_set_entry", mock.Mock(return_value=True))
+    def test_add_loopback_yang_validation(self):
+        config.ADHOC_VALIDATION = False
+        runner = CliRunner()
+        db = Db()
+        obj = {'db':db.cfgdb}
+
+        result = runner.invoke(config.config.commands["loopback"].commands["add"], ["Loopback12"], obj=obj)
+        print(result.exit_code)
+        print(result.output)
+        assert result.exit_code == 0
+
+    def test_add_loopback_adhoc_validation(self):
+        config.ADHOC_VALIDATION = True
+        runner = CliRunner()
+        db = Db()
+        obj = {'db':db.cfgdb}
+
+        result = runner.invoke(config.config.commands["loopback"].commands["add"], ["Loopback12"], obj=obj)
+        print(result.exit_code)
+        print(result.output)
+        assert result.exit_code == 0
+
+    @classmethod
+    def teardown_class(cls):
+        print("TEARDOWN")

tests/portchannel_test.py

@@ -30,7 +30,6 @@ def test_add_portchannel_with_invalid_name_yang_validation(self):
         print(result.output)
         assert result.exit_code != 0
         assert "Error: PortChan005 is invalid!, name should have prefix 'PortChannel' and suffix '<0-9999>'" in result.output
-
 
     def test_add_portchannel_with_invalid_name_adhoc_validation(self):
         config.ADHOC_VALIDATION = True
@@ -45,6 +44,20 @@ def test_add_portchannel_with_invalid_name_adhoc_validation(self):
         assert result.exit_code != 0
         assert "Error: PortChan005 is invalid!, name should have prefix 'PortChannel' and suffix '<0-9999>'" in result.output
 
+    @patch("config.validated_config_db_connector.validated_set_entry", mock.Mock(side_effect=JsonPatchConflict))
+    def test_delete_nonexistent_portchannel_yang_validation(self):
+        config.ADHOC_VALIDATION = False
+        runner = CliRunner()
+        db = Db()
+        obj = {'db':db.cfgdb}
+
+        # delete a portchannel with invalid name
+        result = runner.invoke(config.config.commands["portchannel"].commands["del"], ["PortChan005"], obj=obj)
+        print(result.exit_code)
+        print(result.output)
+        assert result.exit_code != 0
+        assert "PortChan005 is not present" in result.output
+
     def test_delete_portchannel_with_invalid_name_adhoc_validation(self):
         config.ADHOC_VALIDATION = True
         runner = CliRunner()

tests/validated_config_db_connector_test.py

@@ -23,7 +23,7 @@ class TestValidatedConfigDBConnector(TestCase):
         Test Class for validated_config_db_connector.py
 
     '''
-    def test_validated_config_db_connector(self): 
+    def test_validated_config_db_connector_empty_table(self): 
         mock_generic_updater = mock.Mock()
         mock_generic_updater.apply_patch = mock.Mock(side_effect=EmptyTableError)
         with mock.patch('validated_config_db_connector.GenericUpdater', return_value=mock_generic_updater):
@@ -32,5 +32,3 @@ def test_validated_config_db_connector(self):
             with mock.patch('validated_config_db_connector.ConfigDBConnector', return_value=db.cfgdb):
                 validated_config_db_connector.validated_set_entry(SAMPLE_TABLE, SAMPLE_KEY, SAMPLE_VALUE_EMPTY)
                 assert db.cfgdb.get_entry(SAMPLE_TABLE, SAMPLE_KEY) == {}
-
-