diff --git a/changelogs/fragments/164-fix-xml-element-comparison-util.yml b/changelogs/fragments/164-fix-xml-element-comparison-util.yml new file mode 100644 index 00000000..b84fa7f0 --- /dev/null +++ b/changelogs/fragments/164-fix-xml-element-comparison-util.yml @@ -0,0 +1,3 @@ +--- +minor_changes: + - plugins.module_utils.xml_utils:elements_equal - Fix element content comparison behaviour diff --git a/plugins/module_utils/xml_utils.py b/plugins/module_utils/xml_utils.py index d3a80d42..ed8570e4 100644 --- a/plugins/module_utils/xml_utils.py +++ b/plugins/module_utils/xml_utils.py @@ -187,17 +187,6 @@ def etree_to_dict(input_etree: Element) -> dict: return {input_etree.tag: result} -def _is_whitespace_or_none(text) -> bool: - """ - Checks if a given string is a string of whitespace characters or None. - Args: - text: the string to perform the check on. - Returns: - bool: True if the 'text' string is None or a whitespace string. - """ - return text is None or text.strip() == "" - - def elements_equal(e1, e2) -> bool: """ Compare two XML elements for equality. @@ -216,10 +205,13 @@ def elements_equal(e1, e2) -> bool: if len(e1) == 0 and len(e2) == 0: # 1. Check if texts are exactly the same (ignoring whitespaces and None) # 2. or check if one text is '1' and the other is None with no children + e1_text: Optional[str] = "" if e1.text is None else str(e1.text).strip() + e2_text: Optional[str] = "" if e2.text is None else str(e2.text).strip() + return ( - (_is_whitespace_or_none(e1.text) == _is_whitespace_or_none(e2.text)) - or (e1.text == "1" and not e2.text and not e2) - or (e2.text == "1" and not e1.text and not e1) + e1_text == e2_text + or (e1_text == "1" and e2_text == "") + or (e2_text == "1" and e1_text == "") ) # Tags have children diff --git a/tests/unit/plugins/module_utils/test_xml_utils.py b/tests/unit/plugins/module_utils/test_xml_utils.py index 47dba969..49c5ff47 100644 --- a/tests/unit/plugins/module_utils/test_xml_utils.py +++ b/tests/unit/plugins/module_utils/test_xml_utils.py @@ -415,6 +415,21 @@ def test_elements_equal_without_children_whitespace_matches(): assert xml_utils.elements_equal(e1, e2) +def test_elements_equal_str_num_and_int(): + """ + Test that elements_equal function considers a string number equal to an integer number + + eg: + == + """ + e1 = Element("test") + e1.text = 1 + e2 = Element("test") + e2.text = "1" + + assert xml_utils.elements_equal(e1, e2) + + def test_elements_equal_without_children_none_and_1(): """ Tests elements_equal function matches a boolean flag "1" the same as an empty element. @@ -433,6 +448,7 @@ def test_elements_equal_without_children_none_and_1(): def test_elements_equal_with_children(): """ Tests elements_equal function matches recursively equal elements. + Elements having children with the same content should be equal. """ e1 = Element("test") e1c1 = Element("child_1") @@ -440,10 +456,34 @@ def test_elements_equal_with_children(): e1c2 = Element("child_2") e1c2.text = "some_text_as_well" e1.extend([e1c1, e1c2]) + e2 = Element("test") e2c1 = Element("child_1") e2c1.text = "some_text \n" e2c2 = Element("child_2") - e2c2.text = "\n some_text \n" + e2c2.text = "\n some_text_as_well \n" e2.extend([e2c1, e2c2]) + assert xml_utils.elements_equal(e1, e2) + + +def test_elements_not_equal_with_children(): + """ + Tests elements_equal function checks elements recursively. + Elements having children with different content should not be equal. + """ + e1 = Element("test") + e1c1 = Element("child_1") + e1c1.text = "some_text" + e1c2 = Element("child_2") + e1c2.text = "some_text_as_well" + e1.extend([e1c1, e1c2]) + + e2 = Element("test") + e2c1 = Element("child_1") + e2c1.text = "some_text \n" + e2c2 = Element("child_2") + e2c2.text = "\n wrong_text \n" + e2.extend([e2c1, e2c2]) + + assert not xml_utils.elements_equal(e1, e2)