Re: [pulseaudio-discuss] [PATCH] tests: make 'check' optional
Thanks a lot for the follow-ups. 2012/10/4 Tanu Kaskinen : > On Mon, 2012-08-20 at 16:24 +0800, rong deng wrote: >> ping. > > Sorry to keep you waiting. I've pushed the patch now (I needed it in my > own packaging work). Thanks for the contribution! > > -- > Tanu > ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH] tests: make 'check' optional
On Mon, 2012-08-20 at 16:24 +0800, rong deng wrote: > ping. Sorry to keep you waiting. I've pushed the patch now (I needed it in my own packaging work). Thanks for the contribution! -- Tanu ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH] tests: make 'check' optional
ping.
2012/8/13 rong deng :
> 2012/8/13 Deng Zhengrong :
>> ---
>> configure.ac| 17 +++--
>> src/Makefile.am | 14 ++
>> 2 files changed, 29 insertions(+), 2 deletions(-)
>>
>> diff --git a/configure.ac b/configure.ac
>> index ffb2a35..43ccf44 100644
>> --- a/configure.ac
>> +++ b/configure.ac
>> @@ -579,12 +579,23 @@ fi
>>
>> AC_CHECK_HEADERS_ONCE([valgrind/memcheck.h])
>>
>> - check test framework
>> + check unit tests
>> +
>> +AC_ARG_ENABLE([tests],
>> +AS_HELP_STRING([--disable-tests],[Disable unit tests]))
>> +
>> +AS_IF([test "x$enable_tests" != "xno"],
>> +[PKG_CHECK_MODULES(LIBCHECK, [ check ], HAVE_LIBCHECK=1,
>> HAVE_LIBCHECK=0)],
>> +HAVE_LIBCHECK=0)
>>
>> -PKG_CHECK_MODULES(LIBCHECK, [ check ])
>> AC_SUBST(LIBCHECK_CFLAGS)
>> AC_SUBST(LIBCHECK_LIBS)
>>
>> +AS_IF([test "x$enable_tests" = "xyes" && test "x$HAVE_LIBCHECK" = "x0"],
>> +[AC_MSG_ERROR([*** check library not found])])
>> +
>> +AM_CONDITIONAL([HAVE_TESTS], [test "x$HAVE_LIBCHECK" = x1])
>
> Note the above code that when HAVE_LIBCHECK is therer, HAVE_TESTS is
> 1, I don't want to use HAVE_TESTS in PKG_CHECK_MODULES() because I try
> to differentiate. and LIBCHECK_LIBS and LIBCHECK_CFLAGS are more clear
> than TESTS_LIBS and TESTS_CFLAGS.
>
>> +
>> json parsing
>>
>> PKG_CHECK_MODULES(LIBJSON, [ json >= 0.9 ])
>> @@ -1393,6 +1404,7 @@ AS_IF([test "x$HAVE_SIMPLEDB" = "x1"],
>> ENABLE_SIMPLEDB=yes, ENABLE_SIMPLEDB=no)
>> AS_IF([test "x$HAVE_ESOUND" = "x1"], ENABLE_ESOUND=yes, ENABLE_ESOUND=no)
>> AS_IF([test "x$HAVE_ESOUND" = "x1" -a "x$USE_PER_USER_ESOUND_SOCKET" =
>> "x1"], ENABLE_PER_USER_ESOUND_SOCKET=yes, ENABLE_PER_USER_ESOUND_SOCKET=no)
>> AS_IF([test "x$HAVE_GCOV" = "x1"], ENABLE_GCOV=yes, ENABLE_GCOV=no)
>> +AS_IF([test "x$HAVE_LIBCHECK" = "x1"], ENABLE_TESTS=yes, ENABLE_TESTS=no)
>> AS_IF([test "x$enable_legacy_database_entry_format" != "xno"],
>> ENABLE_LEGACY_DATABASE_ENTRY_FORMAT=yes,
>> ENABLE_LEGACY_DATABASE_ENTRY_FORMAT=no)
>>
>> echo "
>> @@ -1440,6 +1452,7 @@ echo "
>> Enable speex (resampler, AEC): ${ENABLE_SPEEX}
>> Enable WebRTC echo canceller: ${ENABLE_WEBRTC}
>> Enable gcov coverage: ${ENABLE_GCOV}
>> +Enable unit tests: ${ENABLE_TESTS}
>> Database
>>tdb: ${ENABLE_TDB}
>>gdbm:${ENABLE_GDBM}
>> diff --git a/src/Makefile.am b/src/Makefile.am
>> index 2f20df2..f7f8333 100644
>> --- a/src/Makefile.am
>> +++ b/src/Makefile.am
>> @@ -297,6 +297,7 @@ TESTS_norun += \
>> alsa-time-test
>> endif
>>
>> +if HAVE_TESTS
>> TESTS_ENVIRONMENT=MAKE_CHECK=1
>> TESTS = $(TESTS_default)
>>
>> @@ -309,6 +310,19 @@ endif
>> check-daemon: $(TESTS_daemon)
>> PATH=$(builddir):${PATH} $(top_srcdir)/src/tests/test-daemon.sh
>> $(TESTS_daemon)
>>
>> +else
>> +TESTS_ENVIRONMENT=
>> +TESTS =
>> +noinst_PROGRAMS =
>> +check_PROGRAMS =
>> +
>> +check-daemon:
>> + @echo "Tests are disabled!"
>> + @echo "Pass option \"--enable-tests\" to configure and install
>> \"check\" library properly!"
>> + false
>> +
>> +endif
>> +
>> mainloop_test_SOURCES = tests/mainloop-test.c
>> mainloop_test_CFLAGS = $(AM_CFLAGS) $(LIBCHECK_CFLAGS)
>> mainloop_test_LDADD = $(AM_LDADD) libpulse.la
>> libpulsecommon-@PA_MAJORMINOR@.la
>> --
>> 1.7.7.6
>>
___
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH] tests: make 'check' optional
2012/8/13 Deng Zhengrong :
> ---
> configure.ac| 17 +++--
> src/Makefile.am | 14 ++
> 2 files changed, 29 insertions(+), 2 deletions(-)
>
> diff --git a/configure.ac b/configure.ac
> index ffb2a35..43ccf44 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -579,12 +579,23 @@ fi
>
> AC_CHECK_HEADERS_ONCE([valgrind/memcheck.h])
>
> - check test framework
> + check unit tests
> +
> +AC_ARG_ENABLE([tests],
> +AS_HELP_STRING([--disable-tests],[Disable unit tests]))
> +
> +AS_IF([test "x$enable_tests" != "xno"],
> +[PKG_CHECK_MODULES(LIBCHECK, [ check ], HAVE_LIBCHECK=1,
> HAVE_LIBCHECK=0)],
> +HAVE_LIBCHECK=0)
>
> -PKG_CHECK_MODULES(LIBCHECK, [ check ])
> AC_SUBST(LIBCHECK_CFLAGS)
> AC_SUBST(LIBCHECK_LIBS)
>
> +AS_IF([test "x$enable_tests" = "xyes" && test "x$HAVE_LIBCHECK" = "x0"],
> +[AC_MSG_ERROR([*** check library not found])])
> +
> +AM_CONDITIONAL([HAVE_TESTS], [test "x$HAVE_LIBCHECK" = x1])
Note the above code that when HAVE_LIBCHECK is therer, HAVE_TESTS is
1, I don't want to use HAVE_TESTS in PKG_CHECK_MODULES() because I try
to differentiate. and LIBCHECK_LIBS and LIBCHECK_CFLAGS are more clear
than TESTS_LIBS and TESTS_CFLAGS.
> +
> json parsing
>
> PKG_CHECK_MODULES(LIBJSON, [ json >= 0.9 ])
> @@ -1393,6 +1404,7 @@ AS_IF([test "x$HAVE_SIMPLEDB" = "x1"],
> ENABLE_SIMPLEDB=yes, ENABLE_SIMPLEDB=no)
> AS_IF([test "x$HAVE_ESOUND" = "x1"], ENABLE_ESOUND=yes, ENABLE_ESOUND=no)
> AS_IF([test "x$HAVE_ESOUND" = "x1" -a "x$USE_PER_USER_ESOUND_SOCKET" =
> "x1"], ENABLE_PER_USER_ESOUND_SOCKET=yes, ENABLE_PER_USER_ESOUND_SOCKET=no)
> AS_IF([test "x$HAVE_GCOV" = "x1"], ENABLE_GCOV=yes, ENABLE_GCOV=no)
> +AS_IF([test "x$HAVE_LIBCHECK" = "x1"], ENABLE_TESTS=yes, ENABLE_TESTS=no)
> AS_IF([test "x$enable_legacy_database_entry_format" != "xno"],
> ENABLE_LEGACY_DATABASE_ENTRY_FORMAT=yes,
> ENABLE_LEGACY_DATABASE_ENTRY_FORMAT=no)
>
> echo "
> @@ -1440,6 +1452,7 @@ echo "
> Enable speex (resampler, AEC): ${ENABLE_SPEEX}
> Enable WebRTC echo canceller: ${ENABLE_WEBRTC}
> Enable gcov coverage: ${ENABLE_GCOV}
> +Enable unit tests: ${ENABLE_TESTS}
> Database
>tdb: ${ENABLE_TDB}
>gdbm:${ENABLE_GDBM}
> diff --git a/src/Makefile.am b/src/Makefile.am
> index 2f20df2..f7f8333 100644
> --- a/src/Makefile.am
> +++ b/src/Makefile.am
> @@ -297,6 +297,7 @@ TESTS_norun += \
> alsa-time-test
> endif
>
> +if HAVE_TESTS
> TESTS_ENVIRONMENT=MAKE_CHECK=1
> TESTS = $(TESTS_default)
>
> @@ -309,6 +310,19 @@ endif
> check-daemon: $(TESTS_daemon)
> PATH=$(builddir):${PATH} $(top_srcdir)/src/tests/test-daemon.sh
> $(TESTS_daemon)
>
> +else
> +TESTS_ENVIRONMENT=
> +TESTS =
> +noinst_PROGRAMS =
> +check_PROGRAMS =
> +
> +check-daemon:
> + @echo "Tests are disabled!"
> + @echo "Pass option \"--enable-tests\" to configure and install
> \"check\" library properly!"
> + false
> +
> +endif
> +
> mainloop_test_SOURCES = tests/mainloop-test.c
> mainloop_test_CFLAGS = $(AM_CFLAGS) $(LIBCHECK_CFLAGS)
> mainloop_test_LDADD = $(AM_LDADD) libpulse.la
> libpulsecommon-@PA_MAJORMINOR@.la
> --
> 1.7.7.6
>
___
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH] tests: make 'check' optional
On Thu, 2012-08-09 at 10:01 +0800, Deng Zhengrong wrote:
> ---
> configure.ac| 15 ++-
> src/Makefile.am | 12
> 2 files changed, 26 insertions(+), 1 deletions(-)
>
> diff --git a/configure.ac b/configure.ac
> index ffb2a35..9cb5aa6 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -581,10 +581,21 @@ AC_CHECK_HEADERS_ONCE([valgrind/memcheck.h])
>
> check test framework
>
> -PKG_CHECK_MODULES(LIBCHECK, [ check ])
> +AC_ARG_ENABLE([check],
> +AS_HELP_STRING([--disable-check],[Disable unit tests]))
If the help text is "Disable unit tests" (which is good in my opinion),
I think the AC_ARG_ENABLE parameter should be "tests" instead of
"check", and "--disable-check" should be "--disable-tests".
> +
> +AS_IF([test "x$enable_check" != "xno"],
> +[PKG_CHECK_MODULES(LIBCHECK, [ check ], HAVE_LIBCHECK=1,
> HAVE_LIBCHECK=0)],
> +HAVE_LIBCHECK=0)
> +
> +AS_IF([test "x$enable_check" = "xyes" && test "x$HAVE_LIBCHECK" = "x0"],
> +[AC_MSG_ERROR([*** check library not found])])
> +
> AC_SUBST(LIBCHECK_CFLAGS)
> AC_SUBST(LIBCHECK_LIBS)
>
> +AM_CONDITIONAL([HAVE_LIBCHECK], [test "x$HAVE_LIBCHECK" = x1])
> +
> json parsing
>
> PKG_CHECK_MODULES(LIBJSON, [ json >= 0.9 ])
> @@ -1393,6 +1404,7 @@ AS_IF([test "x$HAVE_SIMPLEDB" = "x1"],
> ENABLE_SIMPLEDB=yes, ENABLE_SIMPLEDB=no)
> AS_IF([test "x$HAVE_ESOUND" = "x1"], ENABLE_ESOUND=yes, ENABLE_ESOUND=no)
> AS_IF([test "x$HAVE_ESOUND" = "x1" -a "x$USE_PER_USER_ESOUND_SOCKET" =
> "x1"], ENABLE_PER_USER_ESOUND_SOCKET=yes, ENABLE_PER_USER_ESOUND_SOCKET=no)
> AS_IF([test "x$HAVE_GCOV" = "x1"], ENABLE_GCOV=yes, ENABLE_GCOV=no)
> +AS_IF([test "x$HAVE_LIBCHECK" = "x1"], ENABLE_LIBCHECK=yes,
> ENABLE_LIBCHECK=no)
> AS_IF([test "x$enable_legacy_database_entry_format" != "xno"],
> ENABLE_LEGACY_DATABASE_ENTRY_FORMAT=yes,
> ENABLE_LEGACY_DATABASE_ENTRY_FORMAT=no)
>
> echo "
> @@ -1440,6 +1452,7 @@ echo "
> Enable speex (resampler, AEC): ${ENABLE_SPEEX}
> Enable WebRTC echo canceller: ${ENABLE_WEBRTC}
> Enable gcov coverage: ${ENABLE_GCOV}
> +Enable check unit tests: ${ENABLE_LIBCHECK}
Since you disable all tests if the check framework is not available,
regardless of whether the tests use the check framework or not, I think
the message should be just "Enable unit tests", without the "check"
specifier. And instead of printing the ENABLE_LIBCHECK value, you should
print the enable_tests value (or maybe there should be a new variable
ENABLE_TESTS to be more consistent).
> Database
>tdb: ${ENABLE_TDB}
>gdbm:${ENABLE_GDBM}
> diff --git a/src/Makefile.am b/src/Makefile.am
> index 2f20df2..dfa055e 100644
> --- a/src/Makefile.am
> +++ b/src/Makefile.am
> @@ -297,6 +297,7 @@ TESTS_norun += \
> alsa-time-test
> endif
>
> +if HAVE_LIBCHECK
> TESTS_ENVIRONMENT=MAKE_CHECK=1
> TESTS = $(TESTS_default)
>
> @@ -309,6 +310,17 @@ endif
> check-daemon: $(TESTS_daemon)
> PATH=$(builddir):${PATH} $(top_srcdir)/src/tests/test-daemon.sh
> $(TESTS_daemon)
>
> +else
> +TESTS_ENVIRONMENT=
> +TESTS =
> +noinst_PROGRAMS =
> +check_PROGRAMS =
I think you should check ENABLE_TESTS instead of HAVE_LIBCHECK, if
you're going to disable all tests.
> +
> +check-daemon:
> + @echo "Please don't specify \"--disable-check\" and make sure check
> library is installed!"
I think "Tests are disabled" would be a more appropriate message,
because that's the problem. After stating the problem, giving hints for
how to resolve the problem would be fine, though.
Also, I think that make should be somehow informed that an error
happened. Maybe add a call to false (I mean /bin/false, in case it was
unclear) after the echo? It feels a bit clumsy to me, though, but I'm
not aware of other ways to do it (I'm not really a make expert).
--
Tanu
___
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH] tests: make 'check' optional
2012/8/9, David Henningsson : > Seems to do what it should. Sorry if I was a little harsh on you > yesterday, it's just that bringing in new dependencies is nothing to > take lightly. It brings work upon others, e g packagers. Most > dependencies also means that size increases, which means that on an > ever-overflowing Live-CD we have to throw something else out. (Check > being the exception I assume, as it is just a build-time and not a > run-time dependency.) > > So, Thanks for this patch, and > Acked-by: David Henningsson Thanks for your quick review. This is what this mailing list is aimed to do. We argue, we learn, we improve. :) deng ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH] tests: make 'check' optional
Seems to do what it should. Sorry if I was a little harsh on you
yesterday, it's just that bringing in new dependencies is nothing to
take lightly. It brings work upon others, e g packagers. Most
dependencies also means that size increases, which means that on an
ever-overflowing Live-CD we have to throw something else out. (Check
being the exception I assume, as it is just a build-time and not a
run-time dependency.)
So, Thanks for this patch, and
Acked-by: David Henningsson
Because I'm not a build system expert, and because I tend to make stupid
mistakes today, I'll like somebody else have a look (and commit) as well.
On 08/09/2012 04:01 AM, Deng Zhengrong wrote:
---
configure.ac| 15 ++-
src/Makefile.am | 12
2 files changed, 26 insertions(+), 1 deletions(-)
diff --git a/configure.ac b/configure.ac
index ffb2a35..9cb5aa6 100644
--- a/configure.ac
+++ b/configure.ac
@@ -581,10 +581,21 @@ AC_CHECK_HEADERS_ONCE([valgrind/memcheck.h])
check test framework
-PKG_CHECK_MODULES(LIBCHECK, [ check ])
+AC_ARG_ENABLE([check],
+AS_HELP_STRING([--disable-check],[Disable unit tests]))
+
+AS_IF([test "x$enable_check" != "xno"],
+[PKG_CHECK_MODULES(LIBCHECK, [ check ], HAVE_LIBCHECK=1, HAVE_LIBCHECK=0)],
+HAVE_LIBCHECK=0)
+
+AS_IF([test "x$enable_check" = "xyes" && test "x$HAVE_LIBCHECK" = "x0"],
+[AC_MSG_ERROR([*** check library not found])])
+
AC_SUBST(LIBCHECK_CFLAGS)
AC_SUBST(LIBCHECK_LIBS)
+AM_CONDITIONAL([HAVE_LIBCHECK], [test "x$HAVE_LIBCHECK" = x1])
+
json parsing
PKG_CHECK_MODULES(LIBJSON, [ json >= 0.9 ])
@@ -1393,6 +1404,7 @@ AS_IF([test "x$HAVE_SIMPLEDB" = "x1"],
ENABLE_SIMPLEDB=yes, ENABLE_SIMPLEDB=no)
AS_IF([test "x$HAVE_ESOUND" = "x1"], ENABLE_ESOUND=yes, ENABLE_ESOUND=no)
AS_IF([test "x$HAVE_ESOUND" = "x1" -a "x$USE_PER_USER_ESOUND_SOCKET" = "x1"],
ENABLE_PER_USER_ESOUND_SOCKET=yes, ENABLE_PER_USER_ESOUND_SOCKET=no)
AS_IF([test "x$HAVE_GCOV" = "x1"], ENABLE_GCOV=yes, ENABLE_GCOV=no)
+AS_IF([test "x$HAVE_LIBCHECK" = "x1"], ENABLE_LIBCHECK=yes, ENABLE_LIBCHECK=no)
AS_IF([test "x$enable_legacy_database_entry_format" != "xno"],
ENABLE_LEGACY_DATABASE_ENTRY_FORMAT=yes, ENABLE_LEGACY_DATABASE_ENTRY_FORMAT=no)
echo "
@@ -1440,6 +1452,7 @@ echo "
Enable speex (resampler, AEC): ${ENABLE_SPEEX}
Enable WebRTC echo canceller: ${ENABLE_WEBRTC}
Enable gcov coverage: ${ENABLE_GCOV}
+Enable check unit tests: ${ENABLE_LIBCHECK}
Database
tdb: ${ENABLE_TDB}
gdbm:${ENABLE_GDBM}
diff --git a/src/Makefile.am b/src/Makefile.am
index 2f20df2..dfa055e 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -297,6 +297,7 @@ TESTS_norun += \
alsa-time-test
endif
+if HAVE_LIBCHECK
TESTS_ENVIRONMENT=MAKE_CHECK=1
TESTS = $(TESTS_default)
@@ -309,6 +310,17 @@ endif
check-daemon: $(TESTS_daemon)
PATH=$(builddir):${PATH} $(top_srcdir)/src/tests/test-daemon.sh
$(TESTS_daemon)
+else
+TESTS_ENVIRONMENT=
+TESTS =
+noinst_PROGRAMS =
+check_PROGRAMS =
+
+check-daemon:
+ @echo "Please don't specify \"--disable-check\" and make sure check library
is installed!"
+
+endif
+
mainloop_test_SOURCES = tests/mainloop-test.c
mainloop_test_CFLAGS = $(AM_CFLAGS) $(LIBCHECK_CFLAGS)
mainloop_test_LDADD = $(AM_LDADD) libpulse.la
libpulsecommon-@PA_MAJORMINOR@.la
--
David Henningsson, Canonical Ltd.
https://launchpad.net/~diwic
___
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
