[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
Thu, 20 Dec 2018 09:33:04 +0000
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.
[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)