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

["Marc-André Lureau" <marcandre.lureau@redhat.com>]

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 :|
>