[RFC PATCH] tests: be a bit more strict cleaning up fifos

[RFC PATCH] tests: be a bit more strict cleaning up fifos

[Alex Bennée]

When we re-factored we dropped the unlink() step which turns out to be
required for rmdir to do its thing. If we had been checking the return
value we would have noticed so lets do that with this fix.

Fixes: 68406d1085 (tests/unit: cleanups for test-io-channel-command)
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Suggested-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 tests/unit/test-io-channel-command.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/tests/unit/test-io-channel-command.c b/tests/unit/test-io-channel-command.c
index 425e2f5594..c2179a6462 100644
--- a/tests/unit/test-io-channel-command.c
+++ b/tests/unit/test-io-channel-command.c
@@ -42,6 +42,7 @@ static void test_io_channel_command_fifo(bool async)
     g_auto(GStrv) dstargv = g_strsplit(dstargs, " ", -1);
     QIOChannel *src, *dst;
     QIOChannelTest *test;
+    int err;
 
     if (mkfifo(fifo, 0600)) {
         g_error("mkfifo: %s", strerror(errno));
@@ -61,7 +62,10 @@ static void test_io_channel_command_fifo(bool async)
     object_unref(OBJECT(src));
     object_unref(OBJECT(dst));
 
-    g_rmdir(tmpdir);
+    err = g_unlink(fifo);
+    g_assert(err == 0);
+    err = g_rmdir(tmpdir);
+    g_assert(err == 0);
 }
 
 static void test_io_channel_command_fifo_async(void)
-- 
2.39.1

Re: [RFC PATCH] tests: be a bit more strict cleaning up fifos

[Philippe Mathieu-Daudé]

On 6/2/23 15:10, Alex Bennée wrote:
> When we re-factored we dropped the unlink() step which turns out to be
> required for rmdir to do its thing. If we had been checking the return
> value we would have noticed so lets do that with this fix.
> 
> Fixes: 68406d1085 (tests/unit: cleanups for test-io-channel-command)
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> Suggested-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   tests/unit/test-io-channel-command.c | 6 +++++-
>   1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/tests/unit/test-io-channel-command.c b/tests/unit/test-io-channel-command.c
> index 425e2f5594..c2179a6462 100644
> --- a/tests/unit/test-io-channel-command.c
> +++ b/tests/unit/test-io-channel-command.c
> @@ -42,6 +42,7 @@ static void test_io_channel_command_fifo(bool async)
>       g_auto(GStrv) dstargv = g_strsplit(dstargs, " ", -1);
>       QIOChannel *src, *dst;
>       QIOChannelTest *test;
> +    int err;
>   
>       if (mkfifo(fifo, 0600)) {
>           g_error("mkfifo: %s", strerror(errno));
> @@ -61,7 +62,10 @@ static void test_io_channel_command_fifo(bool async)
>       object_unref(OBJECT(src));
>       object_unref(OBJECT(dst));
>   
> -    g_rmdir(tmpdir);
> +    err = g_unlink(fifo);
> +    g_assert(err == 0);
> +    err = g_rmdir(tmpdir);
> +    g_assert(err == 0);
>   }

Thanks for the patch, but unfortunately this doesn't help:

Unknown TAP version. The first line MUST be `TAP version <int>`. 
Assuming version 12.

71/93 qemu:unit / test-io-channel-command             ERROR 
1.06s   killed by signal 13 SIGPIPE
 >>> G_TEST_BUILDDIR=/Users/philmd/source/qemu/build/tests/unit 
G_TEST_SRCDIR=/Users/philmd/source/qemu/tests/unit MALLOC_PERTURB_=27 
/Users/philmd/source/qemu/build/tests/unit/test-io-channel-command --tap -k
――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――― 
✀ 
――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――
stderr:
2023/02/06 15:46:03 socat[88615] E read(5, 0x13480c000, 8192): Bad file 
descriptor

(test program exited with status code -13)

TAP parsing error: Too few tests run (expected 4, got 0)

