On Wed, Mar 14, 2012 at 11:44 AM, Greg Stein <gst...@gmail.com> wrote:
> I don't see how this affects the logging at all. Before/after, you're
> still calling _log_exception(), but now without a specified message.
That isn't completely true. _log_exception() only gets called once
per exception.
The original code was:
except Failure, ex:
result = svntest.testcase.RESULT_FAIL
# We captured Failure and its subclasses. We don't want to print
# anything for plain old Failure since that just indicates test
# failure, rather than relevant information. However, if there
# *is* information in the exception's arguments, then print it.
if ex.__class__ != Failure or ex.args:
ex_args = str(ex)
print('CWD: %s' % os.getcwd())
if ex_args:
print('EXCEPTION: %s: %s' % (ex.__class__.__name__, ex_args))
else:
print('EXCEPTION: %s' % ex.__class__.__name__)
traceback.print_exc(file=sys.stdout)
sys.stdout.flush()
It might not be readily apparent, but the indentation is such that the
traceback.print_exc() is called (only once) outside of the if-block.
The new code (after r1300623) is:
except Failure, ex:
result = svntest.testcase.RESULT_FAIL
# We captured Failure and its subclasses. We don't want to print
# anything for plain old Failure since that just indicates test
# failure, rather than relevant information. However, if there
# *is* information in the exception's arguments, then print it.
if ex.__class__ != Failure or ex.args:
ex_args = str(ex)
logger.warn('CWD: %s' % os.getcwd())
if ex_args:
_log_exception('EXCEPTION: %s: %s',
ex.__class__.__name__, ex_args)
else:
_log_exception('EXCEPTION: %s', ex.__class__.__name__)
else:
_log_exception()
To log the exception with the message, we moved the _log_exception
inside if-block, meaning that the traceback is no longer printed
unconditionally. To fix it, I added the extra _log_exception() so
that _log_exception() is now called just once no matter the branch
taken through the if-block.
Granted, it could probably be cleaner, no doubt. But it's still correct.
-Hyrum
> In fact, won't this (now) generate an empty log message, followed by
> the traceback?
>
> Maybe you need to do something like:
>
> msg = traceback.format_exc()
> if fmt:
> logger.warn(fmt, *args)
> logger.warn(msg)
>
> It may be that the first logger.warn() erased the traceback information.
>
> Cheers,
> -g
>
> On Wed, Mar 14, 2012 at 12:32, <hwri...@apache.org> wrote:
>> Author: hwright
>> Date: Wed Mar 14 16:32:20 2012
>> New Revision: 1300623
>>
>> URL: http://svn.apache.org/viewvc?rev=1300623&view=rev
>> Log:
>> Make sure we print exception stack on all test failures.
>>
>> * subversion/tests/cmdline/svntest/main.py
>> (_log_exception): Don't require a message.
>> (TestRunner.run): Log the exception stack on a generic failure.
>>
>> 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=1300623&r1=1300622&r2=1300623&view=diff
>> ==============================================================================
>> --- subversion/trunk/subversion/tests/cmdline/svntest/main.py (original)
>> +++ subversion/trunk/subversion/tests/cmdline/svntest/main.py Wed Mar 14
>> 16:32:20 2012
>> @@ -994,7 +994,7 @@ def use_editor(func):
>> os.environ['SVNTEST_EDITOR_FUNC'] = func
>> os.environ['SVN_TEST_PYTHON'] = sys.executable
>>
>> -def _log_exception(fmt, *args):
>> +def _log_exception(fmt='', *args):
>> logger.warn(fmt, *args)
>> logger.warn(traceback.format_exc())
>>
>> @@ -1345,6 +1345,8 @@ class TestRunner:
>> ex.__class__.__name__, ex_args)
>> else:
>> _log_exception('EXCEPTION: %s', ex.__class__.__name__)
>> + else:
>> + _log_exception()
>> except KeyboardInterrupt:
>> logger.error('Interrupted')
>> sys.exit(0)
>>
>>
--
uberSVN: Apache Subversion Made Easy
http://www.uberSVN.com/
