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

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(a)redhat.com&gt; > > --- > > 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.

> > > + > > + 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(a)redhat.com&gt; > > Jano