Re: [PATCH 09/15] virbitmaptest: Add few more cases for virBitmapToString

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

 



On a Friday in 2020, Peter Krempa wrote:
On Fri, Oct 02, 2020 at 12:47:48 +0200, Ján Tomko wrote:
On a Friday in 2020, Peter Krempa wrote:
> Test an empty bitmap including it's extension via the self-expanding
> APIs and and a "0" and "" strings when converting the string back and
> forth.
>
> Signed-off-by: Peter Krempa <pkrempa@xxxxxxxxxx>
> ---
> tests/virbitmaptest.c | 35 ++++++++++++++++++++++++++++++++++-
> 1 file changed, 34 insertions(+), 1 deletion(-)
>
> diff --git a/tests/virbitmaptest.c b/tests/virbitmaptest.c
> index 56110971c9..c14a6c7e26 100644
> --- a/tests/virbitmaptest.c
> +++ b/tests/virbitmaptest.c
> @@ -605,7 +605,7 @@ test12(const void *opaque G_GNUC_UNUSED)
> static int
> test13(const void *opaque G_GNUC_UNUSED)
> {
> -    const char *strings[] = { "1234feebee", "000c0fefe" };
> +    const char *strings[] = { "1234feebee", "000c0fefe", "0", "" };
>     size_t i = 0;
>
>     for (i = 0; i < G_N_ELEMENTS(strings); i++) {
> @@ -684,6 +684,36 @@ test15(const void *opaque)
> }
>
>
> +/* virBitmapNewEmpty + virBitmapToString */
> +static int
> +test16(const void *opaque G_GNUC_UNUSED)
> +{
> +    g_autoptr(virBitmap) map = virBitmapNewEmpty();
> +    g_autofree char *res_empty = NULL;
> +    g_autofree char *res_set = NULL;
> +
> +    if (!(res_empty = virBitmapToString(map)) ||
> +        STRNEQ_NULLABLE(res_empty, "")) {
> +        fprintf(stderr, "\n expected bitmap string '%s' actual string '%s'\n",
> +                "", NULLSTR(res_empty));
> +        return -1;
> +    }
> +
> +    if (virBitmapSetBitExpand(map, 2) < 0 ||
> +        virBitmapSetBitExpand(map, 11) < 0)
> +        abort();

While this should be dead code with the current virBitmap APIs,
the point of the test is to check that they APIs behave correctly.
We should report a proper error instead of aborting.

There are already tests which check proper extension of the bitmap. This
is merely to extend it for testing that the bitmap is stringified
correctly so I wouldn't consider this a part of the test.


I can lessen my requirement to 'return -1;' here, without bothering to
write an error. But we should not abort on other than allocation
errors in the tests - one pass through the test suite should execute
all the tests instead of having developers dig through possible errors
one-by-one. (Yes, this is currently the last tests, but sets a bad
example)

Jano


> +
> +    if (!(res_set = virBitmapToString(map)) ||
> +        STRNEQ_NULLABLE(res_set, "804")) {
> +        fprintf(stderr, "\n expected bitmap string '%s' actual string '%s'\n",
> +                "804", NULLSTR(res_set));
> +        return -1;
> +    }
> +
> +    return 0;
> +}
> +
> +
> #define TESTBINARYOP(A, B, RES, FUNC) \
>     testBinaryOpData.a = A; \
>     testBinaryOpData.b = B; \

With the error message added:
Reviewed-by: Ján Tomko <jtomko@xxxxxxxxxx>

Jano


Attachment: signature.asc
Description: PGP signature


[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux