Re: [PATCH v3 13/49] tests/qtest/qmp-test: don't depend on human-monitor-command

On 17/8/26 10:11, Marc-André Lureau wrote:
> Hi
> 
> On Mon, Aug 17, 2026 at 7:56 AM Philippe Mathieu-Daudé
> <[email protected]> wrote:
>>
>> On 16/8/26 21:12, Marc-André Lureau wrote:
>>> The test was using 'human-monitor-command' (which requires HMP) to
>>> verify that an integer 'id' is echoed back on failure. Replace with
>>> 'query-name' for the success case and 'block_resize' (missing
>>> required args) for the failure case, so the test works regardless of
>>> HMP availability.
>>>
>>> Reviewed-by: Daniel P. Berrangé <[email protected]>
>>> Signed-off-by: Marc-André Lureau <[email protected]>
>>> ---
>>>    tests/qtest/qmp-test.c | 13 +++++++++++--
>>>    1 file changed, 11 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/tests/qtest/qmp-test.c b/tests/qtest/qmp-test.c
>>> index 3a325a04e07b..927ab053f8ab 100644
>>> --- a/tests/qtest/qmp-test.c
>>> +++ b/tests/qtest/qmp-test.c
>>> @@ -151,9 +151,14 @@ static void test_qmp_protocol(void)
>>>        g_assert_cmpstr(qdict_get_try_str(resp, "id"), ==, "cookie#1");
>>>        qobject_unref(resp);
>>>
>>> -    /* Test command failure with 'id' */
>>> -    resp = qtest_qmp(qts, "{ 'execute': 'human-monitor-command', 'id': 2 }");
>>> +    /* Test integer 'id' is echoed back on success */
>>> +    resp = qtest_qmp(qts, "{ 'execute': 'query-name', 'id': 2 }");
>>>        g_assert_cmpint(qdict_get_int(resp, "id"), ==, 2);
>>> +    qobject_unref(resp);
>>> +
>>> +    /* Test integer 'id' is echoed back on failure */
>>> +    resp = qtest_qmp(qts, "{ 'execute': 'block_resize', 'id': 3 }");
>>> +    g_assert_cmpint(qdict_get_int(resp, "id"), ==, 3);
>>>        qmp_expect_error_and_unref(resp, "GenericError");
>>>
>>>        qtest_quit(qts);
>>> @@ -487,6 +492,7 @@ static void test_qmp_monitor_remove_cli(void)
>>>        qtest_quit(qts);
>>>    }
>>>
>>> +#ifdef CONFIG_HMP
>>>    static void test_qmp_monitor_remove_hmp(void)
>>>    {
>>>        QTestState *qts;
>>> @@ -504,6 +510,7 @@ static void test_qmp_monitor_remove_hmp(void)
>>>
>>>        qtest_quit(qts);
>>>    }
>>> +#endif
>>>
>>>    int main(int argc, char *argv[])
>>>    {
>>> @@ -521,7 +528,9 @@ int main(int argc, char *argv[])
>>>        qtest_add_func("qmp/monitor-chardev-in-use",
>>>                        test_qmp_monitor_chardev_in_use);
>>>        qtest_add_func("qmp/monitor-remove-cli", test_qmp_monitor_remove_cli);
>>> +#ifdef CONFIG_HMP
>>>        qtest_add_func("qmp/monitor-remove-hmp", test_qmp_monitor_remove_hmp);
>>> +#endif
>>
>> Should we re-order #26 "build-sys: add 'hmp' option" or add a temporary
>> #define CONFIG_HMP?
> 
> Let's add a temporary CONFIG_HMP, rather than a temporary non-working
> --disable-hmp. That should also help with ordering of other later
> patches which may not be optimal.

LGTM.