[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 2/3] qemuagenttest: Test timeout of agent commands



On 08/01/2013 07:24 AM, Peter Krempa wrote:
> If VIR_TEST_EXPENSIVE is enabled, test timeout of agent commands. This
> test takes 6 seconds to finish.
> ---
>  tests/qemuagenttest.c | 60 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 60 insertions(+)

In my v2 patch for setting up VIR_TEST_EXPENSIVE, I guaranteed that:

make VIR_TEST_EXPENSIVE=0     => getenv("VIR_TEST_EXPENSIVE") sees "0"
make VIR_TEST_EXPENSIVE=1     => getenv("VIR_TEST_EXPENSIVE") sees "1"
make                          => getenv("VIR_TEST_EXPENSIVE") sees
either "0" or "1", based on configure options

as well as a fourth possibility:

cd tests; ./qemuagenttest     => getenv("VIR_TEST_EXPENSIVE") sees
whatever is in your environment (usually NULL)

> +
> +static int
> +testQemuAgentTimeout(const void *data)
> +{
> +    virDomainXMLOptionPtr xmlopt = (virDomainXMLOptionPtr)data;
> +    qemuMonitorTestPtr test = qemuMonitorTestNewAgent(xmlopt);
> +    char *reply = NULL;
> +    int ret = -1;
> +
> +    if (!test)
> +        return -1;
> +
> +    if (!getenv("VIR_TEST_EXPENSIVE"))
> +        return EXIT_AM_SKIP;

Hence, this is prone to false negatives (when expensive tests are
disabled, getenv still returns a value when invoked through make). I'd
better add a patch 2/2 on my proposal that tweaks testutils.h to provide
a helper function (so we aren't repeating the logic of HOW to tell if we
are in expensive mode), and this patch should be rebased on top of that
to just call the helper function.

Everything else looked okay, so ACK once we get the expensive skipping
mechanics sorted.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]