[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [PATCH v4 3/5] iotests: change qmp_log filters to expec
Re: [Qemu-block] [PATCH v4 3/5] iotests: change qmp_log filters to expect QMP objects only
Wed, 19 Dec 2018 14:52:23 -0500
Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.3.1
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.
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.
[Qemu-block] [PATCH v4 2/5] iotests: remove default filters from qmp_log, John Snow, 2018/12/18
[Qemu-block] [PATCH v4 3/5] iotests: change qmp_log filters to expect QMP objects only, John Snow, 2018/12/18
Re: [Qemu-block] [PATCH v4 3/5] iotests: change qmp_log filters to expect QMP objects only, John Snow, 2018/12/19
Re: [Qemu-block] [PATCH v4 3/5] iotests: change qmp_log filters to expect QMP objects only, Vladimir Sementsov-Ogievskiy, 2018/12/20
[Qemu-block] [PATCH v4 4/5] iotests: implement pretty-print for log and qmp_log, John Snow, 2018/12/18
[Qemu-block] [PATCH v4 5/5] iotests: add iotest 236 for testing bitmap merge, John Snow, 2018/12/18
- Re: [Qemu-block] [PATCH v4 1/5] iotests: add qmp recursive sorting function, (continued)