qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH v6 2/4] qapi: Use an explicit input file


From: Lluís Vilanova
Subject: Re: [Qemu-devel] [PATCH v6 2/4] qapi: Use an explicit input file
Date: Tue, 01 Apr 2014 16:05:03 +0200
User-agent: Gnus/5.13 (Gnus v5.13) Emacs/24.3 (gnu/linux)

Eric Blake writes:

> On 03/31/2014 01:16 PM, Lluís Vilanova wrote:
[...]
>> @@ -368,7 +368,8 @@ check-tests/test-qapi.py: tests/test-qapi.py
>> 
>> .PHONY: $(patsubst %, check-%, $(check-qapi-schema-y))
>> $(patsubst %, check-%, $(check-qapi-schema-y)): check-%.json: 
>> $(SRC_PATH)/%.json
>> -    $(call quiet-command, PYTHONPATH=$(SRC_PATH)/scripts $(PYTHON) 
>> $(SRC_PATH)/tests/qapi-schema/test-qapi.py <$^ >$*.test.out 2>$*.test.err; 
>> echo $$? >$*.test.exit, "  TEST  $*.out")
>> +    $(call quiet-command, PYTHONPATH=$(SRC_PATH)/scripts $(PYTHON) 
>> $(SRC_PATH)/tests/qapi-schema/test-qapi.py "$^" >$*.test.out 2>$*.test.err; 
>> echo $$? >$*.test.exit, "  TEST  $*.out")

> Why is this line still long?  Shouldn't it have been split in 1/4?  And
> why is this form using an argument of the input file, instead of adding
> a -i option like all the others?

The "test-qapi.py" was already not using arguments, so I did not add support for
it, since it has one single mandatory argument.


[...]
>> +++ b/tests/qapi-schema/test-qapi.py
>> @@ -12,10 +12,11 @@
>> 
>> from qapi import *
>> from pprint import pprint
>> +import os
>> import sys
>> 
>> try:
>> -    exprs = parse_schema(sys.stdin)
>> +    exprs = parse_schema(sys.argv[1])

> Here's where I found it inconsistent with the rest of the patch.  It
> seems it would be better to either use -i everywhere, or use sys.argv[1]
> everywhere.

I just did not want to add argument parsing when there's only one mandatory
argument.


>> +++ b/tests/qapi-schema/trailing-comma-list.err
>> @@ -1 +1 @@
>> -<stdin>:2:36: Expected "{", "[" or string
>> +tests/qapi-schema/trailing-comma-list.json:2:36: Expected "{", "[" or string

> These are some long error messages; is it also worth trimming the
> leading "tests/qapi-schema/" in the sed script where you massage the
> data before doing the diff on expected errors?

I'm neutral about this, since the user will barely ever look at the test
contents.


Lluis

-- 
 "And it's much the same thing with knowledge, for whenever you learn
 something new, the whole world becomes that much richer."
 -- The Princess of Pure Reason, as told by Norton Juster in The Phantom
 Tollbooth



reply via email to

[Prev in Thread] Current Thread [Next in Thread]