Eric Blake <ebl...@redhat.com> writes:
> On 07/28/2017 01:53 PM, Markus Armbruster wrote:
>> Eric Blake <ebl...@redhat.com> writes:
>>
>>> Now that we have the qmp_cmd() helper, we can further simplify
>>> some of the tests by using it.
>>>
>>> Signed-off-by: Eric Blake <ebl...@redhat.com>
>>> ---
>
>>> }
>>> - resp = qmp("{'execute': 'qom-list-types',"
>>> - " 'arguments': %p }", args);
>>> + resp = qmp_cmd("qom-list-types", QOBJECT(args));
>>
>> This one's a clear win.
>
> Well, it'd be even NICER if I could pass QDict instead of QObject around.
>
>
>>> +++ b/tests/ide-test.c
>>> @@ -651,7 +651,7 @@ static void test_retry_flush(const char *machine)
>>> qmp_eventwait("STOP");
>>>
>>> /* Complete the command */
>>> - qmp_discard_response("{'execute':'cont' }");
>>> + qmp_cmd_discard_response("cont", NULL);
>>
>> This one isn't.
>
> Fair enough.
>
>>> @@ -86,7 +87,7 @@ void set_context(QOSState *s)
>>>
>>> static QDict *qmp_execute(const char *command)
>>> {
>>> - return qmp("{ 'execute': %s }", command);
>>> + return qmp_cmd(command, NULL);
>>
>> Marginal.
>>
>>> }
>>>
>>> void migrate(QOSState *from, QOSState *to, const char *uri)
>>> @@ -106,7 +107,7 @@ void migrate(QOSState *from, QOSState *to, const char
>>> *uri)
>>> QDECREF(rsp);
>>>
>>> /* Issue the migrate command. */
>>> - rsp = qmp("{ 'execute': 'migrate', 'arguments': { 'uri': %s } }", uri);
>>> + rsp = qmp_cmd("migrate", qobject_from_jsonf("{ 'uri': %s }", uri));
>>
>> Not a simplification. The opposite, I'm afraid.
>>
>> I could become one if qmp_cmd() worked like this:
>>
>> rsp = qmp_cmd("migrate", "{ 'uri': %s }", uri));
>
> Oh, nice. But it makes qmp_cmd become varargs, at which point...
>
>>
>> Even better if qmp_cmd("cont", "") just works. Hmm, unles gcc whines
>> about the empty format string.
>
> yeah, we'd have to figure out a way to shut up gcc when no arguments are
> wanted. Here's a compromise that does the best for all three categories
> you pointed out above:
>
> qmp_cmd("cont");
> qmp_cmd_args("migrate", "{ 'uri': %s }", uri);
> qmp_cmd_dict("qom-list-types", args);
>
> Sounds like I have a plan of attack! Also, since I'm somewhat churning
> on the stuff you did in a previous patch, should we reorder any of this
> series (add the helper first, then a single fix the callers that benefit
> from the helpers; instead of fixing callers twice in three patches)?
If easier review and cleaner history can justify the effort to rework.
Your call.
>>> g_assert(qdict_haskey(rsp, "return"));
>>> QDECREF(rsp);
>>>
>> [More of the same snipped...]
>>
>
> And, as I said in the cover letter, there's probably a lot more we could
> touch in the testsuite if we like the new pattern.
One step at a time.
