[cfe-dev] add a valgrind option to "make test"

[Gabor Greif]

Sam Bishop wrote:

> I've tweaked this a bit to better handle tests that fail both with
> incorrect data and valgrind errors.
>
> Sam

This is great, Sam! The linux folks will love it.

In the last days I have tweaked a lot on "make test", but now it should
be stable enough for you to get this in. My apologies not coming back to you
earlier.

Please find my comments below:


> Index: test/TestRunner.sh
> ===================================================================
> --- test/TestRunner.sh	(revision 48417)
> +++ test/TestRunner.sh	(working copy)
> @@ -39,18 +39,38 @@
>     exit 1
>  )
>
> +# Run clang under Valgrind, if asked to by the user.
> +if [ -n "$VG" ]; then
> +  CLANG="valgrind --leak-check=full -q --log-file-exactly=$OUTPUT.vg clang"
> +else
> +  CLANG="clang"
> +fi
> +

I guess this is ok.


>  SCRIPT=$OUTPUT.script
> -grep 'RUN:' $FILENAME | sed "s|^.*RUN:\(.*\)$|\1|g;s|%s|$SUBST|g;s|%llvmgcc|llvm-gcc -emit-llvm|g;s|%llvmgxx|llvm-g++ -emit-llvm|g;s|%prcontext|prcontext.tcl|g" > $SCRIPT
> +grep 'RUN:' $FILENAME | \
> +  sed -e "s|^.*RUN:\(.*\)$|\1|g" \
> +      -e "s|%s|$SUBST|g" \
> +      -e "s|%llvmgcc|llvm-gcc -emit-llvm|g" \
> +      -e "s|%llvmgxx|llvm-g++ -emit-llvm|g" \
> +      -e "s|%prcontext|prcontext.tcl|g" \
> +      -e "s|clang|$CLANG|g" > $SCRIPT

nice cleanup. meanwhile I have added a new %t substitution, please add that too.

>
>  grep -q XFAIL $FILENAME && (printf "XFAILED '$TESTNAME': "; grep XFAIL $FILENAME)
>
> -/bin/sh $SCRIPT > $OUTPUT 2>&1 || (
> +/bin/sh $SCRIPT > $OUTPUT 2>&1
> +STATUS=$?
> +if [ $STATUS -ne 0 -o -n "$VG" -a -s $OUTPUT.vg ]; then
>    echo "******************** TEST '$TESTNAME' FAILED! ********************"
> -  echo "Command: "
> +  echo "Command:"
>    cat $SCRIPT
> -  echo "Output:"
> -  cat $OUTPUT
> -  rm $OUTPUT
> +  if [ $STATUS -ne 0 ]; then
> +    echo "Incorrect Output:"
> +    cat $OUTPUT
> +  fi
> +  if [ -n "$VG" -a -s $OUTPUT.vg ]; then
> +    echo "Valgrind Errors:"
> +    cat $OUTPUT.vg
> +  fi
>    echo "******************** TEST '$TESTNAME' FAILED! ********************"
> -)
> +fi

OK. I guess seeing "Incorrect Output:" instead of "Output:" will disturb nobody.

>
> Index: test/Makefile
> ===================================================================
> --- test/Makefile	(revision 48417)
> +++ test/Makefile	(working copy)
> @@ -9,6 +9,6 @@
>  endif
>
>  all::
> -	PATH=$$PATH:$(ToolDir):$(LLVM_SRC_ROOT)/test/Scripts \
> +	PATH=$$PATH:$(ToolDir):$(LLVM_SRC_ROOT)/test/Scripts VG=${VG} \
>            find $(TESTDIRS) \( -name '*.c' -or -name '*.cpp' -or -name '*.m' \) \
>  	      -print -exec ./TestRunner.sh {} \;

Why do you use wiggly brackets in ${VG}. The file uses parentheses, let's stick to them.

Btw. test/Makefile is obsolete and will go away soon. Please apply the
change to test/Makefile.parallel too.


Can you resubmit your updated patch? I would gladly check this in.

Cheers,

	Gabor