classification
Title: Migrate test_minidom.py to unittest
Type: Stage:
Components: Library (Lib) Versions: Python 2.6
process
Status: closed Resolution: rejected
Dependencies: Superseder:
Assigned To: Nosy List: collinwinter, jorend, nnorwitz (3)
Priority: normal Keywords: patch

Created on 2007-03-30 04:15 by jorend, last changed 2007-04-20 11:48 by jorend.

Files
File name Uploaded Description Edit Remove
test_minidom.patch jorend, 2007-03-30 04:15 Patch for Lib/test/test_minidom.py *and* Lib/xml/dom/expatbuilder.py
test_minidom.second.patch jorend, 2007-04-01 21:18 Patch for Lib/test/test_minidom.py (based on r54643)
expatbuilder.second.patch jorend, 2007-04-01 21:18 Patch for Lib/xml/dom/expatbuilder.py (based on r54643)
Messages (7)
msg52335 - (view) Author: Jason Orendorff (jorend) Date: 2007-03-30 04:15 
Update Lib/test/test_minidom.py to use the unittest module.

Also:  I noticed the test_minidom.py contains some code (now long dead) to check for reference cycles.  The code is active only when Node.allnodes exists; this hasn't existed for over four years now.

After re-implementing that test using gc.DEBUG_SAVEALL, I found that many tests failed because of missing unlink() calls.  I added them.

After that, one test was still failing.  This is because expatbuilder.parseString() and friends do not clean up some cyclic references on error.  The patch includes a fix for that (in Lib/xml/dom/expatbuilder.py).  An alternative would be to ditch the cycle-checking test entirely.  I can submit a patch that does that instead, if desired.
msg52336 - (view) Author: Collin Winter (collinwinter) Date: 2007-03-30 06:49 
test_minidom already uses unittest (as of patch #1683397, SVN r54603). Could you update your patch to the latest trunk revision?
msg52337 - (view) Author: Jason Orendorff (jorend) Date: 2007-04-01 21:18 
OK, I've attached two new patches.

test_minidom.second.patch - Changes "confirm" to "assert_" or "assertEquals", breaks multi-line assertions into many one-line assertions, adds convenience methods "assertSameNode" and "assertNotSameNode", etc.  Also replaces the Node.allnodes test with gc-based test and adds a few calls to unlink().

expatbuilder.second.patch - Fixes the test failure discovered by the new gc testing.

File Added: test_minidom.second.patch
msg52338 - (view) Author: Jason Orendorff (jorend) Date: 2007-04-01 21:18 
File Added: expatbuilder.second.patch
msg52339 - (view) Author: Jason Orendorff (jorend) Date: 2007-04-05 02:01 
Collin:  Thanks for taking the time to look at this.

I don't think this change is worth making.  I've started working on some new features in minidom and it seems like every time I exercise a new piece of code, the gc finds a small number of objects in cycles.  For example, parseFragmentString() seems to leave a few objects (even with expatbuilder.second.patch).  We're just going to have to live with it--  I don't think anyone has time or desire to hunt down them all down.
msg52340 - (view) Author: Neal Norwitz (nnorwitz) Date: 2007-04-19 06:29 
Jason, can you clarify your last comment.  Wrt the "change [not] worth making", do you mean this patch or a different change?  I just want to know what in this patch is still active and needs review before spending time.  Thanks.
msg52341 - (view) Author: Jason Orendorff (jorend) Date: 2007-04-20 11:48 
There's nothing active here.  This should just be Closed Rejected.
History
Date User Action Args
2007-03-30 04:15:07jorendcreate