Re: [Qemu-devel] [PATCH] tests: use static qga config file

[Michael Roth <mdroth@linux.vnet.ibm.com>]

Quoting Marc-André Lureau (2016-06-13 07:30:59)
> Hi
> 
> ----- Original Message -----
> > From: Marc-André Lureau <marcandre.lureau@redhat.com>
> > 
> > Do not create a leaking temporary file, but use a static file instead.
> > 
> > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > Reported-by: Peter Maydell <peter.maydell@linaro.org>
> > ---
> >  tests/test-qga-config |  8 ++++++++
> >  tests/test-qga.c      | 27 ++++-----------------------
> >  2 files changed, 12 insertions(+), 23 deletions(-)
> >  create mode 100644 tests/test-qga-config
> > 
> > diff --git a/tests/test-qga-config b/tests/test-qga-config
> > new file mode 100644
> > index 0000000..4bb721a
> > --- /dev/null
> > +++ b/tests/test-qga-config
> > @@ -0,0 +1,8 @@
> > +[general]
> > +daemon=false
> > +method=virtio-serial
> > +path=/path/to/org.qemu.guest_agent.0
> > +pidfile=/var/foo/qemu-ga.pid
> > +statedir=/var/state
> > +verbose=true
> > +blacklist=guest-ping;guest-get-time
> > diff --git a/tests/test-qga.c b/tests/test-qga.c
> > index 251b201..7d206ec 100644
> > --- a/tests/test-qga.c
> > +++ b/tests/test-qga.c
> > @@ -691,28 +691,11 @@ static void test_qga_blacklist(gconstpointer data)
> >  static void test_qga_config(gconstpointer data)
> >  {
> >      GError *error = NULL;
> > -    char *cwd, *cmd, *out, *err, *str, **strv, *conf, **argv = NULL;
> > +    char *cwd, *cmd, *out, *err, *str, **strv, **argv = NULL;
> >      char *env[2];
> > -    int status, tmp;
> > +    int status;
> >      gsize n;
> >      GKeyFile *kf;
> > -    const char *qga_config =
> > -        "[general]\n"
> > -        "daemon=false\n"
> > -        "method=virtio-serial\n"
> > -        "path=/path/to/org.qemu.guest_agent.0\n"
> > -        "pidfile=/var/foo/qemu-ga.pid\n"
> > -        "statedir=/var/state\n"
> > -        "verbose=true\n"
> > -        "blacklist=guest-ping;guest-get-time\n";
> > -
> > -    tmp = g_file_open_tmp(NULL, &conf, &error);
> > -    g_assert_no_error(error);
> > -    g_assert_cmpint(tmp, >=, 0);
> > -    g_assert_cmpstr(conf, !=, "");
> > -
> > -    g_file_set_contents(conf, qga_config, -1, &error);
> > -    g_assert_no_error(error);
> >  
> >      cwd = g_get_current_dir();
> >      cmd = g_strdup_printf("%s%cqemu-ga -D",
> > @@ -720,7 +703,8 @@ static void test_qga_config(gconstpointer data)
> >      g_shell_parse_argv(cmd, NULL, &argv, &error);
> >      g_assert_no_error(error);
> >  
> > -    env[0] = g_strdup_printf("QGA_CONF=%s", conf);
> > +    env[0] = g_strdup_printf("QGA_CONF=%s%ctests%ctest-qga-config",
> > +                             cwd, G_DIR_SEPARATOR, G_DIR_SEPARATOR);
> 
> Actually instead of cwd here, it would be nice to have srcdir or testdir. There is g_test_build_filename() in glib 2.38, not sure what we should do without it. Ideas?

Maybe just via environment variables?

--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -660,6 +662,7 @@ $(patsubst %, check-%, $(check-unit-y)): check-%: %
    $(if $(CONFIG_GCOV),@rm -f *.gcda */*.gcda */*/*.gcda */*/*/*.gcda,)
    $(call quiet-command, \
        MALLOC_PERTURB_=$${MALLOC_PERTURB_:-$$((RANDOM % 255 + 1))} \
+       QEMU_TEST_SRC_PATH=$(SRC_PATH)/tests \
        gtester $(GTESTER_OPTIONS) -m=$(SPEED) $*,"GTESTER $*")
    $(if $(CONFIG_GCOV),@for f in $(gcov-files-$(subst tests/,,$*)-y) $(gcov-files-generic-y); do \
      echo Gcov report for $$f:;\

We seem to do the same with MALLOC_PERTURB_ already (though there's probably
less options there), and it's consistent with how we control
QGA_TEST_SIDE_EFFECTING currently.

Also, if we default to cwd things should still work for running the test
executable directly.

g_test_build_filename seems to do similar under the covers as well,
though it is a bit more tedious to have to add to every test target we
want access environment variables in. Would be nice if
tests/Makefile.include could do it automatically via common
dependencies, but I'm not sure if that's doable.

> 
> >      env[1] = NULL;
> >      g_spawn_sync(NULL, argv, env, 0,
> >                   NULL, NULL, &out, &err, &status, &error);
> > @@ -775,11 +759,8 @@ static void test_qga_config(gconstpointer data)
> >  
> >      g_free(out);
> >      g_free(err);
> > -    g_free(conf);
> >      g_free(env[0]);
> >      g_key_file_free(kf);
> > -
> > -    close(tmp);
> >  }
> >  
> >  static void test_qga_fsfreeze_status(gconstpointer fix)
> > --
> > 2.7.4
> > 
> > 
>