[Spice-devel] [spice-streaming-agent PATCH v4] catch: check for any of catch and catch2

[Frediano Ziglio]

> 
> Just fixing the subject (spice-streaming-agent patch v4)
> Uri
> 
> On 2/7/19 2:02 PM, Uri Lublin wrote:
> > Catch2 is now in upstream (github) and Fedora (since Fedora 27)
> > 
> > Signed-off-by: Uri Lublin <uril at redhat.com>
> > ---
> > 
> > Since v3 (local only):
> >    - Use AC_CHECK_HEADERS instead of AC_CHECK_HEADER
> > Since v2:
> >    - Do not assume the .h files are under /usr/include
> > Since v1:
> >    - Check both Catch2 and Catch
> > 
> > ---
> >   configure.ac                          |  6 ++++--
> >   src/unittests/Makefile.am             |  2 ++
> >   src/unittests/spice-catch.hpp         | 18 ++++++++++++++++++
> >   src/unittests/test-mjpeg-fallback.cpp |  2 +-
> >   src/unittests/test-stream-port.cpp    |  2 +-
> >   5 files changed, 26 insertions(+), 4 deletions(-)
> >   create mode 100644 src/unittests/spice-catch.hpp
> > 
> > diff --git a/configure.ac b/configure.ac
> > index c259f7e..1c7788b 100644
> > --- a/configure.ac
> > +++ b/configure.ac
> > @@ -119,9 +119,11 @@ case "$enable_tests" in
> >     *) AC_MSG_ERROR([bad value ${enable_tests} for enable-tests option]) ;;
> >   esac
> >   AS_IF([test "x$enable_tests" != "xno"],
> > -      [AC_CHECK_HEADER([catch/catch.hpp],have_check="yes",)])
> > +      [AC_CHECK_HEADERS([catch/catch.hpp],  have_check="yes")
> > +       AC_CHECK_HEADERS([catch2/catch.hpp], have_check="yes")])

I would check "catch2" before "catch" as newer versions use "catch2".

> > +
> >   AS_IF([test "x$enable_tests" = "xyes" && test "x$have_check" != "xyes"],
> > -      [AC_MSG_ERROR([Could not find Catch dependency header
> > (catch/catch.hpp)])])
> > +      [AC_MSG_ERROR([Could not find Catch dependency header
> > (catch.hpp)])])
> >   AM_CONDITIONAL([ENABLE_TESTS], [test "x$have_check" = "xyes"])
> >   
> >   AC_DEFINE_DIR([BINDIR], [bindir], [Where binaries are installed.])
> > diff --git a/src/unittests/Makefile.am b/src/unittests/Makefile.am
> > index 8ce1f7a..556b885 100644
> > --- a/src/unittests/Makefile.am
> > +++ b/src/unittests/Makefile.am
> > @@ -49,6 +49,7 @@ test_mjpeg_fallback_SOURCES = \
> >   	../mjpeg-fallback.cpp \
> >   	../utils.cpp \
> >   	../x11-display-info.cpp \
> > +	spice-catch.hpp \
> >   	$(NULL)
> >   
> >   test_mjpeg_fallback_LDADD = \
> > @@ -61,6 +62,7 @@ test_mjpeg_fallback_LDADD = \
> >   test_stream_port_SOURCES = \
> >   	test-stream-port.cpp \
> >   	../stream-port.cpp \
> > +	spice-catch.hpp \
> >   	$(NULL)
> >   
> >   EXTRA_DIST = \
> > diff --git a/src/unittests/spice-catch.hpp b/src/unittests/spice-catch.hpp
> > new file mode 100644
> > index 0000000..69aac90
> > --- /dev/null
> > +++ b/src/unittests/spice-catch.hpp
> > @@ -0,0 +1,18 @@
> > +/*
> > + * Include catch/catch.hpp or catch2/catch.hpp
> > + * according to what configure found
> > + *
> > + * \copyright
> > + * Copyright 2019 Red Hat Inc. All rights reserved.
> > + */
> > +
> > +#ifndef SPICE_CATCH_HPP
> > +#include <config.h>
> > +

Could be seen weird to include config.h in an header but
as is limited to test director I don't find it an issue.

Could be an option to have

#if !defined(HAVE_CATCH2_CATCH_HPP) && !defined(HAVE_CATCH_CATCH_HPP)
#include <config.h>
#endif

so you can avoid to include from module and header.

> > +#if   defined(HAVE_CATCH2_CATCH_HPP)
> > +#include <catch2/catch.hpp>
> > +#elif defined(HAVE_CATCH_CATCH_HPP)
> > +#include <catch/catch.hpp>
> > +#endif
> > +
> > +#endif // SPICE_CATCH_HPP
> > diff --git a/src/unittests/test-mjpeg-fallback.cpp
> > b/src/unittests/test-mjpeg-fallback.cpp
> > index e39dc49..49eef98 100644
> > --- a/src/unittests/test-mjpeg-fallback.cpp
> > +++ b/src/unittests/test-mjpeg-fallback.cpp
> > @@ -1,5 +1,5 @@
> >   #define CATCH_CONFIG_MAIN
> > -#include "catch/catch.hpp"
> > +#include "spice-catch.hpp"
> >   
> >   #include "mjpeg-fallback.hpp"
> >   
> > diff --git a/src/unittests/test-stream-port.cpp
> > b/src/unittests/test-stream-port.cpp
> > index e7b7b89..8c77979 100644
> > --- a/src/unittests/test-stream-port.cpp
> > +++ b/src/unittests/test-stream-port.cpp
> > @@ -5,7 +5,7 @@
> >    */
> >   
> >   #define CATCH_CONFIG_MAIN
> > -#include <catch/catch.hpp>
> > +#include "spice-catch.hpp"
> >   #include <sys/socket.h>
> >   #include <signal.h>
> >   
> > 

Otherwise,

Acked-by: Frediano Ziglio <fziglio at redhat.com>

Frediano