bug#72368: srfi-64: test-begin does not set test-runner-test-name

[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.
>
> The specification says the following regarding the test-runner-test-name:
>
> > Returns the name of the current test or test group, as a string. During
> > execution of test-begin this is the name of the test group [..]
> >       
> Thus I believe that following should print `x':
>
>     (use-modules (srfi srfi-64))
>     (let ((r (test-runner-null)))
>       (test-runner-current r)
>       (test-runner-on-group-begin! r
>         (λ (runner suite-name count)
>           (pk (test-runner-test-name r))))
>       (test-begin "x"))
>
> However it does not:
>
>     ;;; ("")
>
> Have a nice day
> Tomas Volf
>

I think the spec *may* have meant to say "between test-begin and test-end" or "during execution of a test group" rather than "during execution of test-begin." The reason I think so is that the on-group-begin handler is also called before the new name is added to the stack. So, still having the old name at that point would be consistent. But this may also have all been unintentional, because the implementation of test-runner-test-name is kind of hacky (uses the name from the test-result alist).

If we take the spec literally, you are right in that it should set the name before calling the on-group-begin handler. And I think that's more intuitive anyway. So, I agree that this is best seen a bug, and I believe I've now fixed it in my implementation with this commit:

    https://codeberg.org/taylan/scheme-srfis/commit/16ef0d987c99df7d488e6f4b3fef41d972a7552c

Now my code behaves as follows:

scheme@(guile-user)> ,use (srfi srfi-64)
;;; note: source file /home/taylan/src/scheme-srfis/guile-srfi-64/srfi/srfi-64.scm
;;;       newer than compiled /usr/lib/x86_64-linux-gnu/guile/3.0/ccache/srfi/srfi-64.go
;;; found fresh local cache at /home/taylan/.cache/guile/ccache/3.0-LE-8-4.6/home/taylan/src/scheme-srfis/guile-srfi-64/srfi/srfi-64.scm.go
scheme@(guile-user)> (let ((r (test-runner-null)))
  (test-runner-current r)
  (test-runner-on-group-begin! r
    (λ (runner suite-name count)
      (pk 'group-begin (test-runner-test-name r))))
  (test-runner-on-test-begin! r
    (λ (runner)
      (pk 'test-begin (test-runner-test-name r))))
  (test-runner-on-test-end! r
    (λ (runner)
      (pk 'test-end (test-runner-test-name r))))
  (test-runner-on-group-end! r
    (λ (runner)
      (pk 'group-end (test-runner-test-name r))))
  (test-runner-on-final! r
    (λ (runner)
      (pk 'final (test-runner-test-name r))))
  (pk 'before-begin (test-runner-test-name r))
  (test-begin "x")
  (pk 'within-group (test-runner-test-name r))
  (test-assert "x1" #t)
  (pk 'within-group (test-runner-test-name r))
  (test-end "x")
  (pk 'after-end (test-runner-test-name r)))

;;; (before-begin "")

;;; (group-begin "x")

;;; (within-group "x")

;;; (test-begin "x1")

;;; (test-end "x1")

;;; (within-group "x")

;;; (group-end "x")

;;; (final "")

;;; (after-end "")
$1 = ""
scheme@(guile-user)>

Further, to make the point at which the name is changed consistent with the point at which the group stack is changed, I've modified my implementation to call the on-group-begin handler *after* the stack is modified:

    https://codeberg.org/taylan/scheme-srfis/commit/9f247f9b8d3bca2c9f9fe26581cd8f695714a8aa

From what I can tell, this doesn't contradict anything in the spec, though it is a deviation from what the reference implementation does in practice. I think it's better this way, because otherwise the current name will be inconsistent with the topmost name in the group stack during on-group-begin.

The downside is that there could theoretically be code out there that relies on the behavior of the reference implementation, but I see this as being quite unlikely and it would be easy to fix such code. Opinions welcome.

- Taylan