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

Re: [libvirt] [PATCH 3/3] test-lib.sh: Update helper for VIR_TEST_EXPENSIVE and use it in virsh-all



On 08/01/2013 07:24 AM, Peter Krempa wrote:
> When the test-lib for shell tests was introduced it did think of
> expensive tests although this option was never used.

More historically accurate: the shell script was lifted verbatim from
GNU coreutils, back in the days when Jim Meyering was actively
contributing here, where coreutils DOES have expensive tests.  We have
just never marked a test expensive in libvirt until now.  But our test
driver shell script diverged from coreutils long enough ago that I'm
fine tweaking the script for our own needs instead of trying to resync
from coreutils, so making further tweaks isn't going to make it harder
to converge.

> Update the code for
> the new env variable name.
> 
> Use this function in the virsh-all test that  blindly runs all virsh
> commands without any arguments and thus it's rather time consuming than

s/rather/more/

> useful. Mark it as expensive to skip this test in normal cases.
> ---
>  tests/test-lib.sh | 10 +++++-----
>  tests/virsh-all   |  2 ++
>  2 files changed, 7 insertions(+), 5 deletions(-)
> 
> diff --git a/tests/test-lib.sh b/tests/test-lib.sh
> index 918bf73..6c5049a 100644
> --- a/tests/test-lib.sh
> +++ b/tests/test-lib.sh
> @@ -158,15 +158,15 @@ require_selinux_()
>    esac
>  }
> 
> -very_expensive_()
> +test_expensive()
>  {
> -  if test "$RUN_VERY_EXPENSIVE_TESTS" != yes; then
> +  if test -z "$VIR_TEST_EXPENSIVE"; then

This test isn't quite right compared to the semantics I gave
VIR_TEST_EXPENSIVE.  I think I'll just fold the correct semantics into
my 2/2 patch on that series.

>      skip_test_ '
>  This test is very expensive, so it is disabled by default.
> -To run it anyway, rerun make check with the RUN_VERY_EXPENSIVE_TESTS
> -environment variable set to yes.  E.g.,
> +To run it anyway, rerun make check with the VIR_TEST_EXPENSIVE
> +environment variable se.  E.g.,
> 
> -  env RUN_VERY_EXPENSIVE_TESTS=yes make check
> +  env RUN_VERY_EXPENSIVE_TESTS=1 make check

Wrong variable name :) But again, I'll cover this part.

>  '
>    fi
>  }
> diff --git a/tests/virsh-all b/tests/virsh-all
> index d4e2633..4e456c6 100755
> --- a/tests/virsh-all
> +++ b/tests/virsh-all
> @@ -21,6 +21,8 @@ test -z "$srcdir" && srcdir=$(pwd)
> 
>  . "$srcdir/test-lib.sh"
> 
> +test_expensive
> +

That leaves just this part of your patch, once you rebase on top of my
fixes.  I agree with making this test marked expensive, and with doing
it as a separate patch on top of the framework, so ACK to this portion
once rebased on top of my patches.

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