GNU bug report logs - #74385
[PATCH 0/4] Patches for SRFI-64

Previous Next

To reply to this bug, email your comments to 74385 AT debbugs.gnu.org.

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

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

From: Tomas Volf <~@wolfsden.cz>
To: bug-guile <at> gnu.org
Cc: Tomas Volf <~@wolfsden.cz>
Subject: [PATCH 0/4] Patches for SRFI-64
Date: Sat, 16 Nov 2024 18:39:54 +0100

Few patches resolving either problems I have noticed or that were
reported on the mailing list.

Tomas Volf (4):
  srfi-64: Fix maybe-print-prop.
  srfi-64: Use ~s when printing some properties.
  srfi-64: Export define-equality-test.
  srfi-64: Report failed tests in (standards)Errors format.

 module/srfi/srfi-64.scm | 65 +++++++++++++++++++++++++----------------
 1 file changed, 40 insertions(+), 25 deletions(-)

--
2.46.0

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

From: Tomas Volf <~@wolfsden.cz>
To: 74385 <at> debbugs.gnu.org
Cc: Tomas Volf <~@wolfsden.cz>
Subject: [PATCH 2/4] srfi-64: Use ~s when printing some properties.
Date: Sat, 16 Nov 2024 18:42:05 +0100

This will help to properly debug failing tests like:

    (test-equal "some failing test" "a b " "a b")

Before there was no way to tell that one "a b" as extra trailing space, now
there is.

