Bug 93976

Summary: cmake 'make check' support for test/name-test directory
Product: dbus Reporter: Ralf Habacker <ralf.habacker>
Component: coreAssignee: D-Bus Maintainers <dbus>
Status: RESOLVED MOVED QA Contact: D-Bus Maintainers <dbus>
Severity: normal    
Priority: medium CC: ralf.habacker
Version: 1.10   
Hardware: Other   
OS: Linux (All)   
Whiteboard:
i915 platform: i915 features:
Bug Depends on: 88966, 92899    
Bug Blocks:    
Attachments: Add cmake support for running testcases in test/name-test.
Add cmake support for running testcases in test/name-test. (update 2)
Add tool dbus-run-session on linux to cmake build system.

Description Ralf Habacker 2016-02-03 07:49:56 UTC
Current cmake build system is not able to run the tests in test/name-test subdirectory of dbus source on linux.
Comment 1 Ralf Habacker 2016-02-03 09:42:58 UTC
Also add multithread test case from bug 93464.
Comment 2 Ralf Habacker 2016-02-03 14:43:09 UTC
Created attachment 121491 [details] [review]
Add cmake support for running testcases in test/name-test.

Bug:
Comment 3 Simon McVittie 2016-02-08 17:57:29 UTC
Comment on attachment 121491 [details] [review]
Add cmake support for running testcases in test/name-test.

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

::: cmake/CMakeLists.txt
@@ +13,4 @@
>  endif(COMMAND cmake_policy)
>  
>  if(CMAKE_MAJOR_VERSION GREATER 2)
> +    cmake_policy(SET CMP0026 OLD)

What does this policy do, and why are you going back to the OLD (deprecated?) interpretation?

::: cmake/modules/Macros.cmake
@@ +50,5 @@
>      target_link_libraries(${_target} ${ARGN})
>      if (CMAKE_CROSSCOMPILING AND CMAKE_SYSTEM_NAME STREQUAL "Windows")
> +        if(NOT HAVE_BINFMT_WINE_SUPPORT)
> +            set(DBUS_TEST_DAEMON_WRAPPER ${WINE_EXECUTABLE})
> +            set(_wrapper_args "${_wrapper} ${_wrapper_args}")

Where would a non-empty _wrapper_args come from before this line? Did you mean _args?

::: cmake/tools/CMakeLists.txt
@@ +77,5 @@
>  target_link_libraries(dbus-monitor ${DBUS_LIBRARIES})
>  install_targets(/bin dbus-monitor )
>  
> +add_executable(dbus-run-session ../../tools/dbus-run-session.c)
> +target_link_libraries(dbus-run-session ${DBUS_LIBRARIES} dbus-internal)

This bit should surely be in a separate commit?

If you make it conditional on Unix, it can go in separately before either this or the Windows implementation of dbus-run-session. If not, it can go in when the Windows implementation of dbus-run-session is ready.
Comment 4 Ralf Habacker 2016-02-11 02:30:58 UTC
Created attachment 121664 [details] [review]
Add cmake support for running testcases in test/name-test. (update 2)

(In reply to Simon McVittie from comment #3)
> Comment on attachment 121491 [details] [review] [review]
> Add cmake support for running testcases in test/name-test.
> 
> Review of attachment 121491 [details] [review] [review]:
> -----------------------------------------------------------------
> 
> ::: cmake/CMakeLists.txt
> @@ +13,4 @@
> >  endif(COMMAND cmake_policy)
> >  
> >  if(CMAKE_MAJOR_VERSION GREATER 2)
> > +    cmake_policy(SET CMP0026 OLD)
https://cmake.org/cmake/help/v3.0/policy/CMP0026.html
In dbus we can live with the old behavior.

> 
> What does this policy do, and why are you going back to the OLD
> (deprecated?) interpretation?
> 
> ::: cmake/modules/Macros.cmake
> @@ +50,5 @@
> >      target_link_libraries(${_target} ${ARGN})
> >      if (CMAKE_CROSSCOMPILING AND CMAKE_SYSTEM_NAME STREQUAL "Windows")
> > +        if(NOT HAVE_BINFMT_WINE_SUPPORT)
> > +            set(DBUS_TEST_DAEMON_WRAPPER ${WINE_EXECUTABLE})
> > +            set(_wrapper_args "${_wrapper} ${_wrapper_args}")
> 
> Where would a non-empty _wrapper_args come from before this line? Did you
> mean _args?
fixed
> 
> ::: cmake/tools/CMakeLists.txt
> @@ +77,5 @@
> >  target_link_libraries(dbus-monitor ${DBUS_LIBRARIES})
> >  install_targets(/bin dbus-monitor )
> >  
> > +add_executable(dbus-run-session ../../tools/dbus-run-session.c)
> > +target_link_libraries(dbus-run-session ${DBUS_LIBRARIES} dbus-internal)
> 
> This bit should surely be in a separate commit?
sure
Comment 5 Ralf Habacker 2016-02-11 02:31:21 UTC
Created attachment 121665 [details] [review]
Add tool dbus-run-session on linux to cmake build system.
Comment 6 Ralf Habacker 2016-02-11 02:37:07 UTC
With both patches applied I get failures on running testcases 

    Start 1: test-pending-call-dispatch
1/7 Test #1: test-pending-call-dispatch .......***Failed    0.02 sec
    Start 2: test-pending-call-timeout
2/7 Test #2: test-pending-call-timeout ........***Failed    0.01 sec
    Start 3: test-thread-init
3/7 Test #3: test-thread-init .................***Failed    0.02 sec
    Start 4: test-ids
4/7 Test #4: test-ids .........................   Passed    0.01 sec
    Start 5: test-shutdown
5/7 Test #5: test-shutdown ....................   Passed    0.01 sec
    Start 6: test-privserver-client
6/7 Test #6: test-privserver-client ...........***Failed    0.02 sec
    Start 7: test-autolaunch
7/7 Test #7: test-autolaunch ..................   Passed    0.01 sec

The first issue that the service file expects the executable in <build-root>/bin/name-test, while it is located in <build-root>/bin

cat ../data/valid-service-files/org.freedesktop.DBus.TestSuite.PrivServer.service 
D-BUS Service]
Name=org.freedesktop.DBus.TestSuite.PrivServer
Exec=/home/ralf/src/dbus-2-cmake-build/bin/name-test/test-privserver
                                           ^^^^^

