qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Peter Xu <peterx@redhat.com>
To: Eric Blake <eblake@redhat.com>
Cc: qemu-devel@nongnu.org, mdroth@linux.vnet.ibm.com,
	Markus Armbruster <armbru@redhat.com>
Subject: Re: [Qemu-devel] [PATCH 2/3] tests: add oob-test for qapi-schema
Date: Thu, 22 Mar 2018 11:43:37 +0800	[thread overview]
Message-ID: <20180322034337.GD32362@xz-mi> (raw)
In-Reply-To: <5f5af472-1606-ebe1-47c6-9b6bdc26cad9@redhat.com>

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

  reply	other threads:[~2018-03-22  3:44 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-03-21  6:55 [Qemu-devel] [PATCH 0/3] tests: trivial enhancements for OOB Peter Xu
2018-03-21  6:55 ` [Qemu-devel] [PATCH 1/3] tests: let qapi-schema tests detect oob Peter Xu
2018-03-21 12:47   ` Eric Blake
2018-03-21  6:55 ` [Qemu-devel] [PATCH 2/3] tests: add oob-test for qapi-schema Peter Xu
2018-03-21 12:52   ` Eric Blake
2018-03-22  3:43     ` Peter Xu [this message]
2018-03-21  6:55 ` [Qemu-devel] [PATCH 3/3] tests: more strict command batching test Peter Xu
2018-03-21 12:55   ` Eric Blake
2018-03-22  3:48     ` Peter Xu
2018-03-21 12:45 ` [Qemu-devel] [PATCH 0/3] tests: trivial enhancements for OOB Eric Blake

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180322034337.GD32362@xz-mi \
    --to=peterx@redhat.com \
    --cc=armbru@redhat.com \
    --cc=eblake@redhat.com \
    --cc=mdroth@linux.vnet.ibm.com \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).