[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v3 1/9] tests/qtest: add various qtest_qmp_assert_success() v
From: |
Daniel P . Berrangé |
Subject: |
Re: [PATCH v3 1/9] tests/qtest: add various qtest_qmp_assert_success() variants |
Date: |
Thu, 1 Jun 2023 13:48:49 +0100 |
User-agent: |
Mutt/2.2.9 (2022-11-12) |
On Thu, Jun 01, 2023 at 11:23:01AM +0200, Thomas Huth wrote:
> On 31/05/2023 15.23, Daniel P. Berrangé wrote:
> > Add several counterparts of qtest_qmp_assert_success() that can
> >
> > * Use va_list instead of ...
> > * Accept a list of FDs to send
> > * Return the response data
> >
> > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> > ---
> > tests/qtest/libqtest.c | 99 +++++++++++++++++++++++++++++++++--
> > tests/qtest/libqtest.h | 115 +++++++++++++++++++++++++++++++++++++++++
> > 2 files changed, 209 insertions(+), 5 deletions(-)
> > +#ifndef _WIN32
> > +QDict *qtest_vqmp_fds_assert_success_ref(QTestState *qts, int *fds, size_t
> > nfds,
> > + const char *fmt, va_list args)
> > +{
> > + QDict *response;
> > + QDict *ret;
> > +
> > + response = qtest_vqmp_fds(qts, fds, nfds, fmt, args);
> > +
> > + g_assert(response);
> > + if (!qdict_haskey(response, "return")) {
> > + GString *s = qobject_to_json_pretty(QOBJECT(response), true);
> > + g_test_message("%s", s->str);
> > + g_string_free(s, true);
> > + }
> > + g_assert(qdict_haskey(response, "return"));
> > + ret = qdict_get_qdict(response, "return");
> > + qobject_ref(ret);
> > + qobject_unref(response);
> > +
> > + return ret;
> > +}
> > +
> > +void qtest_vqmp_fds_assert_success(QTestState *qts, int *fds, size_t nfds,
> > + const char *fmt, va_list args)
> > +{
> > + QDict *response;
> > + response = qtest_vqmp_fds_assert_success_ref(qts, fds, nfds, fmt,
> > args);
> > qobject_unref(response);
> > }
>
> <bikeshedding>The ordering is a little bit inconsistent ... for some pairs,
> you do the _success() function first, and for some others you do the
> _success_ref() function first. IMHO it would be nicer to have the same
> ordering everywhere, preferably with the _success_ref() function first
> (since that's the one that is called from the other).</bikeshedding>
Sure, will do.
>
> > +#endif /* !_WIN32 */
> > +
> > +void qtest_qmp_assert_success(QTestState *qts, const char *fmt, ...)
> > +{
> > + QDict *response;
> > + va_list ap;
> > + va_start(ap, fmt);
> > + response = qtest_vqmp_assert_success_ref(qts, fmt, ap);
>
> You could use qtest_vqmp_assert_success() instead, I think, and dro the
> qobject_unref() below.
Agreed
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|