From 84e53eedefdff042f88391d4f279216eed00821d Mon Sep 17 00:00:00 2001 From: xincunli-sonic Date: Tue, 2 Apr 2024 14:33:39 -0700 Subject: [PATCH] Add more test cases. --- generic_config_updater/change_applier.py | 13 ++- tests/config_test.py | 34 ++++++++ .../multiasic_change_applier_test.py | 83 +++++++++++++++++++ 3 files changed, 127 insertions(+), 3 deletions(-) diff --git a/generic_config_updater/change_applier.py b/generic_config_updater/change_applier.py index 8e4cff9f5b..7738ef1427 100644 --- a/generic_config_updater/change_applier.py +++ b/generic_config_updater/change_applier.py @@ -169,10 +169,17 @@ def remove_backend_tables_from_config(self, data): def _get_running_config(self): (_, fname) = tempfile.mkstemp(suffix="_changeApplier") + command = "sonic-cfggen -d --print-data" + if self.namespace is not None and self.namespace != multi_asic.DEFAULT_NAMESPACE: - os.system("sonic-cfggen -d --print-data -n {} > {}".format(self.namespace, fname)) - else: - os.system("sonic-cfggen -d --print-data > {}".format(fname)) + command += " -n {}".format(self.namespace) + command += " > {}".format(fname) + + ret_code = os.system(command) + if ret_code != 0: + # Handle the error appropriately, e.g., raise an exception or log an error + raise RuntimeError("sonic-cfggen command failed with return code {}".format(ret_code)) + run_data = {} with open(fname, "r") as s: run_data = json.load(s) diff --git a/tests/config_test.py b/tests/config_test.py index eb073b9672..c0cdea48d1 100644 --- a/tests/config_test.py +++ b/tests/config_test.py @@ -2698,6 +2698,40 @@ def test_apply_patch_multiasic(self): # Verify mocked_open was called as expected mocked_open.assert_called_with(self.patch_file_path, 'r') + def test_apply_patch_dryrun_multiasic(self): + # Mock open to simulate file reading + with patch('builtins.open', mock_open(read_data=json.dumps(self.patch_content)), create=True) as mocked_open: + # Mock GenericUpdater to avoid actual patch application + with patch('config.main.GenericUpdater') as mock_generic_updater: + mock_generic_updater.return_value.apply_patch = MagicMock() + + # Mock ConfigDBConnector to ensure it's not called during dry-run + with patch('config.main.ConfigDBConnector') as mock_config_db_connector: + + print("Multi ASIC: {}".format(multi_asic.is_multi_asic())) + # Invocation of the command with the CliRunner + result = self.runner.invoke(config.config.commands["apply-patch"], + [self.patch_file_path, + "--format", ConfigFormat.SONICYANG.name, + "--dry-run", + "--ignore-non-yang-tables", + "--ignore-path", "/ANY_TABLE", + "--ignore-path", "/ANY_OTHER_TABLE/ANY_FIELD", + "--ignore-path", "", + "--verbose"], + catch_exceptions=False) + + print("Exit Code: {}, output: {}".format(result.exit_code, result.output)) + # Assertions and verifications + self.assertEqual(result.exit_code, 0, "Command should succeed") + self.assertIn("Patch applied successfully.", result.output) + + # Verify mocked_open was called as expected + mocked_open.assert_called_with(self.patch_file_path, 'r') + + # Ensure ConfigDBConnector was never instantiated or called + mock_config_db_connector.assert_not_called() + @classmethod def teardown_class(cls): print("TEARDOWN") diff --git a/tests/generic_config_updater/multiasic_change_applier_test.py b/tests/generic_config_updater/multiasic_change_applier_test.py index d7123b6596..9ffc30a750 100644 --- a/tests/generic_config_updater/multiasic_change_applier_test.py +++ b/tests/generic_config_updater/multiasic_change_applier_test.py @@ -98,3 +98,86 @@ def test_apply_change_given_namespace(self, mock_ConfigDBConnector, mock_json_lo # Assert ConfigDBConnector called with the correct namespace mock_ConfigDBConnector.assert_called_once_with(use_unix_socket_path=True, namespace="asic0") + + @patch('generic_config_updater.change_applier.os.system', autospec=True) + @patch('generic_config_updater.change_applier.json.load', autospec=True) + @patch('generic_config_updater.change_applier.ConfigDBConnector', autospec=True) + def test_apply_change_failure(self, mock_ConfigDBConnector, mock_json_load, mock_os_system): + # Setup mock for ConfigDBConnector + mock_db = MagicMock() + mock_ConfigDBConnector.return_value = mock_db + + # Setup mock for json.load to return some running configuration + mock_json_load.return_value = { + "tables": { + "ACL_TABLE": { + "services_to_validate": ["aclservice"], + "validate_commands": ["acl_loader show table"] + }, + "PORT": { + "services_to_validate": ["portservice"], + "validate_commands": ["show interfaces status"] + } + }, + "services": { + "aclservice": { + "validate_commands": ["acl_loader show table"] + }, + "portservice": { + "validate_commands": ["show interfaces status"] + } + } + } + + # Setup mock for os.system to simulate failure, such as a command returning non-zero status + mock_os_system.return_value = 1 # Non-zero return value indicates failure + + # Instantiate ChangeApplier with a specific namespace to simulate applying changes in a multi-asic environment + namespace = "asic0" + applier = generic_config_updater.change_applier.ChangeApplier(namespace=namespace) + + # Prepare a change object or data that applier.apply would use + change = MagicMock() + + # Test the behavior when os.system fails + with self.assertRaises(Exception) as context: + applier.apply(change) + + # Optionally, assert specific error message if your method raises a custom exception + # self.assertTrue("Expected error message" in str(context.exception)) + + # Assert that os.system was called, indicating the command was attempted + mock_os_system.assert_called() + + @patch('generic_config_updater.change_applier.os.system', autospec=True) + @patch('generic_config_updater.change_applier.json.load', autospec=True) + @patch('generic_config_updater.change_applier.ConfigDBConnector', autospec=True) + def test_apply_patch_with_empty_tables_failure(self, mock_ConfigDBConnector, mock_json_load, mock_os_system): + # Setup mock for ConfigDBConnector + mock_db = MagicMock() + mock_ConfigDBConnector.return_value = mock_db + + # Setup mock for json.load to simulate configuration where crucial tables are unexpectedly empty + mock_json_load.return_value = { + "tables": { + # Simulate empty tables or missing crucial configuration + }, + "services": { + # Normally, services would be listed here + } + } + + # Setup mock for os.system to simulate command execution success + mock_os_system.return_value = 0 + + # Instantiate ChangeApplier with a specific namespace to simulate applying changes in a multi-asic environment + applier = generic_config_updater.change_applier.ChangeApplier(namespace="asic0") + + # Prepare a change object or data that applier.apply would use, simulating a patch that requires non-empty tables + change = MagicMock() + + # Apply the patch + assert(applier.apply(change) != 0) + + # Verify that the system attempted to retrieve the configuration despite the missing data + mock_json_load.assert_called()