On Wed, Mar 21, 2018 at 07:52:06AM -0500, Eric Blake wrote: > On 03/21/2018 01:55 AM, Peter Xu wrote: > > It simply tests the new OOB capability, and make sure the QAPISchema can > > parse it correctly. > > We also want negative tests that cover any new error messages in the qapi > generator (such as 'allow-oob':'bad' diagnosing a non-bool, or > 'allow-oob':false giving an error message that false is already the default > such that only 'allow-oob':true makes sense). > > Also, it's often easier to merge the positive test into the giant existing > qapi-schema-test.json rather than creating a new positive test.
It seems that for one QAPI schema negative test only the first error will be reported, then the script halts (so the 2nd negative test in the same .json won't be reported). To make it simple - I'll put the positive test into qapi-schema-test.json, and add one negative test in oob-test.json to check again strings (though in the code I'll only allow 'false'). (Actually I'll need one liner change to check that value when parsing since it was not checked before...) > > > +++ b/tests/qapi-schema/oob-test.out > > @@ -0,0 +1,6 @@ > > +object q_empty > > +enum QType ['none', 'qnull', 'qnum', 'qstring', 'qdict', 'qlist', 'qbool'] > > + prefix QTYPE > > +module oob-test.json > > +command an-oob-command None -> None > > + gen=True success_response=True boxed=False oob=True > > At any rate, the positive test addition is good. I may beat you to > submitting a v2 patch that covers the error messages that I'm thinking of. Will post another version (and I'll see whether I should queue more to fix existing reported OOB problems). Thanks, -- Peter Xu
