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
