Re: [Python-Dev] Question over splitting unittest into a package

2009/12/30 Martin (gzlist) <gzlist@googlemail.com>:
Hi Benjamin,
Hi!
In rev 74094 of Python, you split the unittest module up, could you point me at any bug entries or discussion over this revision so I can catch up?
This was mostly a discussion on IRC between Michael Foord and myself.
As a side-effect you seem to have changed the method of marking a module as not worth including in a traceback to be no longer extensible.
Before: <http://svn.python.org/view/python/trunk/Lib/unittest.py?view=markup&pathrev=74094>
A global was set at the top of the module:
__unittest = 1
Which is then checked for when constructing traceback output:
def _is_relevant_tb_level(self, tb): return '__unittest' in tb.tb_frame.f_globals
After: <http://svn.python.org/view/python/trunk/Lib/unittest/__init__.py?revision=74095&view=markup>
def _is_relevant_tb_level(self, tb): globs = tb.tb_frame.f_globals is_relevant = '__name__' in globs and \ globs["__name__"].startswith("unittest") del globs return is_relevant
Only packages actually named "unittest" can be excluded.
What is now the prefered method of marking a module as test-internal? Overriding the leading-underscore _is_relevant_tb_level method? How can this be done cooperatively by different packages?
When I made that change, I didn't know that the __unittest "hack" was being used elsewhere outside of unittest, so I felt fine replacing it with another. While I still consider it an implementation detail, I would be ok with exposing an "official" API for this. Perhaps __unittest_ignore_traceback?
I would have CCed a mailinglist with this question but don't like getting yelled at for posting on the wrong one, please feel free to do so with your reply if you feel it's appropriate (the CCing, not the yelling).
python-dev is perfect for this discussion. -- Regards, Benjamin

On Wed, Dec 30, 2009 at 3:04 PM, Benjamin Peterson <benjamin@python.org> wrote:
2009/12/30 Martin (gzlist) <gzlist@googlemail.com>:
Hi Benjamin,
Hi!
In rev 74094 of Python, you split the unittest module up,
+1
could you point me at any bug entries or discussion over this revision so I can catch up?
This was mostly a discussion on IRC between Michael Foord and myself.
... and there was a previous thread about that some time ago here in python-dev ;o)
def _is_relevant_tb_level(self, tb): return '__unittest' in tb.tb_frame.f_globals
After: <http://svn.python.org/view/python/trunk/Lib/unittest/__init__.py?revision=74095&view=markup>
def _is_relevant_tb_level(self, tb): globs = tb.tb_frame.f_globals is_relevant = '__name__' in globs and \ globs["__name__"].startswith("unittest") del globs return is_relevant
Had not seen this ... Hmmm ... -1
Only packages actually named "unittest" can be excluded.
What is now the prefered method of marking a module as test-internal? Overriding the leading-underscore _is_relevant_tb_level method? How can this be done cooperatively by different packages?
When I made that change, I didn't know that the __unittest "hack" was being used elsewhere outside of unittest, so I felt fine replacing it with another. While I still consider it an implementation detail, I would be ok with exposing an "official" API for this. Perhaps __unittest_ignore_traceback?
Hmmm ... One of the issues I didn't notice ... IMO +1 for leaving it as it was before (i.e. __unittest) because : - the double underscore should mean (CMIIW) that it's an implementation detail - not few libs use that name already ;o) +0.5 for adding `__unittest_ignore_traceback` or something shorter (e.g. `__unittest_ignore`) too ... +1 for documenting that «hack» PS: `assertRaises` context managers are great !!! BTW ;o) -- Regards, Olemis. Blog ES: http://simelo-es.blogspot.com/ Blog EN: http://simelo-en.blogspot.com/ Featured article: Assessment of unittest 2.7 API : new features and opinions - http://feedproxy.google.com/~r/simelo-en/~3/cVOgG8NIBFY/assessment-of-unitte...

2009/12/30 Olemis Lang <olemis@gmail.com>:
Hmmm ... One of the issues I didn't notice ...
IMO +1 for leaving it as it was before (i.e. __unittest) because :
- the double underscore should mean (CMIIW) that it's an implementation detail - not few libs use that name already ;o)
+0.5 for adding `__unittest_ignore_traceback` or something shorter (e.g. `__unittest_ignore`) too ...
+1 for documenting that «hack»
Someone should probably file a tracker request about this. -- Regards, Benjamin

Olemis Lang wrote:
PS: `assertRaises` context managers are great !!! BTW ;o)
The detailed comparison methods added for 2.7 are really nice too. It's great getting error messages that tell me what I broke rather than "you broke it!" :) Cheers, Nick. -- Nick Coghlan | ncoghlan@gmail.com | Brisbane, Australia ---------------------------------------------------------------

