GNOME Bugzilla – Bug 743398
Add support for installed-tests
Last modified: 2015-01-23 14:07:31 UTC
Add support for installing the folks unit tests, using: https://wiki.gnome.org/Initiatives/GnomeGoals/InstalledTests This will improve CI by allowing the tests to be re-run whenever a dependency changes; e.g. if EDS changes its behaviour, we can catch that without having to rebuild folks.
Created attachment 295268 [details] [review] build: Local change to git.mk to ignore installed-test metadata files Submitted upstream as: https://github.com/behdad/git.mk/pull/28/.
Created attachment 295269 [details] [review] tests: Allow pkg-config calls to be overridden by environment variables When running installed tests, the pkg-config files for our dependencies might not be available, so the runtime pkg-config calls will fail and hence the tests will fail. Allow overriding the pkg-config calls with environment variables which will be set in the .test files for the installed tests.
Created attachment 295270 [details] [review] tests: Fix load path for a Telepathy test file The path works fine when running the tests in the source directory, but not for installed tests. Also, it seems a bit pointless to keep the file in its own subdirectory; and this will cause problems when installing it for installed-tests.
Created attachment 295271 [details] [review] tests: Port to use installed-tests Port the unit tests to follow the installed-tests standard, installing the test binaries and generated .test metadata files. This removes the existing --enable-tests configure option, replacing it with the standard: • --enable-modular-tests • --enable-installed-tests options. --enable-modular-tests is a direct replacement for --enable-tests, controlling whether tests are built at compile time, or only when run under `make check`. --enable-installed-tests controls whether tests will be installed on the system. If the tests are installed, use gnome-desktop-testing-runner folks to run them all.
Review of attachment 295271 [details] [review]: All patches above here are fine. This one is OK, but I don't like pre-expanding installation directories at configure time. It's a minor thing - all it breaks is the ability to "make prefix=/opt", but that is technically something that is meant to work in Automake. ::: configure.ac @@ +574,3 @@ + ac_expand=`eval echo [$]ac_expand` + $3=`eval echo [$]ac_expand` +]) Is this basically AS_AC_EXPAND? That tends to be a red flag for "you're doing it wrong" - it hard-codes something that is conceptually build-time as it was at configure-time. @@ +577,3 @@ + +dnl FULL_LIBEXECDIR is used for X-GNOME-Bugzilla-ExtraInfoScript expansion +dnl in data/org.gnome.Totem.desktop.in.in.in Nitpicking: I think you'll find it's used for something other than that :-P @@ +588,3 @@ + [Installation path for test binaries]) +AC_DEFINE_UNQUOTED([INSTALLED_TESTS_META_DIR],["$installed_tests_meta_dir"], + [Installation path for test metadata (.test files)]) You could replace the AC_DEFINE_UNQUOTED with the CPPFLAGS -DINSTALLED_TESTS_DIR=\"${INSTALLED_TESTS_DIR}\" in the appropriate Makefile(s) (test.mk?) to avoid having to do the expansions.
Review of attachment 295268 [details] [review]: .
Review of attachment 295269 [details] [review]: looks like changing patch status without a comment crashes Splinter so here is a comment
Review of attachment 295270 [details] [review]: yes
Review of attachment 295271 [details] [review]: fine to commit tbh
(In reply to comment #5) > You could replace the AC_DEFINE_UNQUOTED with the CPPFLAGS > > -DINSTALLED_TESTS_DIR=\"${INSTALLED_TESTS_DIR}\" > > in the appropriate Makefile(s) (test.mk?) to avoid having to do the expansions. I'd be fine with you doing this as an additional commit, or saying "no".
Created attachment 295273 [details] [review] tests: Port to use installed-tests Port the unit tests to follow the installed-tests standard, installing the test binaries and generated .test metadata files. This removes the existing --enable-tests configure option, replacing it with the standard: • --enable-modular-tests • --enable-installed-tests options. --enable-modular-tests is a direct replacement for --enable-tests, controlling whether tests are built at compile time, or only when run under `make check`. --enable-installed-tests controls whether tests will be installed on the system. If the tests are installed, use gnome-desktop-testing-runner folks to run them all.
Pushed with a fix for my braindeadness. Thanks for the review. Attachment 295268 [details] pushed as 3a95647 - build: Local change to git.mk to ignore installed-test metadata files Attachment 295269 [details] pushed as 324f868 - tests: Allow pkg-config calls to be overridden by environment variables Attachment 295270 [details] pushed as 98fe783 - tests: Fix load path for a Telepathy test file Attachment 295273 [details] pushed as bc77be3 - tests: Port to use installed-tests