Re: [Qemu-devel] [PATCH v2 21/23] libqtest: Remove qtest_qmp_discard_response() & friends

[Thomas Huth]

On 07/30/2018 08:32 AM, Markus Armbruster wrote:
> Eric Blake <address@hidden> writes:
> 
>> On 07/27/2018 11:46 AM, Thomas Huth wrote:
>>> On 07/27/2018 05:13 PM, Markus Armbruster wrote:
>>>> qtest_qmp_discard_response(...) is shorthand for
>>>> qobject_unref(qtest_qmp(...), except it's not actually shorter.
>>>
>>> But the latter is IMHO harder to read.
> 
> Doing things sloppily looks a bit uglier now.  That's a feature.
> 
>> Maybe, but then it lends itself well to:
>>
>> QObject *rsp = qtest_qmp(...);
>> qobject_unref(rsp);
>>
>> which is where you do insert tests for valid responses.
>>
>>> And it might be shorter in the compiled binary (one function call vs. two).
> 
> I'd be quite sympathetic to this argument...
> 
>> The size of the test binaries is not our biggest concern.
> 
> ... outside tests/.
> 
>>>> Moreover, the presence of these functions encourage sloppy testing.
>>>
>>> Shouldn't we then rather fix the tests to check for valid responses
>>> instead of replacing this function with harder-to-read code?
> 
> I'd welcome such patches, but this series is already pretty long.

Then maybe rather drop this patch from this series, and fix the issues
in a separate series instead?

 Thomas