You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by hw...@apache.org on 2012/03/13 05:55:26 UTC
svn commit: r1299972 -
/subversion/trunk/subversion/tests/cmdline/svntest/main.py
Author: hwright
Date: Tue Mar 13 04:55:26 2012
New Revision: 1299972
URL: http://svn.apache.org/viewvc?rev=1299972&view=rev
Log:
In the test suite, print out exceptions with logging level of WARNING, rather
than ERROR.
* subversion/tests/cmdline/svntest/main.py
(_log_exception): New helper.
(TestRunner): Use the helper to emit exception contents.
Modified:
subversion/trunk/subversion/tests/cmdline/svntest/main.py
Modified: subversion/trunk/subversion/tests/cmdline/svntest/main.py
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/cmdline/svntest/main.py?rev=1299972&r1=1299971&r2=1299972&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/cmdline/svntest/main.py (original)
+++ subversion/trunk/subversion/tests/cmdline/svntest/main.py Tue Mar 13 04:55:26 2012
@@ -36,6 +36,7 @@ import optparse
import xml
import urllib
import logging
+import cStringIO as StringIO
try:
# Python >=3.0
@@ -993,6 +994,15 @@ def use_editor(func):
os.environ['SVNTEST_EDITOR_FUNC'] = func
os.environ['SVN_TEST_PYTHON'] = sys.executable
+def _log_exception():
+ import traceback
+
+ o = StringIO.StringIO()
+ ei = sys.exc_info()
+ traceback.print_exception(ei[0], ei[1], ei[2], None, o)
+ logger.warn(o.getvalue())
+ o.close()
+
def mergeinfo_notify_line(revstart, revend, target=None):
"""Return an expected output line that describes the beginning of a
mergeinfo recording notification on revisions REVSTART through REVEND."""
@@ -1336,10 +1346,10 @@ class TestRunner:
ex_args = str(ex)
logger.warn('CWD: %s' % os.getcwd())
if ex_args:
- logger.exception('EXCEPTION: %s: %s' % (ex.__class__.__name__,
- ex_args))
+ logger.warn('EXCEPTION: %s: %s' % (ex.__class__.__name__, ex_args))
else:
- logger.exception('EXCEPTION: %s' % ex.__class__.__name__)
+ logger.warn('EXCEPTION: %s' % ex.__class__.__name__)
+ _log_exception()
except KeyboardInterrupt:
logger.error('Interrupted')
sys.exit(0)
@@ -1350,8 +1360,8 @@ class TestRunner:
except:
result = svntest.testcase.RESULT_FAIL
logger.warn('CWD: %s' % os.getcwd())
- logger.exception('UNEXPECTED EXCEPTION:')
- sys.stdout.flush()
+ logger.warn('UNEXPECTED EXCEPTION:')
+ _log_exception()
os.chdir(saved_dir)
exit_code, result_text, result_benignity = self.pred.results(result)
Re: svn commit: r1299972 - /subversion/trunk/subversion/tests/cmdline/svntest/main.py
Posted by Greg Stein <gs...@gmail.com>.
On Tue, Mar 13, 2012 at 09:45, Greg Stein <gs...@gmail.com> wrote:
>
> On Mar 13, 2012 9:34 AM, "Hyrum K Wright" <hy...@wandisco.com> wrote:
>>
>> On Tue, Mar 13, 2012 at 1:09 AM, Greg Stein <gs...@gmail.com> wrote:
>> > On Tue, Mar 13, 2012 at 00:55, <hw...@apache.org> wrote:
>> >>...
>> >> +++ subversion/trunk/subversion/tests/cmdline/svntest/main.py Tue Mar
>> >> 13 04:55:26 2012
>> >> @@ -36,6 +36,7 @@ import optparse
>> >> import xml
>> >> import urllib
>> >> import logging
>> >> +import cStringIO as StringIO
>> >
>> > No need to rename this module, as is typical with back-compat importing.
>>
>> This isn't backward compat, it's an alternative implementation that is
>> a drop-in replacement for the StringIO module, hence the desire to
>> alias it.
>
> Dude. I *know* what it is. My point is that it is silly to rename it when it
> has been a standard module for *years*. Nobody really uses StringIO any
> more. It is all cStringIO, possibly renamed thru import logic that is used
> for back-compat. The rename is superfluous gunk nowadays.
cStringIO has been around since 1998.
Re: svn commit: r1299972 - /subversion/trunk/subversion/tests/cmdline/svntest/main.py
Posted by Greg Stein <gs...@gmail.com>.
On Tue, Mar 13, 2012 at 10:12, Hyrum K Wright <hy...@wandisco.com> wrote:
>...
> I've no doubt *you* know what it is, but this is a public list. And
Ah. Didn't realize we'd moved from code review to public discussion :-P
Re: svn commit: r1299972 - /subversion/trunk/subversion/tests/cmdline/svntest/main.py
Posted by Hyrum K Wright <hy...@wandisco.com>.
On Tue, Mar 13, 2012 at 8:45 AM, Greg Stein <gs...@gmail.com> wrote:
>
> On Mar 13, 2012 9:34 AM, "Hyrum K Wright" <hy...@wandisco.com> wrote:
>>
>> On Tue, Mar 13, 2012 at 1:09 AM, Greg Stein <gs...@gmail.com> wrote:
>> > On Tue, Mar 13, 2012 at 00:55, <hw...@apache.org> wrote:
>> >>...
>> >> +++ subversion/trunk/subversion/tests/cmdline/svntest/main.py Tue Mar
>> >> 13 04:55:26 2012
>> >> @@ -36,6 +36,7 @@ import optparse
>> >> import xml
>> >> import urllib
>> >> import logging
>> >> +import cStringIO as StringIO
>> >
>> > No need to rename this module, as is typical with back-compat importing.
>>
>> This isn't backward compat, it's an alternative implementation that is
>> a drop-in replacement for the StringIO module, hence the desire to
>> alias it.
>
> Dude. I *know* what it is. My point is that it is silly to rename it when it
> has been a standard module for *years*. Nobody really uses StringIO any
> more. It is all cStringIO, possibly renamed thru import logic that is used
> for back-compat. The rename is superfluous gunk nowadays.
I've no doubt *you* know what it is, but this is a public list. And
out test suite still uses vanilla StringIO, in both factory.py and
tree.py (though I plan to change that shortly).
This feels like a bikeshed, and I'll shortly do away with it by
importing the StringIO class from $MODULE, as we do in other modules
in our tree.
-Hyrum
--
uberSVN: Apache Subversion Made Easy
http://www.uberSVN.com/
Re: svn commit: r1299972 - /subversion/trunk/subversion/tests/cmdline/svntest/main.py
Posted by Greg Stein <gs...@gmail.com>.
On Mar 13, 2012 9:34 AM, "Hyrum K Wright" <hy...@wandisco.com> wrote:
>
> On Tue, Mar 13, 2012 at 1:09 AM, Greg Stein <gs...@gmail.com> wrote:
> > On Tue, Mar 13, 2012 at 00:55, <hw...@apache.org> wrote:
> >>...
> >> +++ subversion/trunk/subversion/tests/cmdline/svntest/main.py Tue Mar
13 04:55:26 2012
> >> @@ -36,6 +36,7 @@ import optparse
> >> import xml
> >> import urllib
> >> import logging
> >> +import cStringIO as StringIO
> >
> > No need to rename this module, as is typical with back-compat importing.
>
> This isn't backward compat, it's an alternative implementation that is
> a drop-in replacement for the StringIO module, hence the desire to
> alias it.
Dude. I *know* what it is. My point is that it is silly to rename it when
it has been a standard module for *years*. Nobody really uses StringIO any
more. It is all cStringIO, possibly renamed thru import logic that is used
for back-compat. The rename is superfluous gunk nowadays.
-g
Re: svn commit: r1299972 - /subversion/trunk/subversion/tests/cmdline/svntest/main.py
Posted by Hyrum K Wright <hy...@wandisco.com>.
On Tue, Mar 13, 2012 at 1:09 AM, Greg Stein <gs...@gmail.com> wrote:
> On Tue, Mar 13, 2012 at 00:55, <hw...@apache.org> wrote:
>>...
>> +++ subversion/trunk/subversion/tests/cmdline/svntest/main.py Tue Mar 13 04:55:26 2012
>> @@ -36,6 +36,7 @@ import optparse
>> import xml
>> import urllib
>> import logging
>> +import cStringIO as StringIO
>
> No need to rename this module, as is typical with back-compat importing.
This isn't backward compat, it's an alternative implementation that is
a drop-in replacement for the StringIO module, hence the desire to
alias it. Though I've also noticed that in some places we *do* import
just the StringIO class from those modules, and I'm tempted to do that
here for consistency.
Actually, it's moot because of the other changes made to fix the
following issues.
>
>>
>> try:
>> # Python >=3.0
>> @@ -993,6 +994,15 @@ def use_editor(func):
>> os.environ['SVNTEST_EDITOR_FUNC'] = func
>> os.environ['SVN_TEST_PYTHON'] = sys.executable
>>
>> +def _log_exception():
>> + import traceback
>> +
>> + o = StringIO.StringIO()
>> + ei = sys.exc_info()
>> + traceback.print_exception(ei[0], ei[1], ei[2], None, o)
>> + logger.warn(o.getvalue())
>> + o.close()
>
> You could simplify: logger.warn(traceback.format_exc())
>
> I would also recommend importing traceback at the module level, rather
> than within this function. I've seen cases where the "import
> traceback" will *overwrite* the exception registered on the current
> thread. Something as simple as Python looking for the traceback module
> and doing an open("/some/weird/path/traceback.py") and getting an
> IOError. The above code likely *happens* to work because traceback is
> already registered in sys.modules, so a true import doesn't have to
> happen.
>
> I would also recommend that _log_exception() takes parameters so that
> you can collapse the logging into a single invocation, all going to
> logger.warn(). The current code allows the caller and this function to
> (accidentally) use different logging levels.
>
> def _log_exception(fmt, *args):
> logger.warn(fmt, *args)
> ...
This stuff fixed in r1300118.
-Hyrum
--
uberSVN: Apache Subversion Made Easy
http://www.uberSVN.com/
Re: svn commit: r1299972 - /subversion/trunk/subversion/tests/cmdline/svntest/main.py
Posted by Greg Stein <gs...@gmail.com>.
On Tue, Mar 13, 2012 at 00:55, <hw...@apache.org> wrote:
>...
> +++ subversion/trunk/subversion/tests/cmdline/svntest/main.py Tue Mar 13 04:55:26 2012
> @@ -36,6 +36,7 @@ import optparse
> import xml
> import urllib
> import logging
> +import cStringIO as StringIO
No need to rename this module, as is typical with back-compat importing.
>
> try:
> # Python >=3.0
> @@ -993,6 +994,15 @@ def use_editor(func):
> os.environ['SVNTEST_EDITOR_FUNC'] = func
> os.environ['SVN_TEST_PYTHON'] = sys.executable
>
> +def _log_exception():
> + import traceback
> +
> + o = StringIO.StringIO()
> + ei = sys.exc_info()
> + traceback.print_exception(ei[0], ei[1], ei[2], None, o)
> + logger.warn(o.getvalue())
> + o.close()
You could simplify: logger.warn(traceback.format_exc())
I would also recommend importing traceback at the module level, rather
than within this function. I've seen cases where the "import
traceback" will *overwrite* the exception registered on the current
thread. Something as simple as Python looking for the traceback module
and doing an open("/some/weird/path/traceback.py") and getting an
IOError. The above code likely *happens* to work because traceback is
already registered in sys.modules, so a true import doesn't have to
happen.
I would also recommend that _log_exception() takes parameters so that
you can collapse the logging into a single invocation, all going to
logger.warn(). The current code allows the caller and this function to
(accidentally) use different logging levels.
def _log_exception(fmt, *args):
logger.warn(fmt, *args)
...
>...