Re: [Qemu-devel] [PATCH 04/20] libqtest: Clean up how we read the QMP greeting

[Markus Armbruster]

Thomas Huth <address@hidden> writes:

> On 12.07.2018 13:12, Markus Armbruster wrote:
>> qtest_init() still uses the qtest_qmp_discard_response(s, "") hack to
>> receive the greeting, even though we have qtest_qmp_receive() since
>> commit 66e0c7b187e.  Put it to use.
>> 
>> Bonus: gets rid of an empty format string.  A step towards
>> compile-time format string checking without triggering
>> -Wformat-zero-length.
>> 
>> Signed-off-by: Markus Armbruster <address@hidden>
>> ---
>>  tests/libqtest.c | 4 +++-
>>  1 file changed, 3 insertions(+), 1 deletion(-)
>> 
>> diff --git a/tests/libqtest.c b/tests/libqtest.c
>> index 071d7eb7b1..c2c08a890c 100644
>> --- a/tests/libqtest.c
>> +++ b/tests/libqtest.c
>> @@ -249,9 +249,11 @@ QTestState *qtest_init_without_qmp_handshake(bool 
>> use_oob,
>>  QTestState *qtest_init(const char *extra_args)
>>  {
>>      QTestState *s = qtest_init_without_qmp_handshake(false, extra_args);
>> +    QDict *greeting;
>>  
>>      /* Read the QMP greeting and then do the handshake */
>> -    qtest_qmp_discard_response(s, "");
>> +    greeting = qtest_qmp_receive(s);
>> +    qobject_unref(greeting);
>>      qtest_qmp_discard_response(s, "{ 'execute': 'qmp_capabilities' }");
>>  
>>      return s;
>
> I wonder whether we should actually check the greeting for some expected
> information? Anyway, that's something for later, and not related to your
> patch.

This kind of sloppy testing is quite common.

Checking test results manually in code maximizes flexibility.  It also
maximizes temptation to cut corners, because doing a complete job is so
tedious.  One reason I prefer "normalize and diff against expected
results".  The closest we got to support for that way of testing is
qlit_equal_qobject().

Digression: QOM also maximizes flexibility by doing stuff in code rather
than data.  The two go back to the same stratum of QEMU development.
Hardly coincidence, in my opinion.  We've since extended QOM to support
doing more in data, but there's still plenty of old code doing it in
code, and plenty of new code following that old code's lead.  Recovering
from such damage is hard work.

> Reviewed-by: Thomas Huth <address@hidden>

Thanks!