Bug 66291 - check DBUS_BUILD_EMBEDDED_TESTS instead of DBUS_BUILD_TESTS
Summary: check DBUS_BUILD_EMBEDDED_TESTS instead of DBUS_BUILD_TESTS
Status: RESOLVED FIXED
Alias: None
Product: dbus
Classification: Unclassified
Component: core (show other bugs)
Version: 1.5
Hardware: Other All
: medium normal
Assignee: Havoc Pennington
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-06-28 01:50 UTC by Chengwei Yang
Modified: 2013-06-28 11:43 UTC (History)
1 user (show)

See Also:
i915 platform:
i915 features:


Attachments
[PATCH 1/5] tests to embedded tests: replaced in automake files (3.84 KB, patch)
2013-06-28 08:37 UTC, Chengwei Yang
Details | Splinter Review
[PATCH 2/5] tests to embedded tests: replaced in cmake files (4.06 KB, patch)
2013-06-28 08:38 UTC, Chengwei Yang
Details | Splinter Review
[PATCH 3/5] tests to embedded tests: replaced in dbus-daemon (8.52 KB, patch)
2013-06-28 08:38 UTC, Chengwei Yang
Details | Splinter Review
[PATCH 4/5] tests to embedded tests: replaced in libdbus (35.96 KB, patch)
2013-06-28 08:38 UTC, Chengwei Yang
Details | Splinter Review
[PATCH 5/5] tests to embedded tests: replaced in tools (1.13 KB, patch)
2013-06-28 08:39 UTC, Chengwei Yang
Details | Splinter Review

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.