[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [Qemu-devel] [PATCH v3 5/5] tests: Add check-qobject fo
From: |
Max Reitz |
Subject: |
Re: [Qemu-block] [Qemu-devel] [PATCH v3 5/5] tests: Add check-qobject for equality tests |
Date: |
Mon, 3 Jul 2017 18:13:50 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.2.0 |
On 2017-07-03 16:15, Eric Blake wrote:
> On 07/03/2017 07:25 AM, Max Reitz wrote:
>> Add a new test file (check-qobject.c) for unit tests that concern
>> QObjects as a whole.
>>
>> Its only purpose for now is to test the qobject_is_equal() function.
>>
>> Signed-off-by: Max Reitz <address@hidden>
>> ---
>> tests/Makefile.include | 4 +-
>> tests/check-qobject.c | 312
>> +++++++++++++++++++++++++++++++++++++++++++++++++
>> 2 files changed, 315 insertions(+), 1 deletion(-)
>> create mode 100644 tests/check-qobject.c
>>
>
>> + * Note that qobject_is_equal() is not really an equivalence relation,
>> + * so this function may not be used for all objects (reflexivity is
>> + * not guaranteed).
>
> May be worth expanding the comment to mention NaN in QNum as the culprit
> for this fact.
True.
>> +static void do_test_equality(bool expected, ...)
>> +{
>> + va_list ap_count, ap_extract;
>> + QObject **args;
>> + int arg_count = 0;
>> + int i, j;
>> +
>> + va_start(ap_count, expected);
>> + va_copy(ap_extract, ap_count);
>> + while (va_arg(ap_count, QObject *) != &_test_equality_end_of_arguments)
>> {
>
> Here, you're careful to allow a NULL argument,
>
>
>> +static void do_free_all(int _, ...)
>> +{
>> + va_list ap;
>> + QObject *obj;
>> +
>> + va_start(ap, _);
>> + while ((obj = va_arg(ap, QObject *)) != NULL) {
>> + qobject_decref(obj);
>> + }
>
> Here, you stop on the first NULL, and aren't checking for the special
> sentinel that can't be freed.
>
>> + va_end(ap);
>> +}
>> +
>> +#define free_all(...) \
>> + do_free_all(0, __VA_ARGS__, NULL)
>
> Then again, you don't pass the special sentinel. So as long as NULL is
> the last argument(s) on any test that passes NULL (rather than an
> intermediate argument), you don't need to use the sentinel, and stopping
> iteration on the first NULL is okay. A bit magic, but I can live with it.
I don't mind using the sentinel here, too, but passing NULL doesn't make
much sense here. It does for test_equality(), though.
>> +
>> +static void qobject_is_equal_null_test(void)
>> +{
>> + test_equality(false, qnull(), NULL);
>> +}
>
> Where do you test_equality(true, NULL, NULL) ?
Automatically in do_test_equality().
(It automatically tests whether all arguments are equal to itself --
which is why I can't use it to test NaN.)
>> +
>> +static void qobject_is_equal_num_test(void)
>> +{
>> + QNum *u0, *i0, *d0, *d0p25, *dnan, *um42, *im42, *dm42;
>> + QString *s0, *s_empty;
>> + QBool *bfalse;
>> +
>> + u0 = qnum_from_uint(0u);
>> + i0 = qnum_from_int(0);
>> + d0 = qnum_from_double(0.0);
>> + d0p25 = qnum_from_double(0.25);
>> + dnan = qnum_from_double(0.0 / 0.0);
>
> Ah, so you ARE testing NaN as a QNum, even though it can't occur in
> JSON. Might be worth a comment.
Sure, why not.
>> +
>> + /* Containing an NaN value will make this dict compare unequal to
>
> s/an/a/ (if you pronounce it "nan" or "not-a-number") (but I guess no
> change if you pronounce it "en-a-en")
I pronounce it en-a-en, but I don't know whether that's the usual way. ;-)
> There might be some improvements to add, but as written the test is
> reasonable, and some testing is better than none, so:
> Reviewed-by: Eric Blake <address@hidden>
Thanks!
Max
signature.asc
Description: OpenPGP digital signature