$ tests/unit/test-io-channel-command
# random seed: R02Se92d5500c30bbf1797b1047cd480607c
1..4
# Start of io tests
# Start of channel tests
# Start of command tests
# Start of fifo tests
2023/02/06 15:47:31 socat[88651] E read(5, 0x12000c000, 8192): Bad file 
descriptor
$ echo $?
141

Should we add 'socat' as testing dependency in lci-tool?

Re: [RFC PATCH] tests: be a bit more strict cleaning up fifos

[Alex Bennée]

Philippe Mathieu-Daudé <philmd@linaro.org> writes:

> On 6/2/23 15:10, Alex Bennée wrote:
>> When we re-factored we dropped the unlink() step which turns out to be
>> required for rmdir to do its thing. If we had been checking the return
>> value we would have noticed so lets do that with this fix.
>> Fixes: 68406d1085 (tests/unit: cleanups for test-io-channel-command)
>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>> Suggested-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>>   tests/unit/test-io-channel-command.c | 6 +++++-
>>   1 file changed, 5 insertions(+), 1 deletion(-)
>> diff --git a/tests/unit/test-io-channel-command.c
>> b/tests/unit/test-io-channel-command.c
>> index 425e2f5594..c2179a6462 100644
>> --- a/tests/unit/test-io-channel-command.c
>> +++ b/tests/unit/test-io-channel-command.c
>> @@ -42,6 +42,7 @@ static void test_io_channel_command_fifo(bool async)
>>       g_auto(GStrv) dstargv = g_strsplit(dstargs, " ", -1);
>>       QIOChannel *src, *dst;
>>       QIOChannelTest *test;
>> +    int err;
>>         if (mkfifo(fifo, 0600)) {
>>           g_error("mkfifo: %s", strerror(errno));
>> @@ -61,7 +62,10 @@ static void test_io_channel_command_fifo(bool async)
>>       object_unref(OBJECT(src));
>>       object_unref(OBJECT(dst));
>>   -    g_rmdir(tmpdir);
>> +    err = g_unlink(fifo);
>> +    g_assert(err == 0);
>> +    err = g_rmdir(tmpdir);
>> +    g_assert(err == 0);
>>   }
>
> Thanks for the patch, but unfortunately this doesn't help:
>
> Unknown TAP version. The first line MUST be `TAP version <int>`.
> Assuming version 12.
>
> 71/93 qemu:unit / test-io-channel-command             ERROR 1.06s
> killed by signal 13 SIGPIPE
>>>> G_TEST_BUILDDIR=/Users/philmd/source/qemu/build/tests/unit
>     G_TEST_SRCDIR=/Users/philmd/source/qemu/tests/unit
>     MALLOC_PERTURB_=27
>     /Users/philmd/source/qemu/build/tests/unit/test-io-channel-command
>    --tap -k
> ―――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――
> ✀
> ――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――
> stderr:
> 2023/02/06 15:46:03 socat[88615] E read(5, 0x13480c000, 8192): Bad
> file descriptor
>
> (test program exited with status code -13)
>
> TAP parsing error: Too few tests run (expected 4, got 0)

Ahh that is a different error.

>
> $ tests/unit/test-io-channel-command
> # random seed: R02Se92d5500c30bbf1797b1047cd480607c
> 1..4
> # Start of io tests
> # Start of channel tests
> # Start of command tests
> # Start of fifo tests
> 2023/02/06 15:47:31 socat[88651] E read(5, 0x12000c000, 8192): Bad
> file descriptor
> $ echo $?
> 141
>
> Should we add 'socat' as testing dependency in lci-tool?

Yes, and maybe this is triggering on FreeBSD as well?

  https://cdn.artifacts.gitlab-static.net/7e/5d/7e5de39c75978325e6b9b68dd0f992f487a1c862c6dff2cc867723e4c306e820/2023_02_05/3717196650/4070740647/job.log?response-content-type=text%2Fplain%3B%20charset%3Dutf-8&response-content-disposition=inline&Expires=1675696399&KeyName=gprd-artifacts-cdn&Signature=B4AVL9qee1_jd8hmwvEkyVepy38=

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro