[PATCH 0/7] Allow tests to run in parallel

[PATCH 0/7] Allow tests to run in parallel

[Rob Browning]

This series switches Guile to the Automake parallel test harness so
that commands like "make -j4 check" can run tests concurrently:
https://www.gnu.org/software/automake/manual/html_node/Parallel-Test-Harness.html.

Here it cuts the check time in half.

Right now I just wanted to see if this might be interesting.  If so, I
can make whatever adjustments are desired (guessing perhaps changelog
entries, etc.).

I suspect eventually we might also want further adjustments to the
output, but that might or might not be important in the first pass.

Rob Browning (7):
  srfi-10.test: add missing (test-suite lib) dependency
  interp.test: add missing (test-suite lib) dependency
  guile-test: set declarative #f to eliminate warning
  guile-test: support automake parallel test harness via --trs-file
  check-guile.in: improve quoting (e.g. paths with spaces)
  check-guile.in: exit 2 on errors and direct output to stderr
  Switch to the preferred parallel automake test harness

 Makefile.am                            |  3 --
 check-guile.in                         | 26 ++++-------
 configure.ac                           |  5 +--
 test-suite/Makefile.am                 | 18 +++++---
 test-suite/driver                      | 60 ++++++++++++++++++++++++++
 test-suite/guile-test                  | 30 ++++++++++---
 test-suite/test-suite/lib/automake.scm | 54 +++++++++++++++++++++++
 test-suite/tests/interp.test           |  3 ++
 test-suite/tests/srfi-10.test          |  4 +-
 9 files changed, 165 insertions(+), 38 deletions(-)
 create mode 100755 test-suite/driver
 create mode 100644 test-suite/test-suite/lib/automake.scm

-- 
2.39.2

[PATCH 1/7] srfi-10.test: add missing (test-suite lib) dependency

[Rob Browning]

---
 test-suite/tests/srfi-10.test | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/test-suite/tests/srfi-10.test b/test-suite/tests/srfi-10.test
index c379d0bac..bc7a79157 100644
--- a/test-suite/tests/srfi-10.test
+++ b/test-suite/tests/srfi-10.test
@@ -17,7 +17,9 @@
 ;;;; License along with this library; if not, write to the Free Software
 ;;;; Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
 
