Always use run-test.sh to run tests

There are three modes here:

- make check: run tests in ${srcdir} against MC in ${builddir}
- make installcheck: run tests in ${srcdir} against MC in
  ${DESTDIR}${prefix}
- ${mctestsdir}/twisted/run-test.sh: run tests in ${mctestsdir}
  against MC in ${prefix}

Also simplify the last of those cases, and avoid a potential symlink
attack, by requiring that the current working directory for run-test.sh
is a directory controlled by us. In practice we already required that,
because with-session-bus.sh creates files in the current directory.

Bug: https://bugs.freedesktop.org/show_bug.cgi?id=69542
Reviewed-by: Guillaume Desmottes <guillaume.desmottes@collabora.co.uk>

3 files changed, 151 insertions, 126 deletions

diff --git a/tests/twisted/Makefile.am b/tests/twisted/Makefile.am
index e2f6d05a..40d327b9 100644
--- a/tests/twisted/Makefile.am
+++ b/tests/twisted/Makefile.am
@@ -1,5 +1,5 @@
-TWISTED_TESTS =
-
+# Tests that don't need their own MC instance, although in practice
+# we currently run them in their own MC instance anyway.
 TWISTED_BASIC_TESTS = \
 	account-manager/account-basics.py \
 	account-manager/avatar.py \
@@ -79,9 +79,11 @@ TWISTED_SPECIAL_BUILD_TESTS = \
 	account-storage/default-keyring-storage.py \
 	account-storage/diverted-storage.py
 
+# Tests that are usually too slow to run.
 TWISTED_SLOW_TESTS = \
 	account-manager/server-drops-us.py
 
+# Tests that need their own MC instance.
 TWISTED_SEPARATE_TESTS = \
 	account-manager/auto-connect.py \
 	account-manager/avatar-persist.py \
@@ -91,7 +93,12 @@ TWISTED_SEPARATE_TESTS = \
 	crash-recovery/crash-recovery.py \
 	dispatcher/create-at-startup.py
 
-TWISTED_SEPARATE_TESTS += $(TWISTED_SPECIAL_BUILD_TESTS)
+# All the tests that are run by "make check"
+TWISTED_TESTS = \
+	$(TWISTED_BASIC_TESTS) \
+	$(TWISTED_SEPARATE_TESTS) \
+	$(TWISTED_SPECIAL_BUILD_TESTS) \
+	$(NULL)
 
 # other files used by the twisted tests, but are not tests and are not built
 # source
@@ -111,13 +118,15 @@ TWISTED_OTHER_FILES = \
 	telepathy/managers/README \
 	$(NULL)
 
+# This deliberately doesn't include the TWISTED_SPECIAL_BUILD_TESTS
+# since it's testing the real binary, not the "debug" binary.
 mc-twisted-tests.list:
-	$(AM_V_GEN)echo $(TWISTED_BASIC_TESTS) > $@
+	$(AM_V_GEN)echo $(TWISTED_BASIC_TESTS) $(TWISTED_SEPARATE_TESTS) > $@
 
 run-test.sh: run-test.sh.in Makefile
 	$(AM_V_GEN)sed \
 		-e "s|[@]mctestsdir[@]|@mctestsdir@|g" \
-		-e "s|[@]PYTHON[@]|$(PYTHON)|g" \
+		-e "s|[@]TEST_PYTHON[@]|$(PYTHON)|g" \
 		$< > $@
 	@chmod +x $@
 
@@ -130,6 +139,7 @@ nobase_nodist_twistedtests_SCRIPTS = \
 	$(NULL)
 nobase_dist_twistedtests_DATA = \
 	$(TWISTED_BASIC_TESTS) \
+	$(TWISTED_SEPARATE_TESTS) \
 	$(TWISTED_OTHER_FILES) \
 	$(NULL)
 nobase_nodist_twistedtests_DATA = \
@@ -222,44 +232,6 @@ INCLUDES = \
 
 TESTS =
 
