Re: [libvirt] [PATCH 3/3] When testing qemu, unset the SDL driver environment variables.

On Mon, Dec 28, 2009 at 04:35:56PM +0100, Matthias Bolte wrote: > 2009/12/23 Diego Elio Pettenò <flameeyes(a)gmail.com&gt;: > > With these variables present, the tests would fail because they are not > > expected. > > --- > >  tests/qemuxml2argvtest.c |    2 ++ > >  1 files changed, 2 insertions(+), 0 deletions(-) > > > > diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c > > index 3b0aa2b..b64dc6a 100644 > > --- a/tests/qemuxml2argvtest.c > > +++ b/tests/qemuxml2argvtest.c > > @@ -188,6 +188,8 @@ mymain(int argc, char **argv) > >     unsetenv("TMPDIR"); > >     unsetenv("LD_PRELOAD"); > >     unsetenv("LD_LIBRARY_PATH"); > > +    unsetenv("SDL_AUDIODRIVER"); > > +    unsetenv("SDL_VIDEODRIVER"); > > > >     DO_TEST("minimal", QEMUD_CMD_FLAG_NAME); > >     DO_TEST("machine-aliases1", 0); > > -- > > 1.6.6.rc4 > > > > You're right, all envvars copied in qemudBuildCommandLine should be > unset or set to known values in this testcase to make sure the test is > not affected by the actual envvar values. > > But SDL_VIDEODRIVER is not copied in qemudBuildCommandLine and > QEMU_AUDIO_DRV is copied but not unset in the test. > > I suggest the attached patch. > > Matthias > From 891e786c4e0863668b3b5b2e1be4f1acfd2b0f97 Mon Sep 17 00:00:00 2001 > From: Matthias Bolte <matthias.bolte(a)googlemail.com&gt; > Date: Mon, 28 Dec 2009 16:21:15 +0100 > Subject: [PATCH] Unset copied environment variables in qemuxml2argvtest > > The test expected all environment variables copied in qemudBuildCommandLine > to have known values. So all of them have to be either set to a known value > or be unset. SDL_VIDEODRIVER and QEMU_AUDIO_DRV are not handled at all but > should be handled. Unset both, otherwise the test will fail if they are set > in the testing environment. > > * src/qemu/qemu_conf.c: add a comment about copied environment variables >   and qemuxml2argvtest > * tests/qemuxml2argvtest.c: unset SDL_VIDEODRIVER and QEMU_AUDIO_DRV > --- >  src/qemu/qemu_conf.c     |    2 ++ >  tests/qemuxml2argvtest.c |    5 +++++ >  2 files changed, 7 insertions(+), 0 deletions(-) > > diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c > index f53c1f7..1f22826 100644 > --- a/src/qemu/qemu_conf.c > +++ b/src/qemu/qemu_conf.c > @@ -2034,6 +2034,8 @@ int qemudBuildCommandLine(virConnectPtr conn, >          qenv[qenvc++] = envval;                                         \ >      } while (0) > > +    /* Make sure to unset or set all envvars in qemuxml2argvtest.c that > +     * are copied here using this macro, otherwise the test may fail */ >  #define ADD_ENV_COPY(envname)                                           \ >      do {                                                                \ >          char *val = getenv(envname);                                    \ > diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c > index 3b0aa2b..4e97294 100644 > --- a/tests/qemuxml2argvtest.c > +++ b/tests/qemuxml2argvtest.c > @@ -181,6 +181,9 @@ mymain(int argc, char **argv) >  #define DO_TEST(name, extraFlags)                       \ >          DO_TEST_FULL(name, extraFlags, NULL) > > +    /* Unset or set all envvars here that are copied in qemudBuildCommandLine > +     * using ADD_ENV_COPY, otherwise these tests may fail due to unexpected > +     * values for these envvars */ >      setenv("PATH", "/bin", 1); >      setenv("USER", "test", 1); >      setenv("LOGNAME", "test", 1); > @@ -188,6 +191,8 @@ mymain(int argc, char **argv) >      unsetenv("TMPDIR"); >      unsetenv("LD_PRELOAD"); >      unsetenv("LD_LIBRARY_PATH"); > +    unsetenv("QEMU_AUDIO_DRV"); > +    unsetenv("SDL_AUDIODRIVER"); > >      DO_TEST("minimal", QEMUD_CMD_FLAG_NAME); >      DO_TEST("machine-aliases1", 0);  ACK, Daniel