[Bug 41448] Installing unit tests

bugzilla-daemon at freedesktop.org bugzilla-daemon at freedesktop.org
Mon Oct 10 18:49:21 CEST 2011


Marco Barisione <marco.barisione at collabora.co.uk> changed:

           What    |Removed                     |Added
             Status|NEW                         |ASSIGNED
  Status Whiteboard|                            |review-
         AssignedTo|telepathy-bugs at lists.freede |alban.crequy at collabora.co.u
                   |sktop.org                   |k

--- Comment #1 from Marco Barisione <marco.barisione at collabora.co.uk> 2011-10-10 09:49:20 PDT ---
-Y´tests: add --enable-install-tests¡

Did you test what happens if $TEST_PYTHON is false (i.e. if python or twisted
or similar are missing)?

+    [Enable installation of twisted tests]),

Only twisted ones?
The existing ones use a lower care first letter. I would keep the string just
as ´install tests¡ or ´make tests installable¡.


Why not ENABLE_INSTALL_TESTS like the option name?

+      [Installation of test files])

The documentation string is not very clear

+twistedcapstestsdir = $(GABBLE_TESTS_PATH)/twisted/caps

Are you sure there is no way to avoid having an entry per subdirectory? :(

+BUILT_SOURCES += $(service_files) $(conf_files) tools/exec-with-log.sh

And run-gabble.sh?


How was it working before without this?

+script_fullname=`readlink -e "@abs_top_tests_installdir@/twisted/run-test.sh"`
+if [ `readlink -e "$0"` != "$script_fullname" ] ; then
+  echo "This script is meant to be installed"
+  exit 1


+    /usr/bin/python @abs_top_tests_installdir@/twisted/$i

You should not rely on the python executable to be /usr/bin/python. I think
that AM_PATH_PYTHON provides some magic to detect the python executable.


I think it's a bit weird that you changed the purpose of the .service.in file
in tools/ and moved the old one to tools/servicedir-uninstalled.
Why not keeping the old files as they were and having another dir for the
installed ones? Or maybe having an uninstalled/ and an installed/ directories.

+export GABBLE_PLUGIN_DIR="usr/lib/telepathy/gabble-0"

It's missing a ´/¡ at the beginning, but anyway you should not rely on stuff
being in /usr.

+  <!-- This is included last so local configuration can override what's 
+       in this standard file -->

Too many empty lines :P

´tests/twisted/Makefile.am: fix make dist¡
´fix confusion between both dbus config files¡

I think you should squash these patches with the previous one.

Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug.
You are the assignee for the bug.

More information about the telepathy-bugs mailing list