bug#72371: srfi-64: test marked for skip and as expected failure has wrong result-kind in on-test-begin-function

bug#72371: srfi-64: test marked for skip and as expected failure has wrong result-kind in on-test-begin-function

[Tomas Volf]

Hello,

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

The specification says the following regarding the test-result-kind:

> If we've started on a new test, but don't have a result yet, then the result
> kind is 'xfail if the test is expected to fail, 'skip if the test is supposed
> to be skipped, or #f otherwise.

Thus I believe that following should print `xfail':

    (use-modules (srfi srfi-64))
    (test-begin "x")

    (test-runner-on-test-begin! (test-runner-current)
      (λ (runner)
        (pk (test-result-kind))))

    (test-skip 1)
    (test-expect-fail 1)
    (test-assert #t)

    (test-end)

However it does not:

    ;;; (skip)

Have a nice day
Tomas Volf

bug#72371: srfi-64: test marked for skip and as expected failure has wrong result-kind in on-test-begin-function

[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-result-kind:
>
>> If we've started on a new test, but don't have a result yet, then the result
>> kind is 'xfail if the test is expected to fail, 'skip if the test is supposed
>> to be skipped, or #f otherwise.
> Thus I believe that following should print `xfail':
>
>     (use-modules (srfi srfi-64))
>     (test-begin "x")
>
>     (test-runner-on-test-begin! (test-runner-current)
>       (λ (runner)
>         (pk (test-result-kind))))
>
>     (test-skip 1)
>     (test-expect-fail 1)
>     (test-assert #t)
>
>     (test-end)
>
> However it does not:
>
>     ;;; (skip)
>
> Have a nice day
> Tomas Volf
>
I think this is a case where the spec didn't actually consider what should happen if skip and expect-fail are combined. Otherwise, I would expect to see a more explicit description of what should happen in such cases.

In other words, I think the English description of what's supposed to happen, that you've quoted, is *not* intended to be read like procedural pseudo-code: "If expected to fail, return 'xfail; if supposed to be skipped, return 'skip." The reference implementation does it the exact other way around, in a rather straightforward manner (two consecutive clasuses of a cond expression), so I don't think it's a bug.

Intuitively, I also think it makes the most sense to treat skipping as a higher priority. While an xfail test is still executed, a skipped test is not executed at all, which is a more significant change in the test suite's behavior and should be honored IMO. If I've marked a test to be skipped, it could be because executing it currently leads to a crash or an infinite loop, so it would be important to skip it even if it's marked as xfail.

So, I think the observed behavior is probably best, and intended. Opinions welcome.

- Taylan