This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

Author karlcow
Recipients Julian.Gindi, ajitesh.gupta, karlcow, mark.dickinson, python-dev, r.david.murray, serhiy.storchaka, zach.ware
Date 2019-10-03.00:47:54
SpamBayes Score -1.0
Marked as misclassified Yes
Message-id <1570063676.44.0.221790770593.issue19683@roundup.psfhosted.org>
In-reply-to
Content
@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?
History
Date User Action Args
2019-10-03 00:47:56karlcowsetrecipients: + karlcow, mark.dickinson, r.david.murray, python-dev, zach.ware, serhiy.storchaka, Julian.Gindi, ajitesh.gupta
2019-10-03 00:47:56karlcowsetmessageid: <1570063676.44.0.221790770593.issue19683@roundup.psfhosted.org>
2019-10-03 00:47:56karlcowlinkissue19683 messages
2019-10-03 00:47:54karlcowcreate