Re: [KVM-AUTOTEST PATCH v2 3/6] [RFC] Introduce exception context strings

[Michael Goldish <mgoldish@redhat.com>]

On 01/05/2011 06:21 PM, Avi Kivity wrote:
> On 01/05/2011 06:12 PM, Avi Kivity wrote:
>> On 01/05/2011 05:45 PM, Michael Goldish wrote:
>>> In complex tests (KVM) an exception string is often not informative
>>> enough and
>>> the traceback and source code have to be examined in order to figure
>>> out what
>>> caused the exception.  Context strings are a way for tests to provide
>>> information about what they're doing, so that when an exception is
>>> raised, this
>>> information will be embedded in the exception string.  The result is
>>> a concise
>>> yet highly informative exception string, which should make it very
>>> easy to
>>> figure out where/when the exception was raised.
>>>
>>> A typical example for a test where this may be useful is KVM's reboot
>>> test.
>>> Some exceptions can be raised either before or after the VM is
>>> rebooted (e.g.
>>> logging into the guest can fail) and whether they are raised before
>>> or after
>>> is critical to the understanding of the failure.  Normally the
>>> traceback would
>>> have to be examined, but the proposed method makes it easy to know
>>> where the
>>> exception is raised without doing so.  To achieve this, the reboot
>>> test should
>>> place calls to error.context() as follows:
>>>
>>> error.context("before reboot")
>>> <carry out pre-reboot actions>
>>> error.context("sending reboot command")
>>> <send the reboot command>
>>> error.context("after reboot")
>>> <carry out post-reboot actions>
>>>
>>> If login fails in the pre-reboot section, the resulting exception
>>> string can
>>> can have something like "context: before reboot" embedded in it. 
>>> (The actual
>>> embedding is done in the next patch in the series.)
>>
>> It would be nice to make the error context a stack, and to use the
>> with statement to manage the stack:
>>
>>
>>    with error.context("main test"):
>>        foo()
>>        with error.context("before reboot"):
>>            bar()
>>
>> If foo() throws an exception, the context would be "main test", while
>> if bar() throws an exception, the context would be "before reboot" in
>> "main test".

This seems like the best solution and it's unfortunate that we can't use it.

> btw, you can have a decorator for enclosing an entire function in an
> error context:
> 
>    @function_error_context('migration test')
>    def migration_test(...):
>        ...
> 
> anything in migration_test() is enclosed in that context.  But we're
> just repeating the ordinary stack trace with something more readable.

The problem is that the string passed to function_error_context can't be
based on function parameters, so declaring a context like 'migrating
vm1' is impossible.

I do think we can benefit from 2 context levels per function though:

@context_aware
def migrate(...):
    base_context("migrating %s" % vm.name)
    context("collecting parameters")
    ...
    context("sending monitor command")
    ...
    context("cleanup")
    ...

base_context() and context() will just be joined together using ' --> '
like regular contexts.  base_context() can be useful for long utility
functions.  Does this sound like a reasonable solution, or do you think
it's cleaner to always define a new nested function for each context level?