* module/srfi/srfi-64.scm (test-on-test-end-simple)['expected-value]
['expected-error, 'actual-value, 'actual-error]: Print using ~s.
[maybe-print-prop]: Take the code for format as a parameter.
---
 module/srfi/srfi-64.scm | 25 +++++++++----------------
 1 file changed, 9 insertions(+), 16 deletions(-)

diff --git a/module/srfi/srfi-64.scm b/module/srfi/srfi-64.scm
index 13ae26d48..7b3341bf0 100644
--- a/module/srfi/srfi-64.scm
+++ b/module/srfi/srfi-64.scm
@@ -27,7 +27,6 @@
   #:use-module (ice-9 exceptions)
   #:use-module (ice-9 format)
   #:use-module (ice-9 match)
-  #:use-module (ice-9 pretty-print)
   #:use-module (srfi srfi-1)
   #:use-module (srfi srfi-9)
   #:use-module (srfi srfi-26)
@@ -417,17 +416,11 @@ instead."
 
 (define (test-on-test-end-simple runner)
   "Log that test is done."
-  (define (maybe-print-prop prop pretty?)
+  (define (maybe-print-prop prop pretty? code)
     (let* ((default (list))
            (val (test-result-ref runner prop default)))
       (unless (eq? val default)
-        (let ((val (string-trim-both
-                    (with-output-to-string
-                      (λ ()
-                        (if pretty?
-                            (pretty-print val #:per-line-prefix "             ")
-                            (display val)))))))
-          (format #t "~a: ~a~%" prop val)))))
+        (format #t "~a: ~@?~&" prop code val))))
 
   (let ((result-kind (test-result-kind runner)))
     ;; Skip tests not executed due to run list.
@@ -436,13 +429,13 @@ instead."
               result-kind
               (test-runner-test-name runner))
       (unless (member result-kind '(pass xfail))
-        (maybe-print-prop 'source-file    #f)
-        (maybe-print-prop 'source-line    #f)
-        (maybe-print-prop 'source-form    #t)
-        (maybe-print-prop 'expected-value #f)
-        (maybe-print-prop 'expected-error #t)
-        (maybe-print-prop 'actual-value   #f)
-        (maybe-print-prop 'actual-error   #t)))))
+        (maybe-print-prop 'source-file    #f "~a")
+        (maybe-print-prop 'source-line    #f "~a")
+        (maybe-print-prop 'source-form    #t "~y")
+        (maybe-print-prop 'expected-value #f "~s")
+        (maybe-print-prop 'expected-error #t "~s")
+        (maybe-print-prop 'actual-value   #f "~s")
+        (maybe-print-prop 'actual-error   #t "~s")))))
 
 (define (test-runner-simple)
   "Creates a new simple test-runner, that prints errors and a summary on the
-- 
2.46.0

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

From: Tomas Volf <~@wolfsden.cz>
To: 74385 <at> debbugs.gnu.org
Cc: Tomas Volf <~@wolfsden.cz>
Subject: [PATCH 1/4] srfi-64: Fix maybe-print-prop.
Date: Sat, 16 Nov 2024 18:42:04 +0100

Previously it always printed the property, regardless of whether it was set or
not.

* module/srfi/srfi-64.scm (test-on-test-end-simple)[maybe-print-prop]:
Print only set properties.
---
 module/srfi/srfi-64.scm | 19 ++++++++++---------
 1 file changed, 10 insertions(+), 9 deletions(-)

diff --git a/module/srfi/srfi-64.scm b/module/srfi/srfi-64.scm
index 98fcef645..13ae26d48 100644
--- a/module/srfi/srfi-64.scm
+++ b/module/srfi/srfi-64.scm
@@ -418,15 +418,16 @@ instead."
 (define (test-on-test-end-simple runner)
   "Log that test is done."
   (define (maybe-print-prop prop pretty?)
-    (let* ((val (test-result-ref runner prop))
-           (val (string-trim-both
-                 (with-output-to-string
-                   (λ ()
-                     (if pretty?
-                         (pretty-print val #:per-line-prefix "             ")
-                         (display val)))))))
-      (when val
-        (format #t "~a: ~a~%" prop val))))
+    (let* ((default (list))
+           (val (test-result-ref runner prop default)))
+      (unless (eq? val default)
+        (let ((val (string-trim-both
+                    (with-output-to-string
+                      (λ ()
+                        (if pretty?
+                            (pretty-print val #:per-line-prefix "             ")
+                            (display val)))))))
+          (format #t "~a: ~a~%" prop val)))))
 
   (let ((result-kind (test-result-kind runner)))
     ;; Skip tests not executed due to run list.
-- 
2.46.0

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

From: Tomas Volf <~@wolfsden.cz>
To: 74385 <at> debbugs.gnu.org
Cc: Tomas Volf <~@wolfsden.cz>, Janneke Nieuwenhuizen <janneke <at> gnu.org>
Subject: [PATCH 3/4] srfi-64: Export define-equality-test.
Date: Sat, 16 Nov 2024 18:42:06 +0100

Interest was expressed on the mailing list to have %test-2 as a part of the
public API.  So rename it and export from the module.

* module/srfi/srfi-64.scm (define-equality-test): Rename from %test-2.
(%test-2): Rename from %%test-2.
(test-eq, test-eqv, test-equal): Adjust.
(define-module)<#:export>: Export it.

Reported-by: Janneke Nieuwenhuizen <janneke <at> gnu.org>
---
 module/srfi/srfi-64.scm | 30 +++++++++++++++++++++++-------
 1 file changed, 23 insertions(+), 7 deletions(-)

diff --git a/module/srfi/srfi-64.scm b/module/srfi/srfi-64.scm
index 7b3341bf0..203db49ea 100644
--- a/module/srfi/srfi-64.scm
+++ b/module/srfi/srfi-64.scm
@@ -118,6 +118,8 @@
    test-procedure?
    test-thunk
 
+   define-equality-test
+
    &bad-end-name
    bad-end-name?
    bad-end-name-begin-name
@@ -728,7 +730,7 @@ to invoke @code{test-assert} if there is no current test runner.
 
 @end defspec")
 
-(define-syntax %%test-2
+(define-syntax %test-2
   (λ (x)
     (syntax-case x ()
       ((_ syn test-proc test-name expected test-expr)
@@ -742,20 +744,34 @@ to invoke @code{test-assert} if there is no current test runner.
                          (test-result-set! r 'actual-value   a)
                          (test-proc e a))))))))
 
-(define-syntax %test-2
+(define-syntax define-equality-test
   (syntax-rules ()
     ((_ name test-proc)
      (define-syntax name
        (λ (x)
          (syntax-case x ()
            ((_ test-name expected test-expr)
-            #`(%%test-2 #,x test-proc test-name expected test-expr))
+            #`(%test-2 #,x test-proc test-name expected test-expr))
            ((_ expected test-expr)
-            #`(%%test-2 #,x test-proc #f        expected test-expr))))))))
+            #`(%test-2 #,x test-proc #f        expected test-expr))))))))
+(set-documentation! 'define-equality-test
+  "@defspec define-equality-test identifier proc
+Define a new test form named @var{identifier} with same signature and usage as
+@code{test-eq} but using @var{proc} instead of @code{eq?}.
 
-(%test-2 test-eq    eq?)
-(%test-2 test-eqv   eqv?)
-(%test-2 test-equal equal?)
+For example, the provided equality checks are defined as:
+
+@lisp
+(define-equality-test test-eq    eq?)
+(define-equality-test test-eqv   eqv?)
+(define-equality-test test-equal equal?)
+@end lisp
+
+@end defspec")
+
+(define-equality-test test-eq    eq?)
+(define-equality-test test-eqv   eqv?)
+(define-equality-test test-equal equal?)
 
 (set-documentation! 'test-eq
   "@defspec test-eq test-name expected test-expr
-- 
2.46.0

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

From: Tomas Volf <~@wolfsden.cz>
To: 74385 <at> debbugs.gnu.org
Cc: Tomas Volf <~@wolfsden.cz>, Janneke Nieuwenhuizen <janneke <at> gnu.org>
Subject: [PATCH 4/4] srfi-64: Report failed tests in (standards)Errors format.
Date: Sat, 16 Nov 2024 18:42:07 +0100

There is a page in the GNU Standards document regarding the format of error
messages.  Both GNU Emacs and Vim are able to parse it and support jumping to
next/previous error.  My version did not produce a line in this format for
failed tests and this commit rectifies that.

* module/srfi/srfi-64.scm (test-on-test-end-simple)[non-passed]: Write
out (standards)Errors compatible line.

Reported-by: Janneke Nieuwenhuizen <janneke <at> gnu.org>
---
 module/srfi/srfi-64.scm | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/module/srfi/srfi-64.scm b/module/srfi/srfi-64.scm
index 203db49ea..98f6c8114 100644
--- a/module/srfi/srfi-64.scm
+++ b/module/srfi/srfi-64.scm
@@ -28,6 +28,7 @@
   #:use-module (ice-9 format)
   #:use-module (ice-9 match)
   #:use-module (srfi srfi-1)
+  #:use-module (srfi srfi-2)
   #:use-module (srfi srfi-9)
   #:use-module (srfi srfi-26)
   #:use-module (srfi srfi-71)
@@ -431,6 +432,10 @@ instead."
               result-kind
               (test-runner-test-name runner))
       (unless (member result-kind '(pass xfail))
+        (and-let* ((file (test-result-ref runner 'source-file))
+                   (line (test-result-ref runner 'source-line)))
+          ;; Satisfy (standards)Errors
+          (format #t "~a:~a: unexpected result~%" file line))
         (maybe-print-prop 'source-file    #f "~a")
         (maybe-print-prop 'source-line    #f "~a")
         (maybe-print-prop 'source-form    #t "~y")
-- 
2.46.0

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

From: Ludovic Courtès <ludo <at> gnu.org>
To: Tomas Volf <~@wolfsden.cz>
Cc: 74385 <at> debbugs.gnu.org, Janneke Nieuwenhuizen <janneke <at> gnu.org>
Subject: Re: bug#74385: [PATCH 3/4] srfi-64: Export define-equality-test.
Date: Mon, 09 Dec 2024 18:01:15 +0100

Hi Tomas,

Tomas Volf <~@wolfsden.cz> skribis:

> +(define-syntax define-equality-test
>    (syntax-rules ()
>      ((_ name test-proc)
>       (define-syntax name
>         (λ (x)
>           (syntax-case x ()
>             ((_ test-name expected test-expr)
> -            #`(%%test-2 #,x test-proc test-name expected test-expr))
> +            #`(%test-2 #,x test-proc test-name expected test-expr))
>             ((_ expected test-expr)
> -            #`(%%test-2 #,x test-proc #f        expected test-expr))))))))
> +            #`(%test-2 #,x test-proc #f        expected test-expr))))))))
> +(set-documentation! 'define-equality-test
> +  "@defspec define-equality-test identifier proc
> +Define a new test form named @var{identifier} with same signature and usage as
> +@code{test-eq} but using @var{proc} instead of @code{eq?}.

I didn’t notice earlier, but you can add docstrings like this:

  (define-syntax define-equality-test
    (syntax-rules ()
      "This is the docstring."
      …))

As for exporting ‘define-equality-test’, I would clearly mark it as a
“GNU extension”.

The way this was done before is by having more exports in a separate
module, like (srfi srfi-9 gnu).

Whether or not you pick this approach, please make sure to document it
in the manual and to prominently mark it as a GNU extension.

Ludo’.

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

From: Ludovic Courtès <ludo <at> gnu.org>
To: Tomas Volf <~@wolfsden.cz>
Cc: 74385 <at> debbugs.gnu.org, Janneke Nieuwenhuizen <janneke <at> gnu.org>
Subject: Re: bug#74385: [PATCH 4/4] srfi-64: Report failed tests in
 (standards)Errors format.
Date: Mon, 09 Dec 2024 18:02:57 +0100

Tomas Volf <~@wolfsden.cz> skribis:

> There is a page in the GNU Standards document regarding the format of error
> messages.  Both GNU Emacs and Vim are able to parse it and support jumping to
> next/previous error.  My version did not produce a line in this format for
> failed tests and this commit rectifies that.
>
> * module/srfi/srfi-64.scm (test-on-test-end-simple)[non-passed]: Write
> out (standards)Errors compatible line.
>
> Reported-by: Janneke Nieuwenhuizen <janneke <at> gnu.org>

I personally like this but my gut feeling is that we may want to stick
to whatever the previous SRFI-64 implementation was doing, to avoid
disruption or breakage for users (remember we’re applying this to a
stable series).

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

From: Ludovic Courtès <ludo <at> gnu.org>
To: Tomas Volf <~@wolfsden.cz>
Cc: 74385 <at> debbugs.gnu.org
Subject: Re: bug#74385: [PATCH 1/4] srfi-64: Fix maybe-print-prop.
Date: Mon, 09 Dec 2024 18:04:01 +0100

Tomas Volf <~@wolfsden.cz> skribis:

> Previously it always printed the property, regardless of whether it was set or
> not.
>
> * module/srfi/srfi-64.scm (test-on-test-end-simple)[maybe-print-prop]:
> Print only set properties.

[...]

> This will help to properly debug failing tests like:
>
>     (test-equal "some failing test" "a b " "a b")
>
> Before there was no way to tell that one "a b" as extra trailing space, now
> there is.
>
> * module/srfi/srfi-64.scm (test-on-test-end-simple)['expected-value]
> ['expected-error, 'actual-value, 'actual-error]: Print using ~s.
> [maybe-print-prop]: Take the code for format as a parameter.

These two LGTM.  Thanks!

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

From: Tomas Volf <~@wolfsden.cz>
To: Ludovic Courtès <ludo <at> gnu.org>
Cc: 74385 <at> debbugs.gnu.org, Janneke Nieuwenhuizen <janneke <at> gnu.org>
Subject: Re: bug#74385: [PATCH 4/4] srfi-64: Report failed tests in
 (standards)Errors format.
Date: Fri, 13 Dec 2024 17:05:59 +0100

[Message part 1 (text/plain, inline)]

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

> Tomas Volf <~@wolfsden.cz> skribis:
>
>> There is a page in the GNU Standards document regarding the format of error
>> messages.  Both GNU Emacs and Vim are able to parse it and support jumping to
>> next/previous error.  My version did not produce a line in this format for
>> failed tests and this commit rectifies that.
>>
>> * module/srfi/srfi-64.scm (test-on-test-end-simple)[non-passed]: Write
>> out (standards)Errors compatible line.
>>
>> Reported-by: Janneke Nieuwenhuizen <janneke <at> gnu.org>
>
> I personally like this but my gut feeling is that we may want to stick
> to whatever the previous SRFI-64 implementation was doing, to avoid
> disruption or breakage for users (remember we’re applying this to a
> stable series).

I agree that the output of the simple test runner of the current and the
reference versions do differ, by necessity.  I tested the change in
Emacs and it seems to work out of the box, the compilation buffer
correctly navigates to the locations.

However I understand your worries regarding the stable series.  What I
would suggest is that I can extract the test runner from the reference
implementation, and package it as reference/test-runner-simple.  User
then could, with a simple code change, use it instead of the new
compliant runner.

I would even go as far as to provide a mechanism to select the runner by
environment, so GUILE_SRFI64_DEFAULT_TEST_RUNNER=reference would cause
test-runner-factory to return reference/test-runner-simple.  If unset or
empty value, it would use the new runner as it does now.  This will
allow restoring the previous output format even with no code changes.

Combined with NEWS entry, that could be sufficient?  What do you think?

Tomas

-- 
There are only two hard things in Computer Science:
cache invalidation, naming things and off-by-one errors.

[signature.asc (application/pgp-signature, inline)]

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

From: Tomas Volf <~@wolfsden.cz>
To: Ludovic Courtès <ludo <at> gnu.org>
Cc: 74385 <at> debbugs.gnu.org, Janneke Nieuwenhuizen <janneke <at> gnu.org>
Subject: Re: bug#74385: [PATCH 3/4] srfi-64: Export define-equality-test.
Date: Fri, 13 Dec 2024 17:16:47 +0100

[Message part 1 (text/plain, inline)]

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

> Hi Tomas,
>
> Tomas Volf <~@wolfsden.cz> skribis:
>
>> +(define-syntax define-equality-test
>>    (syntax-rules ()
>>      ((_ name test-proc)
>>       (define-syntax name
>>         (λ (x)
>>           (syntax-case x ()
>>             ((_ test-name expected test-expr)
>> -            #`(%%test-2 #,x test-proc test-name expected test-expr))
>> +            #`(%test-2 #,x test-proc test-name expected test-expr))
>>             ((_ expected test-expr)
>> -            #`(%%test-2 #,x test-proc #f        expected test-expr))))))))
>> +            #`(%test-2 #,x test-proc #f        expected test-expr))))))))
>> +(set-documentation! 'define-equality-test
>> +  "@defspec define-equality-test identifier proc
>> +Define a new test form named @var{identifier} with same signature and usage as
>> +@code{test-eq} but using @var{proc} instead of @code{eq?}.
>
> I didn’t notice earlier, but you can add docstrings like this:
>
>   (define-syntax define-equality-test
>     (syntax-rules ()
>       "This is the docstring."
>       …))

Ah, good catch.  Since that works just for syntax-rules and not
syntax-case, I probably did not moved the documentation string around
when switching between them.  Will update.

>
> As for exporting ‘define-equality-test’, I would clearly mark it as a
> “GNU extension”.
>
> The way this was done before is by having more exports in a separate
> module, like (srfi srfi-9 gnu).
>
> Whether or not you pick this approach, please make sure to document it
> in the manual and to prominently mark it as a GNU extension.

I personally think that leaving it in one module is fine, and I agree it
should be documented.  There are other GNU extensions already in the new
SRFI-64, and I plan to document all once #71300 is merged.  I will wait
for v2 until that happens and will include the documentation changes
required.

Have a nice day,
Tomas

-- 
There are only two hard things in Computer Science:
cache invalidation, naming things and off-by-one errors.

[signature.asc (application/pgp-signature, inline)]

Previous Next

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