Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove check_string and check_mapping from test_xml_etree #100933

Closed
sobolevn opened this issue Jan 11, 2023 · 1 comment
Closed

Remove check_string and check_mapping from test_xml_etree #100933

sobolevn opened this issue Jan 11, 2023 · 1 comment
Assignees
Labels
tests Tests in the Lib/test dir topic-XML type-bug An unexpected behavior, bug, or error

Comments

@sobolevn
Copy link
Member

sobolevn commented Jan 11, 2023

Right now one test in test_xml_etree contains two helpers called check_mapping and check_string:

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:

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

@sobolevn sobolevn added type-bug An unexpected behavior, bug, or error tests Tests in the Lib/test dir labels Jan 11, 2023
@sobolevn sobolevn self-assigned this Jan 11, 2023
sobolevn added a commit to sobolevn/cpython that referenced this issue Jan 11, 2023
@AlexWaygood AlexWaygood changed the title Remove check_string and chec_mapping from test_xml_etree Remove check_string and check_mapping from test_xml_etree Jan 11, 2023
ambv pushed a commit that referenced this issue Feb 8, 2023
Items checked by this test are always `str` and `dict` instances.
miss-islington pushed a commit to miss-islington/cpython that referenced this issue Feb 8, 2023
…ythonGH-100934)

Items checked by this test are always `str` and `dict` instances.
(cherry picked from commit eb49d32)

Co-authored-by: Nikita Sobolev <mail@sobolevn.me>
miss-islington pushed a commit to miss-islington/cpython that referenced this issue Feb 8, 2023
…ythonGH-100934)

Items checked by this test are always `str` and `dict` instances.
(cherry picked from commit eb49d32)

Co-authored-by: Nikita Sobolev <mail@sobolevn.me>
ambv pushed a commit that referenced this issue Feb 8, 2023
…H-100934) (#101686)

Items checked by this test are always `str` and `dict` instances.
(cherry picked from commit eb49d32)

Co-authored-by: Nikita Sobolev <mail@sobolevn.me>
ambv pushed a commit that referenced this issue Feb 8, 2023
…H-100934) (#101687)

Items checked by this test are always `str` and `dict` instances.
(cherry picked from commit eb49d32)

Co-authored-by: Nikita Sobolev <mail@sobolevn.me>
@hauntsaninja
Copy link
Contributor

Thank you!

carljm added a commit to carljm/cpython that referenced this issue Feb 9, 2023
* main: (82 commits)
  pythongh-101670: typo fix in PyImport_ExtendInittab() (python#101723)
  pythonGH-99293: Document that `Py_TPFLAGS_VALID_VERSION_TAG` shouldn't be used. (#pythonGH-101736)
  no-issue: Add Dong-hee Na as the cjkcodecs codeowner (pythongh-101731)
  pythongh-101678: Merge math_1_to_whatever() and math_1() (python#101730)
  pythongh-101678: refactor the math module to use special functions from c11 (pythonGH-101679)
  pythongh-85984: Remove legacy Lib/pty.py code. (python#92365)
  pythongh-98831: Use opcode metadata for stack_effect() (python#101704)
  pythongh-101283: Version was just released, so should be changed in 3.11.3 (pythonGH-101719)
  pythongh-101283: Fix use of unbound variable (pythonGH-101712)
  pythongh-101283: Improved fallback logic for subprocess with shell=True on Windows (pythonGH-101286)
  pythongh-101277: Port more itertools static types to heap types (python#101304)
  pythongh-98831: Modernize CALL and family (python#101508)
  pythonGH-101696: invalidate type version tag in `_PyStaticType_Dealloc` (python#101697)
  pythongh-100221: Fix creating dirs in `make sharedinstall` (pythonGH-100329)
  pythongh-101670: typo fix in PyImport_AppendInittab() (pythonGH-101672)
  pythongh-101196: Make isdir/isfile/exists faster on Windows (pythonGH-101324)
  pythongh-101614: Don't treat python3_d.dll as a Python DLL when checking extension modules for incompatibility (pythonGH-101615)
  pythongh-100933: Improve `check_element` helper in `test_xml_etree` (python#100934)
  pythonGH-101578: Normalize the current exception (pythonGH-101607)
  pythongh-47937: Note that Popen attributes are read-only (python#93070)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tests Tests in the Lib/test dir topic-XML type-bug An unexpected behavior, bug, or error
Projects
None yet
Development

No branches or pull requests

3 participants