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

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.