Re: [PATCH] tests: Ensure TAP version is printed before other messages

[Thomas Huth <thuth@redhat.com>]

On 01/03/2023 11.52, Richard W.M. Jones wrote:
> On Tue, Feb 28, 2023 at 09:30:56PM +0100, Thomas Huth wrote:
>> On 27/02/2023 18.40, Richard W.M. Jones wrote:
>>> These two tests were failing with this error:
>>>
>>>    stderr:
>>>    TAP parsing error: version number must be on the first line
>>>    [...]
>>>    Unknown TAP version. The first line MUST be `TAP version <int>`. Assuming version 12.
>>>
>>> This can be fixed by ensuring we always call g_test_init first in the
>>> body of main.
>>>
>>> Thanks: Daniel Berrange, for diagnosing the problem
>>> Signed-off-by: Richard W.M. Jones <rjones@redhat.com>
>>> ---
>>>   tests/qtest/fuzz-lsi53c895a-test.c | 4 ++--
>>>   tests/qtest/rtl8139-test.c         | 5 +++--
>>>   2 files changed, 5 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/tests/qtest/fuzz-lsi53c895a-test.c b/tests/qtest/fuzz-lsi53c895a-test.c
>>> index a9254b455d..2012bd54b7 100644
>>> --- a/tests/qtest/fuzz-lsi53c895a-test.c
>>> +++ b/tests/qtest/fuzz-lsi53c895a-test.c
>>> @@ -112,12 +112,12 @@ static void test_lsi_do_dma_empty_queue(void)
>>>   int main(int argc, char **argv)
>>>   {
>>> +    g_test_init(&argc, &argv, NULL);
>>> +
>>>       if (!qtest_has_device("lsi53c895a")) {
>>>           return 0;
>>
>> Could you please double-check that the !lsi53c895a case works fine,
>> too? (just temporarily change it into a "if (1) { ..." statement)
>> ... I'm a little bit afraid that the TAP protocol might be
>> incomplete without the g_test_run() at the end otherwise. If so, you
>> might now need a "goto out" instead of the "return 0" here...
> 
> Applying ...
> 
> diff --git a/tests/qtest/fuzz-lsi53c895a-test.c b/tests/qtest/fuzz-lsi53c895a-test.c
> index 2012bd54b7..e0c902aac4 100644
> --- a/tests/qtest/fuzz-lsi53c895a-test.c
> +++ b/tests/qtest/fuzz-lsi53c895a-test.c
> @@ -114,7 +114,7 @@ int main(int argc, char **argv)
>   {
>       g_test_init(&argc, &argv, NULL);
>   
> -    if (!qtest_has_device("lsi53c895a")) {
> +    if (1) {
>           return 0;
>       }
>   
> ... and rerunning the tests, everything still passes.
> 
> The stdout of the test after this change is:
> 
> TAP version 13
> # random seed: R02S1c1f371a09fbfdf0dd747f898d55fe97
> 
> but apparently this version of TAP doesn't care (perhaps because the
> number of tests "1..2" is never printed?)
> 
> Anyway it doesn't appear to be a problem.

Ok, thanks for checking! I just also checked 
https://testanything.org/tap-version-13-specification.html and it says "The 
plan is optional", so this should be fine.

Reviewed-by: Thomas Huth <thuth@redhat.com>