-TMPSUFFIX = foo
-
-BASIC_TESTS_ENVIRONMENT = \
-	PYTHONPATH=@abs_top_srcdir@/tests/twisted:@abs_top_builddir@/tests/twisted \
-	MC_DEBUG=all \
-	MC_FILTER_PLUGIN_DIR=@abs_top_builddir@/tests/twisted/.libs \
-	GIO_EXTRA_MODULES=@abs_top_builddir@/tests/twisted/.libs \
-	MC_ACCOUNT_DIR=@abs_top_builddir@/tests/twisted/tmp-$(TMPSUFFIX)/mc-account-dir \
-	XDG_CONFIG_HOME=@abs_top_builddir@/tests/twisted/tmp-$(TMPSUFFIX)/config \
-	XDG_CONFIG_DIRS=@abs_top_srcdir@/tests/twisted \
-	XDG_DATA_HOME=@abs_top_builddir@/tests/twisted/tmp-$(TMPSUFFIX)/localshare \
-	XDG_DATA_DIRS=@abs_top_builddir@/tests/twisted/tmp-$(TMPSUFFIX)/share:@abs_top_srcdir@/tests/twisted \
-	XDG_CACHE_DIR=@abs_top_builddir@/tests/twisted/tmp-$(TMPSUFFIX)/cache \
-	XDG_CACHE_HOME=@abs_top_builddir@/tests/twisted/tmp-$(TMPSUFFIX)/cache \
-	MC_CLIENTS_DIR=@abs_top_srcdir@/tests/twisted/telepathy/clients \
-	MC_MANAGER_DIR=@abs_top_srcdir@/tests/twisted/telepathy/managers \
-	G_DEBUG=fatal_criticals
-
-if ENABLE_LIBACCOUNTS_SSO
-BASIC_TESTS_ENVIRONMENT += AG_DEBUG=all \
-	ACCOUNTS=@abs_top_builddir@/tests/twisted/tmp-$(TMPSUFFIX) \
-	AG_SERVICES=@abs_top_builddir@/tests/twisted/tmp-$(TMPSUFFIX)
-endif
-
-WITH_SESSION_BUS = \
-	sh $(srcdir)/tools/with-session-bus.sh \
-	--also-for-system \
-	--config-file=tools/servicedir-uninstalled/tmp-session-bus.conf --
-
-COMBINED_TESTS_ENVIRONMENT = \
-	$(BASIC_TESTS_ENVIRONMENT) \
-	$(PYTHON) -u
-
-SEPARATE_TESTS_ENVIRONMENT = \
-	$(BASIC_TESTS_ENVIRONMENT) \
-	$(WITH_SESSION_BUS) \
-	$(PYTHON)
-
 if WANT_TWISTED_TESTS
 check-local: check-twisted
 
@@ -271,69 +243,32 @@ installcheck-twisted:
 
 endif # WANT_TWISTED_TESTS
 
-check-twisted:
+CHECK_TWISTED_SLOW =
+
+check-twisted: $(BUILT_SOURCES)
 	$(MAKE) -C tools
 	rm -f tools/core
 	rm -f tools/missioncontrol-*.log
-	mkdir tmp-$$$$ && { \
-		$(MAKE) check-combined TMPSUFFIX=$$$$; \
-		e=$$?; \
-		rm -rf tmp-$$$$; \
-	exit $$e; }
-	for TESTS in $(TWISTED_SEPARATE_TESTS); do export TESTS; ( \
-	rm -rf tmp-$$$$ && mkdir tmp-$$$$ && { \
-		e=0; \
-		$(MAKE) check-separate \
-			TMPSUFFIX=$$$$ \
-			TESTS=$$TESTS \
-		|| e=$$?; \
-		rm -rf tmp-$$$$; \
-	}; exit $$e ) || exit $$?; done
-	@if test -e tools/core; then\
-		echo "Core dump exists: tools/core";\
-		exit 1;\
-	fi
-
-check-torture:
-	mkdir tmp-$$$$ && { \
-		env $(BASIC_TESTS_ENVIRONMENT) $(WITH_SESSION_BUS) \
-			$(MAKE) _check-torture TMPSUFFIX=$$$$; \
-		e=$$?; \
-		rm -rf tmp-$$$$; \
-	exit $$e; }
-
-TORTURE_REPEATS = 100
-
-_check-torture:
-	for i in `seq 1 $(TORTURE_REPEATS)`; do \
-		$(MAKE) check-TESTS \
-			TESTS='$$(TWISTED_BASIC_TESTS) $$(TWISTED_SLOW_TESTS)'\
-			TESTS_ENVIRONMENT='$$(COMBINED_TESTS_ENVIRONMENT)'; \
-		e=$$?; \
-		test z$$e = z0 || break; \
-	done
-
-CHECK_TWISTED_SLOW =
-
-check-combined:
+	failed=0; \
 	if test x$(CHECK_TWISTED_SLOW) = x; then \
 		extra_tests= ; \
 	else \
 		extra_tests=' $$(TWISTED_SLOW_TESTS)'; \
 	fi; \
