Issue 15083: Rewrite ElementTree tests in a cleaner and safer way (original) (raw)

Created on 2012-06-16 03:52 by eli.bendersky, last changed 2022-04-11 14:57 by admin. This issue is now closed.

Files
File name Uploaded Description Edit
test_xml_etree.patch serhiy.storchaka,2013-02-25 11:17 review
Pull Requests
URL Status Linked Edit
PR 906 merged serhiy.storchaka,2017-03-30 14:49
Messages (14)
msg162952 - (view) Author: Eli Bendersky (eli.bendersky) * (Python committer) Date: 2012-06-16 03:52
As #15075 demonstrated, the ET tests are sensitive to execution order because of the way they operate. Two sets of tests (one for the C module and one for the pure Python module) operate from the same test code, monkey-patching the imported module. This sometimes breaks. A more robust solution is needed that is completely deterministic and does not rely on import artifacts.
msg179801 - (view) Author: Eli Bendersky (eli.bendersky) * (Python committer) Date: 2013-01-12 14:39
It should be noted that the doctests complicate things considerably, and should be rewritten to be unittest, which are easier to manipulate in terms of modules used.
msg179815 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2013-01-12 15:45
New changeset f9d1d120c19e by Eli Bendersky in branch '3.3': Issues #15083 and #16992: port find.* method tests to unittest http://hg.python.org/cpython/rev/f9d1d120c19e New changeset 18b16104166c by Eli Bendersky in branch 'default': Issues #15083 and #16992: port find.* method tests to unittest http://hg.python.org/cpython/rev/18b16104166c
msg182929 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2013-02-25 11:17
Here is a patch which converts all etree doctests to unittests.
msg182937 - (view) Author: Eli Bendersky (eli.bendersky) * (Python committer) Date: 2013-02-25 13:59
Serhiy, thanks for working on this! I didn't read the whole patch yet - will tinker with it a bit more when applying. Did you prepare the patch vs. 3.3 or default? The two are still synced and I'd be happy to apply it to both branches. Now, the real reason I wanted to get rid of doctests is to make the global environment clean in test_xml_etree. The next logical step is to make all test classes in test_xml_etree accept the ET module in some way and store it, using it to get classes & function. I.e. no more global "ET" and "pyET" at all. Would you like to add this to your patch?
msg182942 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2013-02-25 14:11
This patch is for default branch. It applied to 3.3 too. > The next logical step is to make all test classes in test_xml_etree accept the ET module in some way and store it, using it to get classes & function. I.e. no more global "ET" and "pyET" at all. Would you like to add this to your patch? I was going to do this on the next step, but if you prefer I can prepare a single patch. But it seems to me that more simple patches are easier to review.
msg182944 - (view) Author: Eli Bendersky (eli.bendersky) * (Python committer) Date: 2013-02-25 14:26
No problems. You can go ahead and commit this patch to 3.3 and default, then. I will review it post-commit, since I wanted to tweak some things around anyway.
msg182947 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2013-02-25 15:23
New changeset af570205b978 by Serhiy Storchaka in branch '3.3': Issue #15083: Convert ElementTree doctests to unittests. http://hg.python.org/cpython/rev/af570205b978 New changeset 5eefc85b8be8 by Serhiy Storchaka in branch 'default': Issue #15083: Convert ElementTree doctests to unittests. http://hg.python.org/cpython/rev/5eefc85b8be8
msg183058 - (view) Author: Eli Bendersky (eli.bendersky) * (Python committer) Date: 2013-02-26 14:04
Great, thanks. Now looking forward to the patch getting rid of the module-level globals. One idea is explicitly pass the module into each testing class. The classes should not rely on anything global in this respect.
msg183110 - (view) Author: Ezio Melotti (ezio.melotti) * (Python committer) Date: 2013-02-27 04:08
> The next logical step is to make all test classes in test_xml_etree > accept the ET module in some way and store it, using it to get classes > & function. I.e. no more global "ET" and "pyET" at all. The idiom suggested by PEP 399 has the two modules (cmod and pymod) as globals, and then simply sets them as class attributes. Is there any reason why this should be avoided in this case?
msg183111 - (view) Author: Eli Bendersky (eli.bendersky) * (Python committer) Date: 2013-02-27 04:40
On Tue, Feb 26, 2013 at 8:08 PM, Ezio Melotti <report@bugs.python.org>wrote: > > Ezio Melotti added the comment: > > > The next logical step is to make all test classes in test_xml_etree > > accept the ET module in some way and store it, using it to get classes > > & function. I.e. no more global "ET" and "pyET" at all. > > The idiom suggested by PEP 399 has the two modules (cmod and pymod) as > globals, and then simply sets them as class attributes. Is there any > reason why this should be avoided in this case? > I'm just not sure how the globals help except for saving 2-3 lines of code. I do see how they can cause problems because even when I just want to run the pure Python code, the C module gets imported. Why should it be? I really don't want it to, I want to isolate things as much as possible (after all, testing the pure Python module actually tests a scenario where there is no C module). Pickle is one concrete place that can cause problems with this. We talked about related things in Issue #15083 and AFAIR Eric's and Brett's proposals move these modules away from the global namespace.
msg183112 - (view) Author: Ezio Melotti (ezio.melotti) * (Python committer) Date: 2013-02-27 05:12
> I'm just not sure how the globals help It doesn't, but if it works fine there's probably no reason to complicate (if it's complicated at all) things to change this. > even when I just want to run the pure Python code, the C module gets > imported. Why should it be? Doesn't the test always run both? (assuming the C module is available -- if it's not it won't be imported anyway) > Pickle is one concrete place that can cause problems with this. This might be an actual reason to avoid globals, but I don't know the details.
msg290844 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2017-03-30 14:55
It is hard to backport bugfixes to 2.7 since tests are too different. Due to this I backported some bugfixes without tests and omitted backporting other bugfixes. PR 906 converts doctests in 2.7 to unittests. This will help backporting bugfixes too much. Actually I have backported tests from 3.5 and checked that all old tests are present in new tests. Perhaps I found a bug in ElementTree in 2.7. Will open an issue after merging tests.
msg291037 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2017-04-02 13:55
New changeset 68903b656d4e1011525a46cbd1338c6cbab83d6d by Serhiy Storchaka in branch '2.7': bpo-15083: Convert ElementTree doctests to unittests. (#906) https://github.com/python/cpython/commit/68903b656d4e1011525a46cbd1338c6cbab83d6d
History
Date User Action Args
2022-04-11 14:57:31 admin set github: 59288
2017-04-02 14:26:36 serhiy.storchaka set status: open -> closedstage: patch review -> resolvedresolution: fixedversions: + Python 2.7
2017-04-02 13:55:45 serhiy.storchaka set messages: +
2017-03-30 15:03:26 serhiy.storchaka link issue29948 dependencies
2017-03-30 14:56:47 serhiy.storchaka link issue27863 dependencies
2017-03-30 14:55:08 serhiy.storchaka set messages: +
2017-03-30 14:49:14 serhiy.storchaka set pull_requests: + <pull%5Frequest806>
2013-02-27 05:12:30 ezio.melotti set messages: +
2013-02-27 04:40:59 eli.bendersky set messages: +
2013-02-27 04:08:52 ezio.melotti set messages: +
2013-02-26 14:04:06 eli.bendersky set messages: +
2013-02-25 15:23:11 python-dev set messages: +
2013-02-25 14:26:52 eli.bendersky set versions: + Python 3.3
2013-02-25 14:26:46 eli.bendersky set messages: +
2013-02-25 14:11:16 serhiy.storchaka set messages: +
2013-02-25 13:59:09 eli.bendersky set messages: +
2013-02-25 11:17:38 serhiy.storchaka set files: + test_xml_etree.patchnosy: + serhiy.storchakamessages: + keywords: + patchstage: needs patch -> patch review
2013-01-12 15:45:10 python-dev set nosy: + python-devmessages: +
2013-01-12 14:39:23 eli.bendersky set messages: +
2012-12-30 17:05:38 asvetlov set nosy: + asvetlov
2012-12-30 00:22:24 Arfrever set nosy: + Arfrever
2012-12-29 16:20:13 danielsh set nosy: + danielsh
2012-07-15 03:45:42 eli.bendersky set versions: - Python 3.3
2012-06-22 18:28:12 tshepang set nosy: + tshepang
2012-06-22 16:09:46 ezio.melotti set nosy: + ezio.melotti
2012-06-16 03:53:16 eli.bendersky link issue15075 superseder
2012-06-16 03:52:39 eli.bendersky create