qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
To: John Snow <jsnow@redhat.com>, Eric Blake <eblake@redhat.com>,
	"qemu-devel@nongnu.org" <qemu-devel@nongnu.org>,
	"qemu-block@nongnu.org" <qemu-block@nongnu.org>
Cc: Markus Armbruster <armbru@redhat.com>,
	Kevin Wolf <kwolf@redhat.com>, Max Reitz <mreitz@redhat.com>
Subject: Re: [Qemu-devel] [PATCH v4 3/5] iotests: change qmp_log filters to expect QMP objects only
Date: Thu, 20 Dec 2018 09:33:04 +0000	[thread overview]
Message-ID: <001b68c9-bfdc-e709-27cc-f3d11a069820@virtuozzo.com> (raw)
In-Reply-To: <aae520a3-2728-1269-8e87-99b2f4ef151c@redhat.com>

19.12.2018 22:52, John Snow wrote:
> 
> 
> On 12/19/18 2:01 PM, Eric Blake wrote:
>> On 12/19/18 5:27 AM, Vladimir Sementsov-Ogievskiy wrote:
>>
>>> But still not sure that it worth it. Isn't it better to just remove
>>> fields from dict,
>>> which are unpredictable, instead of substituting them..
>>
>> For getting the test to pass when we have a key:unpredictable value in
>> the dict, you are right that both changing it to key:SUBST or removing
>> key work at producing reproducible output. But when it comes to
>> debugging test failure, having key:SUBST in the logs gives you a hint at
>> what else to look at, whereas omitting key altogether may make the
>> reason for the failure completely disappear from the logs.
>>
>> Thus, I would argue that even though it is more complex to write a
>> filter that can recursively substitute, the resulting output is easier
>> to debug if a test starts failing - and that if the work in doing the
>> more complex filtering has already been submitted and is not too much of
>> a burden to maintain, then we might as well use it rather than going
>> with the simpler case of just eliding the problematic keys or using just
>> textual filtering.
>>
>> However, I'm not in a good position to argue whether there is a
>> reasonable maintenance burden with the patches in this series, vs. what
>> it would take to rewrite 206 to do just textual filtering instead of QMP
>> dict substitution filtering.
>>
> 
> My reasoning is this:
> 
> (1) It would be good if QMP filters behaved similarly to their plaintext
> companions, as this is the least surprising behavior, and
> 
> (2) It would be best to log the keys provided in responses in case we
> wish to test for their presence (and that their values match something
> we were able to predict via a pattern), and
> 
> (3) Not arbitrarily changing the nature of the response invisibly in a
> way that obscures it from log files to aid debugging, like you say.

Yes, at least (2-3) makes sense for me.

> 
> 
> 
> I offer some ideas for how to add text filtering for QMP objects in
> iotests in some of my replies, but it's not going to happen in 2018,
> IMO. I want pretty-printing of QMP commands more than I want text
> filtering of serialized QMP objects.
> 


-- 
Best regards,
Vladimir

  reply	other threads:[~2018-12-20  9:33 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-12-19  1:52 [Qemu-devel] [PATCH v4 0/5] bitmaps: remove x- prefix from QMP api Part2 John Snow
2018-12-19  1:52 ` [Qemu-devel] [PATCH v4 1/5] iotests: add qmp recursive sorting function John Snow
2018-12-19 10:20   ` Vladimir Sementsov-Ogievskiy
2018-12-19 17:55     ` John Snow
2018-12-19 18:50       ` Eric Blake
2018-12-19 18:52   ` Eric Blake
2018-12-19 18:57     ` John Snow
2018-12-19 19:19       ` Eric Blake
2018-12-19 19:47         ` John Snow
2018-12-19  1:52 ` [Qemu-devel] [PATCH v4 2/5] iotests: remove default filters from qmp_log John Snow
2018-12-19 10:58   ` Vladimir Sementsov-Ogievskiy
2018-12-19  1:52 ` [Qemu-devel] [PATCH v4 3/5] iotests: change qmp_log filters to expect QMP objects only John Snow
2018-12-19 11:07   ` Vladimir Sementsov-Ogievskiy
2018-12-19 11:27     ` Vladimir Sementsov-Ogievskiy
2018-12-19 17:29       ` John Snow
2018-12-19 19:01       ` Eric Blake
2018-12-19 19:52         ` John Snow
2018-12-20  9:33           ` Vladimir Sementsov-Ogievskiy [this message]
2018-12-19 18:35     ` John Snow
2018-12-20  9:11       ` Vladimir Sementsov-Ogievskiy
2018-12-19  1:52 ` [Qemu-devel] [PATCH v4 4/5] iotests: implement pretty-print for log and qmp_log John Snow
2018-12-19  1:52 ` [Qemu-devel] [PATCH v4 5/5] iotests: add iotest 236 for testing bitmap merge John Snow
2018-12-19 19:34   ` Eric Blake
2018-12-20  2:01     ` John Snow

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=001b68c9-bfdc-e709-27cc-f3d11a069820@virtuozzo.com \
    --to=vsementsov@virtuozzo.com \
    --cc=armbru@redhat.com \
    --cc=eblake@redhat.com \
    --cc=jsnow@redhat.com \
    --cc=kwolf@redhat.com \
    --cc=mreitz@redhat.com \
    --cc=qemu-block@nongnu.org \
    --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).