-	env $(BASIC_TESTS_ENVIRONMENT) $(WITH_SESSION_BUS) \
-	$(MAKE) check-TESTS \
-		TESTS='$$(TWISTED_BASIC_TESTS)'"$${extra_tests}" \
-		TESTS_ENVIRONMENT='$$(COMBINED_TESTS_ENVIRONMENT)'
-
-check-separate:
-	echo "Running $(TESTS) in tmp-$(TMPSUFFIX)"
-	$(MAKE) check-TESTS \
-		TESTS_ENVIRONMENT='$$(SEPARATE_TESTS_ENVIRONMENT)'
+	MC_TEST_UNINSTALLED=1 \
+	  MC_ABS_TOP_SRCDIR=@abs_top_srcdir@ \
+	  MC_ABS_TOP_BUILDDIR=@abs_top_builddir@ \
+	  sh run-test.sh "${TWISTED_TESTS}${extra_tests}"
+	if test -e tools/core; then\
+		echo "Core dump exists: tools/core";\
+		exit 1;\
+	fi
 
 EXTRA_DIST = \
 	$(TWISTED_BASIC_TESTS) \
 	$(TWISTED_SEPARATE_TESTS) \
+	$(TWISTED_SLOW_TESTS) \
+	$(TWISTED_SPECIAL_BUILD_TESTS) \
 	$(TWISTED_OTHER_FILES) \
 	accounts/README \
 	run-test.sh.in \
diff --git a/tests/twisted/README b/tests/twisted/README
index 56f664a8..6fdc7ce3 100644
--- a/tests/twisted/README
+++ b/tests/twisted/README
@@ -5,14 +5,12 @@ To run Twisted tests:
 To run an individual Twisted test:
 
   make -C tests/twisted check-twisted \
-        TWISTED_BASIC_TESTS= \
-        TWISTED_SEPARATE_TESTS=dispatcher/create-at-startup.py
+        TWISTED_TESTS=dispatcher/create-at-startup.py
 
 To run with debug information:
 
   make -C tests/twisted check-twisted \
-        TWISTED_BASIC_TESTS= \
-        TWISTED_SEPARATE_TESTS=dispatcher/create-at-startup.py \
+        TWISTED_TESTS=dispatcher/create-at-startup.py \
         CHECK_TWISTED_VERBOSE=1
 
 To debug an individual test you can set one of the following env variable:
diff --git a/tests/twisted/run-test.sh.in b/tests/twisted/run-test.sh.in
index 27277462..cbd2e773 100644
--- a/tests/twisted/run-test.sh.in
+++ b/tests/twisted/run-test.sh.in
@@ -1,44 +1,136 @@
 #!/bin/sh
 
-script_fullname=`readlink -e "@mctestsdir@/twisted/run-test.sh"`
-if [ `readlink -e "$0"` != "$script_fullname" ] ; then
-  echo "This script is meant to be installed"
-  exit 1
+# This script assumes that it is run in a temporary directory where it can
+# create and delete subdirectories for files, logs, etc., but other users
+# cannot write (for instance, /tmp is unsuitable, but
+# the directory created by `mktemp -d /tmp/test.XXXXXXXXXX` is fine).
+#
+# During "make check" or "make installcheck" it runs in
+# ${top_builddir}/tests/twisted.
+#
+# During installed testing, the test environment must run it in a
+# suitable location.
+
+set -e
+
+if test "x$MC_TEST_UNINSTALLED" = x; then
+  script_fullname=`readlink -e "@mctestsdir@/twisted/run-test.sh"`
+  if [ `readlink -e "$0"` != "$script_fullname" ] ; then
+    echo "This script is meant to be installed at $script_fullname" >&2
+    exit 1
+  fi
+
+  test_src="@mctestsdir@"
+  test_build="@mctestsdir@"
+  config_file="@mctestsdir@/twisted/tools/servicedir-installed/tmp-session-bus.conf"
+  plugins="@mctestsdir@/twisted/plugins"
+
+  PYTHONPATH="@mctestsdir@/twisted"
+  export PYTHONPATH
+
+  MC_TWISTED_PATH="@mctestsdir@/twisted"
+  export MC_TWISTED_PATH
+else
+  if test -z "$MC_ABS_TOP_SRCDIR"; then
+    echo "MC_ABS_TOP_SRCDIR must be set" >&2
+    exit 1
+  fi
+  if test -z "$MC_ABS_TOP_BUILDDIR"; then
+    echo "MC_ABS_TOP_BUILDDIR must be set" >&2
+    exit 1
+  fi
+
+  test_src="${MC_ABS_TOP_SRCDIR}/tests"
+  test_build="${MC_ABS_TOP_BUILDDIR}/tests"
+  config_file="${test_build}/twisted/tools/servicedir-uninstalled/tmp-session-bus.conf"
+  plugins="${test_build}/twisted/.libs"
+
+  PYTHONPATH="${test_src}/twisted:${test_build}/twisted"
+  export PYTHONPATH
+
+  MC_TWISTED_PATH="${test_src}/twisted"
+  export MC_TWISTED_PATH
 fi
 
