On Tue, Aug 18, 2009 at 10:32:27AM -0400, Stephen Gallagher wrote:
> This patch resolves
https://fedorahosted.org/sssd/ticket/116
>
> --with-tests was confusing. Since we now build our tests only with
> 'make check', it doesn't make sense for this to be a configure-
> time option. We will detect during configure whether the 'check'
> package is available and we will use them if so. Otherwise, we
> will only build and execute any test suites that do not rely on
> the 'check' framework.
>
This patch works nicely for me with and without check/check-devel
installed. Neverteless I have a few comments.
I think it would be nice to print a message when 'make check' is called
and check is not installed that not all test are executed and that
people should install check to run all tests.
More comments inline.
bye,
Sumit
> Stephen Gallagher
> RHCE 804006346421761
>
> Looking to carve out IT costs?
>
www.redhat.com/carveoutcosts/
> From a44d2add848578509538427da5533067069727fc Mon Sep 17 00:00:00 2001
> From: Stephen Gallagher <sgallagh(a)redhat.com>
> Date: Tue, 18 Aug 2009 10:29:53 -0400
> Subject: [PATCH] Eliminate the --with-tests configure flag
>
> --with-tests was confusing. Since we now build our tests only with
> 'make check', it doesn't make sense for this to be a configure-
> time option. We will detect during configure whether the 'check'
> package is available and we will use them if so. Otherwise, we
> will only build and execute any test suites that do not rely on
> the 'check' framework.
> ---
> server/Makefile.am | 26 ++++++++++++++++++--------
> server/conf_macros.m4 | 18 ------------------
> server/configure.ac | 8 ++++----
> 3 files changed, 22 insertions(+), 30 deletions(-)
>
> diff --git a/server/Makefile.am b/server/Makefile.am
> index 7ef7eb7..ce5978b 100644
> --- a/server/Makefile.am
> +++ b/server/Makefile.am
> @@ -49,12 +49,21 @@ sssdlibexec_PROGRAMS = \
> $(sssd_pk) \
> $(sssd_info)
>
> -if BUILD_TESTS
> check_PROGRAMS = \
> - sysdb-tests \
> - stress-tests \
> - resolv-tests
> -TESTS = sysdb-tests resolv-tests
> + stress-tests
> +
> +if HAVE_CHECK
> + check_PROGRAMS += \
> + sysdb-tests \
> + resolv-tests
additional whitespace ^
> +endif
> +
> +TESTS =
> +
> +if HAVE_CHECK
> + TESTS += \
> + sysdb-tests \
> + resolv-tests
> endif
>
I would prefer to only one place to list the check based tests like
non_interactive_check_based_tests = \
sysdb-tests \
resolv-tests
and include this variable at the right places.
_______________________________________________
sssd-devel mailing list
sssd-devel(a)lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/sssd-devel
New patch attached. We will now print a warning while running
'configure' if the CHECK libraries are not printed. The other review
recommendations have also been implemented.
--
Stephen Gallagher
RHCE 804006346421761
Looking to carve out IT costs?