Re: PATCH: Use macros to remove duplicate test code

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

 



"Daniel P. Berrange" <berrange@xxxxxxxxxx> wrote:
...
> Index: test_conf.sh
> ===================================================================
> RCS file: /data/cvs/libvirt/tests/test_conf.sh,v
> retrieving revision 1.4
> diff -u -p -r1.4 test_conf.sh
> --- test_conf.sh	5 Feb 2008 19:27:37 -0000	1.4
> +++ test_conf.sh	18 Apr 2008 00:29:55 -0000
> @@ -1,8 +1,15 @@
>  #!/bin/bash
> -set -x
> +#set -x
> +set -e
> +if [ -z "$abs_srcdir" ]; then
> +  abs_srcdir=`pwd`
> +fi
> +
>  NOK=0
> -for f in $abs_top_srcdir/tests/confdata/*.conf
> +i=1
> +for f in $abs_srcdir/confdata/*.conf
>  do
> +    n=`echo $f | sed -e "s,$abs_srcdir/confdata/,,"`
>      ./conftest $f > conftest.$$
>      outfile=`echo "$f" | sed s+\.conf$+\.out+`
>      diff $outfile conftest.$$ > /dev/null
> @@ -11,11 +18,12 @@ do
>          if [ -n "$DEBUG_TESTS" ]; then
>              diff -u $outfile conftest.$$
>          fi
> -        echo "$f					FAILED"
> +        printf "%2d) %-60s      ... %s\n" $i $n "FAILED"
>          NOK=1
>      else
> -        echo "$f					OK"
> +        printf "%2d) %-60s      ... %s\n" $i $n "OK"
>      fi
> +    i=`expr $i + 1`
>  done
>  rm -f conftest.$$
>  exit $NOK

Actually, I went to factor out the duplicate printf uses, then
noticed some underquoting problems (i.e., when $abs_srcdir expands
to something pathological, including e.g., semicolons or other shell
meta-characters), so started to fix those, and then went ahead and
converted to using test-lib.sh, so that this test now creates temporaries
in its own private directory (better for running tests in parallel).
As such, it doesn't have to use temp file names including ".$$" and
no longer has to remove them explicitly.  Also, there's no need to
rely on /bin/bash in this particular case.

Here's the new version.
It also required the following small changes to TEST_ENVIRONMENT
in Makefile.am.
(however, if an older automake we care about doesn't have abs_srcdir,
the RHS will have to be `cd '$(srcdir)'; pwd` instead)

Dan, you're welcome to fold these changes into your patch.
Or just commit what you have and I'll do this separately.

----------------------------------
#!/bin/sh

if test "$VERBOSE" = yes; then
  set -x
  virsh --version
fi

. $srcdir/test-lib.sh

set -e
if test "x$abs_srcdir" = x; then
  abs_srcdir=`pwd`
  abs_builddir=`pwd`
fi

fail=0
i=1
data_dir=$abs_srcdir/confdata
for f in $(cd "$data_dir" && echo *.conf)
do
    "$abs_builddir/conftest" "$data_dir/$f" > "$f-actual"
    expected="$data_dir"/`echo "$f" | sed s+\.conf$+\.out+`
    if compare "$expected" "$f-actual"; then
        msg=OK
    else
        msg=FAILED
        fail=1
    fi
    printf "%2d) %-60s      ... %s\n" $i "$f" $msg
    i=`expr $i + 1`
done

(exit $fail); exit $fail
----------------------------------


diff --git a/tests/Makefile.am b/tests/Makefile.am
index ca12b84..95ca4e5 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -64,6 +64,8 @@ path_add = $$abs_top_builddir/src$(PATH_SEPARATOR)$$abs_top_builddir/qemud
 # abs_top_{src/build}dir variables, so don't rely
 # on them here. Fake them with 'pwd'
 TESTS_ENVIRONMENT =				\
+  abs_srcdir='$(abs_srcdir)'			\
+  abs_builddir=`cd '$(builddir)'; pwd`		\
   abs_top_builddir=`cd '$(top_builddir)'; pwd`	\
   abs_top_srcdir=`cd '$(top_srcdir)'; pwd`	\
   PATH="$(path_add)$(PATH_SEPARATOR)$$PATH"	\

--
Libvir-list mailing list
Libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]