Libguestfs - Jul 2020 - Re: [nbdkit PATCH] RFC tests: Avoid odd test behavior under NDEBUG

While we support compilation with CFLAGS=-DNDEBUG for the brave user
desiring to avoid assertion overhead, this causes the tests to be less
powerful.  Fortunately, a quick test of './configure CFLAGS=-DNDEBUG'
didn't hit any test failures, but it seems better to just ensure that
assertions always work in our tests, even if they are turned off for
the main binary.

Signed-off-by: Eric Blake <eblake@redhat.com>
---

I don't know if I like this one.  Since 'make check' passed in a
build
with CFLAGS=-DNDEBUG, I'm not sure we need it.

 common/include/test-ascii-ctype.c      | 1 +
 common/include/test-ascii-string.c     | 1 +
 common/include/test-byte-swapping.c    | 1 +
 common/include/test-current-dir-name.c | 1 +
 common/include/test-isaligned.c        | 1 +
 common/include/test-ispowerof2.c       | 1 +
 common/include/test-iszero.c           | 1 +
 common/include/test-minmax.c           | 1 +
 common/include/test-nextnonzero.c      | 1 +
 common/bitmap/test-bitmap.c            | 1 +
 common/utils/test-quotes.c             | 1 +
 common/utils/test-vector.c             | 1 +
 tests/test-layers-filter.c             | 1 +
 tests/test-pause.c                     | 1 +
 tests/test-stdio-plugin.c              | 1 +
 15 files changed, 15 insertions(+)

diff --git a/common/include/test-ascii-ctype.c
b/common/include/test-ascii-ctype.c
index 85a78fa5..6e0b6626 100644
--- a/common/include/test-ascii-ctype.c
+++ b/common/include/test-ascii-ctype.c
@@ -34,6 +34,7 @@

 #include <stdio.h>
 #include <stdlib.h>
+#undef NDEBUG /* We want the test to fail even if nbdkit disables assertions */
 #include <assert.h>

 #include "ascii-ctype.h"
diff --git a/common/include/test-ascii-string.c
b/common/include/test-ascii-string.c
index 0fa4a483..6bbf9997 100644
--- a/common/include/test-ascii-string.c
+++ b/common/include/test-ascii-string.c
@@ -34,6 +34,7 @@

 #include <stdio.h>
 #include <stdlib.h>
+#undef NDEBUG /* We want the test to fail even if nbdkit disables assertions */
 #include <assert.h>

 #include "ascii-string.h"
diff --git a/common/include/test-byte-swapping.c
b/common/include/test-byte-swapping.c
index e95a17db..a5e61a1f 100644
--- a/common/include/test-byte-swapping.c
+++ b/common/include/test-byte-swapping.c
@@ -36,6 +36,7 @@
 #include <stdlib.h>
 #include <stdint.h>
 #include <string.h>
+#undef NDEBUG /* We want the test to fail even if nbdkit disables assertions */
 #include <assert.h>

 #include "byte-swapping.h"
diff --git a/common/include/test-current-dir-name.c
b/common/include/test-current-dir-name.c
index bb2bfd12..40872f2b 100644
--- a/common/include/test-current-dir-name.c
+++ b/common/include/test-current-dir-name.c
@@ -37,6 +37,7 @@
 #include <stdint.h>
 #include <string.h>
 #include <unistd.h>
+#undef NDEBUG /* We want the test to fail even if nbdkit disables assertions */
 #include <assert.h>

 #include "get-current-dir-name.h"
diff --git a/common/include/test-isaligned.c b/common/include/test-isaligned.c
index adad3266..45415e37 100644
--- a/common/include/test-isaligned.c
+++ b/common/include/test-isaligned.c
@@ -36,6 +36,7 @@
 #include <stdlib.h>
 #include <stdint.h>
 #include <string.h>
+#undef NDEBUG /* We want the test to fail even if nbdkit disables assertions */
 #include <assert.h>

 #include "isaligned.h"
