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