-(use-modules (srfi srfi-10))
+(use-modules
+ (srfi srfi-10)
+ ((test-suite lib) #:select (pass-if with-test-prefix)))
 
 (define-reader-ctor 'rx make-regexp)
 
-- 
2.39.2

[PATCH 2/7] interp.test: add missing (test-suite lib) dependency

[Rob Browning]

---
 test-suite/tests/interp.test | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/test-suite/tests/interp.test b/test-suite/tests/interp.test
index 5f3e2aaf7..7497094a8 100644
--- a/test-suite/tests/interp.test
+++ b/test-suite/tests/interp.test
@@ -16,6 +16,9 @@
 ;;;; License along with this library; if not, write to the Free Software
 ;;;; Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
 
+(define-module (test-suite test-interp)
+  #:use-module (test-suite lib))
+
 (pass-if "Internal defines 1"
 	 (letrec ((foo (lambda (arg)
 			 (or arg (and (procedure? foo)
-- 
2.39.2

[PATCH 3/7] guile-test: set declarative #f to eliminate warning

[Rob Browning]

---
 test-suite/guile-test | 1 +
 1 file changed, 1 insertion(+)

diff --git a/test-suite/guile-test b/test-suite/guile-test
index 2d4e94171..e0c4333f7 100755
--- a/test-suite/guile-test
+++ b/test-suite/guile-test
@@ -81,6 +81,7 @@
     (apply (module-ref module 'main) args)))
 
 (define-module (test-suite guile-test)
+  :declarative? #f
   :use-module (test-suite lib)
   :use-module (ice-9 getopt-long)
   :use-module (ice-9 and-let-star)
-- 
2.39.2

[PATCH 4/7] guile-test: support automake parallel test harness via --trs-file

[Rob Browning]

Support an optional --trs-file PATH argument that causes guile-test to
write the status information expected by the automake parallel test
harness to PATH.

In addition, when --trs-file is specified, suppress the final test
summary (via print-counts) since it would be repeated per-test-file
when running in parallel, the automake harness prints its own summary.

cf. https://www.gnu.org/software/automake/manual/html_node/API-for-Custom-Test-Drivers.html
---
 test-suite/guile-test                  | 29 +++++++++++---
 test-suite/test-suite/lib/automake.scm | 54 ++++++++++++++++++++++++++
 2 files changed, 77 insertions(+), 6 deletions(-)
 create mode 100644 test-suite/test-suite/lib/automake.scm

diff --git a/test-suite/guile-test b/test-suite/guile-test
index e0c4333f7..6090efc35 100755
--- a/test-suite/guile-test
+++ b/test-suite/guile-test
@@ -89,6 +89,7 @@
   :use-module (system vm coverage)
   :use-module (srfi srfi-11)
   :use-module (system vm vm)
+  :use-module ((test-suite lib automake) :prefix automake/)
   :export (main data-file-name test-file-name))
 
 \f
@@ -184,7 +185,9 @@
                                 (coverage
                                  (single-char #\c))
 				(debug
-				 (single-char #\d))))))
+				 (single-char #\d))
+                                (trs-file
+                                 (value #t))))))
     (define (opt tag default)
       (let ((pair (assq tag options)))
 	(if pair (cdr pair) default)))
@@ -207,11 +210,16 @@
 	      (if (null? foo)
 		  (enumerate-tests test-suite)
 		  foo)))
-	   (log-file
-	    (opt 'log-file "guile.log")))
+	   (log-file (opt 'log-file "guile.log"))
+           (trs-file (opt 'trs-file #f)))
 
       ;; Open the log file.
-      (let ((log-port (open-output-file log-file)))
+      (let ((log-port (open-output-file log-file))
+            (trs-port (and trs-file
+                           (let ((p (open-output-file trs-file)))
+                             (set-port-encoding! p "UTF-8")
+                             (display ":copy-in-global-log: no\n" p)
+                             p))))
 
         ;; Allow for arbitrary Unicode characters in the log file.
         (set-port-encoding! log-port "UTF-8")
@@ -223,9 +231,11 @@
 	;; Register some reporters.
 	(let ((global-pass #t)
 	      (counter (make-count-reporter)))
+          (when trs-port
+            (register-reporter (automake/reporter trs-port)))
 	  (register-reporter (car counter))
 	  (register-reporter (make-log-reporter log-port))
-	  (register-reporter user-reporter)
+          (register-reporter user-reporter)
 	  (register-reporter (lambda results
 			       (case (car results)
                                  ((unresolved)
@@ -255,10 +265,17 @@
 	  ;; Display the final counts, both to the user and in the log
 	  ;; file.
 	  (let ((counts ((cadr counter))))
-	    (print-counts counts)
+	    (unless trs-port
+              (print-counts counts))
 	    (print-counts counts log-port))
 
 	  (close-port log-port)
+
+          (when trs-port
+            (when global-pass (display ":recheck: no\n" trs-port))
+            (display ":test-global-result: umm, ok?\n" trs-port)
+            (close-port trs-port))
+
 	  (quit global-pass))))))
 
 \f
diff --git a/test-suite/test-suite/lib/automake.scm b/test-suite/test-suite/lib/automake.scm
new file mode 100644
index 000000000..237a89d65
--- /dev/null
+++ b/test-suite/test-suite/lib/automake.scm
@@ -0,0 +1,54 @@
+;;;; test-suite/lib/automake.scm --- support for automake driven tests
+;;;; Copyright (C) 2023 Free Software Foundation, Inc.
+;;;;
+;;;; This program is free software; you can redistribute it and/or
+;;;; modify it under the terms of the GNU Lesser General Public
+;;;; License as published by the Free Software Foundation; either
+;;;; version 3, or (at your option) any later version.
+;;;;
+;;;; This program is distributed in the hope that it will be useful,
+;;;; but WITHOUT ANY WARRANTY; without even the implied warranty of
+;;;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+;;;; GNU Lesser General Public License for more details.
+;;;;
+;;;; You should have received a copy of the GNU Lesser General Public
+;;;; License along with this software; see the file COPYING.LESSER.
+;;;; If not, write to the Free Software Foundation, Inc., 51 Franklin
+;;;; Street, Fifth Floor, Boston, MA 02110-1301 USA
+
+(define-module (test-suite lib automake)
+  :use-module ((ice-9 match))
+  :use-module ((srfi srfi-1) :select (drop-right last))
+  :export (reporter))
+
+(define (display->str x)
+  (call-with-output-string (lambda (port) (display x port))))
+
+(define (write->str x)
+  (call-with-output-string (lambda (port) (write x port))))
+
+(define (show port . args)
+  (for-each (lambda (x) (display x port)) args))
+
+(define (render-name name)
+  (string-join (append (map display->str (drop-right name 1))
+                       ;; Because for some tests, say via pass-if or
+                       ;; pass-if-equal with no explict name, it's an
+                       ;; arbirary form, possibly including null chars,
+                       ;; etc.
+                       (list (write->str (last name))))
+               ": "))
+
+(define (reporter trs-port)
+  (match-lambda*
+    (('pass name) (show trs-port ":test-result: PASS " (render-name name) "\n"))
+    (('upass name) (show trs-port ":test-result: XPASS " (render-name name) "\n"))
+    (('fail name) (show trs-port ":test-result: FAIL " (render-name name) "\n"))
+    (('xfail name . args) (show trs-port ":test-result: XFAIL " (render-name name) "\n"))
+    (('untested name) (show trs-port ":test-result: SKIP " (render-name name) "\n"))
+    (('unsupported name) (show trs-port ":test-result: SKIP " (render-name name) "\n"))
+    (('unresolved name) (show trs-port ":test-result: SKIP " (render-name name) "\n"))
+    (('error name . args)
+     (show trs-port ":test-result: ERROR " (render-name name) " ")
+     (write args trs-port)
+     (newline trs-port))))
-- 
2.39.2

[PATCH 5/7] check-guile.in: improve quoting (e.g. paths with spaces)

[Rob Browning]

---
 check-guile.in | 23 +++++++++--------------
 1 file changed, 9 insertions(+), 14 deletions(-)

diff --git a/check-guile.in b/check-guile.in
index 214deec16..09d95a03c 100644
--- a/check-guile.in
+++ b/check-guile.in
@@ -14,25 +14,22 @@ set -e
 top_builddir=@top_builddir_absolute@
 top_srcdir=@top_srcdir_absolute@
 
-TEST_SUITE_DIR=${top_srcdir}/test-suite
-export TEST_SUITE_DIR
+export TEST_SUITE_DIR="${top_srcdir}/test-suite"
 
 if [ x"$1" = x-i ] ; then
-    guile=$2
-    shift
-    shift
+    guile="$2"
+    shift 2
 else
-    guile=${top_builddir}/meta/guile
+    guile="${top_builddir}/meta/guile"
 fi
 
-GUILE_LOAD_PATH=$TEST_SUITE_DIR
-export GUILE_LOAD_PATH
+export GUILE_LOAD_PATH="$TEST_SUITE_DIR"
 
 if [ -f "$guile" -a -x "$guile" ] ; then
-    echo Testing $guile ... "$@"
-    echo with GUILE_LOAD_PATH=$GUILE_LOAD_PATH
+    echo "Testing $guile ..." "$@"
+    echo "with GUILE_LOAD_PATH=$GUILE_LOAD_PATH"
 else
-    echo ERROR: Cannot execute $guile
+    echo "ERROR: Cannot execute $guile"
     exit 1
 fi
 
@@ -41,11 +38,9 @@ if [ ! -f guile-procedures.txt ] ; then
    @LN_S@ libguile/guile-procedures.txt .
 fi
 
-exec $guile \
+exec "$guile" \
     --debug \
     -L "$TEST_SUITE_DIR" \
     --no-auto-compile -e main -s "$TEST_SUITE_DIR/guile-test" \
     --test-suite "$TEST_SUITE_DIR/tests" \
     --log-file check-guile.log "$@"
-
-# check-guile ends here
-- 
2.39.2

[PATCH 6/7] check-guile.in: exit 2 on errors and direct output to stderr

[Rob Browning]

Return 2 rather than 1 for errors so that 1 will be available for any
future boolean tests (as with say grep).

Direct error message to stderr rather than stdout.
---
 check-guile.in | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/check-guile.in b/check-guile.in
index 09d95a03c..31bdce340 100644
--- a/check-guile.in
+++ b/check-guile.in
@@ -29,8 +29,8 @@ if [ -f "$guile" -a -x "$guile" ] ; then
     echo "Testing $guile ..." "$@"
     echo "with GUILE_LOAD_PATH=$GUILE_LOAD_PATH"
 else
-    echo "ERROR: Cannot execute $guile"
-    exit 1
+    echo "ERROR: Cannot execute $guile" 1>&2
+    exit 2
 fi
 
 # documentation searching ignores GUILE_LOAD_PATH.
-- 
2.39.2

[PATCH 7/7] Switch to the preferred parallel automake test harness

[Rob Browning]

Here, this allows make -j4 check to run in about half the time that it
does with the deprecated serial harness.
---
 Makefile.am            |  3 ---
 check-guile.in         |  5 +---
 configure.ac           |  5 +---
 test-suite/Makefile.am | 18 ++++++++-----
 test-suite/driver      | 60 ++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 73 insertions(+), 18 deletions(-)
 create mode 100755 test-suite/driver

diff --git a/Makefile.am b/Makefile.am
index eb3541c34..b2ac5539e 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -85,9 +85,6 @@ EXTRA_DIST = LICENSE HACKING GUILE-VERSION			\
 	     .guix/modules/guile-package.scm			\
 	     .guix/manifest.scm
 
-TESTS = check-guile
-TESTS_ENVIRONMENT = @LOCALCHARSET_TESTS_ENVIRONMENT@
-
 ACLOCAL_AMFLAGS = -I m4
 
 CLEANFILES = libguile/guile-procedures.txt
diff --git a/check-guile.in b/check-guile.in
index 31bdce340..137fe6d40 100644
--- a/check-guile.in
+++ b/check-guile.in
@@ -25,10 +25,7 @@ fi
 
 export GUILE_LOAD_PATH="$TEST_SUITE_DIR"
 
-if [ -f "$guile" -a -x "$guile" ] ; then
-    echo "Testing $guile ..." "$@"
-    echo "with GUILE_LOAD_PATH=$GUILE_LOAD_PATH"
-else
+if ! [ -f "$guile" -a -x "$guile" ] ; then
     echo "ERROR: Cannot execute $guile" 1>&2
     exit 2
 fi
diff --git a/configure.ac b/configure.ac
index d0a2dc79b..28f5d4166 100644
--- a/configure.ac
+++ b/configure.ac
@@ -33,11 +33,8 @@ AC_CONFIG_SRCDIR(GUILE-VERSION)
 
 AC_CANONICAL_TARGET
 
-dnl Use `serial-tests' so the output `check-guile' is not hidden
-dnl (`parallel-tests' is the default in Automake 1.13.)
-dnl `serial-tests' was introduced in Automake 1.12.
 AM_INIT_AUTOMAKE([1.12 gnu no-define -Wall -Wno-override \
-  serial-tests color-tests dist-lzip dist-xz])
+  color-tests dist-lzip dist-xz])
 m4_ifdef([AM_SILENT_RULES], [AM_SILENT_RULES([yes])], [AC_SUBST([AM_DEFAULT_VERBOSITY],1)])
 
 AC_COPYRIGHT(GUILE_CONFIGURE_COPYRIGHT)
diff --git a/test-suite/Makefile.am b/test-suite/Makefile.am
index 81e63bce2..b00d598a5 100644
--- a/test-suite/Makefile.am
+++ b/test-suite/Makefile.am
@@ -77,9 +77,7 @@ SCM_TESTS = tests/00-initial-env.test		\
 	    tests/list.test			\
 	    tests/load.test			\
 	    tests/match.test			\
-	    tests/match.test.upstream		\
 	    tests/modules.test			\
-	    tests/multilingual.nottest		\
 	    tests/net-db.test			\
 	    tests/numbers.test			\
 	    tests/optargs.test			\
@@ -93,7 +91,6 @@ SCM_TESTS = tests/00-initial-env.test		\
 	    tests/procs.test			\
 	    tests/poe.test			\
 	    tests/popen.test			\
-	    tests/popen-child.scm		\
 	    tests/ports.test			\
 	    tests/posix.test			\
 	    tests/q.test			\
@@ -206,6 +203,9 @@ EXTRA_DIST = \
 	guile-test \
 	test-suite/lib.scm \
 	$(SCM_TESTS) \
+	tests/match.test.upstream \
+	tests/multilingual.nottest \
+	tests/popen-child.scm \
 	tests/rnrs-test-a.scm \
 	tests/srfi-64-test.scm \
 	ChangeLog-2008
@@ -248,9 +248,13 @@ LALR_EXTRA +=					\
   lalr/glr-test.scm				\
   lalr/run-guile-test.sh
 
-TESTS = $(LALR_TESTS)
-TESTS_ENVIRONMENT =				\
-  @LOCALCHARSET_TESTS_ENVIRONMENT@		\
-  $(top_builddir)/meta/guile --no-auto-compile
+TESTS = $(LALR_TESTS) $(SCM_TESTS)
+
+AM_TESTS_ENVIRONMENT = @LOCALCHARSET_TESTS_ENVIRONMENT@
+LOG_COMPILER = $(top_builddir)/meta/guile
+AM_LOG_FLAGS = --no-auto-compile
+
+TEST_EXTENSIONS = .test
+TEST_LOG_DRIVER = ./driver
 
 EXTRA_DIST += $(LALR_EXTRA) $(LALR_TESTS) tests/sxml-match-tests.ss
diff --git a/test-suite/driver b/test-suite/driver
new file mode 100755
index 000000000..be71a2e63
--- /dev/null
+++ b/test-suite/driver
@@ -0,0 +1,60 @@
+#!/bin/sh
+
+set -ue
+
+script_home="$(cd "$(dirname "$0")" && pwd)"
+
+usage()
+{
+    cat <<-EOF
+	Usage: driver [OPTION ...] -- TEST [ARG ...]
+          This is a test
+	  Options:
+	    --log-file PATH
+	    --trs-file PATH
+	    --color-tests (yes|no)           currently ignored
+	    --expect-failure (yes|no)        currently ignored
+	    --enable-hard-errors (yes|no)    currently ignored
+	    --test-name NAME                 currently ignored
+
+	  This command provides an Automake parallel test harness
+	  compatible driver for running TEST with Guile.  This is
+	  essentially an adapter for check-guile and it currently
+	  assumes that check-guile is in the parent directory.
+
+	EOF
+}
+
+misuse() { usage 1>&2; exit 2; }
+
+test_name=''
+log_file=''
+trs_file=''
+
+while test $# -gt 0; do
+    case "$1" in
+        --test-name) test $# -gt 1 || misuse; test_name="$2"; shift 2 ;;
+        --log-file) test $# -gt 1 || misuse; log_file="$2"; shift 2 ;;
+        --trs-file) test $# -gt 1 || misuse; trs_file="$2"; shift 2 ;;
+        --color-tests|--expect-failure|--enable-hard-errors) shift 2 ;;
+        --) shift; break ;;
+        *) break ;;
+    esac
+done
+
+test "$test_name" || misuse
+test "$log_file" || misuse
+test "$trs_file" || misuse
+
+test $# -gt 0 || misuse
+program="$1"
+shift
+
+# REVIEW: check test-name vs program...
+
+cd ..
+
+exec ./check-guile \
+    --log-file "test-suite/$log_file" \
+    --trs-file "test-suite/$trs_file" \
+    "$(basename "$program")"
-- 
2.39.2

Re: [PATCH 0/7] Allow tests to run in parallel

[Rob Browning]

Rob Browning <rlb@defaultvalue.org> writes:

> This series switches Guile to the Automake parallel test harness so
> that commands like "make -j4 check" can run tests concurrently:
> https://www.gnu.org/software/automake/manual/html_node/Parallel-Test-Harness.html.
>
> Here it cuts the check time in half.
>
> Right now I just wanted to see if this might be interesting.  If so, I
> can make whatever adjustments are desired (guessing perhaps changelog
> entries, etc.).
>
> I suspect eventually we might also want further adjustments to the
> output, but that might or might not be important in the first pass.

Now also available for evaluation via
https://codeberg.org/rlb/guile/pulls/2 and the corresponding rev branch.

Thanks
-- 
Rob Browning
rlb @defaultvalue.org and @debian.org
GPG as of 2011-07-10 E6A9 DA3C C9FD 1FF8 C676 D2C4 C0F0 39E9 ED1B 597A
GPG as of 2002-11-03 14DD 432F AE39 534D B592 F9A0 25C8 D377 8C7E 73A4