diff mbox series

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

Message ID 20230206141051.4088777-1-alex.bennee@linaro.org (mailing list archive)
State New, archived
Headers show
Series [RFC] tests: be a bit more strict cleaning up fifos | expand

Commit Message

Alex Bennée Feb. 6, 2023, 2:10 p.m. UTC
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(-)

Comments

Philippe Mathieu-Daudé Feb. 6, 2023, 2:54 p.m. UTC | #1
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?
Alex Bennée Feb. 6, 2023, 3:15 p.m. UTC | #2
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=
diff mbox series

Patch

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)