Re: [PATCH 6/6] Add a simple test script, embed expected results into test files

[Josh Triplett <josh@freedesktop.org>]

[-- Attachment #1: Type: text/plain, Size: 3715 bytes --]

Awesome!  I plan to merge this tomorrow.

I did notice a few issues.  If you get to them before tomorrow then please
submit an updated patch, but otherwise I will go ahead and fix them myself and
apply the patch; I really want to see this go into Sparse as soon as possible.

* The files starting with "preprocessor" shouldn't run through all of Sparse,
  just the preprocessor (sparse -E).  I've moved them to a
  validation/preprocessor/ subdirectory to keep them separated.

* Ideally, the test suite should support a SPARSE_FLAGS comment, so you can
  specify different -Wfoo options and the corresponding output.  This would
  also handle the preprocessor tests.

Pavel Roskin wrote:
> diff --git a/validation/preprocessor1.c b/validation/preprocessor1.c
> index 5ae20aa..5e9f234 100644
> --- a/validation/preprocessor1.c
> +++ b/validation/preprocessor1.c
> @@ -12,3 +12,8 @@
>  #define bar func(
>  #define foo bar foo
>  foo )
> +
> +/* SPARSE
> +builtin:0:0: error: Expected ; at end of declaration
> +builtin:0:0: error: got end-of-input
> +   SPARSE */

Errors in builtin seem strange and probably broken.

> diff --git a/validation/preprocessor10.c b/validation/preprocessor10.c
> index 7fcac36..137bafd 100644
> --- a/validation/preprocessor10.c
> +++ b/validation/preprocessor10.c
> @@ -11,3 +11,8 @@ defined
>  #else
>  undefined
>  #endif
> +
> +/* SPARSE
> +builtin:0:0: error: Expected ; at end of declaration
> +builtin:0:0: error: got end-of-input
> +   SPARSE */

Ditto.

> diff --git a/validation/preprocessor4.c b/validation/preprocessor4.c
> index 8b8c4da..1620a8b 100644
> --- a/validation/preprocessor4.c
> +++ b/validation/preprocessor4.c
> @@ -8,3 +8,8 @@
>  
>  mac(foo)
>  
> +
> +/* SPARSE
> +builtin:0:0: error: Expected ; at end of declaration
> +builtin:0:0: error: got end-of-input
> +   SPARSE */

Ditto

> diff --git a/validation/run-tests b/validation/run-tests
> new file mode 100755
> index 0000000..ebe8ffc
> --- /dev/null
> +++ b/validation/run-tests
> @@ -0,0 +1,58 @@
> +#!/bin/sh
> +
> +: ${SPARSE=../sparse}
> +: ${SPARSE_FLAGS=}
> +
> +# Use "--update" to update test files with the actual output
> +update=
> +if test "$1" = "--update"; then
> +	update=1
> +	shift
> +fi
> +
> +if test -n "$1"; then
> +	tests="$@"
> +else
> +	tests=`echo *.c`
> +fi
> +
> +bad=
> +for test in $tests; do
> +	base="`basename $test .c`"
> +	exp="$base.exp"
> +	res="$base.res"
> +	diff="$base.diff"
> +	$SPARSE $SPARSE_FLAGS "$test" 2>"$res"
> +	if test "$?" != 0; then
> +		echo "FATAL: $test"
> +		bad="$bad $test"
> +		continue
> +	fi
> +
> +	if test -n "$update"; then
> +		sed -i '/SPARSE/,/SPARSE/d' "$test"
> +		if test -s "$res"; then
> +			echo "/* SPARSE" >>"$test"
> +			cat "$res" >>"$test"
> +			echo "   SPARSE */" >>"$test"
> +		fi
> +	fi

Rather than treating the absence of a SPARSE comment as meaning that the test
has no output, I think I'd rather explicitly include an empty SPARSE comment
in such tests, and then make the absence of a SPARSE comment an error.

> +	sed -n '/SPARSE/,/SPARSE/p' "$test" |grep -v SPARSE >"$exp"
> +	diff -u "$exp" "$res" >"$diff"
> +	if test -s "$diff"; then
> +		echo "FAIL: $test"
> +		bad="$bad $test"
> +		continue
> +	fi
> +
> +	rm -f "$exp" "$res" "$diff"
> +done
> +
> +if test -n "$bad"; then
> +	echo "Failed tests:$bad"
> +	exit 1
> +fi

How about a --verbose that prints the diffs?  I'd prefer that with automated
build systems, for example, so that I can immediately see the test suite
failure in the logs.  Also, --verbose --update should print the diffs it
applies.

- Josh Triplett


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 252 bytes --]