[issue19683] test_minidom has many empty tests

Zachary Ware report at bugs.python.org
Thu Dec 26 16:03:04 CET 2013


Zachary Ware added the comment:

Some refactoring of the tests is certainly acceptable.  If there are some tests that can be merged together, it is fine to do so; also for removing ones that don't make any sense (it's not like they've ever tested anything anyway :)).  We don't have anyone listed as an expert on xml.dom.minidom (or the xml package as a whole), so we kind of have to just muddle along on our own with this.  Any tests you come up with to fill the empty ones will be better than no tests at all.  If someone with more experience with minidom comes along later and improves your tests, that will be great; but considering how long there have been this many empty tests in the file, I don't think that's terribly likely.

In fact, having looked at the test module in a bit more detail, it's in pretty sore need of an overall modernization.  The 'confirm' method is just a thin wrapper around assertTrue with an extremely unhelpful default message, and is used almost exclusively for all tests in the file.  So currently if anything breaks the tests will say "this failed, but I won't tell you why.  Good luck figuring it out!"  'confirm' should be removed, and all of the huge conditions passed into it throughout the file should be converted into individual assert*() calls.  It also looks like we could make use of setUp/tearDown to eliminate a lot of repetition (such as creating a base Document and subsequently removing it).

----------

_______________________________________
Python tracker <report at bugs.python.org>
<http://bugs.python.org/issue19683>
_______________________________________


More information about the Python-bugs-list mailing list