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

[Frediano Ziglio]

> 
> On Thu, Aug 03, 2017 at 09:24:51AM -0400, Frediano Ziglio wrote:
> > > 
> > > 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)
> > > 
> > 
> > Yes, I know.
> > 
> > > I'd prefer to keep this line near the top, which means the if
> > > HAVE_GDK_PIXBUF block not moving.
> > > 
> > 
> > I didn't propose to move it... to remove it (the top one).
> 
> If your suggestion is
> 
> TESTS = test_logging test_marshallers
> noinst_PROGRAMS = $(TESTS)
> 
> ...
> 
> if HAVE_GDK_PIXBUF
> 
> TESTS += test_quic
> test_quic_SOURCES = ...
> test_quic_CFLAGS = ...
> 
> endif
> 
> Then noinst_PROGRAMS is not going to contain "test_quic"

it will

> I quickly tested it and this does not seem to create issues, but this
> still does not feel correct.
> 

Makefile rules :-)

> Is this what you were suggesting, or did I misunderstand?
> 
> Christophe
> 

Frediano