Thanks for the quick response. On 30/12/2009, Benjamin Peterson <benjamin@python.org> wrote:
When I made that change, I didn't know that the __unittest "hack" was being used elsewhere outside of unittest, so I felt fine replacing it with another. While I still consider it an implementation detail, I would be ok with exposing an "official" API for this. Perhaps __unittest_ignore_traceback?
Well, bazaar has had the trick for a couple of years, and googling around now turns up some other projects using it or thinking about it: <http://github.com/gfxmonk/mocktest/commit/b5e94f7ee06ab627cea2c9cf90d0aeb63e...> <http://bitbucket.org/uche/amara/changeset/eeaf69f48271/> <http://twistedmatrix.com/trac/ticket/4127> I get the impression Robert doesn't like it much though, and seemed to have wanted something more targeted in the past as well: <http://bugs.python.org/issue1705520> Reinstating the old implementation (with the same name) would mean that existing code would keep working with Python 2.7 but maybe a discussion could start about a new, less hacky, way of doing the same thing. May not be worthwhile making life more complicated though, there aren't *that* many unittest-extending projects. Martin

On Thu, Dec 31, 2009 at 10:30 AM, Martin (gzlist) <gzlist@googlemail.com> wrote:
Thanks for the quick response.
On 30/12/2009, Benjamin Peterson <benjamin@python.org> wrote:
When I made that change, I didn't know that the __unittest "hack" was being used elsewhere outside of unittest, so I felt fine replacing it with another. While I still consider it an implementation detail, I would be ok with exposing an "official" API for this. Perhaps __unittest_ignore_traceback?
Well, bazaar has had the trick for a couple of years, and googling around now turns up some other projects using it or thinking about it: <http://github.com/gfxmonk/mocktest/commit/b5e94f7ee06ab627cea2c9cf90d0aeb63e...> <http://bitbucket.org/uche/amara/changeset/eeaf69f48271/> <http://twistedmatrix.com/trac/ticket/4127>
Add `dutest` and probably `nose` [2]_ and ...
Reinstating the old implementation (with the same name) would mean that existing code would keep working with Python 2.7
IOW ... if it is removed all the libraries will have to change and check for the Py version and ... (probably not a big deal depending on the details of the «solution»)
but maybe a discussion could start about a new, less hacky, way of doing the same
I am strongly -1 for modifying the classes in «traditional» unittest module [2]_ (except that I am strongly +1 for the package structure WITHOUT TOUCHING anything else ...) , and the more I think about it I am more convinced ... but anyway, this not a big deal (and in the end what I think is not that relevant either ... :o). So ... pass
May not be worthwhile making life more complicated though, there aren't *that* many unittest-extending projects.
But every library extending `unittest` will do it or otherwise not-so-useful error messages will be displayed ;o) PS: Happy New Year ! BTW .. [1] [feature] nosexml should omit trace frames where `__unittest... (http://code.google.com/p/python-nosexml/issues/detail?id=4#c0) .. [2] Assessment of unittest 2.7 API : new features and opinions (http://simelo-en.blogspot.com/2009/12/assessment-of-unittest-27-api-new.html) -- Regards, Olemis. Blog ES: http://simelo-es.blogspot.com/ Blog EN: http://simelo-en.blogspot.com/ Featured article: Free new ripit 1.3.3 Download - mac software - http://feedproxy.google.com/~r/TracGViz-full/~3/KFwyUTKH0vI/

On Mon, Jan 4, 2010 at 9:24 AM, Olemis Lang <olemis@gmail.com> wrote:
On Thu, Dec 31, 2009 at 10:30 AM, Martin (gzlist) <gzlist@googlemail.com> wrote:
Thanks for the quick response.
On 30/12/2009, Benjamin Peterson <benjamin@python.org> wrote:
but maybe a discussion could start about a new, less hacky, way of doing the same
I am strongly -1 for modifying the classes in «traditional» unittest module [2]_ (except that I am strongly +1 for the package structure WITHOUT TOUCHING anything else ...) , and the more I think about it I am more convinced ... but anyway, this not a big deal (and in the end what I think is not that relevant either ... :o). So ...
IOW, if I had all the freedom to implement it, after the pkg structure I'd do something like : {{{ #!python class TestResult: # Everything just the same def _is_relevant_tb_level(self, tb): return '__unittest' in tb.tb_frame.f_globals class BetterTestResult(TestResult): # Further code ... maybe ;o) # def _is_relevant_tb_level(self, tb): # This or anything else you might want to do ;o) # globs = tb.tb_frame.f_globals is_relevant = '__name__' in globs and \ globs["__name__"].startswith("unittest") del globs return is_relevant }}} that's what inheritance is for ;o) ... but quite probably that's not gonna happen, just a comment . -- Regards, Olemis. Blog ES: http://simelo-es.blogspot.com/ Blog EN: http://simelo-en.blogspot.com/ Featured article: Ubuntu sustituye GIMP por F-Spot - http://feedproxy.google.com/~r/simelo-es/~3/-g48D6T6Ojs/ubuntu-sustituye-gim...
participants (4)
-
Benjamin Peterson
-
Martin (gzlist)
-
Nick Coghlan
-
Olemis Lang