diff --git a/common/include/test-ispowerof2.c b/common/include/test-ispowerof2.c
index af3e3b9b..4ea7fdde 100644
--- a/common/include/test-ispowerof2.c
+++ b/common/include/test-ispowerof2.c
@@ -36,6 +36,7 @@
 #include <stdlib.h>
 #include <stdint.h>
 #include <string.h>
+#undef NDEBUG /* We want the test to fail even if nbdkit disables assertions */
 #include <assert.h>

 #include "ispowerof2.h"
diff --git a/common/include/test-iszero.c b/common/include/test-iszero.c
index 1a4f09ff..d53efb4f 100644
--- a/common/include/test-iszero.c
+++ b/common/include/test-iszero.c
@@ -36,6 +36,7 @@
 #include <stdlib.h>
 #include <stdint.h>
 #include <string.h>
+#undef NDEBUG /* We want the test to fail even if nbdkit disables assertions */
 #include <assert.h>

 #include "iszero.h"
diff --git a/common/include/test-minmax.c b/common/include/test-minmax.c
index 7d4173ca..ce559b91 100644
--- a/common/include/test-minmax.c
+++ b/common/include/test-minmax.c
@@ -36,6 +36,7 @@
 #include <stdlib.h>
 #include <stdint.h>
 #include <limits.h>
+#undef NDEBUG /* We want the test to fail even if nbdkit disables assertions */
 #include <assert.h>
 #include <float.h>

diff --git a/common/include/test-nextnonzero.c
b/common/include/test-nextnonzero.c
index 86aee5ec..0c3ad5e3 100644
--- a/common/include/test-nextnonzero.c
+++ b/common/include/test-nextnonzero.c
@@ -36,6 +36,7 @@
 #include <stdlib.h>
 #include <stdint.h>
 #include <string.h>
+#undef NDEBUG /* We want the test to fail even if nbdkit disables assertions */
 #include <assert.h>

 #include "nextnonzero.h"
diff --git a/common/bitmap/test-bitmap.c b/common/bitmap/test-bitmap.c
index cbbbbef8..2323bb7f 100644
--- a/common/bitmap/test-bitmap.c
+++ b/common/bitmap/test-bitmap.c
@@ -40,6 +40,7 @@
 #include <stdarg.h>
 #include <string.h>
 #include <errno.h>
+#undef NDEBUG /* We want the test to fail even if nbdkit disables assertions */
 #include <assert.h>

 #include <nbdkit-plugin.h>
diff --git a/common/utils/test-quotes.c b/common/utils/test-quotes.c
index 72e54fc7..0783be27 100644
--- a/common/utils/test-quotes.c
+++ b/common/utils/test-quotes.c
@@ -37,6 +37,7 @@
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
+#undef NDEBUG /* We want the test to fail even if nbdkit disables assertions */
 #include <assert.h>
 #include <stdbool.h>

diff --git a/common/utils/test-vector.c b/common/utils/test-vector.c
index a2c0db10..be3696b7 100644
--- a/common/utils/test-vector.c
+++ b/common/utils/test-vector.c
@@ -35,6 +35,7 @@
 #include <stdio.h>
 #include <stdlib.h>
 #include <stdint.h>
+#undef NDEBUG /* We want the test to fail even if nbdkit disables assertions */
 #include <assert.h>

 #include "vector.h"
diff --git a/tests/test-layers-filter.c b/tests/test-layers-filter.c
index 66d80835..b77df258 100644
--- a/tests/test-layers-filter.c
+++ b/tests/test-layers-filter.c
@@ -35,6 +35,7 @@
 #include <stdio.h>
 #include <stdlib.h>
 #include <stdint.h>
+#undef NDEBUG /* We want the test to fail even if nbdkit disables assertions */
 #include <assert.h>

 #include <nbdkit-filter.h>
