Re: [PATCH v3 5/5] tests/unit: make test-io-channel-command work on win32

[Daniel P . Berrangé]

On Thu, Oct 06, 2022 at 12:12:22PM +0400, marcandre.lureau@redhat.com wrote:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
> 
> This has been tested under msys2 & windows 11. I haven't tried to make
> it work with other environments yet, but that should be enough to
> validate the channel-command implementation anyway.
> 
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  tests/unit/test-io-channel-command.c | 32 ++++++++++++----------------
>  tests/unit/meson.build               |  2 +-
>  2 files changed, 15 insertions(+), 19 deletions(-)
> 
> diff --git a/tests/unit/test-io-channel-command.c 
> b/tests/unit/test-io-channel-command.c
> index aa09c559cd..be98c3452a 100644
> --- a/tests/unit/test-io-channel-command.c
> +++ b/tests/unit/test-io-channel-command.c
> @@ -24,29 +24,27 @@
>  #include "qapi/error.h"
>  #include "qemu/module.h"
>  
> -#ifndef WIN32
> +#define TEST_PATH "test-io-channel-command.fifo"
> +
> +#define SOCAT_SRC "PIPE:" TEST_PATH ",wronly"
> +#define SOCAT_DST "PIPE:" TEST_PATH ",rdonly"
> +
>  static void test_io_channel_command_fifo(bool async)
>  {
> -#define TEST_FIFO "tests/test-io-channel-command.fifo"
>      QIOChannel *src, *dst;
>      QIOChannelTest *test;
> -    const char *srcfifo = "PIPE:" TEST_FIFO ",wronly";
> -    const char *dstfifo = "PIPE:" TEST_FIFO ",rdonly";
>      const char *srcargv[] = {
> -        "/bin/socat", "-", srcfifo, NULL,
> +        g_getenv("SOCAT"), "-", SOCAT_SRC, NULL,

Please don't rely on env variables, as it complicates the ability to
invoke the test directly, without the meson harness. Either pass the
path from meson at compile time in config-host.h, or make this code
use an unqualified path, so it honours $PATH at runtime.

>      };
>      const char *dstargv[] = {
> -        "/bin/socat", dstfifo, "-", NULL,
> +        g_getenv("SOCAT"), SOCAT_DST, "-", NULL,
>      };
>  
> -    unlink(TEST_FIFO);
> -    if (access("/bin/socat", X_OK) < 0) {
> +    unlink(TEST_PATH);
> +    if (!g_file_test(g_getenv("SOCAT"), G_FILE_TEST_IS_EXECUTABLE)) {
>          g_test_skip("socat is missing");
>          return;
>      }
> -    if (mkfifo(TEST_FIFO, 0600) < 0) {
> -        abort();
> -    }
>      src = QIO_CHANNEL(qio_channel_command_new_spawn(srcargv,
>                                                      O_WRONLY,
>                                                      &error_abort));
> @@ -61,7 +59,7 @@ static void test_io_channel_command_fifo(bool async)
>      object_unref(OBJECT(src));
>      object_unref(OBJECT(dst));
>  
> -    unlink(TEST_FIFO);
> +    unlink(TEST_PATH);
>  }
>  
>  
> @@ -81,11 +79,12 @@ static void test_io_channel_command_echo(bool async)
>      QIOChannel *ioc;
>      QIOChannelTest *test;
>      const char *socatargv[] = {
> -        "/bin/socat", "-", "-", NULL,
> +        g_getenv("SOCAT"), "-", "-", NULL,
>      };
>  
> -    if (access("/bin/socat", X_OK) < 0) {
> -        return; /* Pretend success if socat is not present */
> +    if (!g_file_test(g_getenv("SOCAT"), G_FILE_TEST_IS_EXECUTABLE)) {
> +        g_test_skip("socat is missing");
> +        return;
>      }
>  
>      ioc = QIO_CHANNEL(qio_channel_command_new_spawn(socatargv,
> @@ -108,7 +107,6 @@ static void test_io_channel_command_echo_sync(void)
>  {
>      test_io_channel_command_echo(false);
>  }
> -#endif
>  
>  int main(int argc, char **argv)
>  {
> @@ -116,7 +114,6 @@ int main(int argc, char **argv)
>  
>      g_test_init(&argc, &argv, NULL);
>  
> -#ifndef WIN32
>      g_test_add_func("/io/channel/command/fifo/sync",
>                      test_io_channel_command_fifo_sync);
>      g_test_add_func("/io/channel/command/fifo/async",
> @@ -125,7 +122,6 @@ int main(int argc, char **argv)
>                      test_io_channel_command_echo_sync);
>      g_test_add_func("/io/channel/command/echo/async",
>                      test_io_channel_command_echo_async);
> -#endif
>  
>      return g_test_run();
>  }
> diff --git a/tests/unit/meson.build b/tests/unit/meson.build
> index b497a41378..42e8218ac2 100644
> --- a/tests/unit/meson.build
> +++ b/tests/unit/meson.build
> @@ -1,4 +1,3 @@
> -
>  testblock = declare_dependency(dependencies: [block], sources: 'iothread.c')
>  
>  tests = {

Spurious line deletion.


> @@ -164,6 +163,7 @@ endif
>  test_env = environment()
>  test_env.set('G_TEST_SRCDIR', meson.current_source_dir())
>  test_env.set('G_TEST_BUILDDIR', meson.current_build_dir())
> +test_env.set('SOCAT', find_program('socat').full_path())
>  
>  slow_tests = {
>    'test-crypto-tlscredsx509': 45,
> -- 
> 2.37.3
> 

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|