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

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

[Maxim Cournoyer]

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

[bug#45948] [PATCH 1/5] build: test-driver.scm Make output redirection optional.

[Maxim Cournoyer]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=yes, Size: 3978 bytes --]

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@gnu.org>
+;;; Copyright © 2021 Maxim Cournoyer <maxim.cournoyer@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"))
                        "^[[m")          ;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

[bug#45948] [PATCH 2/5] build: test-driver.scm: Define the --test-name option as required.

[Maxim Cournoyer]

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

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

[Maxim Cournoyer]

Hello,

Maxim Cournoyer <maxim.cournoyer@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

[bug#45948] [PATCH 3/5] build: test-driver.scm: Enable colored test results by default.

[Maxim Cournoyer]

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

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

[Ludovic Courtès]

Maxim Cournoyer <maxim.cournoyer@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’.

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

[Maxim Cournoyer]

Hi Ludo,

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

> Maxim Cournoyer <maxim.cournoyer@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

[bug#45948] [PATCH 4/5] build: test-driver.scm: Add test cases filtering options.

[Maxim Cournoyer]

* 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"))
                        "^[[m")          ;no color
         result)))
 
+\f
+;;;
+;;; 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))
 
+\f
+;;;
+;;; 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))
+
 \f
 ;;;
 ;;; 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

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

[Maxim Cournoyer]

Hello,

Maxim Cournoyer <maxim.cournoyer@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

[bug#45948] [PATCH 4/5] build: test-driver.scm: Provide the ability to filter on test case names.

[Maxim Cournoyer]

* 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"))
                        "^[[m")          ;no color
         result)))
 
+\f
+;;;
+;;; 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))
 
+\f
+;;;
+;;; 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))
+
 \f
 ;;;
 ;;; 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

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

[Ludovic Courtès]

Hi!

Maxim Cournoyer <maxim.cournoyer@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’.

bug#45948: [PATCH 0/5] Improvements to the Automake SRFI 64 test driver.

[Maxim Cournoyer]

Hello,

Ludovic Courtès <ludo@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

[bug#45948] [PATCH 5/5] build: test-driver.scm: Add a new '--errors-only' option.

[Maxim Cournoyer]

* 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

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

[Ludovic Courtès]

Hi!

Maxim Cournoyer <maxim.cournoyer@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’.

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

[Ludovic Courtès]

Hello!

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

> Ludovic Courtès <ludo@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’.

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

[Maxim Cournoyer]

Hi Ludo,

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

> Hello!
>
> Maxim Cournoyer <maxim.cournoyer@gmail.com> skribis:
>
>> Ludovic Courtès <ludo@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

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

[zimoun]

Hi,

On Tue, 02 Feb 2021 at 00:47, Maxim Cournoyer <maxim.cournoyer@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

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

[Maxim Cournoyer]

Hello Simon!

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

> Hi,
>
> On Tue, 02 Feb 2021 at 00:47, Maxim Cournoyer <maxim.cournoyer@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