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