bug#72365: srfi-64: test-on-bad-end-name-simple is not allowed to raise an exception

[Taylan Kammer]

On 30.07.2024 21:51, Tomas Volf wrote:
> Hello,
>
> I think I found a bug in (srfi srfi-64) module shipped with GNU Guile.

Hi Tomas,

Thanks for stress-testing the SRFI 64 spec & implementation and reporting all 
these discrepancies. :-)

Firstly, to reiterate some things I've already mentioned in the thread on bug 
71300, just so it goes on record here as well:

I have a SRFI 64 implementation of my own. I hope Guile will switch to it 
eventually because I find the upstream reference implementation to be somewhat 
unpleasant to work with. (It's monolithic, and not the cleanest code.)

Until then, my implementation can be used by following these steps:

1. Cloning this repo:

    https://codeberg.org/taylan/scheme-srfis/

2. Running Guile like so:

    GUILE_LOAD_PATH=/path/to/scheme-srfis/guile-srfi-64 guile

(Replacing /path/to/scheme-srfis with the actual path to wherein the repo was 
cloned, of course.)

Then, loading SRFI-64 the regular way should load my implementation rather than 
the one that ships with Guile (which is the reference implementation from the 
SRFI author).

I'll respond to your reports one by one, treating them like bug reports towards 
my own implementation, since it was originally derived from the reference 
implementation and has probably inherited some of the bugs. Unfortunately, I'm 
not motivated to work on the implementation that's in Guile, because I find it 
too cumbersome to navigate its code and the unclean coding practices too 
distracting.

> The specification says the following about the simple test runner:
>
>> Creates a new simple test-runner, that prints errors and a summary on the
>> standard output port.
> It does not mention that it can signal errors, so I believe the following 
> should
> just print to the terminal:
>
>     (use-modules (srfi srfi-64))
>     (test-on-bad-end-name-simple (test-runner-null) "x" "y")
>
> However is signals an error instead:
>
>     Backtrace:
>     In ice-9/boot-9.scm:
>       1752:10  6 (with-exception-handler _ _ #:unwind? _ #:unwind-for-type _)
>     In unknown file:
>                5 (apply-smob/0 #<thunk 7f6cf05a9300>)
>     In ice-9/boot-9.scm:
>         724:2  4 (call-with-prompt ("prompt") #<procedure 7f6cf05b6280 at 
> ice-9/eval.scm:330:…> …)
>     In ice-9/eval.scm:
>         619:8  3 (_ #(#(#<directory (guile-user) 7f6cf05acc80>)))
>     In ice-9/boot-9.scm:
>        2836:4  2 (save-module-excursion #<procedure 7f6cf059d300 at 
> ice-9/boot-9.scm:4393:3 ()>)
>       4388:12  1 (_)
>     In srfi/srfi-64/testing.scm:
>        375:14  0 (test-on-bad-end-name-simple _ _ _)
>
>     srfi/srfi-64/testing.scm:375:14: In procedure test-on-bad-end-name-simple:
>     test-end x does not match test-begin y

The spec is very sparse on what the simple test runner does, so I'm not sure if 
the intention is to imply that it does nothing other than what's stated.

In one case, the reference implementation clearly violates the specification: 
The simple test runner uses the `aux` field which the spec claims it doesn't 
use. (My implementation fixes this.) However, in this case it's not that 
clear-cut.

In this case, I think raising an error is good default behavior, since the 
mismatched end name indicates a problem with the test suite itself rather than 
the code being tested. If it poses a problem to the user, one can override that 
callback with the `test-runner-on-bad-end-name!` setter.

What do you think?

- Taylan