diff --git a/tests/test-pause.c b/tests/test-pause.c
index ba772d4f..09f76c76 100644
--- a/tests/test-pause.c
+++ b/tests/test-pause.c
@@ -40,6 +40,7 @@
 #include <string.h>
 #include <unistd.h>
 #include <time.h>
+#undef NDEBUG /* We want the test to fail even if nbdkit disables assertions */
 #include <assert.h>
 #include <sys/types.h>
 #include <sys/socket.h>
diff --git a/tests/test-stdio-plugin.c b/tests/test-stdio-plugin.c
index 6af1ba90..f29bf3f7 100644
--- a/tests/test-stdio-plugin.c
+++ b/tests/test-stdio-plugin.c
@@ -32,6 +32,7 @@

 #include <config.h>

+#undef NDEBUG /* We want the test to fail even if nbdkit disables assertions */
 #include <assert.h>
 #include <fcntl.h>
 #include <stdbool.h>
-- 
2.27.0

On Tue, Jul 07, 2020 at 09:51:53AM -0500, Eric Blake
wrote:
> While we support compilation with CFLAGS=-DNDEBUG for the brave user
> desiring to avoid assertion overhead, this causes the tests to be less
> powerful.  Fortunately, a quick test of './configure
CFLAGS=-DNDEBUG'
> didn't hit any test failures, but it seems better to just ensure that
> assertions always work in our tests, even if they are turned off for
> the main binary.
> 
> Signed-off-by: Eric Blake <eblake@redhat.com>
> ---
> 
> I don't know if I like this one.  Since 'make check' passed in
a build
> with CFLAGS=-DNDEBUG, I'm not sure we need it.
I think we actually do need it.  The tests "fail open" if someone
compiles with -DNDEBUG so we should ensure that something is being
tested even under these conditions.

ACK

