Re: [Qemu-devel] [PATCH RFC v4 05/32] tests/qapi-schema: Convert test harness to QAPISchemaVisitor

[Markus Armbruster]

Eric Blake <address@hidden> writes:

> On 09/03/2015 08:29 AM, Markus Armbruster wrote:
>> The old code prints the result of parsing (list of expression
>> dictionaries), and partial results of semantic analysis (list of enum
>> dictionaries, list of struct dictionaries).
>> 
>> The new code prints a trace of a schema visit, i.e. what the back-ends
>> are going to use.  Built-in and array types are omitted, because
>> they're boring.
>> 
>> Signed-off-by: Markus Armbruster <address@hidden>
>> Reviewed-by: Eric Blake <address@hidden>
>> ---
>
>> +++ b/tests/qapi-schema/test-qapi.py
>> @@ -15,11 +15,35 @@ from pprint import pprint
>>  import os
>>  import sys
>>  
>> -try:
>> -    exprs = QAPISchema(sys.argv[1]).get_exprs()
>> -except SystemExit:
>> -    raise
>
> Don't we still need a try wrapped around...
>
>> +schema = QAPISchema(sys.argv[1])
>> +schema.visit(QAPISchemaTestVisitor())
>
> ...this, to guarantee non-zero exit status if an exception caused by
> refactoring breaks the pretty-printing?  (It happened to me when I tried
> to rename to _tag_name, and my only indication that something was wrong
> was that qapi-tests/qapi-schema-test.out was truncated due to this
> script aborting early after trying to access the old spelling tag_name).

The new version doesn't catch exceptions, and uncaught exceptions
terminate the program unsucsessfully.  Anything wrong with that?

The old version hides exceptions other than SystemExit.  Why?  I believe
it's mostly historical accident.  Accidents mostly cause by me.

I created the file in commit 9862657.  I wrapped parse_schema() in a
misguided attempt to keep the .err files concise:

    try:
        exprs = parse_schema(sys.stdin)
    except:
        print >>sys.stderr, "Crashed:", sys.exc_info()[0]
        exit(1)

This misreported errors as crashes, so I amended it in commit 2caba36:

    try:
        exprs = parse_schema(sys.stdin)
    except SystemExit:
        raise
    except:
        print >>sys.stderr, "Crashed:", sys.exc_info()[0]
        exit(1)

Lluís then ripped out the original misguided blanket exception catching,
but left in the amend (commit 98c1200):

    try:
        exprs = parse_schema(sys.stdin)
    except SystemExit:
        raise