Re: [spice-streaming-agent 5/6] build: Add --enable-tests

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, 2018-03-01 at 16:27 +0100, Christophe Fergeau wrote:
> Tests are enabled by default, but are expensive to compile, so they can be
> disabled if that's what one wants. They will also be enabled/disabled by
> default depending on the availability of 'catch'

Thanks again for this :)

I'd still say the "tests are expensive to compile" argument applies
only if we want to build tests during `make all` (which I really think
we shouldn't). Otherwise, the user chose to run `make check` and the
long compilation can't be avoided.

> Signed-off-by: Christophe Fergeau <cfergeau@xxxxxxxxxx>
> ---
>  configure.ac    | 21 +++++++++++++++++++--
>  src/Makefile.am |  3 +++
>  2 files changed, 22 insertions(+), 2 deletions(-)
> 
> diff --git a/configure.ac b/configure.ac
> index ce83153..aa2b179 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -38,8 +38,6 @@ PKG_CHECK_MODULES(XFIXES, xfixes)
>  
>  PKG_CHECK_MODULES(JPEG, libjpeg)
>  
> -AC_CHECK_HEADER([catch/catch.hpp],,[AC_MSG_ERROR([Could not find Catch dependency header (catch/catch.hpp)])])
> -
>  dnl ===========================================================================
>  dnl check compiler flags
>  
> @@ -50,6 +48,25 @@ LIBVIRT_LINKER_NO_INDIRECT
>  AC_SUBST(WARN_CFLAGS)
>  AC_SUBST(WARN_CXXFLAGS)
>  
> +dnl =========================================================================
> +dnl tests
> +AC_ARG_ENABLE([tests],
> +              AS_HELP_STRING([--disable-tests=@<:@yes/no@:>@],
> +                             [Disable tests which can be expensive to compile and require 'catch' @<:@default=auto@:>@]),

In light of the above, I'd consider removing the "expensive to compile"
part.

> +                             [],
> +                             [enable_tests="auto"])
> +case "$enable_tests" in
> +  0|no) enable_tests="no" ;;
> +  1|yes) enable_tests="yes" ;;
> +  auto) enable_tests="auto" ;;
> +  *) 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",)])
> +AS_IF([test "x$enable_tests" = "xyes" && test "x$have_check" != "xyes"],
> +      [AC_MSG_ERROR([Could not find Catch dependency header (catch/catch.hpp)])])
> +AM_CONDITIONAL([ENABLE_TESTS], [test "x$have_check" = "xyes"])
> +
>  AC_DEFINE_DIR([BINDIR], [bindir], [Where binaries are installed to.])
>  
>  AC_OUTPUT([
> diff --git a/src/Makefile.am b/src/Makefile.am
> index 857d763..606f51a 100644
> --- a/src/Makefile.am
> +++ b/src/Makefile.am
> @@ -4,7 +4,10 @@
>  # Copyright 2016-2017 Red Hat Inc. All rights reserved.
>  
>  NULL =
> +
> +if ENABLE_TESTS
>  SUBDIRS = . unittests
> +endif
>  
>  AM_CPPFLAGS = \
>  	-DSPICE_STREAMING_AGENT_PROGRAM \

Otherwise, seems to be doing what we want, but I can't vouch for
correctness :)

Cheers,
Lukas
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]     [Monitors]