bug#72367: srfi-64: nested group not counted as one test

bug#72367: srfi-64: nested group not counted as one test

[Tomas Volf]

Hello,

I think I found a bug in (srfi srfi-64) module shipped with GNU Guile.

The specification says the following about the count argument to the test-begin
form:

> The optional count must match the number of test-cases executed by this
> group. (Nested test groups count as a single test case for this count.)

Thus I believe that following should *not* call on-bad-count-function.

    (use-modules (srfi srfi-64))
    (let ((r (test-runner-null)))
      (test-runner-current r)
      (test-runner-on-bad-count! r (λ (runner actual-count expected-count)
                                     (pk (test-runner-group-stack runner))
                                     (pk actual-count expected-count)))
      (test-begin "x" 1)
      (test-begin "y" 3)
      (test-assert #t)
      (test-assert #t)
      (test-assert #t)
      (test-end)
      (test-end))

However as we can see it does:

    ;;; (("x"))

    ;;; (3 1)

Have a nice day
Tomas Volf

bug#72367: srfi-64: nested group not counted as one test

[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 about the count argument to the test-begin
> form:
>
>> The optional count must match the number of test-cases executed by this
>> group. (Nested test groups count as a single test case for this count.)
> Thus I believe that following should *not* call on-bad-count-function.
>
>     (use-modules (srfi srfi-64))
>     (let ((r (test-runner-null)))
>       (test-runner-current r)
>       (test-runner-on-bad-count! r (λ (runner actual-count expected-count)
>                                      (pk (test-runner-group-stack runner))
>                                      (pk actual-count expected-count)))
>       (test-begin "x" 1)
>       (test-begin "y" 3)
>       (test-assert #t)
>       (test-assert #t)
>       (test-assert #t)
>       (test-end)
>       (test-end))
>
> However as we can see it does:
>
>     ;;; (("x"))
>
>     ;;; (3 1)
>
> Have a nice day
> Tomas Volf

Looks like a bug indeed. I believe I was able to fix it in my implementation with this commit:

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

An equivalent fix could be applied to the upstream reference implementation / current Guile implementation.

- Taylan

bug#72367: srfi-64: nested group not counted as one test

[Taylan Kammer]

On 30.09.2024 16:42, Taylan Kammer wrote:
> 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 about the count argument to the test-begin
>> form:
>>
>>> The optional count must match the number of test-cases executed by this
>>> group. (Nested test groups count as a single test case for this count.)
>> Thus I believe that following should *not* call on-bad-count-function.
>>
>>     (use-modules (srfi srfi-64))
>>     (let ((r (test-runner-null)))
>>       (test-runner-current r)
>>       (test-runner-on-bad-count! r (λ (runner actual-count expected-count)
>>                                      (pk (test-runner-group-stack runner))
>>                                      (pk actual-count expected-count)))
>>       (test-begin "x" 1)
>>       (test-begin "y" 3)
>>       (test-assert #t)
>>       (test-assert #t)
>>       (test-assert #t)
>>       (test-end)
>>       (test-end))
>>
>> However as we can see it does:
>>
>>     ;;; (("x"))
>>
>>     ;;; (3 1)
>>
>> Have a nice day
>> Tomas Volf
> Looks like a bug indeed. I believe I was able to fix it in my implementation with this commit:
>
>     https://codeberg.org/taylan/scheme-srfis/commit/c49e367fbf292c6ee66ff435c7daa1f4ae5a47fa
>
> An equivalent fix could be applied to the upstream reference implementation / current Guile implementation.
>
I've also discovered another pair of bugs related to nested test groups, but only when using `test-group`.

1. Skipping a `test-group` by name was not working. The "meta test suite" of SRFI-64 had an associated test case marked as an expected failure with the comment "???". I've found why it wasn't working, and fixed it.

2. Had the skipping actually worked, the code would have failed to increment the skip-count and total-count. I've made sure these are incremented appropriately when a `test-group` is skipped.

One quirk remains, but is difficult to fix without substantial changes:

A skipped `test-group` will not invoke any handlers whatsoever, so the runner can't detect this case to write something into the log. Normally, a skipped test would cause something to be logged in my implementation, but a skipped `test-group` will be skipped silently and only appear as a statistic in the summary of the counts at the end.

Fixing this would require "privileging" my `test-runner-simple` implementation by giving it access to undocumented APIs, or explicitly expanding the SRFI-64 API.

Another option would be to slightly change the semantics of `test-group`, such that the on-group-begin and on-group-end handlers are always invoked, and only the body of the group skipped if the group is supposed to be skipped. This is how skipping of individual tests is handled: The on-test-begin and on-test-end handlers *are* called for skipped tests. So, this change would make the API more coherent. It would probably also simplify the implementation. The downside is that it would be a clear contradiction of the spec.

It seems like skipping of test groups was kind of an afterthought and wasn't integrated cleanly with the rest of SRFI-64. Opinions welcome on whether I should simply make my implementation break off from the spec here. I imagine there isn't much code out there that strictly requires group-begin/group-end to *not* be called in case of a `test-group` that's meant to be skipped.

- Taylan