GNU bug report logs - #45948
[PATCH 0/5] Improvements to the Automake SRFI 64 test driver.

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 45948 in the body.
You can then email your comments to 45948 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
To: guix-patches <at> gnu.org
Cc: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
Subject: [PATCH 0/5] Improvements to the Automake SRFI 64 test driver.
Date: Mon, 18 Jan 2021 01:18:53 -0500

This series make the SRFI 64 test driver easier to hack on, and add support to
select individual test cases (rather than test files) as well as a way to make
the output concise enough to make it more useful in a test driven development
context.

Maxim Cournoyer (5):
  build: test-driver.scm Make output redirection optional.
  build: test-driver.scm: Define the --test-name option as required.
  build: test-driver.scm: Enable colored test results by default.
  build: test-driver.scm: Add test cases filtering options.
  build: test-driver.scm: Add a new '--errors-only' option.

 build-aux/test-driver.scm | 144 +++++++++++++++++++++++++++-----------
 doc/guix.texi             |  24 +++++++
 2 files changed, 127 insertions(+), 41 deletions(-)

-- 
2.29.2

Message #8 received at 45948 <at> debbugs.gnu.org (full text, mbox):

From: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
To: 45948 <at> debbugs.gnu.org
Cc: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
Subject: [PATCH 1/5] build: test-driver.scm Make output redirection optional.
Date: Mon, 18 Jan 2021 01:24:56 -0500

This makes it easier (and less surprising) for users to experiment with the
custom Scheme test driver directly.  The behavior is unchanged from Automake's
point of view.

* build-aux/test-driver.scm (main): Make the --log-file and --trs-file
arguments optional.  Only open, redirect and close a port to a log file when
the --log-file option is provided.  Only open and close a port to a trs file
when the --trs-file option is provided.
(test-runner-gnu): Set OUT-PORT parameter default value to the current output
port.  Set the TRS-PORT parameter default value to a void port.  Update doc.
---
 build-aux/test-driver.scm | 34 ++++++++++++++++++++--------------
 1 file changed, 20 insertions(+), 14 deletions(-)

diff --git a/build-aux/test-driver.scm b/build-aux/test-driver.scm
index 52af1e9be7..b7622c3ed2 100644
--- a/build-aux/test-driver.scm
+++ b/build-aux/test-driver.scm
@@ -1,8 +1,9 @@
 ;;;; test-driver.scm - Guile test driver for Automake testsuite harness
 
-(define script-version "2017-03-22.13") ;UTC
+(define script-version "2021-01-18.06") ;UTC
 
 ;;; Copyright © 2015, 2016 Mathieu Lirzin <mthl <at> gnu.org>
+;;; Copyright © 2021 Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
 ;;;
 ;;; This program is free software; you can redistribute it and/or modify it
 ;;; under the terms of the GNU General Public License as published by
@@ -75,11 +76,14 @@ The '--test-name', '--log-file' and '--trs-file' options are mandatory.\n"))
                        "")          ;no color
         result)))
 
