diff --git a/generic_config_updater/gu_common.py b/generic_config_updater/gu_common.py index d7eef0f1b141..7fd45f618e38 100644 --- a/generic_config_updater/gu_common.py +++ b/generic_config_updater/gu_common.py @@ -414,11 +414,14 @@ def _find_leafref_paths(self, path, config): ref_xpaths.extend(sy.find_data_dependencies(xpath)) ref_paths = [] + ref_paths_set = set() for ref_xpath in ref_xpaths: ref_path = self.convert_xpath_to_path(ref_xpath, config, sy) - ref_paths.append(ref_path) + if ref_path not in ref_paths_set: + ref_paths.append(ref_path) + ref_paths_set.add(ref_path) - return set(ref_paths) + return ref_paths def _get_inner_leaf_xpaths(self, xpath, sy): if xpath == "/": # Point to Root element which contains all xpaths diff --git a/tests/generic_config_updater/files/patch_sorter_test_success.json b/tests/generic_config_updater/files/patch_sorter_test_success.json index 8d7b92d32fb7..14506503c7d5 100644 --- a/tests/generic_config_updater/files/patch_sorter_test_success.json +++ b/tests/generic_config_updater/files/patch_sorter_test_success.json @@ -698,7 +698,7 @@ [ { "op": "remove", - "path": "/LOOPBACK_INTERFACE/Loopback1|2200:2::32~1128" + "path": "/LOOPBACK_INTERFACE/Loopback1|20.2.0.32~132" } ], [ @@ -752,7 +752,7 @@ [ { "op": "add", - "path": "/LOOPBACK_INTERFACE/Loopback1|2200:2::32~1128", + "path": "/LOOPBACK_INTERFACE/Loopback1|20.2.0.32~132", "value": {} } ] @@ -1331,16 +1331,42 @@ "path": "/PORT/Ethernet3" } ], + [ + { + "op": "remove", + "path": "/ACL_TABLE/NO-NSW-PACL-V4/ports" + } + ], [ { "op": "remove", "path": "/VLAN_MEMBER" } ], + [ + { + "op": "add", + "path": "/ACL_TABLE/NO-NSW-PACL-V4/ports", + "value": [ + "Ethernet0" + ] + } + ], [ { "op": "remove", - "path": "/ACL_TABLE/NO-NSW-PACL-V4/ports" + "path": "/ACL_TABLE" + } + ], + [ + { + "op": "add", + "path": "/ACL_TABLE", + "value": { + "NO-NSW-PACL-V4": { + "type": "L3" + } + } } ], [ @@ -1349,6 +1375,13 @@ "path": "/PORT" } ], + [ + { + "op": "add", + "path": "/ACL_TABLE/NO-NSW-PACL-V4/policy_desc", + "value": "NO-NSW-PACL-V4" + } + ], [ { "op": "add", @@ -2465,19 +2498,19 @@ [ { "op": "remove", - "path": "/LOOPBACK_INTERFACE/Loopback1|2200:2::32~1128" + "path": "/LOOPBACK_INTERFACE/Loopback1|20.2.0.32~132" } ], [ { "op": "remove", - "path": "/LOOPBACK_INTERFACE/Loopback1|20.2.0.32~132" + "path": "/LOOPBACK_INTERFACE/Loopback1|2200:2::32~1128" } ], [ { "op": "add", - "path": "/LOOPBACK_INTERFACE/Loopback1|2200:2::32~1128", + "path": "/LOOPBACK_INTERFACE/Loopback1|20.2.0.32~132", "value": {} } ], @@ -2490,20 +2523,20 @@ [ { "op": "add", - "path": "/LOOPBACK_INTERFACE/Loopback1|20.2.0.32~132", + "path": "/LOOPBACK_INTERFACE/Loopback1|2200:2::32~1128", "value": {} } ], [ { "op": "remove", - "path": "/LOOPBACK_INTERFACE/Loopback1|2200:2::32~1128" + "path": "/LOOPBACK_INTERFACE/Loopback1|20.2.0.32~132" } ], [ { "op": "remove", - "path": "/LOOPBACK_INTERFACE/Loopback1|20.2.0.32~132" + "path": "/LOOPBACK_INTERFACE/Loopback1|2200:2::32~1128" } ], [ diff --git a/tests/generic_config_updater/gu_common_test.py b/tests/generic_config_updater/gu_common_test.py index 52cf9a818de1..083546c98012 100644 --- a/tests/generic_config_updater/gu_common_test.py +++ b/tests/generic_config_updater/gu_common_test.py @@ -590,7 +590,7 @@ def test_find_ref_paths__ref_is_the_whole_key__returns_ref_paths(self): actual = self.path_addressing.find_ref_paths(path, Files.CROPPED_CONFIG_DB_AS_JSON) # Assert - self.assertCountEqual(expected, actual) + self.assertEqual(expected, actual) def test_find_ref_paths__ref_is_a_part_of_key__returns_ref_paths(self): # Arrange @@ -605,7 +605,7 @@ def test_find_ref_paths__ref_is_a_part_of_key__returns_ref_paths(self): actual = self.path_addressing.find_ref_paths(path, Files.CROPPED_CONFIG_DB_AS_JSON) # Assert - self.assertCountEqual(expected, actual) + self.assertEqual(expected, actual) def test_find_ref_paths__ref_is_in_multilist__returns_ref_paths(self): # Arrange @@ -619,7 +619,7 @@ def test_find_ref_paths__ref_is_in_multilist__returns_ref_paths(self): actual = self.path_addressing.find_ref_paths(path, Files.CONFIG_DB_WITH_INTERFACE) # Assert - self.assertCountEqual(expected, actual) + self.assertEqual(expected, actual) def test_find_ref_paths__ref_is_in_leafref_union__returns_ref_paths(self): # Arrange @@ -632,47 +632,47 @@ def test_find_ref_paths__ref_is_in_leafref_union__returns_ref_paths(self): actual = self.path_addressing.find_ref_paths(path, Files.CONFIG_DB_WITH_PORTCHANNEL_AND_ACL) # Assert - self.assertCountEqual(expected, actual) + self.assertEqual(expected, actual) def test_find_ref_paths__path_is_table__returns_ref_paths(self): # Arrange path = "/PORT" expected = [ - "/ACL_TABLE/DATAACL/ports/0", - "/ACL_TABLE/EVERFLOW/ports/0", - "/ACL_TABLE/EVERFLOWV6/ports/0", - "/ACL_TABLE/EVERFLOWV6/ports/1", "/ACL_TABLE/NO-NSW-PACL-V4/ports/0", "/VLAN_MEMBER/Vlan1000|Ethernet0", + "/ACL_TABLE/DATAACL/ports/0", + "/ACL_TABLE/EVERFLOWV6/ports/0", "/VLAN_MEMBER/Vlan1000|Ethernet4", - "/VLAN_MEMBER/Vlan1000|Ethernet8", + "/ACL_TABLE/EVERFLOW/ports/0", + "/ACL_TABLE/EVERFLOWV6/ports/1", + "/VLAN_MEMBER/Vlan1000|Ethernet8" ] # Act actual = self.path_addressing.find_ref_paths(path, Files.CROPPED_CONFIG_DB_AS_JSON) # Assert - self.assertCountEqual(expected, actual) + self.assertEqual(expected, actual) def test_find_ref_paths__whole_config_path__returns_all_refs(self): # Arrange path = "" expected = [ - "/ACL_TABLE/DATAACL/ports/0", - "/ACL_TABLE/EVERFLOW/ports/0", - "/ACL_TABLE/EVERFLOWV6/ports/0", - "/ACL_TABLE/EVERFLOWV6/ports/1", - "/ACL_TABLE/NO-NSW-PACL-V4/ports/0", "/VLAN_MEMBER/Vlan1000|Ethernet0", "/VLAN_MEMBER/Vlan1000|Ethernet4", "/VLAN_MEMBER/Vlan1000|Ethernet8", + "/ACL_TABLE/NO-NSW-PACL-V4/ports/0", + "/ACL_TABLE/DATAACL/ports/0", + "/ACL_TABLE/EVERFLOWV6/ports/0", + "/ACL_TABLE/EVERFLOW/ports/0", + "/ACL_TABLE/EVERFLOWV6/ports/1", ] # Act actual = self.path_addressing.find_ref_paths(path, Files.CROPPED_CONFIG_DB_AS_JSON) # Assert - self.assertCountEqual(expected, actual) + self.assertEqual(expected, actual) def test_convert_path_to_xpath(self): def check(path, xpath, config=None): diff --git a/tests/generic_config_updater/patch_sorter_test.py b/tests/generic_config_updater/patch_sorter_test.py index 710de1860d3e..e79d18163bd2 100644 --- a/tests/generic_config_updater/patch_sorter_test.py +++ b/tests/generic_config_updater/patch_sorter_test.py @@ -1747,6 +1747,9 @@ def verify(self, algo, algo_class): self.assertCountEqual(expected_validator, actual_validators) class TestPatchSorter(unittest.TestCase): + def setUp(self): + self.config_wrapper = ConfigWrapper() + def test_patch_sorter_success(self): # Format of the JSON file containing the test-cases: # @@ -1762,9 +1765,7 @@ def test_patch_sorter_success(self): # . # } data = Files.PATCH_SORTER_TEST_SUCCESS - # TODO: Investigate issue where different runs of patch-sorter generated different but correct steps - # Once investigation is complete remove the flag 'skip_exact_change_list_match' - skip_exact_change_list_match = True + skip_exact_change_list_match = False for test_case_name in data: with self.subTest(name=test_case_name): self.run_single_success_case(data[test_case_name], skip_exact_change_list_match) @@ -1787,7 +1788,7 @@ def run_single_success_case(self, data, skip_exact_change_list_match): simulated_config = current_config for change in actual_changes: simulated_config = change.apply(simulated_config) - self.assertTrue(ConfigWrapper().validate_config_db_config(simulated_config)) + self.assertTrue(self.config_wrapper.validate_config_db_config(simulated_config)) self.assertEqual(target_config, simulated_config) def test_patch_sorter_failure(self): @@ -1831,7 +1832,7 @@ def run_single_failure_case(self, data): def create_patch_sorter(self, config=None): if config is None: config=Files.CROPPED_CONFIG_DB_AS_JSON - config_wrapper = ConfigWrapper() + config_wrapper = self.config_wrapper config_wrapper.get_config_db_as_json = MagicMock(return_value=config) patch_wrapper = PatchWrapper(config_wrapper) operation_wrapper = OperationWrapper()