The second issue is that there is misconfiguration in the related session.conf

Single run of ctest -VV -R ^test-privserver-client$ shows
Test project /home/ralf/src/dbus-2-cmake-build/test/name-test
...
test 6
    Start 6: test-privserver-client

6: Test command: /home/ralf/src/dbus-2-cmake-build/bin/dbus-run-session "--dbus-daemon=/home/ralf/src/dbus-2-cmake-build/bin/dbus-daemon" "--config-file=/home/ralf/src/dbus-2-cmake-build/test/name-test/tmp-session-like-system.conf" "/home/ralf/src/dbus-2-cmake-build/bin/test-privserver-client" "--tap"
6: Environment variables: 
6:  DBUS_TEST_DAEMON=/home/ralf/src/dbus-2-cmake-build/bin/dbus-daemon
6:  DBUS_SESSION_BUS_ADDRESS=
6:  DBUS_FATAL_WARNINGS=0
6:  DBUS_TEST_DATA=/home/ralf/src/dbus-2-cmake-build/test/data
6:  DBUS_TEST_DBUS_LAUNCH=/home/ralf/src/dbus-2-cmake-build/bin/dbus-launch
6:  DBUS_TEST_HOMEDIR=/home/ralf/src/dbus-2-cmake-build/dbus
6:  DBUS_TEST_EXEC=/home/ralf/src/dbus-2-cmake-build/bin
6:  DBUS_TEST_DAEMON=/home/ralf/src/dbus-2-cmake-build/bin/dbus-daemon
...
6: Test timeout computed to be: 9.99988e+06
6: dbus[6673]: [session uid=1002 pid=6673] org.freedesktop.DBus.Error.NotSupported: cannot change fd limit on this platform
6: dbus[6673]: [session uid=1002 pid=6673] Activating service name='org.freedesktop.DBus.TestSuite.PrivServer'
6: Fd 3 did not have the close-on-exec flag set!
6: couldn't request name: Connection ":1.1" is not allowed to own the service "org.freedesktop.DBus.TestSuite.PrivServer" due to security policies in the configuration filedbus[6673]: [session uid=1002 pid=6673] Activated service 'org.freedesktop.DBus.TestSuite.PrivServer' failed: Process org.freedesktop.DBus.TestSuite.PrivServer exited with status 1
6: couldn't send message: Process org.freedesktop.DBus.TestSuite.PrivServer exited with status 1
Comment 7 Ralf Habacker 2016-02-11 02:38:21 UTC
(In reply to Ralf Habacker from comment #6)
> 6:  DBUS_FATAL_WARNINGS=0
I changed that from 1 temporary to avoid an abort on the following message
> 6: Fd 3 did not have the close-on-exec flag set!
Comment 8 Simon McVittie 2016-02-29 08:11:50 UTC
(In reply to Ralf Habacker from comment #6)
> 6: dbus[6673]: [session uid=1002 pid=6673]
> org.freedesktop.DBus.Error.NotSupported: cannot change fd limit on this
> platform

If this is compiling on Linux, for Linux, then there must be a missing feature-test, because Linux can certainly do that. Specifically, we should detect whether the getrlimit() and setrlimit() functions exist, and #define HAVE_GETRLIMIT and HAVE_SETRLIMIT.

The ideal implementation would be the CMake equivalent of AC_CHECK_FUNCS([getrlimit setrlimit]). If that isn't possible, assuming that every non-Windows platform has those functions would be a reasonable approximation (they're in POSIX).

> 6: Fd 3 did not have the close-on-exec flag set!

This probably indicates that there's a missing feature-test somewhere in the CMake stuff, and also that the fallback code path for legacy systems (which would be used here as a result of the missing feature-test) has a bug and doesn't correctly mark the fd close-on-exec.

> 6: couldn't request name: Connection ":1.1" is not allowed to own the
> service "org.freedesktop.DBus.TestSuite.PrivServer" due to security policies
> in the configuration file

I suspect this means that a username or something is being substituted wrong in session.conf under CMake; or maybe the dbus-daemon is using the system-wide installed session.conf instead of the one we just built.
Comment 9 GitLab Migration User 2018-10-12 21:26:25 UTC
-- GitLab Migration Automatic Message --

This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/dbus/dbus/issues/143.

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.