karl <karl+pythonb...@la-grange.net> added the comment:
@zach.ware @r.david.murray So I was looking at that issue. There is a lot of work. I had a couple of questions, because there are different categories # Empty tests for existing functions. This seems to be straightforward as they would complete the module. Example: ```python def testGetAttributeNode(self): pass ``` https://github.com/python/cpython/blob/3e04cd268ee9a57f95dc78d8974b21a6fac3f666/Lib/test/test_minidom.py#L412 which refers to: `GetAttributeNode` https://github.com/python/cpython/blob/3e04cd268ee9a57f95dc78d8974b21a6fac3f666/Lib/xml/dom/minidom.py#L765-L768 https://github.com/python/cpython/blob/3e04cd268ee9a57f95dc78d8974b21a6fac3f666/Lib/test/test_minidom.py#L285-L294 # Tests without any logical reference in the module. This is puzzling because I'm not sure which DOM feature they should be testing. For example: ``` def testGetAttrList(self): pass ``` https://github.com/python/cpython/blob/3e04cd268ee9a57f95dc78d8974b21a6fac3f666/Lib/test/test_minidom.py#L383-L384 Or maybe this is just supposed to test Element.attributes returning a list of attributes, such as `NamedNodeMap [ def="ghi", jkl="mno"]` returned by a browser. ``` >>> import xml.dom.minidom >>> from xml.dom.minidom import parse, Node, Document, parseString >>> from xml.dom.minidom import getDOMImplementation >>> dom = parseString("<abc/>") >>> el = dom.documentElement >>> el.setAttribute("def", "ghi") >>> el.setAttribute("jkl", "mno") >>> el.attributes <xml.dom.minidom.NamedNodeMap object at 0x10b6d6780> ``` or is it supposed to test something like ``` >>> el.attributes.items() [('def', 'ghi'), ('jkl', 'mno')] ``` This is slightly confusing. And the missing docstrings are not making it easier. # Tests which do not really test the module(?) I think for example about this, which is testing that `del` is working, but it doesn't have anything to do with the DOM. ``` def testDeleteAttr(self): dom = Document() child = dom.appendChild(dom.createElement("abc")) self.confirm(len(child.attributes) == 0) child.setAttribute("def", "ghi") self.confirm(len(child.attributes) == 1) del child.attributes["def"] self.confirm(len(child.attributes) == 0) dom.unlink() ``` https://github.com/python/cpython/blob/3e04cd268ee9a57f95dc78d8974b21a6fac3f666/Lib/test/test_minidom.py#L285-L294 Specifically when there is a function for it: `removeAttribute` https://www.w3.org/TR/2000/REC-DOM-Level-2-Core-20001113/core.html#ID-6D6AC0F9 which is tested just below that test. https://github.com/python/cpython/blob/3e04cd268ee9a57f95dc78d8974b21a6fac3f666/Lib/test/test_minidom.py#L296-L305 so I guess these should be removed or do I miss something in the testing logic? # Missing docstrings. Both the testing module and the module lack a lot of docstrings. Would it be good to fix this too, probably in a separate commit. # DOM Level 2 So the module intent is to implement DOM Level 2. but does that make sense in the light of https://dom.spec.whatwg.org/ Should minidom tries to follow the current DOM spec? ---------- nosy: +karlcow _______________________________________ Python tracker <rep...@bugs.python.org> <https://bugs.python.org/issue19683> _______________________________________ _______________________________________________ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
