[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH] test: adding tests to virStrToDouble() inside virstringtest.



Hi John,

Thanks for your points.
The V2 is ready... I will use %lf for now because the intention of output is
show the error with a clear message.

%lf is easier to check, I believe. Well, any other suggestions are welcome.


2017-07-08 10:59 GMT-03:00 John Ferlan <jferlan redhat com>:
>
>
> On 06/24/2017 08:15 PM, Julio Faracco wrote:
>> There are no occurrences of tests related to Strings and Double numbers
>> inside virstringtest.c. This commit introduces some tests to validate the
>> conversion. The test does not include locale changes yet.
>>
>> Signed-off-by: Julio Faracco <jcfaracco gmail com>
>> ---
>>  tests/virstringtest.c | 84 +++++++++++++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 84 insertions(+)
>>
>> diff --git a/tests/virstringtest.c b/tests/virstringtest.c
>> index 97c6e76..32ce79c 100644
>> --- a/tests/virstringtest.c
>> +++ b/tests/virstringtest.c
>> @@ -652,6 +652,52 @@ testStringToLong(const void *opaque)
>>  }
>>
>>
>> +struct stringToDoubleData {
>> +    const char *str;
>> +    const char *end_ptr;
>> +    double res;
>> +};
>> +
>> +/* This test checks if double strings are successfully converted to double
>> + * number considering the byproduct string too. */
>> +static int
>> +testStringToDouble(const void *opaque)
>> +{
>> +    const struct stringToDoubleData *data = opaque;
>> +    int ret = -1;
>> +    char *end_ptr = NULL;
>> +    double res = 0;
>> +
>> +    if (data->end_ptr) {
>> +        ret = virStrToDouble(data->str, &end_ptr, &res);
>> +    } else {
>> +        /* end_ptr returns or a substring or an empty string.
>> +         * It never returns a NULL pointer. */
>> +        ret = virStrToDouble(data->str, NULL, &res);
>> +    }
>
> Not sure the comment makes sense...  Why not just one line:
>
> ret = virStrToDouble(data->str, data->end_ptr ? &end_ptr : NULL, &res);
>
> or combining with the subsequent "if (ret < 0) {" test:
>
> if ((ret = virStrToDouble(data->str, data->end_ptr ? &end_ptr : NULL,
>                           &res)) < 0) {
>
>
>> +
>> +    if (ret < 0) {
>> +        fprintf(stderr, "Convert error of '%s', expected '%f'\n",
>
> Should the format be %lf or %g?  I've see both used within libvirt code
> - search around for VIR_TYPED_PARAM_DOUBLE, _TYPE_DOUBLE, or "param.d"
> printing.
>
> (similarly for the next %f usage as well)
>
>
>> +                data->str, data->res);
>> +        return ret;
>> +    }
>> +
>> +    if (res != data->res) {
>> +        fprintf(stderr, "Returned '%f', expected '%f'\n",
>> +                res, data->res);
>> +        return -1;
>> +    }
>> +
>> +    /* Comparing substrings. */
>> +    if (STRNEQ_NULLABLE(end_ptr, data->end_ptr)) {
>> +        fprintf(stderr, "Expected substring '%s', but got '%s'\n",
>> +                end_ptr, data->end_ptr);
>> +        return -1;
>> +    }
>> +
>> +    return ret;
>> +}
>> +
>>  /* The point of this test is to check whether all members of the array are
>>   * freed. The test has to be checked using valgrind. */
>>  static int
>> @@ -965,6 +1011,44 @@ mymain(void)
>>      TEST_STRTOL("-18446744073709551616", NULL, 0, -1, 0U, -1,
>>                  0LL, -1, 0ULL, -1);
>>
>> +#define TEST_STRTOD(str, end_ptr, res)                                  \
>> +    do {                                                                \
>> +        struct stringToDoubleData data = {                              \
>> +            str, end_ptr, res,                                          \
>> +        };                                                              \
>> +        if (virTestRun("virStringToDouble '" str "'",                   \
>> +                       testStringToDouble, &data) < 0)                  \
>> +            ret = -1;                                                   \
>> +    } while(0)
>
> This fails syntax-check due to no space between while and (0)
>
> I can either make the suggested changes (perhaps someone else has a
> strong feeling of using %g or %lf) or you can post a new patch. Either
> way is fine.
>
> John
>
>> +
>> +    /* Simple numbers. */
>> +    TEST_STRTOD("0.0", NULL, 0);
>> +    TEST_STRTOD("1.0", NULL, 1);
>> +    TEST_STRTOD("3.14159", NULL, 3.14159);
>> +    TEST_STRTOD("0.57721", NULL, 0.57721);
>> +
>> +    /* Testing ending string. */
>> +    TEST_STRTOD("2.718", "", 2.718);
>> +    TEST_STRTOD("2.718 281 828 459", " 281 828 459", 2.718);
>> +    TEST_STRTOD("2.718,281,828,459", ",281,828,459", 2.718);
>> +
>> +    /* Scientific numbers. */
>> +    TEST_STRTOD("3.14159e+000", NULL, 3.14159);
>> +    TEST_STRTOD("2.00600e+003", NULL, 2006);
>> +    TEST_STRTOD("1.00000e-010", NULL, 1e-010);
>> +
>> +    /* Negative numbers. */
>> +    TEST_STRTOD("-1.6180339887", NULL, -1.6180339887);
>> +    TEST_STRTOD("-0.00031e-010", NULL, -0.00031e-010);
>> +
>> +    /* Long numbers. */
>> +    TEST_STRTOD("57089907708238388904078437636832797971793838081897.0",
>> +                NULL,
>> +                57089907708238388904078437636832797971793838081897.0);
>> +    TEST_STRTOD("3.141592653589793238462643383279502884197169399375105",
>> +                NULL,
>> +                3.141592653589793238462643383279502884197169399375105);
>> +
>>      /* test virStringListFreeCount */
>>      if (virTestRun("virStringListFreeCount", testVirStringListFreeCount,
>>                     NULL) < 0)
>>


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]