-export PYTHONPATH=@mctestsdir@/twisted
+MC_DEBUG=all
+export MC_DEBUG
+G_DEBUG=fatal-criticals
+export G_DEBUG
+# for ENABLE_LIBACCOUNTS_SSO
+AG_DEBUG=all
+export AG_DEBUG
 
-export XDG_CONFIG_DIRS=@mctestsdir@/twisted
-export XDG_DATA_DIRS=@mctestsdir@/twisted
+GIO_EXTRA_MODULES="${plugins}"
+export GIO_EXTRA_MODULES
+MC_FILTER_PLUGIN_DIR="${plugins}"
+export MC_FILTER_PLUGIN_DIR
 
-TMP=/tmp/telepathy-mission-control-tests/tmp
-mkdir -p $TMP
-export XDG_CONFIG_HOME=$TMP
-export XDG_DATA_HOME=$TMP
-export XDG_CACHE_DIR=$TMP
-export XDG_CACHE_HOME=$TMP
+XDG_CONFIG_DIRS="${test_src}/twisted"
+export XDG_CONFIG_DIRS
 
-export G_DEBUG=fatal-criticals
-export MC_DEBUG=all
-export MC_FILTER_PLUGIN_DIR=@mctestsdir@/twisted/plugins
-export GIO_EXTRA_MODULES=@mctestsdir@/twisted/plugins
-export MC_ACCOUNT_DIR=/tmp/telepathy-mission-control-tests/accounts
-export MC_CLIENTS_DIR=@mctestsdir@/twisted/telepathy/clients
-export MC_MANAGER_DIR=@mctestsdir@/twisted/telepathy/managers
-export MC_FILTER_PLUGIN_DIR=@mctestsdir@/twisted/plugins
-mkdir -p $MC_ACCOUNT_DIR || exit 1
+MC_CLIENTS_DIR="${test_src}/twisted/telepathy/clients"
+export MC_CLIENTS_DIR
+MC_MANAGER_DIR="${test_src}/twisted/telepathy/managers"
+export MC_MANAGER_DIR
 
 if [ -n "$1" ] ; then
   list="$1"
 else
-  list=$(cat @mctestsdir@/twisted/mc-twisted-tests.list)
+  list=$(cat "${test_build}"/twisted/mc-twisted-tests.list)
 fi
 
-for i in $list ; do 
-  echo "Testing $i"
-  sh @mctestsdir@/twisted/tools/with-session-bus.sh \
-    --config-file=@mctestsdir@/twisted/tools/servicedir-installed/tmp-session-bus.conf \
+any_failed=0
+for i in $list ; do
+  echo "Testing $i ..."
+
+  tmp="`pwd`/tmp-`echo $i | tr ./ __`"
+  rm -fr "$tmp"
+  mkdir "$tmp"
+
+  MC_ACCOUNT_DIR="${tmp}/mc-account-dir"
+  export MC_ACCOUNT_DIR
+  XDG_CONFIG_HOME="${tmp}/config"
+  export XDG_CONFIG_HOME
+  XDG_DATA_HOME="${tmp}/localshare"
+  export XDG_DATA_HOME
+  XDG_DATA_DIRS="${tmp}/share:${test_src}/twisted"
+  export XDG_DATA_DIRS
+  XDG_CACHE_HOME="${tmp}/cache"
+  export XDG_CACHE_HOME
+  XDG_CACHE_DIR="${tmp}/cache"
+  export XDG_CACHE_DIR
+
+  # for ENABLE_LIBACCOUNTS_SSO
+  ACCOUNTS="${tmp}/libaccounts-accounts"
+  export ACCOUNTS
+  AG_SERVICES="${tmp}/libaccounts-services"
+  export AG_SERVICES
+
+  sh "${test_src}/twisted/tools/with-session-bus.sh" \
+    ${MC_TEST_SLEEP} \
+    --also-for-system \
+    --config-file="${config_file}" \
     -- \
-    @PYTHON@ @mctestsdir@/twisted/$i
+    @TEST_PYTHON@ -u "${test_src}/twisted/$i"
+  e=$?
+  case "$e" in
+    (0)
+      echo "PASS: $i"
+      if test -z "MC_TEST_KEEP_TEMP"; then
+        rm -fr "$tmp"
+      fi
+      ;;
+    (77)
+      echo "SKIP: $i"
+      if test -z "MC_TEST_KEEP_TEMP"; then
+        rm -fr "$tmp"
+      fi
+      ;;
+    (*)
+      any_failed=1
+      echo "FAIL: $i ($e)"
+      ;;
+  esac
 done
 
+exit $any_failed