[Spice-devel] [spice-common v2 13/13] quic: Add test case for compression/decompression

[Christophe Fergeau]

On Wed, Aug 02, 2017 at 05:32:04AM -0400, Frediano Ziglio wrote:
> > 
> > This only adds a basic test relying on gdk-pixbuf.
> > The main limitation is that gdk-pixbuf does not handle 16bpp images,
> > nor 32bpp/no alpha images. I should have picked something else instead ;)
> > 
> > This allows at least to exercise the QUIC_IMAGE_TYPE_RGB24 and
> > QUIC_IMAGE_TYPE_RGBA codepaths.
> > 
> > Signed-off-by: Christophe Fergeau <cfergeau at redhat.com>
> > ---
> >  configure.ac      |   1 +
> >  m4/spice-deps.m4  |  15 ++++
> >  tests/Makefile.am |  22 +++++
> >  tests/test-quic.c | 235
> >  ++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  4 files changed, 273 insertions(+)
> >  create mode 100644 tests/test-quic.c
> > 
> > diff --git a/configure.ac b/configure.ac
> > index 3542161..1f2ecc0 100644
> > --- a/configure.ac
> > +++ b/configure.ac
> > @@ -47,6 +47,7 @@ SPICE_CHECK_CELT051
> >  SPICE_CHECK_GLIB2
> >  SPICE_CHECK_OPUS
> >  SPICE_CHECK_OPENSSL
> > +SPICE_CHECK_GDK_PIXBUF
> >  
> >  SPICE_COMMON_CFLAGS='$(PIXMAN_CFLAGS) $(SMARTCARD_CFLAGS) $(CELT051_CFLAGS)
> >  $(GLIB2_CFLAGS) $(OPUS_CFLAGS) $(OPENSSL_CFLAGS)'
> >  SPICE_COMMON_CFLAGS="$SPICE_COMMON_CFLAGS -DG_LOG_DOMAIN=\\\"Spice\\\""
> > diff --git a/m4/spice-deps.m4 b/m4/spice-deps.m4
> > index 68e3091..3fe4a5b 100644
> > --- a/m4/spice-deps.m4
> > +++ b/m4/spice-deps.m4
> > @@ -147,6 +147,21 @@ AC_DEFUN([SPICE_CHECK_GLIB2], [
> >      PKG_CHECK_MODULES(GLIB2, glib-2.0 >= 2.22 gio-2.0 >= 2.22 gthread-2.0 >=
> >      2.22)
> >  ])
> >  
> > +# SPICE_CHECK_GDK_PIXBUF
> > +# ----------------------
> > +# Check for the availability of gdk-pixbuf. If found, it will return the
> > flags to use
> > +# in the GDK_PIXBUF_CFLAGS and GDK_PIXBUF_LIBS variables, and it will define
> > a
> > +# HAVE_GDK_PIXBUF preprocessor symbol as well as a HAVE_GDK_PIXBUF Makefile
> > conditional.
> > +# ----------------
> > +AC_DEFUN([SPICE_CHECK_GDK_PIXBUF], [
> > +    PKG_CHECK_MODULES([GDK_PIXBUF], [gdk-pixbuf-2.0], [have_gdk_pixbuf=yes],
> > [have_gdk_pixbuf=no])

I changed this check to 2.26 due to the use of
gdk_pixbuf_get_byte_length() in the test program.

> > diff --git a/tests/Makefile.am b/tests/Makefile.am
> > index 10033c0..02f679d 100644
> > --- a/tests/Makefile.am
> > +++ b/tests/Makefile.am
> > @@ -1,6 +1,9 @@
> >  NULL =
> >  
> >  TESTS = test_logging test_marshallers
> > +if HAVE_GDK_PIXBUF
> > +TESTS += test_quic
> > +endif
> 
> I would put these with other lines

Note the line just below which uses TESTS:
noinst_PROGRAMS = $(TESTS)

I'd prefer to keep this line near the top, which means the if
HAVE_GDK_PIXBUF block not moving.

> >  

> >  test_logging_SOURCES = test-logging.c
> > @@ -33,6 +36,25 @@ test_marshallers_LDADD =				\
> >  	$(GLIB2_LIBS)			\
> >  	$(NULL)
> >  
> > +
> > +if HAVE_GDK_PIXBUF
> 
> so here
> 
> TESTS += test_quic
> 
> > +test_quic_SOURCES =			\
> > +	test-quic.c			\
> > +	$(NULL)
> > +test_quic_CFLAGS =			\
> > +	-I$(top_srcdir)			\
> > +	$(GLIB2_CFLAGS)			\
> > +	$(GDK_PIXBUF_CFLAGS)		\
> > +	$(PROTOCOL_CFLAGS)		\
> > +	$(NULL)
> > +test_quic_LDADD =					\
> > +	$(top_builddir)/common/libspice-common.la	\
> > +	$(GLIB2_LIBS)					\
> > +	$(GDK_PIXBUF_LIBS)				\
> > +	$(NULL)
> > +endif
> > +
> > +
> 
> Maybe you want also to add test-quic.c to EXTRA_DIST in order to be
> able to build a proper package if gdk-pixbuf is missing.

automake ignore if/else blocks when generating the list of files to put
in the tarball, test-quic.c will be in the tarball regardless of what is
installed on the machine generating it.

> 
> >  # Avoid need for python(pyparsing) by end users
> >  TEST_MARSHALLERS =				\
> >  	generated_test_marshallers.c		\
> > diff --git a/tests/test-quic.c b/tests/test-quic.c
> > new file mode 100644
> > index 0000000..b45794f
> > --- /dev/null
> > +++ b/tests/test-quic.c
> > @@ -0,0 +1,235 @@
> 
> I would put a license.

Yup, fixed all your comments in this file (plus the warnings you pointed
out privately).

Christophe
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/spice-devel/attachments/20170803/5e7a852a/attachment.sig>