Re: [PATCH v3 08/19] qlit: Move qlit_equal_qobject() reference values to array

[Markus Armbruster]

Eduardo Habkost <ehabkost@redhat.com> writes:

> On Tue, Nov 24, 2020 at 10:51:34AM +0100, Markus Armbruster wrote:
>> Eduardo Habkost <ehabkost@redhat.com> writes:
>> 
>> > Add an array of values to qlit_equal_qobject_test(), so we can
>> > extend the test case to compare multiple literals, not just the
>> > ones at the existing `qlit` and `qlit_foo` variables.
>> >
>> > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
>> > ---
>> > This is a new patch added in v3 of this series.
>> > ---
>> >  tests/check-qlit.c | 26 +++++++++++++++++++-------
>> >  1 file changed, 19 insertions(+), 7 deletions(-)
>> >
>> > diff --git a/tests/check-qlit.c b/tests/check-qlit.c
>> > index 24ac21395c..b1cfbddb17 100644
>> > --- a/tests/check-qlit.c
>> > +++ b/tests/check-qlit.c
>> > @@ -29,11 +29,6 @@ static QLitObject qlit = QLIT_QDICT(((QLitDictEntry[]) {
>> >      { },
>> >  }));
>> >  
>> > -static QLitObject qlit_foo = QLIT_QDICT(((QLitDictEntry[]) {
>> > -    { "foo", QLIT_QNUM_INT(42) },
>> > -    { },
>> > -}));
>> > -
>> >  static QObject *make_qobject(void)
>> >  {
>> >      QDict *qdict = qdict_new();
>> > @@ -53,16 +48,33 @@ static QObject *make_qobject(void)
>> >  
>> >  static void qlit_equal_qobject_test(void)
>> >  {
>> > +    /* Each entry in the values[] array should be different from the 
>> > others */
>> > +    QLitObject values[] = {
>> > +        qlit,
>> > +        QLIT_QDICT(((QLitDictEntry[]) {
>> > +            { "foo", QLIT_QNUM_INT(42) },
>> > +            { },
>> > +        })),
>> > +    };
>> > +    int i;
>> >      QObject *qobj = make_qobject();
>> >  
>> >      g_assert(qlit_equal_qobject(&qlit, qobj));
>> >  
>> > -    g_assert(!qlit_equal_qobject(&qlit_foo, qobj));
>> > -
>> >      qdict_put(qobject_to(QDict, qobj), "bee", qlist_new());
>> >      g_assert(!qlit_equal_qobject(&qlit, qobj));
>> >  
>> >      qobject_unref(qobj);
>> > +
>> > +    for (i = 0; i < ARRAY_SIZE(values); i++) {
>> > +        int j;
>> 
>> I'd prefer to declare this one together with @i.
>> 
>> > +        QObject *o = qobject_from_qlit(&values[i]);
>> 
>> Blank line, if you don't mind.
>
> I will surely do it if there's a v4, but I hope you don't make me
> submit v4 just to change these.

That would be a waste of your time and mine :)

If we decide to take the patches through my tree, I can tweak this in my
tree.

If somebody else takes it, and prefers not to tweak, I can tweak on top
if I care.

>
>> 
>> > +        for (j = 0; j < ARRAY_SIZE(values); j++) {
>> > +            g_assert(qlit_equal_qobject(&values[j], o) == (i == j));
>> > +        }
>> > +        qobject_unref(o);
>> > +    }
>> > +
>> >  }
>> >  
>> >  static void qlit_equal_large_qnum_test(void)