Rich.
>  common/include/test-ascii-ctype.c      | 1 +
>  common/include/test-ascii-string.c     | 1 +
>  common/include/test-byte-swapping.c    | 1 +
>  common/include/test-current-dir-name.c | 1 +
>  common/include/test-isaligned.c        | 1 +
>  common/include/test-ispowerof2.c       | 1 +
>  common/include/test-iszero.c           | 1 +
>  common/include/test-minmax.c           | 1 +
>  common/include/test-nextnonzero.c      | 1 +
>  common/bitmap/test-bitmap.c            | 1 +
>  common/utils/test-quotes.c             | 1 +
>  common/utils/test-vector.c             | 1 +
>  tests/test-layers-filter.c             | 1 +
>  tests/test-pause.c                     | 1 +
>  tests/test-stdio-plugin.c              | 1 +
>  15 files changed, 15 insertions(+)
> 
> diff --git a/common/include/test-ascii-ctype.c
b/common/include/test-ascii-ctype.c
> index 85a78fa5..6e0b6626 100644
> --- a/common/include/test-ascii-ctype.c
> +++ b/common/include/test-ascii-ctype.c
> @@ -34,6 +34,7 @@
> 
>  #include <stdio.h>
>  #include <stdlib.h>
> +#undef NDEBUG /* We want the test to fail even if nbdkit disables
assertions */
>  #include <assert.h>
> 
>  #include "ascii-ctype.h"
> diff --git a/common/include/test-ascii-string.c
b/common/include/test-ascii-string.c
> index 0fa4a483..6bbf9997 100644
> --- a/common/include/test-ascii-string.c
> +++ b/common/include/test-ascii-string.c
> @@ -34,6 +34,7 @@
> 
>  #include <stdio.h>
>  #include <stdlib.h>
> +#undef NDEBUG /* We want the test to fail even if nbdkit disables
assertions */
>  #include <assert.h>
> 
>  #include "ascii-string.h"
> diff --git a/common/include/test-byte-swapping.c
b/common/include/test-byte-swapping.c
> index e95a17db..a5e61a1f 100644
> --- a/common/include/test-byte-swapping.c
> +++ b/common/include/test-byte-swapping.c
> @@ -36,6 +36,7 @@
>  #include <stdlib.h>
>  #include <stdint.h>
>  #include <string.h>
> +#undef NDEBUG /* We want the test to fail even if nbdkit disables
assertions */
>  #include <assert.h>
> 
>  #include "byte-swapping.h"
> diff --git a/common/include/test-current-dir-name.c
b/common/include/test-current-dir-name.c
> index bb2bfd12..40872f2b 100644
> --- a/common/include/test-current-dir-name.c
> +++ b/common/include/test-current-dir-name.c
> @@ -37,6 +37,7 @@
>  #include <stdint.h>
>  #include <string.h>
>  #include <unistd.h>
> +#undef NDEBUG /* We want the test to fail even if nbdkit disables
assertions */
>  #include <assert.h>
> 
>  #include "get-current-dir-name.h"
> diff --git a/common/include/test-isaligned.c
b/common/include/test-isaligned.c
> index adad3266..45415e37 100644
> --- a/common/include/test-isaligned.c
> +++ b/common/include/test-isaligned.c
> @@ -36,6 +36,7 @@
>  #include <stdlib.h>
>  #include <stdint.h>
>  #include <string.h>
> +#undef NDEBUG /* We want the test to fail even if nbdkit disables
assertions */
>  #include <assert.h>
> 
>  #include "isaligned.h"
> diff --git a/common/include/test-ispowerof2.c
b/common/include/test-ispowerof2.c
> index af3e3b9b..4ea7fdde 100644
> --- a/common/include/test-ispowerof2.c
> +++ b/common/include/test-ispowerof2.c
> @@ -36,6 +36,7 @@
>  #include <stdlib.h>
>  #include <stdint.h>
>  #include <string.h>
> +#undef NDEBUG /* We want the test to fail even if nbdkit disables
assertions */
>  #include <assert.h>
> 
>  #include "ispowerof2.h"
> diff --git a/common/include/test-iszero.c b/common/include/test-iszero.c
> index 1a4f09ff..d53efb4f 100644
> --- a/common/include/test-iszero.c
> +++ b/common/include/test-iszero.c
> @@ -36,6 +36,7 @@
>  #include <stdlib.h>
>  #include <stdint.h>
>  #include <string.h>
> +#undef NDEBUG /* We want the test to fail even if nbdkit disables
assertions */
>  #include <assert.h>
> 
>  #include "iszero.h"
> diff --git a/common/include/test-minmax.c b/common/include/test-minmax.c
> index 7d4173ca..ce559b91 100644
> --- a/common/include/test-minmax.c
> +++ b/common/include/test-minmax.c
> @@ -36,6 +36,7 @@
>  #include <stdlib.h>
>  #include <stdint.h>
>  #include <limits.h>
> +#undef NDEBUG /* We want the test to fail even if nbdkit disables
assertions */
>  #include <assert.h>
>  #include <float.h>
> 
> diff --git a/common/include/test-nextnonzero.c
b/common/include/test-nextnonzero.c
> index 86aee5ec..0c3ad5e3 100644
> --- a/common/include/test-nextnonzero.c
> +++ b/common/include/test-nextnonzero.c
> @@ -36,6 +36,7 @@
>  #include <stdlib.h>
>  #include <stdint.h>
>  #include <string.h>
> +#undef NDEBUG /* We want the test to fail even if nbdkit disables
assertions */
>  #include <assert.h>
> 
>  #include "nextnonzero.h"
> diff --git a/common/bitmap/test-bitmap.c b/common/bitmap/test-bitmap.c
> index cbbbbef8..2323bb7f 100644
> --- a/common/bitmap/test-bitmap.c
> +++ b/common/bitmap/test-bitmap.c
> @@ -40,6 +40,7 @@
>  #include <stdarg.h>
>  #include <string.h>
>  #include <errno.h>
> +#undef NDEBUG /* We want the test to fail even if nbdkit disables
assertions */
>  #include <assert.h>
> 
>  #include <nbdkit-plugin.h>
> diff --git a/common/utils/test-quotes.c b/common/utils/test-quotes.c
> index 72e54fc7..0783be27 100644
> --- a/common/utils/test-quotes.c
> +++ b/common/utils/test-quotes.c
> @@ -37,6 +37,7 @@
>  #include <stdio.h>
>  #include <stdlib.h>
>  #include <string.h>
> +#undef NDEBUG /* We want the test to fail even if nbdkit disables
assertions */
>  #include <assert.h>
>  #include <stdbool.h>
> 
> diff --git a/common/utils/test-vector.c b/common/utils/test-vector.c
> index a2c0db10..be3696b7 100644
> --- a/common/utils/test-vector.c
> +++ b/common/utils/test-vector.c
> @@ -35,6 +35,7 @@
>  #include <stdio.h>
>  #include <stdlib.h>
>  #include <stdint.h>
> +#undef NDEBUG /* We want the test to fail even if nbdkit disables
assertions */
>  #include <assert.h>
> 
>  #include "vector.h"
> diff --git a/tests/test-layers-filter.c b/tests/test-layers-filter.c
> index 66d80835..b77df258 100644
> --- a/tests/test-layers-filter.c
> +++ b/tests/test-layers-filter.c
> @@ -35,6 +35,7 @@
>  #include <stdio.h>
>  #include <stdlib.h>
>  #include <stdint.h>
> +#undef NDEBUG /* We want the test to fail even if nbdkit disables
assertions */
>  #include <assert.h>
> 
>  #include <nbdkit-filter.h>
> diff --git a/tests/test-pause.c b/tests/test-pause.c
> index ba772d4f..09f76c76 100644
> --- a/tests/test-pause.c
> +++ b/tests/test-pause.c
> @@ -40,6 +40,7 @@
>  #include <string.h>
>  #include <unistd.h>
>  #include <time.h>
> +#undef NDEBUG /* We want the test to fail even if nbdkit disables
assertions */
>  #include <assert.h>
>  #include <sys/types.h>
>  #include <sys/socket.h>
> diff --git a/tests/test-stdio-plugin.c b/tests/test-stdio-plugin.c
> index 6af1ba90..f29bf3f7 100644
> --- a/tests/test-stdio-plugin.c
> +++ b/tests/test-stdio-plugin.c
> @@ -32,6 +32,7 @@
> 
>  #include <config.h>
> 
> +#undef NDEBUG /* We want the test to fail even if nbdkit disables
assertions */
>  #include <assert.h>
>  #include <fcntl.h>
>  #include <stdbool.h>
> -- 
> 2.27.0
> 
> _______________________________________________
> Libguestfs mailing list
> Libguestfs@redhat.com
> https://www.redhat.com/mailman/listinfo/libguestfs
-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
Fedora Windows cross-compiler. Compile Windows programs, test, and
build Windows installers. Over 100 libraries supported.
http://fedoraproject.org/wiki/MinGW

On 7/7/20 11:27 AM, Richard W.M. Jones wrote:
> On Tue, Jul 07, 2020 at 09:51:53AM -0500, Eric Blake wrote:
>> While we support compilation with CFLAGS=-DNDEBUG for the brave user
>> desiring to avoid assertion overhead, this causes the tests to be less
>> powerful.  Fortunately, a quick test of './configure
CFLAGS=-DNDEBUG'
>> didn't hit any test failures, but it seems better to just ensure
that
>> assertions always work in our tests, even if they are turned off for
>> the main binary.
>>
>> Signed-off-by: Eric Blake <eblake@redhat.com>
>> ---
>>
>> I don't know if I like this one.  Since 'make check' passed
in a build
>> with CFLAGS=-DNDEBUG, I'm not sure we need it.
> 
> I think we actually do need it.  The tests "fail open" if someone
> compiles with -DNDEBUG so we should ensure that something is being
> tested even under these conditions.
> 
> ACK
Okay, I'll push it, then.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org