Bug 66291

Summary: check DBUS_BUILD_EMBEDDED_TESTS instead of DBUS_BUILD_TESTS
Product: dbus Reporter: Chengwei Yang <chengwei.yang.cn>
Component: coreAssignee: Havoc Pennington <hp>
Status: RESOLVED FIXED QA Contact:
Severity: normal    
Priority: medium CC: chengwei.yang.cn
Version: 1.5   
Hardware: Other   
OS: All   
Whiteboard:
i915 platform: i915 features:
Attachments: [PATCH 1/5] tests to embedded tests: replaced in automake files
[PATCH 2/5] tests to embedded tests: replaced in cmake files
[PATCH 3/5] tests to embedded tests: replaced in dbus-daemon
[PATCH 4/5] tests to embedded tests: replaced in libdbus
[PATCH 5/5] tests to embedded tests: replaced in tools

Description Chengwei Yang 2013-06-28 01:50:38 UTC
Quote from Simon commont on #bug66257
"
I'd accept patches that move from checking DBUS_BUILD_TESTS to checking DBUS_ENABLE_EMBEDDED_TESTS (but only on master, and only a few related files at a time please - not all at once :-)
"

So this issue used to track patches to do that.
Comment 1 Chengwei Yang 2013-06-28 08:37:48 UTC
Created attachment 81623 [details] [review]
[PATCH 1/5] tests to embedded tests: replaced in automake files
Comment 2 Chengwei Yang 2013-06-28 08:38:14 UTC
Created attachment 81624 [details] [review]
[PATCH 2/5] tests to embedded tests: replaced in cmake files
Comment 3 Chengwei Yang 2013-06-28 08:38:38 UTC
Created attachment 81625 [details] [review]
[PATCH 3/5] tests to embedded tests: replaced in dbus-daemon
Comment 4 Chengwei Yang 2013-06-28 08:38:59 UTC
Created attachment 81626 [details] [review]
[PATCH 4/5] tests to embedded tests: replaced in libdbus
Comment 5 Chengwei Yang 2013-06-28 08:39:20 UTC
Created attachment 81627 [details] [review]
[PATCH 5/5] tests to embedded tests: replaced in tools
Comment 6 Simon McVittie 2013-06-28 11:11:48 UTC
Comment on attachment 81623 [details] [review]
[PATCH 1/5] tests to embedded tests: replaced in automake files

Review of attachment 81623 [details] [review]:
-----------------------------------------------------------------

::: configure.ac
@@ -201,5 @@
>  if test "x$enable_embedded_tests" = xyes; then
>      AC_DEFINE([DBUS_ENABLE_EMBEDDED_TESTS], [1],
>        [Define to build test code into the library and binaries])
> -    AC_DEFINE([DBUS_BUILD_TESTS], [1],
> -      [Define to build test code into the library and binaries])

Not until all of its uses have been replaced.
Comment 7 Simon McVittie 2013-06-28 11:18:39 UTC
Comment on attachment 81624 [details] [review]
[PATCH 2/5] tests to embedded tests: replaced in cmake files

Review of attachment 81624 [details] [review]:
-----------------------------------------------------------------

::: cmake/CMakeLists.txt
@@ +234,5 @@
>   
>  if(DBUS_BUILD_TESTS)
> +    set (DBUS_ENABLE_EMBEDDED_TESTS ON)
> +    set (DBUS_ENABLE_MODULAR_TESTS ON)
> +    add_definitions(-DDBUS_ENABLE_EMBEDDED_TESTS -DDBUS_ENABLE_MODULAR_TESTS)

I don't think the CMake build system has the modular tests yet? But this seems OK to add.
Comment 8 Simon McVittie 2013-06-28 11:21:27 UTC
With some re-ordering so it doesn't break at intermediate points, this all seems good, thanks. In git for 1.7.6.

I would have preferred a sequence more like this, to make it obvious that each intermediate step was correct:

* test DBUS_ENABLE_EMBEDDED_TESTS in C code, but keep defining DBUS_BUILD_TESTS too
* test DBUS_ENABLE_EMBEDDED_TESTS in *.am and equivalent CMake conditionals
* stop defining DBUS_BUILD_TESTS at all
Comment 9 Chengwei Yang 2013-06-28 11:43:30 UTC
(In reply to comment #8)
> With some re-ordering so it doesn't break at intermediate points, this all
> seems good, thanks. In git for 1.7.6.
> 
> I would have preferred a sequence more like this, to make it obvious that
> each intermediate step was correct:
> 
> * test DBUS_ENABLE_EMBEDDED_TESTS in C code, but keep defining
> DBUS_BUILD_TESTS too
> * test DBUS_ENABLE_EMBEDDED_TESTS in *.am and equivalent CMake conditionals
> * stop defining DBUS_BUILD_TESTS at all

Yes, that's the recommended sequence, I was doing in that sequence because I did want a build failure if I lost any thing. :-).

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.