From 2b8535386047de1a97f6dc09800a2895282bfd4b Mon Sep 17 00:00:00 2001 From: Fabio Bertagna Date: Thu, 5 Dec 2024 15:12:43 +0100 Subject: [PATCH 1/6] Make XML element comparison util more generic --- plugins/module_utils/xml_utils.py | 20 ++++--------------- .../plugins/module_utils/test_xml_utils.py | 19 +++++++++++++++++- 2 files changed, 22 insertions(+), 17 deletions(-) diff --git a/plugins/module_utils/xml_utils.py b/plugins/module_utils/xml_utils.py index d3a80d42..ffdfc798 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,11 +205,10 @@ 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 - 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: 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 e1_text == e2_text or e1_text == "1" and e2_text == "" or e2_text == "1" and e1_text == "" # Tags have children return all( diff --git a/tests/unit/plugins/module_utils/test_xml_utils.py b/tests/unit/plugins/module_utils/test_xml_utils.py index 47dba969..d7f479bf 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. @@ -440,10 +455,12 @@ 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) From d18faad5fa6b1448ce29952d177f19a067c6cbc1 Mon Sep 17 00:00:00 2001 From: Fabio Bertagna Date: Thu, 5 Dec 2024 15:18:04 +0100 Subject: [PATCH 2/6] Add changelog fragment --- changelogs/fragments/164-fix-xml-element-comparison-util.yml | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 changelogs/fragments/164-fix-xml-element-comparison-util.yml 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 From d092a67626d1b0b5bbb88c63cde548c862a34e0a Mon Sep 17 00:00:00 2001 From: Fabio Bertagna Date: Thu, 5 Dec 2024 15:21:25 +0100 Subject: [PATCH 3/6] Lint --- plugins/module_utils/xml_utils.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/plugins/module_utils/xml_utils.py b/plugins/module_utils/xml_utils.py index ffdfc798..c5720ce7 100644 --- a/plugins/module_utils/xml_utils.py +++ b/plugins/module_utils/xml_utils.py @@ -208,7 +208,11 @@ def elements_equal(e1, e2) -> bool: 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 e1_text == e2_text or e1_text == "1" and e2_text == "" or e2_text == "1" and e1_text == "" + return ( + e1_text == e2_text + or e1_text == "1" and e2_text == "" + or e2_text == "1" and e1_text == "" + ) # Tags have children return all( From deedbe5a1d70e9eb6ff78b3000f376552ee1b4a7 Mon Sep 17 00:00:00 2001 From: Fabio Bertagna Date: Thu, 5 Dec 2024 15:22:03 +0100 Subject: [PATCH 4/6] Add inverse test to xml util --- .../plugins/module_utils/test_xml_utils.py | 21 +++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/tests/unit/plugins/module_utils/test_xml_utils.py b/tests/unit/plugins/module_utils/test_xml_utils.py index d7f479bf..e2211629 100644 --- a/tests/unit/plugins/module_utils/test_xml_utils.py +++ b/tests/unit/plugins/module_utils/test_xml_utils.py @@ -464,3 +464,24 @@ def test_elements_equal_with_children(): e2.extend([e2c1, e2c2]) assert xml_utils.elements_equal(e1, e2) + + +def test_elements_not_equal_with_children(): + """ + Tests elements_equal function checks recursively elements. + """ + 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) From c210ba4b3796c28df5d689f0a196633a39f9049f Mon Sep 17 00:00:00 2001 From: Fabio Bertagna Date: Thu, 5 Dec 2024 15:46:08 +0100 Subject: [PATCH 5/6] Lint --- plugins/module_utils/xml_utils.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/plugins/module_utils/xml_utils.py b/plugins/module_utils/xml_utils.py index c5720ce7..ed8570e4 100644 --- a/plugins/module_utils/xml_utils.py +++ b/plugins/module_utils/xml_utils.py @@ -210,8 +210,8 @@ def elements_equal(e1, e2) -> bool: return ( e1_text == e2_text - or e1_text == "1" and e2_text == "" - or e2_text == "1" and e1_text == "" + or (e1_text == "1" and e2_text == "") + or (e2_text == "1" and e1_text == "") ) # Tags have children From a5df6b88588469d843365c76e7bc44ac8e84ce65 Mon Sep 17 00:00:00 2001 From: Fabio Bertagna Date: Fri, 6 Dec 2024 10:12:09 +0100 Subject: [PATCH 6/6] Update some unit test descriptions --- tests/unit/plugins/module_utils/test_xml_utils.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/unit/plugins/module_utils/test_xml_utils.py b/tests/unit/plugins/module_utils/test_xml_utils.py index e2211629..49c5ff47 100644 --- a/tests/unit/plugins/module_utils/test_xml_utils.py +++ b/tests/unit/plugins/module_utils/test_xml_utils.py @@ -448,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") @@ -468,7 +469,8 @@ def test_elements_equal_with_children(): def test_elements_not_equal_with_children(): """ - Tests elements_equal function checks recursively elements. + Tests elements_equal function checks elements recursively. + Elements having children with different content should not be equal. """ e1 = Element("test") e1c1 = Element("child_1")