-(define* (test-runner-gnu test-name #:key color? brief? out-port trs-port)
+(define* (test-runner-gnu test-name #:key color? brief?
+                          (out-port (current-output-port))
+                          (trs-port (%make-void-port "w")))
   "Return an custom SRFI-64 test runner.  TEST-NAME is a string specifying the
 file name of the current the test.  COLOR? specifies whether to use colors,
-and BRIEF?, well, you know.  OUT-PORT and TRS-PORT must be output ports.  The
-current output port is supposed to be redirected to a '.log' file."
+and BRIEF?, well, you know.  OUT-PORT and TRS-PORT must be output ports.
+OUT-PORT defaults to the current output port, while TRS-PORT defaults to a
+void port, which means no TRS output is logged."
 
   (define (test-on-test-begin-gnu runner)
     ;; Procedure called at the start of an individual test case, before the
@@ -156,20 +160,22 @@ current output port is supposed to be redirected to a '.log' file."
      ((option 'help #f)    (show-help))
      ((option 'version #f) (format #t "test-driver.scm ~A" script-version))
      (else
-      (let ((log (open-file (option 'log-file "") "w0"))
-            (trs (open-file (option 'trs-file "") "wl"))
-            (out (duplicate-port (current-output-port) "wl")))
-        (redirect-port log (current-output-port))
-        (redirect-port log (current-warning-port))
-        (redirect-port log (current-error-port))
+      (let ((log (and=> (option 'log-file #f) (cut open-file <> "w0")))
+            (trs (and=> (option 'trs-file #f) (cut open-file <> "wl")))
+            (out (duplicate-port (current-output-port) "wl"))
+            (test-name (option 'test-name #f)))
+        (when log
+          (redirect-port log (current-output-port))
+          (redirect-port log (current-warning-port))
+          (redirect-port log (current-error-port)))
         (test-with-runner
-            (test-runner-gnu (option 'test-name #f)
+            (test-runner-gnu test-name
                              #:color? (option->boolean opts 'color-tests)
                              #:brief? (option->boolean opts 'brief)
                              #:out-port out #:trs-port trs)
-          (load-from-path (option 'test-name #f)))
-        (close-port log)
-        (close-port trs)
+          (load-from-path test-name))
+        (and=> log close-port)
+        (and=> trs close-port)
         (close-port out))))
     (exit 0)))
 
-- 
2.29.2

Message #11 received at 45948 <at> debbugs.gnu.org (full text, mbox):

From: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
To: 45948 <at> debbugs.gnu.org
Cc: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
Subject: [PATCH 2/5] build: test-driver.scm: Define the --test-name option as
 required.
Date: Mon, 18 Jan 2021 01:24:57 -0500

This is clearer than failing to open an empty string file name, when the
option is not provided.

* build-aux/test-driver.scm (%options)[test-name]: Set the required? property
to #t.
---
 build-aux/test-driver.scm | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/build-aux/test-driver.scm b/build-aux/test-driver.scm
index b7622c3ed2..5891631da6 100644
--- a/build-aux/test-driver.scm
+++ b/build-aux/test-driver.scm
@@ -39,7 +39,7 @@
 The '--test-name', '--log-file' and '--trs-file' options are mandatory.\n"))
 
 (define %options
-  '((test-name                 (value #t))
+  '((test-name                 (value #t) (required? #t))
     (log-file                  (value #t))
     (trs-file                  (value #t))
     (color-tests               (value #t))
-- 
2.29.2

Message #14 received at 45948 <at> debbugs.gnu.org (full text, mbox):

From: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
To: 45948 <at> debbugs.gnu.org
Cc: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
Subject: [PATCH 4/5] build: test-driver.scm: Add test cases filtering options.
Date: Mon, 18 Jan 2021 01:24:59 -0500

* build-aux/test-driver.scm (show-help): Add help text for the new --select
and --exclude options.
(%options): Add the new select and exclude options.
(test-runner-gnu): Pass them to the test runner.  Update doc.
(test-match-name*, test-match-name*/negated, %test-match-all): New variables.
(main): Compute the test specifier based on the values of the new options and
apply it to the current test runner when running the test file.
* doc/guix.texi (Running the Test Suite): Document the new options.
---
 build-aux/test-driver.scm | 66 ++++++++++++++++++++++++++++++++-------
 doc/guix.texi             | 12 +++++++
 2 files changed, 66 insertions(+), 12 deletions(-)

diff --git a/build-aux/test-driver.scm b/build-aux/test-driver.scm
index 767a9fb25d..3ad6b0c93f 100644
--- a/build-aux/test-driver.scm
+++ b/build-aux/test-driver.scm
@@ -27,6 +27,8 @@
 
 (use-modules (ice-9 getopt-long)
              (ice-9 pretty-print)
+             (ice-9 regex)
+             (srfi srfi-1)
              (srfi srfi-26)
              (srfi srfi-64))
 
@@ -34,14 +36,19 @@
   (display "Usage:
    test-driver --test-name=NAME --log-file=PATH --trs-file=PATH
                [--expect-failure={yes|no}] [--color-tests={yes|no}]
+               [--select=REGEXP] [--exclude=REGEXP]
                [--enable-hard-errors={yes|no}] [--brief={yes|no}}] [--]
                TEST-SCRIPT [TEST-SCRIPT-ARGUMENTS]
-The '--test-name', '--log-file' and '--trs-file' options are mandatory.\n"))
+The '--test-name', '--log-file' and '--trs-file' options are mandatory.
+The '--select' and '--exclude' options allow selecting or excluding individual
+test cases via a regexp, respectively.\n"))
 
 (define %options
   '((test-name                 (value #t) (required? #t))
     (log-file                  (value #t))
     (trs-file                  (value #t))
+    (select                    (value #t))
+    (exclude                   (value #t))
     (color-tests               (value #t))
     (expect-failure            (value #t)) ;XXX: not implemented yet
     (enable-hard-errors        (value #t)) ;not implemented in SRFI-64
@@ -76,14 +83,22 @@ The '--test-name', '--log-file' and '--trs-file' options are mandatory.\n"))
                        "")          ;no color
         result)))
 
+
+;;;
+;;; SRFI 64 custom test runner.
+;;;
+
 (define* (test-runner-gnu test-name #:key color? brief?
                           (out-port (current-output-port))
-                          (trs-port (%make-void-port "w")))
+                          (trs-port (%make-void-port "w"))
+                          select exclude)
   "Return an custom SRFI-64 test runner.  TEST-NAME is a string specifying the
 file name of the current the test.  COLOR? specifies whether to use colors,
 and BRIEF?, well, you know.  OUT-PORT and TRS-PORT must be output ports.
 OUT-PORT defaults to the current output port, while TRS-PORT defaults to a
-void port, which means no TRS output is logged."
+void port, which means no TRS output is logged.  SELECT and EXCLUDE may take a
+regular expression to select or exclude individual test cases based on their
+names."
 
   (define (test-on-test-begin-gnu runner)
     ;; Procedure called at the start of an individual test case, before the
@@ -148,6 +163,24 @@ void port, which means no TRS output is logged."
     (test-runner-on-bad-end-name! runner test-on-bad-end-name-simple)
     runner))
 
+
+;;;
+;;; SRFI 64 test specifiers.
+;;;
+(define (test-match-name* regexp)
+  "Return a test specifier that matches a test name against REGEXP."
+  (lambda (runner)
+    (string-match regexp (test-runner-test-name runner))))
+
+(define (test-match-name*/negated regexp)
+  "Return a negated test specifier version of test-match-name*."
+  (lambda (runner)
+    (not (string-match regexp (test-runner-test-name runner)))))
+
+;;; XXX: test-match-all is a syntax, which isn't convenient to use with a list
+;;; of test specifiers computed at run time.
+(define %test-match-all (@@ (srfi srfi-64) %test-match-all))
+
 
 ;;;
 ;;; Entry point.
@@ -158,15 +191,22 @@ void port, which means no TRS output is logged."
          (option (cut option-ref opts <> <>)))
     (cond
      ((option 'help #f)    (show-help))
-     ((option 'version #f) (format #t "test-driver.scm ~A" script-version))
+     ((option 'version #f) (format #t "test-driver.scm ~A" 'script-version))
      (else
-      (let ((log (and=> (option 'log-file #f) (cut open-file <> "w0")))
-            (trs (and=> (option 'trs-file #f) (cut open-file <> "wl")))
-            (out (duplicate-port (current-output-port) "wl"))
-            (test-name (option 'test-name #f))
-            (color-tests (if (assoc 'color-tests opts)
-                             (option->boolean opts 'color-tests)
-                             #t)))
+      (let* ((log (and=> (option 'log-file #f) (cut open-file <> "w0")))
+             (trs (and=> (option 'trs-file #f) (cut open-file <> "wl")))
+             (out (duplicate-port (current-output-port) "wl"))
+             (test-name (option 'test-name #f))
+             (select (option 'select #f))
+             (exclude (option 'exclude #f))
+             (test-specifiers (filter-map
+                               identity
+                               (list (and=> select test-match-name*)
+                                     (and=> exclude test-match-name*/negated))))
+             (test-specifier (apply %test-match-all test-specifiers))
+             (color-tests (if (assoc 'color-tests opts)
+                              (option->boolean opts 'color-tests)
+                              #t)))
         (when log
           (redirect-port log (current-output-port))
           (redirect-port log (current-warning-port))
@@ -176,7 +216,9 @@ void port, which means no TRS output is logged."
                              #:color? color-tests
                              #:brief? (option->boolean opts 'brief)
                              #:out-port out #:trs-port trs)
-          (load-from-path test-name))
+          (test-apply test-specifier
+                      (lambda _
+                        (load-from-path test-name))))
         (and=> log close-port)
         (and=> trs close-port)
         (close-port out))))
diff --git a/doc/guix.texi b/doc/guix.texi
index 194bb3a314..d59d2806cb 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -915,6 +915,18 @@ the @code{SCM_LOG_DRIVER_FLAGS} makefile variable as in this example:
 make check TESTS="tests/base64.scm" SCM_LOG_DRIVER_FLAGS="--brief=no"
 @end example
 
+The underlying SRFI 64 custom Automake test driver used for the 'check'
+test suite (located at @file{build-aux/test-driver.scm}) also allows
+selecting which test cases to run at a finer level, via its
+@option{--select} and @option{--exclude} options.  Here's an example, to
+run all the test cases from the @file{tests/packages.scm} test file
+whose names start with ``transaction-upgrade-entry'':
+
+@example
+export SCM_LOG_DRIVER_FLAGS="--select=^transaction-upgrade-entry"
+make check TESTS="tests/packages.scm"
+@end example
+
 Upon failure, please email @email{bug-guix@@gnu.org} and attach the
 @file{test-suite.log} file.  Please specify the Guix version being used
 as well as version numbers of the dependencies (@pxref{Requirements}) in
-- 
2.29.2

Message #17 received at 45948 <at> debbugs.gnu.org (full text, mbox):

From: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
To: 45948 <at> debbugs.gnu.org
Cc: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
Subject: [PATCH 3/5] build: test-driver.scm: Enable colored test results by
 default.
Date: Mon, 18 Jan 2021 01:24:58 -0500

The Automake parallel test harness does its own smart detection of the
terminal color capability and always provides the --color-tests argument to
the driver.  This change defaults the --color-tests argument to true when the
test driver is run on its own (not via Automake).

* build-aux/test-driver.scm (main): Set the default value of the --color-tests
argument to true when it's not explicitly provided.
---
 build-aux/test-driver.scm | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/build-aux/test-driver.scm b/build-aux/test-driver.scm
index 5891631da6..767a9fb25d 100644
--- a/build-aux/test-driver.scm
+++ b/build-aux/test-driver.scm
@@ -163,14 +163,17 @@ void port, which means no TRS output is logged."
       (let ((log (and=> (option 'log-file #f) (cut open-file <> "w0")))
             (trs (and=> (option 'trs-file #f) (cut open-file <> "wl")))
             (out (duplicate-port (current-output-port) "wl"))
-            (test-name (option 'test-name #f)))
+            (test-name (option 'test-name #f))
+            (color-tests (if (assoc 'color-tests opts)
+                             (option->boolean opts 'color-tests)
+                             #t)))
         (when log
           (redirect-port log (current-output-port))
           (redirect-port log (current-warning-port))
           (redirect-port log (current-error-port)))
         (test-with-runner
             (test-runner-gnu test-name
-                             #:color? (option->boolean opts 'color-tests)
+                             #:color? color-tests
                              #:brief? (option->boolean opts 'brief)
                              #:out-port out #:trs-port trs)
           (load-from-path test-name))
-- 
2.29.2

Message #20 received at 45948 <at> debbugs.gnu.org (full text, mbox):

From: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
To: 45948 <at> debbugs.gnu.org
Cc: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
Subject: [PATCH 4/5] build: test-driver.scm: Provide the ability to filter on
 test case names.
Date: Mon, 18 Jan 2021 01:25:00 -0500

* build-aux/test-driver.scm (show-help): Add help text for the new --select
and --exclude options.
(%options): Add the new select and exclude options.
(test-runner-gnu): Pass them to the test runner.  Update doc.
(test-match-name*, test-match-name*/negated, %test-match-all): New variables.
(main): Compute the test specifier based on the values of the new options and
apply it to the current test runner when running the test file.
* doc/guix.texi (Running the Test Suite): Document the new options.
---
 build-aux/test-driver.scm | 66 ++++++++++++++++++++++++++++++++-------
 doc/guix.texi             | 12 +++++++
 2 files changed, 66 insertions(+), 12 deletions(-)

diff --git a/build-aux/test-driver.scm b/build-aux/test-driver.scm
index 767a9fb25d..3ad6b0c93f 100644
--- a/build-aux/test-driver.scm
+++ b/build-aux/test-driver.scm
@@ -27,6 +27,8 @@
 
 (use-modules (ice-9 getopt-long)
              (ice-9 pretty-print)
+             (ice-9 regex)
+             (srfi srfi-1)
              (srfi srfi-26)
              (srfi srfi-64))
 
@@ -34,14 +36,19 @@
   (display "Usage:
    test-driver --test-name=NAME --log-file=PATH --trs-file=PATH
                [--expect-failure={yes|no}] [--color-tests={yes|no}]
+               [--select=REGEXP] [--exclude=REGEXP]
                [--enable-hard-errors={yes|no}] [--brief={yes|no}}] [--]
                TEST-SCRIPT [TEST-SCRIPT-ARGUMENTS]
-The '--test-name', '--log-file' and '--trs-file' options are mandatory.\n"))
+The '--test-name', '--log-file' and '--trs-file' options are mandatory.
+The '--select' and '--exclude' options allow selecting or excluding individual
+test cases via a regexp, respectively.\n"))
 
 (define %options
   '((test-name                 (value #t) (required? #t))
     (log-file                  (value #t))
     (trs-file                  (value #t))
+    (select                    (value #t))
+    (exclude                   (value #t))
     (color-tests               (value #t))
     (expect-failure            (value #t)) ;XXX: not implemented yet
     (enable-hard-errors        (value #t)) ;not implemented in SRFI-64
@@ -76,14 +83,22 @@ The '--test-name', '--log-file' and '--trs-file' options are mandatory.\n"))
                        "")          ;no color
         result)))
 
+
+;;;
+;;; SRFI 64 custom test runner.
+;;;
+
 (define* (test-runner-gnu test-name #:key color? brief?
                           (out-port (current-output-port))
-                          (trs-port (%make-void-port "w")))
+                          (trs-port (%make-void-port "w"))
+                          select exclude)
   "Return an custom SRFI-64 test runner.  TEST-NAME is a string specifying the
 file name of the current the test.  COLOR? specifies whether to use colors,
 and BRIEF?, well, you know.  OUT-PORT and TRS-PORT must be output ports.
 OUT-PORT defaults to the current output port, while TRS-PORT defaults to a
-void port, which means no TRS output is logged."
+void port, which means no TRS output is logged.  SELECT and EXCLUDE may take a
+regular expression to select or exclude individual test cases based on their
+names."
 
   (define (test-on-test-begin-gnu runner)
     ;; Procedure called at the start of an individual test case, before the
@@ -148,6 +163,24 @@ void port, which means no TRS output is logged."
     (test-runner-on-bad-end-name! runner test-on-bad-end-name-simple)
     runner))
 
+
+;;;
+;;; SRFI 64 test specifiers.
+;;;
+(define (test-match-name* regexp)
+  "Return a test specifier that matches a test name against REGEXP."
+  (lambda (runner)
+    (string-match regexp (test-runner-test-name runner))))
+
+(define (test-match-name*/negated regexp)
+  "Return a negated test specifier version of test-match-name*."
+  (lambda (runner)
+    (not (string-match regexp (test-runner-test-name runner)))))
+
+;;; XXX: test-match-all is a syntax, which isn't convenient to use with a list
+;;; of test specifiers computed at run time.
+(define %test-match-all (@@ (srfi srfi-64) %test-match-all))
+
 
 ;;;
 ;;; Entry point.
@@ -158,15 +191,22 @@ void port, which means no TRS output is logged."
          (option (cut option-ref opts <> <>)))
     (cond
      ((option 'help #f)    (show-help))
-     ((option 'version #f) (format #t "test-driver.scm ~A" script-version))
+     ((option 'version #f) (format #t "test-driver.scm ~A" 'script-version))
      (else
-      (let ((log (and=> (option 'log-file #f) (cut open-file <> "w0")))
-            (trs (and=> (option 'trs-file #f) (cut open-file <> "wl")))
-            (out (duplicate-port (current-output-port) "wl"))
-            (test-name (option 'test-name #f))
-            (color-tests (if (assoc 'color-tests opts)
-                             (option->boolean opts 'color-tests)
-                             #t)))
+      (let* ((log (and=> (option 'log-file #f) (cut open-file <> "w0")))
+             (trs (and=> (option 'trs-file #f) (cut open-file <> "wl")))
+             (out (duplicate-port (current-output-port) "wl"))
+             (test-name (option 'test-name #f))
+             (select (option 'select #f))
+             (exclude (option 'exclude #f))
+             (test-specifiers (filter-map
+                               identity
+                               (list (and=> select test-match-name*)
+                                     (and=> exclude test-match-name*/negated))))
+             (test-specifier (apply %test-match-all test-specifiers))
+             (color-tests (if (assoc 'color-tests opts)
+                              (option->boolean opts 'color-tests)
+                              #t)))
         (when log
           (redirect-port log (current-output-port))
           (redirect-port log (current-warning-port))
@@ -176,7 +216,9 @@ void port, which means no TRS output is logged."
                              #:color? color-tests
                              #:brief? (option->boolean opts 'brief)
                              #:out-port out #:trs-port trs)
-          (load-from-path test-name))
+          (test-apply test-specifier
+                      (lambda _
+                        (load-from-path test-name))))
         (and=> log close-port)
         (and=> trs close-port)
         (close-port out))))
diff --git a/doc/guix.texi b/doc/guix.texi
index 194bb3a314..d59d2806cb 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -915,6 +915,18 @@ the @code{SCM_LOG_DRIVER_FLAGS} makefile variable as in this example:
 make check TESTS="tests/base64.scm" SCM_LOG_DRIVER_FLAGS="--brief=no"
 @end example
 
+The underlying SRFI 64 custom Automake test driver used for the 'check'
+test suite (located at @file{build-aux/test-driver.scm}) also allows
+selecting which test cases to run at a finer level, via its
+@option{--select} and @option{--exclude} options.  Here's an example, to
+run all the test cases from the @file{tests/packages.scm} test file
+whose names start with ``transaction-upgrade-entry'':
+
+@example
+export SCM_LOG_DRIVER_FLAGS="--select=^transaction-upgrade-entry"
+make check TESTS="tests/packages.scm"
+@end example
+
 Upon failure, please email @email{bug-guix@@gnu.org} and attach the
 @file{test-suite.log} file.  Please specify the Guix version being used
 as well as version numbers of the dependencies (@pxref{Requirements}) in
-- 
2.29.2

Message #23 received at 45948 <at> debbugs.gnu.org (full text, mbox):

From: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
To: 45948 <at> debbugs.gnu.org
Cc: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
Subject: [PATCH 5/5] build: test-driver.scm: Add a new '--errors-only' option.
Date: Mon, 18 Jan 2021 01:25:01 -0500

* build-aux/test-driver.scm (show-help): Add the help text for the
new '--errors-only' option.
(%options): Add the errors-only option.
(test-runner-gnu): Add the errors-only? parameter and update doc.  Move the
logging of the test data after the test has completed, so a choice can be made
whether to keep it or discard it based on the value of the test result.
(main): Pass the errors-only? option to the driver.
* doc/guix.texi (Running the Test Suite): Document the new option.
---
 build-aux/test-driver.scm | 73 ++++++++++++++++++++++-----------------
 doc/guix.texi             | 12 +++++++
 2 files changed, 54 insertions(+), 31 deletions(-)

diff --git a/build-aux/test-driver.scm b/build-aux/test-driver.scm
index 3ad6b0c93f..3f593abc7b 100644
--- a/build-aux/test-driver.scm
+++ b/build-aux/test-driver.scm
@@ -36,12 +36,15 @@
   (display "Usage:
    test-driver --test-name=NAME --log-file=PATH --trs-file=PATH
                [--expect-failure={yes|no}] [--color-tests={yes|no}]
-               [--select=REGEXP] [--exclude=REGEXP]
+               [--select=REGEXP] [--exclude=REGEXP] [--errors-only={yes|no}]
                [--enable-hard-errors={yes|no}] [--brief={yes|no}}] [--]
                TEST-SCRIPT [TEST-SCRIPT-ARGUMENTS]
 The '--test-name', '--log-file' and '--trs-file' options are mandatory.
 The '--select' and '--exclude' options allow selecting or excluding individual
-test cases via a regexp, respectively.\n"))
+test cases via a regexp, respectively.  The '--errors-only' option can be set
+to \"yes\" to limit the logged test case metadata to only those test cases
+that failed.  When set to \"yes\", the '--brief' option disables printing the
+individual test case result to the console.\n"))
 
 (define %options
   '((test-name                 (value #t) (required? #t))
@@ -49,6 +52,7 @@ test cases via a regexp, respectively.\n"))
     (trs-file                  (value #t))
     (select                    (value #t))
     (exclude                   (value #t))
+    (errors-only               (value #t))
     (color-tests               (value #t))
     (expect-failure            (value #t)) ;XXX: not implemented yet
     (enable-hard-errors        (value #t)) ;not implemented in SRFI-64
@@ -88,27 +92,26 @@ test cases via a regexp, respectively.\n"))
 ;;; SRFI 64 custom test runner.
 ;;;
 
-(define* (test-runner-gnu test-name #:key color? brief?
+(define* (test-runner-gnu test-name #:key color? brief? errors-only?
                           (out-port (current-output-port))
                           (trs-port (%make-void-port "w"))
                           select exclude)
   "Return an custom SRFI-64 test runner.  TEST-NAME is a string specifying the
-file name of the current the test.  COLOR? specifies whether to use colors,
-and BRIEF?, well, you know.  OUT-PORT and TRS-PORT must be output ports.
-OUT-PORT defaults to the current output port, while TRS-PORT defaults to a
-void port, which means no TRS output is logged.  SELECT and EXCLUDE may take a
-regular expression to select or exclude individual test cases based on their
-names."
-
-  (define (test-on-test-begin-gnu runner)
-    ;; Procedure called at the start of an individual test case, before the
-    ;; test expression (and expected value) are evaluated.
-    (let ((result (cute assq-ref (test-result-alist runner) <>)))
-      (format #t "test-name: ~A~%" (result 'test-name))
-      (format #t "location: ~A~%"
-              (string-append (result 'source-file) ":"
-                             (number->string (result 'source-line))))
-      (test-display "source" (result 'source-form) #:pretty? #t)))
+file name of the current the test.  COLOR? specifies whether to use colors.
+When BRIEF? is true, the individual test cases results are masked and only the
+summary is shown.  ERRORS-ONLY? reduces the amount of test case metadata
+logged to only that of the failed test cases.  OUT-PORT and TRS-PORT must be
+output ports.  OUT-PORT defaults to the current output port, while TRS-PORT
+defaults to a void port, which means no TRS output is logged.  SELECT and
+EXCLUDE may take a regular expression to select or exclude individual test
+cases based on their names."
+
+  (define (test-skipped? runner)
+    (eq? 'skip (test-result-kind runner)))
+
+  (define (test-failed? runner)
+    (not (or (test-passed? runner)
+             (test-skipped? runner))))
 
   (define (test-on-test-end-gnu runner)
     ;; Procedure called at the end of an individual test case, when the result
@@ -116,21 +119,29 @@ names."
     (let* ((results (test-result-alist runner))
            (result? (cut assq <> results))
            (result  (cut assq-ref results <>)))
-      (unless brief?
+      (unless (or brief? (and errors-only? (test-skipped? runner)))
         ;; Display the result of each test case on the console.
         (format out-port "~A: ~A - ~A~%"
                 (result->string (test-result-kind runner) #:colorize? color?)
                 test-name (test-runner-test-name runner)))
-      (when (result? 'expected-value)
-        (test-display "expected-value" (result 'expected-value)))
-      (when (result? 'expected-error)
-        (test-display "expected-error" (result 'expected-error) #:pretty? #t))
-      (when (result? 'actual-value)
-        (test-display "actual-value" (result 'actual-value)))
-      (when (result? 'actual-error)
-        (test-display "actual-error" (result 'actual-error) #:pretty? #t))
-      (format #t "result: ~a~%" (result->string (result 'result-kind)))
-      (newline)
+
+      (unless (and errors-only? (not (test-failed? runner)))
+        (format #t "test-name: ~A~%" (result 'test-name))
+        (format #t "location: ~A~%"
+                (string-append (result 'source-file) ":"
+                               (number->string (result 'source-line))))
+        (test-display "source" (result 'source-form) #:pretty? #t)
+        (when (result? 'expected-value)
+          (test-display "expected-value" (result 'expected-value)))
+        (when (result? 'expected-error)
+          (test-display "expected-error" (result 'expected-error) #:pretty? #t))
+        (when (result? 'actual-value)
+          (test-display "actual-value" (result 'actual-value)))
+        (when (result? 'actual-error)
+          (test-display "actual-error" (result 'actual-error) #:pretty? #t))
+        (format #t "result: ~a~%" (result->string (result 'result-kind)))
+        (newline))
+
       (format trs-port ":test-result: ~A ~A~%"
               (result->string (test-result-kind runner))
               (test-runner-test-name runner))))
@@ -157,7 +168,6 @@ names."
       #f))
 
   (let ((runner (test-runner-null)))
-    (test-runner-on-test-begin! runner test-on-test-begin-gnu)
     (test-runner-on-test-end! runner test-on-test-end-gnu)
     (test-runner-on-group-end! runner test-on-group-end-gnu)
     (test-runner-on-bad-end-name! runner test-on-bad-end-name-simple)
@@ -215,6 +225,7 @@ names."
             (test-runner-gnu test-name
                              #:color? color-tests
                              #:brief? (option->boolean opts 'brief)
+                             #:errors-only? (option->boolean opts 'errors-only)
                              #:out-port out #:trs-port trs)
           (test-apply test-specifier
                       (lambda _
diff --git a/doc/guix.texi b/doc/guix.texi
index d59d2806cb..ac20a541f5 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -927,6 +927,18 @@ export SCM_LOG_DRIVER_FLAGS="--select=^transaction-upgrade-entry"
 make check TESTS="tests/packages.scm"
 @end example
 
+Those wishing to inspect the results of failed tests directly from the
+command line can add the @option{--errors-only=yes} option to the
+@code{SCM_LOG_DRIVER_FLAGS} makefile variable and set the @code{VERBOSE}
+Automake makefile variable, as in:
+
+@example
+make check SCM_LOG_DRIVER_FLAGS="--brief=no --errors-only=yes" VERBOSE=1
+@end example
+
+@xref{Parallel Test Harness,,,automake,GNU Automake} for more
+information about the Automake Parallel Test Harness.
+
 Upon failure, please email @email{bug-guix@@gnu.org} and attach the
 @file{test-suite.log} file.  Please specify the Guix version being used
 as well as version numbers of the dependencies (@pxref{Requirements}) in
-- 
2.29.2

Message #26 received at 45948 <at> debbugs.gnu.org (full text, mbox):

From: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
To: 45948 <at> debbugs.gnu.org
Subject: Re: bug#45948: [PATCH 0/5] Improvements to the Automake SRFI 64
 test driver.
Date: Mon, 18 Jan 2021 09:37:07 -0500

Hello,

Maxim Cournoyer <maxim.cournoyer <at> gmail.com> writes:

> This is clearer than failing to open an empty string file name, when the
> option is not provided.
>
> * build-aux/test-driver.scm (%options)[test-name]: Set the required? property
> to #t.

I just noticed this doesn't play well with the --help or --version
options.  So this patch should be dropped.

Thanks,

Maxim

Message #29 received at 45948 <at> debbugs.gnu.org (full text, mbox):

From: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
To: 45948 <at> debbugs.gnu.org
Subject: Re: bug#45948: [PATCH 0/5] Improvements to the Automake SRFI 64
 test driver.
Date: Mon, 18 Jan 2021 09:38:38 -0500

Hello,

Maxim Cournoyer <maxim.cournoyer <at> gmail.com> writes:

[...]

>       ((option 'help #f)    (show-help))
> -     ((option 'version #f) (format #t "test-driver.scm ~A" script-version))
> +     ((option 'version #f) (format #t "test-driver.scm ~A" 'script-version))

This part of the diff is erroneous.  Please disregard.

Thanks,

Maxim

Message #32 received at 45948 <at> debbugs.gnu.org (full text, mbox):

From: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
To: 45948 <at> debbugs.gnu.org
Cc: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
Subject: [PATCH 6/5] build: test-driver.scm: Allow running as a standalone
 script.
Date: Mon, 18 Jan 2021 09:42:01 -0500

* build-aux/test-driver.scm: Add an exec-based shebang and set the script
executable bit.
(main): Insert a newline after the version string is printed with --version.
---
 build-aux/test-driver.scm | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)
 mode change 100644 => 100755 build-aux/test-driver.scm

diff --git a/build-aux/test-driver.scm b/build-aux/test-driver.scm
old mode 100644
new mode 100755
index aa34e669ff..2d0649ef37
--- a/build-aux/test-driver.scm
+++ b/build-aux/test-driver.scm
@@ -1,6 +1,9 @@
+#!/bin/sh
+exec guile --no-auto-compile -e main -s "$0" "$@"
+!#
 ;;;; test-driver.scm - Guile test driver for Automake testsuite harness
 
-(define script-version "2021-01-18.06") ;UTC
+(define script-version "2021-01-18.14") ;UTC
 
 ;;; Copyright © 2015, 2016 Mathieu Lirzin <mthl <at> gnu.org>
 ;;; Copyright © 2021 Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
@@ -201,7 +204,7 @@ cases based on their names."
          (option (cut option-ref opts <> <>)))
     (cond
      ((option 'help #f)    (show-help))
-     ((option 'version #f) (format #t "test-driver.scm ~A" script-version))
+     ((option 'version #f) (format #t "test-driver.scm ~A~%" script-version))
      (else
       (let* ((log (and=> (option 'log-file #f) (cut open-file <> "w0")))
              (trs (and=> (option 'trs-file #f) (cut open-file <> "wl")))
-- 
2.29.2

Message #35 received at 45948 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
Cc: 45948 <at> debbugs.gnu.org
Subject: Re: bug#45948: [PATCH 0/5] Improvements to the Automake SRFI 64
 test driver.
Date: Sat, 30 Jan 2021 22:32:37 +0100

Hi!

Maxim Cournoyer <maxim.cournoyer <at> gmail.com> skribis:

> This makes it easier (and less surprising) for users to experiment with the
> custom Scheme test driver directly.  The behavior is unchanged from Automake's
> point of view.
>
> * build-aux/test-driver.scm (main): Make the --log-file and --trs-file
> arguments optional.  Only open, redirect and close a port to a log file when
> the --log-file option is provided.  Only open and close a port to a trs file
> when the --trs-file option is provided.
> (test-runner-gnu): Set OUT-PORT parameter default value to the current output
> port.  Set the TRS-PORT parameter default value to a void port.  Update doc.

Not tested, but LGTM!

Ludo’.

Message #38 received at 45948 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
Cc: 45948 <at> debbugs.gnu.org
Subject: Re: bug#45948: [PATCH 0/5] Improvements to the Automake SRFI 64
 test driver.
Date: Sat, 30 Jan 2021 22:34:49 +0100

Hi!

Maxim Cournoyer <maxim.cournoyer <at> gmail.com> skribis:

> * build-aux/test-driver.scm (show-help): Add help text for the new --select
> and --exclude options.
> (%options): Add the new select and exclude options.
> (test-runner-gnu): Pass them to the test runner.  Update doc.
> (test-match-name*, test-match-name*/negated, %test-match-all): New variables.
> (main): Compute the test specifier based on the values of the new options and
> apply it to the current test runner when running the test file.
> * doc/guix.texi (Running the Test Suite): Document the new options.

I never felt the need for this since most individual files run quickly
enough (and those that don’t should be optimized…), but it can be
useful.

> +;;; XXX: test-match-all is a syntax, which isn't convenient to use with a list
> +;;; of test specifiers computed at run time.
> +(define %test-match-all (@@ (srfi srfi-64) %test-match-all))

Since this is an internal procedure in Guile that could vanish anytime,
I recommend copying it here (it’s just 9 lines).

> +The underlying SRFI 64 custom Automake test driver used for the 'check'
> +test suite (located at @file{build-aux/test-driver.scm}) also allows

Maybe shorten to “The underlying test driver (located at
@file{build-aux/test-driver.scm}) also allows”.

I didn’t test it, but it otherwise LGTM.

Ludo’.

Message #41 received at 45948 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
Cc: 45948 <at> debbugs.gnu.org
Subject: Re: bug#45948: [PATCH 0/5] Improvements to the Automake SRFI 64
 test driver.
Date: Sat, 30 Jan 2021 22:39:17 +0100

Maxim Cournoyer <maxim.cournoyer <at> gmail.com> skribis:

> The Automake parallel test harness does its own smart detection of the
> terminal color capability and always provides the --color-tests argument to
> the driver.  This change defaults the --color-tests argument to true when the
> test driver is run on its own (not via Automake).
>
> * build-aux/test-driver.scm (main): Set the default value of the --color-tests
> argument to true when it's not explicitly provided.

It’s a small change to why not, but…

… the test driver is not meant to be used on its own though, it’s really
a test driver for Automake.

Plus, in most projects, part of the test environment is defined in
‘Makefile.am’; I wouldn’t want to process bug reports because people
thought they could try and run tests without “make check”.

Thoughts?

As a side note, this test driver is bundled in quite a few Guile
packages.  Longer-term, it would be nice to consider maintaining it as
part of Automake.  Now is maybe a good time since there’s on-going
maintenance work happening in Autoconf (and soon Automake and Libtool)
after a long hiatus.

Ludo’.

Message #44 received at 45948 <at> debbugs.gnu.org (full text, mbox):

From: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
To: Ludovic Courtès <ludo <at> gnu.org>
Cc: 45948 <at> debbugs.gnu.org
Subject: Re: bug#45948: [PATCH 0/5] Improvements to the Automake SRFI 64
 test driver.
Date: Sun, 31 Jan 2021 21:47:46 -0500

Hi Ludo,

Ludovic Courtès <ludo <at> gnu.org> writes:

> Maxim Cournoyer <maxim.cournoyer <at> gmail.com> skribis:
>
>> The Automake parallel test harness does its own smart detection of the
>> terminal color capability and always provides the --color-tests argument to
>> the driver.  This change defaults the --color-tests argument to true when the
>> test driver is run on its own (not via Automake).
>>
>> * build-aux/test-driver.scm (main): Set the default value of the --color-tests
>> argument to true when it's not explicitly provided.
>
> It’s a small change to why not, but…
>
> … the test driver is not meant to be used on its own though, it’s really
> a test driver for Automake.

The interface via make is not terribly convenient/discoverable; I find
having the test-driver produce useful output on its own handy for
hacking on the thing hands on.

> Plus, in most projects, part of the test environment is defined in
> ‘Makefile.am’; I wouldn’t want to process bug reports because people
> thought they could try and run tests without “make check”.

The use of the test driver without the Automake build system is
undocumented; I wouldn't be too worried about people suddenly stumbling
on it.

Thanks,

Maxim

Message #49 received at 45948-done <at> debbugs.gnu.org (full text, mbox):

From: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
To: Ludovic Courtès <ludo <at> gnu.org>
Cc: 45948-done <at> debbugs.gnu.org
Subject: Re: bug#45948: [PATCH 0/5] Improvements to the Automake SRFI 64
 test driver.
Date: Sun, 31 Jan 2021 22:44:15 -0500

Hello,

Ludovic Courtès <ludo <at> gnu.org> writes:

[...]

> I never felt the need for this since most individual files run quickly
> enough (and those that don’t should be optimized…), but it can be
> useful.

What triggered it for me was trying to iterate using tests added to the
tests/packages.scm test module:

--8<---------------cut here---------------start------------->8---
$ time make check TESTS=tests/packages.scm VERBOSE=1
SCM_LOG_DRIVER_FLAGS="--brief=no"

time make check TESTS=tests/packages.scm VERBOSE=1 SCM_LOG_DRIVER_FLAGS="--brief=no"
[...]
PASS: tests/packages.scm - package-patched-vulnerabilities
PASS: tests/packages.scm - fold-packages
PASS: tests/packages.scm - fold-packages, hidden package
[... time passes ...]
PASS: tests/packages.scm - fold-available-packages with/without cache
[...]
PASS: tests/packages.scm - find-package-locations with cache
PASS: tests/packages.scm - specification->location
============================================================================
Testsuite summary for GNU Guix UNKNOWN
============================================================================
# TOTAL: 100
# PASS:  100
# SKIP:  0
# XFAIL: 0
# FAIL:  0
# XPASS: 0
# ERROR: 0
============================================================================
[...]
real    0m46.172s
user    1m4.885s
sys     0m0.376s
--8<---------------cut here---------------end--------------->8---

That's on the fastest machine I have access to (on my vintage desktop,
it took nearly 4 minutes).  The slowest test seems to be
'fold-available-packages with/without cache'.

Now with the new select, one can do:

--8<---------------cut here---------------start------------->8---
$ time make check TESTS=tests/packages.scm VERBOSE=1 SCM_LOG_DRIVER_FLAGS="--brief=no --select='bag->derivation' --errors-only=yes"
[...]
PASS: tests/packages.scm - bag->derivation
PASS: tests/packages.scm - bag->derivation, cross-compilation
============================================================================
Testsuite summary for GNU Guix UNKNOWN
============================================================================
# TOTAL: 100
# PASS:  2
# SKIP:  98
# XFAIL: 0
# FAIL:  0
# XPASS: 0
# ERROR: 0
============================================================================
[...]
real    0m1.569s
user    0m2.382s
sys     0m0.154s
--8<---------------cut here---------------end--------------->8---

1.6 s; better than 46 s!

We can also check the time the suspected slow test took:

$ time make check TESTS=tests/packages.scm SCM_LOG_DRIVER_FLAGS="--select='fold-available-packages with/without cache'"
[...]
PASS: tests/packages.scm - fold-available-packages with/without cache
============================================================================
Testsuite summary for GNU Guix UNKNOWN
============================================================================
# TOTAL: 100
# PASS:  1
# SKIP:  99
# XFAIL: 0
# FAIL:  0
# XPASS: 0
# ERROR: 0
============================================================================
[...]
real    0m36.627s
user    0m45.731s
sys     0m0.264s

So yes, this is the most expensive test of tests/packages.scm.

>> +;;; XXX: test-match-all is a syntax, which isn't convenient to use with a list
>> +;;; of test specifiers computed at run time.
>> +(define %test-match-all (@@ (srfi srfi-64) %test-match-all))
>
> Since this is an internal procedure in Guile that could vanish anytime,
> I recommend copying it here (it’s just 9 lines).

Done!

>> +The underlying SRFI 64 custom Automake test driver used for the 'check'
>> +test suite (located at @file{build-aux/test-driver.scm}) also allows
>
> Maybe shorten to “The underlying test driver (located at
> @file{build-aux/test-driver.scm}) also allows”.

I see value in explicitly stating what it is, as it took me some effort
to be able to answer that question when I started looking at it (the
test driver).

I've now pushed this series to master; thank you for the review!

Maxim

Message #52 received at 45948-done <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
Cc: 45948-done <at> debbugs.gnu.org
Subject: Re: bug#45948: [PATCH 0/5] Improvements to the Automake SRFI 64
 test driver.
Date: Mon, 01 Feb 2021 23:18:27 +0100

Hello!

Maxim Cournoyer <maxim.cournoyer <at> gmail.com> skribis:

> Ludovic Courtès <ludo <at> gnu.org> writes:
>
> [...]
>
>> I never felt the need for this since most individual files run quickly
>> enough (and those that don’t should be optimized…), but it can be
>> useful.
>
> What triggered it for me was trying to iterate using tests added to the
> tests/packages.scm test module:

[...]

> 1.6 s; better than 46 s!

Quite a difference, indeed!

Some tests, like ‘tests/store.scm’, invoke the GC (via ‘delete-paths’
calls in particular), and this is a bad idea because it takes ages.
What I meant by “should be optimized” is that these tests should be
tweaked to avoid invoking the GC as much as possible.

That’s not the case of ‘tests/packages.scm’ though.  Which gives me an
idea: what would it take to modify the test driver so it can print the
time spent in each test?  :-)

That would probably prove helpful to optimize core Guix.

In many cases, I also run the one test I’m interested in through Geiser,
which gives an optimally fast feedback loop.

> We can also check the time the suspected slow test took:
>
> $ time make check TESTS=tests/packages.scm SCM_LOG_DRIVER_FLAGS="--select='fold-available-packages with/without cache'"
> [...]
> PASS: tests/packages.scm - fold-available-packages with/without cache

Ah ha!  Turns out a large part of the time was due to the O(n²) behavior
of the various list operations (the list of packages is big enough!).
Fixed in 73744725dd0a65cddaa9251f104f17ca27756479.

>>> +The underlying SRFI 64 custom Automake test driver used for the 'check'
>>> +test suite (located at @file{build-aux/test-driver.scm}) also allows
>>
>> Maybe shorten to “The underlying test driver (located at
>> @file{build-aux/test-driver.scm}) also allows”.
>
> I see value in explicitly stating what it is, as it took me some effort
> to be able to answer that question when I started looking at it (the
> test driver).

Agreed.  It just seemed to me that we were mentioning three new
concepts/tools in passing: SRFI-64, Automake, and test drivers.

> I've now pushed this series to master; thank you for the review!

Thank you!

Ludo’.

Message #55 received at 45948-done <at> debbugs.gnu.org (full text, mbox):

From: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
To: Ludovic Courtès <ludo <at> gnu.org>
Cc: 45948-done <at> debbugs.gnu.org
Subject: Re: bug#45948: [PATCH 0/5] Improvements to the Automake SRFI 64
 test driver.
Date: Tue, 02 Feb 2021 00:47:11 -0500

Hi Ludo,

Ludovic Courtès <ludo <at> gnu.org> writes:

> Hello!
>
> Maxim Cournoyer <maxim.cournoyer <at> gmail.com> skribis:
>
>> Ludovic Courtès <ludo <at> gnu.org> writes:
>>
>> [...]
>>
>>> I never felt the need for this since most individual files run quickly
>>> enough (and those that don’t should be optimized…), but it can be
>>> useful.
>>
>> What triggered it for me was trying to iterate using tests added to the
>> tests/packages.scm test module:
>
> [...]
>
>> 1.6 s; better than 46 s!
>
> Quite a difference, indeed!
>
> Some tests, like ‘tests/store.scm’, invoke the GC (via ‘delete-paths’
> calls in particular), and this is a bad idea because it takes ages.
> What I meant by “should be optimized” is that these tests should be
> tweaked to avoid invoking the GC as much as possible.
>
> That’s not the case of ‘tests/packages.scm’ though.  Which gives me an
> idea: what would it take to modify the test driver so it can print the
> time spent in each test?  :-)
>
> That would probably prove helpful to optimize core Guix.

Good idea!  I've pushed 5e652e94a9 that does this!  Here's a sample,
showing the three slowest test cases of the tests/packages.scm test:

$ export SCM_LOG_DRIVER_FLAGS="--brief=no --show-duration=yes"
$ make check TESTS=tests/packages.scm

--8<---------------cut here---------------start------------->8---
[...]
PASS: tests/packages.scm - fold-available-packages with/without cache [17.121s]
PASS: tests/packages.scm - find-packages-by-name [0.000s]
PASS: tests/packages.scm - find-packages-by-name with version [0.000s]
PASS: tests/packages.scm - find-packages-by-name with cache [9.492s]
PASS: tests/packages.scm - find-packages-by-name + version, with cache [7.412s]
[...]
PASS: tests/packages.scm - find-package-locations with cache [7.484s]
PASS: tests/packages.scm - specification->location [0.000s]
--8<---------------cut here---------------end--------------->8---

Automake could be extended to support this extra data; it would be
interesting to be able to set the number of slowest tests to display,
such as DURATIONS=5, to show only the 5 slowest tests, for example.

> In many cases, I also run the one test I’m interested in through Geiser,
> which gives an optimally fast feedback loop.

I've done that too, although it's somewhat clunky to use: the test
modules name not being mapped to an actual filename hierarchy trips the
module loading mechanism of Guile/Geiser.  I usually fix that such that
(test-packages) -> (tests packages), then I can load the module
normally.  Except that's not what I want to do as it runs all the tests.
So I must copy paste the module declaration to the REPL to get the
symbols imported, then the other required definitions and the
(test-begin... due to SRFI 64 stateful design.  Not my idea of a sleek
test feedback loop, coming from pytest and other modern test frameworks;
still very powerful to be able to do everything at the REPL though!

>> We can also check the time the suspected slow test took:
>>
>> $ time make check TESTS=tests/packages.scm
>> SCM_LOG_DRIVER_FLAGS="--select='fold-available-packages with/without
>> cache'"
>> [...]
>> PASS: tests/packages.scm - fold-available-packages with/without cache
>
> Ah ha!  Turns out a large part of the time was due to the O(n²) behavior
> of the various list operations (the list of packages is big enough!).
> Fixed in 73744725dd0a65cddaa9251f104f17ca27756479.

Woohoo!  That looks clever!  Too clever for me to understand in the 2
minutes I starred at it ;-).  That test now runs almost 6x faster (~8 s
on a fast machine).  Well done!

>>>> +The underlying SRFI 64 custom Automake test driver used for the 'check'
>>>> +test suite (located at @file{build-aux/test-driver.scm}) also allows
>>>
>>> Maybe shorten to “The underlying test driver (located at
>>> @file{build-aux/test-driver.scm}) also allows”.
>>
>> I see value in explicitly stating what it is, as it took me some effort
>> to be able to answer that question when I started looking at it (the
>> test driver).
>
> Agreed.  It just seemed to me that we were mentioning three new
> concepts/tools in passing: SRFI-64, Automake, and test drivers.

Yeah, it's a mouthful, for better or worse.

Thank you,

Maxim

Message #58 received at 45948-done <at> debbugs.gnu.org (full text, mbox):

From: zimoun <zimon.toutoune <at> gmail.com>
To: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>, Ludovic Courtès <ludo <at> gnu.org>
Cc: 45948-done <at> debbugs.gnu.org
Subject: Re: [bug#45948] [PATCH 0/5] Improvements to the Automake SRFI 64
 test driver.
Date: Tue, 02 Feb 2021 09:36:11 +0100

Hi,

On Tue, 02 Feb 2021 at 00:47, Maxim Cournoyer <maxim.cournoyer <at> gmail.com> wrote:

> $ export SCM_LOG_DRIVER_FLAGS="--brief=no --show-duration=yes"
> $ make check TESTS=tests/packages.scm
>
> --8<---------------cut here---------------start------------->8---
> [...]
> PASS: tests/packages.scm - fold-available-packages with/without cache [17.121s]
> PASS: tests/packages.scm - find-packages-by-name [0.000s]
> PASS: tests/packages.scm - find-packages-by-name with version [0.000s]
> PASS: tests/packages.scm - find-packages-by-name with cache [9.492s]
> PASS: tests/packages.scm - find-packages-by-name + version, with cache [7.412s]
> [...]
> PASS: tests/packages.scm - find-package-locations with cache [7.484s]
> PASS: tests/packages.scm - specification->location [0.000s]
> --8<---------------cut here---------------end--------------->8---

Cool!  Thanks Maxim.  I find it really better. :-)


>>> We can also check the time the suspected slow test took:
>>>
>>> $ time make check TESTS=tests/packages.scm
>>> SCM_LOG_DRIVER_FLAGS="--select='fold-available-packages with/without
>>> cache'"
>>> [...]
>>> PASS: tests/packages.scm - fold-available-packages with/without cache
>>
>> Ah ha!  Turns out a large part of the time was due to the O(n²) behavior
>> of the various list operations (the list of packages is big enough!).
>> Fixed in 73744725dd0a65cddaa9251f104f17ca27756479.

Neat!  Cool!


Cheers,
simon

Message #61 received at 45948-done <at> debbugs.gnu.org (full text, mbox):

From: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
To: zimoun <zimon.toutoune <at> gmail.com>
Cc: Ludovic Courtès <ludo <at> gnu.org>, 45948-done <at> debbugs.gnu.org
Subject: Re: [bug#45948] [PATCH 0/5] Improvements to the Automake SRFI 64
 test driver.
Date: Tue, 02 Feb 2021 07:52:59 -0500

Hello Simon!

zimoun <zimon.toutoune <at> gmail.com> writes:

> Hi,
>
> On Tue, 02 Feb 2021 at 00:47, Maxim Cournoyer <maxim.cournoyer <at> gmail.com> wrote:
>
>> $ export SCM_LOG_DRIVER_FLAGS="--brief=no --show-duration=yes"
>> $ make check TESTS=tests/packages.scm
>>
>> --8<---------------cut here---------------start------------->8---
>> [...]
>> PASS: tests/packages.scm - fold-available-packages with/without cache [17.121s]
>> PASS: tests/packages.scm - find-packages-by-name [0.000s]
>> PASS: tests/packages.scm - find-packages-by-name with version [0.000s]
>> PASS: tests/packages.scm - find-packages-by-name with cache [9.492s]
>> PASS: tests/packages.scm - find-packages-by-name + version, with cache [7.412s]
>> [...]
>> PASS: tests/packages.scm - find-package-locations with cache [7.484s]
>> PASS: tests/packages.scm - specification->location [0.000s]
>> --8<---------------cut here---------------end--------------->8---
>
> Cool!  Thanks Maxim.  I find it really better. :-)

I'm glad you find these changes useful!

Cheers,

Maxim

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.