Skip to content

Commit

Permalink
pythongh-93018: Fix for the compatibility problems with expat
Browse files Browse the repository at this point in the history
This is another take on python#93022, fixes python#93018.

1. I believe it is inappropriate to assert on the error message for the
   external API. Error messages are not part of the provided contract
   with its users.
2. I don't think we are really care what exact exception is thrown, it
   is much more important that non-well-formed XML is discovered.
3. Changing tests based on the version of library seems as a code smell
   to me, but that's personal opinion.
  • Loading branch information
mcepl committed Sep 15, 2022
1 parent e37ac5f commit 2733b20
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 15 deletions.
24 changes: 9 additions & 15 deletions Lib/test/test_minidom.py
Original file line number Diff line number Diff line change
Expand Up @@ -1163,14 +1163,10 @@ def testEncodings(self):

# Verify that character decoding errors raise exceptions instead
# of crashing
if pyexpat.version_info >= (2, 4, 5):
self.assertRaises(ExpatError, parseString,
b'<fran\xe7ais></fran\xe7ais>')
self.assertRaises(ExpatError, parseString,
b'<franais>Comment \xe7a va ? Tr\xe8s bien ?</franais>')
else:
self.assertRaises(UnicodeDecodeError, parseString,
b'<fran\xe7ais>Comment \xe7a va ? Tr\xe8s bien ?</fran\xe7ais>')
with self.assertRaises((UnicodeDecodeError, ExpatError)):
parseString(
b'<fran\xe7ais>Comment \xe7a va ? Tr\xe8s bien ?</fran\xe7ais>'
)

doc.unlink()

Expand Down Expand Up @@ -1631,13 +1627,11 @@ def testEmptyXMLNSValue(self):
self.confirm(doc2.namespaceURI == xml.dom.EMPTY_NAMESPACE)

def testExceptionOnSpacesInXMLNSValue(self):
if pyexpat.version_info >= (2, 4, 5):
context = self.assertRaisesRegex(ExpatError, 'syntax error')
else:
context = self.assertRaisesRegex(ValueError, 'Unsupported syntax')

with context:
parseString('<element xmlns:abc="http:abc.com/de f g/hi/j k"><abc:foo /></element>')
with self.assertRaises((ValueError, ExpatError)):
parseString(
'<element xmlns:abc="http:abc.com/de f g/hi/j k">' +
'<abc:foo /></element>'
)

def testDocRemoveChild(self):
doc = parse(tstfile)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix for the compatibility problems with expat made more robust.

0 comments on commit 2733b20

Please sign in to comment.