Description

sobolevn

opened on Jan 11, 2023

Issue body actions

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_string to assertIsInstance(str)
• check_mapping to assertIsInstance(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

• gh-100933: Improve check_element helper in test_xml_etree #100934
• [3.11] gh-100933: Improve check_element helper in test_xml_etree (GH-100934) #101686
• [3.10] gh-100933: Improve check_element helper in test_xml_etree (GH-100934) #101687

Activity

1. sobolevn

   added

   type-bugAn unexpected behavior, bug, or errorAn unexpected behavior, bug, or error

   testsTests in the Lib/test dirTests in the Lib/test dir

   on Jan 11, 2023
2. sobolevn

   self-assigned this

   on Jan 11, 2023
3. sobolevn

   added a commit that references this issue on Jan 11, 2023

   pythongh-100933: Improve `check_element` helper in `test_xml_etree`

   a11b403
4. AlexWaygood

   changed the title [-]Remove `check_string` and `chec_mapping` from `test_xml_etree`[/-] [+]Remove `check_string` and `check_mapping` from `test_xml_etree`[/+] on Jan 11, 2023
5. mdboom

   added

   topic-XML

   on Jan 11, 2023
6. ambv

   added a commit that references this issue on Feb 8, 2023

   gh-100933: Improve `check_element` helper in `test_xml_etree` (#100934)

   eb49d32
7. miss-islington

   added 2 commits that reference this issue on Feb 8, 2023

   pythongh-100933: Improve `check_element` helper in `test_xml_etree` (p…

   46cf240

   pythongh-100933: Improve `check_element` helper in `test_xml_etree` (p…

   513ade4
8. ambv

   added 2 commits that reference this issue on Feb 8, 2023

   [3.11] gh-100933: Improve `check_element` helper in `test_xml_etree` (G…

   5f0b819

   [3.10] gh-100933: Improve `check_element` helper in `test_xml_etree` (G…

   c51cd54
9. hauntsaninja commented on Feb 9, 2023

   

   hauntsaninja

   on Feb 9, 2023

   Contributor

   More actions

   Thank you!
10. hauntsaninja

    closed this as completedon Feb 9, 2023
11. carljm

    added a commit that references this issue on Feb 9, 2023

    Merge branch 'main' into inlinecomp2

    be3becc

Issue actions