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