1. Patchew
  2. QEMU
  3. [PATCH v3 0/5] io/command: implement portable spawn
  4. View patch

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

marcandre.lureau@redhat.com posted 5 patches 3 years, 4 months ago
Maintainers: "Daniel P. Berrangé" <berrange@redhat.com>
There is a newer version of this series
[PATCH v3 5/5] tests/unit: make test-io-channel-command work on win32
Posted by marcandre.lureau@redhat.com 3 years, 4 months ago 
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,
     };
     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 = {
@@ -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
Re: [PATCH v3 5/5] tests/unit: make test-io-channel-command work on win32
Posted by Daniel P. Berrangé 3 years, 4 months ago 
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 :|
Re: [PATCH v3 5/5] tests/unit: make test-io-channel-command work on win32
Posted by Marc-André Lureau 3 years, 4 months ago 
Hi

On Thu, Oct 6, 2022 at 12:42 PM Daniel P. Berrangé <berrange@redhat.com> wrote:
>
> 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.

I tried to pass it through config-host.h, but I dont see a way to
escape the \ is the paths.

Unqualified path doesn't work under msys2.

>
> >      };
> >      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 :|
>
Re: [PATCH v3 5/5] tests/unit: make test-io-channel-command work on win32
Posted by Daniel P. Berrangé 3 years, 4 months ago 
On Thu, Oct 06, 2022 at 12:46:17PM +0400, Marc-André Lureau wrote:
> Hi
> 
> On Thu, Oct 6, 2022 at 12:42 PM Daniel P. Berrangé <berrange@redhat.com> wrote:
> >
> > 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.
> 
> I tried to pass it through config-host.h, but I dont see a way to
> escape the \ is the paths.

This must be possible, as we have lots of strings in config-host.h
that are paths - eg many CONFIG_QEMU_xxxDIR variables


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 :|
Re: [PATCH v3 5/5] tests/unit: make test-io-channel-command work on win32
Posted by Marc-André Lureau 3 years, 4 months ago 
Hi

On Thu, Oct 6, 2022 at 2:14 PM Daniel P. Berrangé <berrange@redhat.com>
wrote:

> On Thu, Oct 06, 2022 at 12:46:17PM +0400, Marc-André Lureau wrote:
> > Hi
> >
> > On Thu, Oct 6, 2022 at 12:42 PM Daniel P. Berrangé <berrange@redhat.com>
> wrote:
> > >
> > > 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.
> >
> > I tried to pass it through config-host.h, but I dont see a way to
> > escape the \ is the paths.
>
> This must be possible, as we have lots of strings in config-host.h
> that are paths - eg many CONFIG_QEMU_xxxDIR variables
>

Those paths, generated by meson I believe, use /-dir separators. But
find_program() returns \-seperated paths.. Maybe the solution is to replace
\ with / ? (hopefully the path is not quoted already...)

There is a proposal for to_quoted():
https://github.com/mesonbuild/meson/issues/10417

I found another solution, using g_find_program_in_path(). See v3.

thanks

-- 
Marc-André Lureau

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.