-
-
Notifications
You must be signed in to change notification settings - Fork 33.2k
Closed
Labels
testsTests in the Lib/test dirTests in the Lib/test dirtopic-XMLtype-bugAn unexpected behavior, bug, or errorAn unexpected behavior, bug, or error
Description
Right now one test in test_xml_etree contains two helpers called check_mapping and check_string:
cpython/Lib/test/test_xml_etree.py
Lines 206 to 241 in 8dd2766
| def check_string(string): | |
| len(string) | |
| for char in string: | |
| self.assertEqual(len(char), 1, | |
| msg="expected one-character string, got %r" % char) | |
| new_string = string + "" | |
| new_string = string + " " | |
| string[:0] | |
| def check_mapping(mapping): | |
| len(mapping) | |
| keys = mapping.keys() | |
| items = mapping.items() | |
| for key in keys: | |
| item = mapping[key] | |
| mapping["key"] = "value" | |
| self.assertEqual(mapping["key"], "value", | |
| msg="expected value string, got %r" % mapping["key"]) | |
| def check_element(element): | |
| self.assertTrue(ET.iselement(element), msg="not an element") | |
| direlem = dir(element) | |
| for attr in 'tag', 'attrib', 'text', 'tail': | |
| self.assertTrue(hasattr(element, attr), | |
| msg='no %s member' % attr) | |
| self.assertIn(attr, direlem, | |
| msg='no %s visible by dir' % attr) | |
| check_string(element.tag) | |
| check_mapping(element.attrib) | |
| if element.text is not None: | |
| check_string(element.text) | |
| if element.tail is not None: | |
| check_string(element.tail) | |
| for elem in element: | |
| check_element(elem) |
It originates from very old code:
cpython/Lib/test/test_xml_etree.py
Lines 140 to 169 in d9a550b
| def check_string(string): | |
| len(string) | |
| for char in string: | |
| if len(char) != 1: | |
| print("expected one-character string, got %r" % char) | |
| new_string = string + "" | |
| new_string = string + " " | |
| string[:0] | |
| def check_mapping(mapping): | |
| len(mapping) | |
| keys = mapping.keys() | |
| items = mapping.items() | |
| for key in keys: | |
| item = mapping[key] | |
| mapping["key"] = "value" | |
| if mapping["key"] != "value": | |
| print("expected value string, got %r" % mapping["key"]) | |
| def check_element(element): | |
| if not ET.iselement(element): | |
| print("not an element") | |
| if not hasattr(element, "tag"): | |
| print("no tag member") | |
| if not hasattr(element, "attrib"): | |
| print("no attrib member") | |
| if not hasattr(element, "text"): | |
| print("no text member") | |
| if not hasattr(element, "tail"): | |
| print("no tail member") |
It is half-baked with lots of obvious things to be improved.
I think that it is safe just to replace them with:
check_stringtoassertIsInstance(str)check_mappingtoassertIsInstance(dict)
It is more correct, because both Python and C implementations only use str and dict for checked attributes. And in this case we can skip re-inventing tests for mapping and string.
Linked PRs
Metadata
Metadata
Assignees
Labels
testsTests in the Lib/test dirTests in the Lib/test dirtopic-XMLtype-bugAn unexpected behavior, bug, or errorAn unexpected behavior, bug, or error