[PATCH] Add a testsuite, call it on "make check"

[PATCH] Add a testsuite, call it on "make check"

[Pavel Roskin]

From: Pavel Roskin <proski@gnu.org>

Signed-off-by: Pavel Roskin <proski@gnu.org>
---

 Makefile             |    3 +
 validation/testsuite |  118 ++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 121 insertions(+), 0 deletions(-)

diff --git a/Makefile b/Makefile
index 2dcd514..40e916b 100644
--- a/Makefile
+++ b/Makefile
@@ -138,4 +138,7 @@ pre-process.h:
 clean:
 	rm -f *.[oasi] core core.[0-9]* $(PROGRAMS) $(SLIB_FILE) pre-process.h
 
+check:
+	cd validation && ./testsuite
+
 % : SCCS/s.%s
diff --git a/validation/testsuite b/validation/testsuite
new file mode 100755
index 0000000..5498290
--- /dev/null
+++ b/validation/testsuite
@@ -0,0 +1,118 @@
+#! /bin/sh
+
+set -e
+
+: ${SPARSE=../sparse}
+: ${SPARSE_FLAGS=}
+
+# sparse should report a problem and it does
+BAD_EXP_BAD="
+bad-array-designated-initializer.c
+bad-assignment.c
+bad-cast.c
+bad-ternary-cond.c
+badtype2.c
+badtype3.c
+badtype4.c
+"
+
+# sparse should not report a problem and it doesn't
+OK_EXP_OK="
+bitfields.c
+field-overlap.c
+foul-bitwise.c
+init-char-array.c
+inline_compound_literals.c
+preprocessor16.c
+preprocessor17.c
+preprocessor6.c
+struct-as.c
+struct-ns1.c
+struct-ns2.c
+struct-size1.c
+test-be.c
+type1.c
+typeconvert.c
+varargs1.c
+phase3/comments
+"
+
+# sparse should report a problem but it doesn't
+# This needs to be fixed!
+BAD_EXP_OK="
+badtype1.c
+"
+
+# sparse should not report a problem but it does
+# This needs to be fixed!
+OK_EXP_BAD="
+builtin_safe1.c
+check_byte_count-ice.c
+choose_expr.c
+cond_expr.c
+context.c
+initializer-entry-defined-twice.c
+noderef.c
+preprocessor1.c
+preprocessor10.c
+preprocessor11.c
+preprocessor12.c
+preprocessor13.c
+preprocessor14.c
+preprocessor15.c
+preprocessor18.c
+preprocessor19.c
+preprocessor2.c
+preprocessor20.c
+preprocessor3.c
+preprocessor4.c
+preprocessor5.c
+preprocessor7.c
+preprocessor8.c
+preprocessor9.c
+phase2/backslash
+"
+
+unexp=0
+
+for t in $BAD_EXP_BAD; do
+	$SPARSE $SPARSE_FLAGS $t 2>test-err
+	if test -s test-err; then
+		echo "PASS: $t"
+	else
+		echo "UNEXPECTED FAIL: $t"
+		unexp=1
+	fi
+done
+
+for t in $OK_EXP_OK; do
+	$SPARSE $SPARSE_FLAGS $t 2>test-err
+	if test -s test-err; then
+		echo "UNEXPECTED FAIL: $t"
+		unexp=1
+	else
+		echo "PASS: $t"
+	fi
+done
+
+for t in $BAD_EXP_OK; do
+	$SPARSE $SPARSE_FLAGS $t 2>test-err
+	if test -s test-err; then
+		echo "UNEXPECTED PASS: $t"
+		unexp=1
+	else
+		echo "(known) FAIL: $t"
+	fi
+done
+
+for t in $OK_EXP_BAD; do
+	$SPARSE $SPARSE_FLAGS $t 2>test-err
+	if test -s test-err; then
+		echo "(known) FAIL: $t"
+	else
+		echo "UNEXPECTED PASS: $t"
+		unexp=1
+	fi
+done
+
+exit $unexp

Re: [PATCH] Add a testsuite, call it on "make check"

[Josh Triplett]

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

You rock.  A few comments:

In the long run, I'd like to check that the expected output matches the actual
output, rather than just checking for the presence of output; we could do this
by storing expected warning/error output in a file matching the test but with
a .err extension.  That won't stop me from committing this test suite
implementation in the meantime, though.

I don't know for sure, since many of the test suite files don't include
documentation, but I believe that some of them (like the preprocessor tests)
only run as far as sparse -E, and they don't actually compile as C.  Thus,
they shouldn't get listed under BAD_EXP_OK.  For these, I'd eventually like to
check both the preprocessed output and the warning/error output against the
expected values.

Several tests appear miscategorized in this test suite.  For example,
struct-ns2.c includes a comment saying "This is not in scope and should barf
loudly.", and includes code which should indeed cause sparse to barf loudly,
but sparse does not output anything when run on that file, even with -Wall.
Conversely, on initializer-entry-defined-twice.c, sparse actually *should*
report a problem here (two problems, in fact), and it does; it just shouldn't
report a problem on the third.  Also, builtin_safe1.c falls in the category of
"doesn't currently pass", as it warns in two places it shouldn't, in addition
to the various places it should; however, this would require checking the
actual warning/error output.  Figuring out which of these various tests should
pass and which should fail will require a fair bit of work.

Ideally, I'd love to see the test suite find new tests automatically, rather
than needing to list them explicitly; with a warning/error output file for
each test, the only difference between an OK_EXP_OK and a BAD_EXP_BAD becomes
"does the .err file have any content", so that just leaves the known failures,
which we can list explicitly since we don't need to make it easy to add more.

Pavel Roskin wrote:
> --- /dev/null
> +++ b/validation/testsuite
> @@ -0,0 +1,118 @@
> +#! /bin/sh
> +
> +set -e

At first, this seemed wrong, since sparse should exit with a non-zero exit
code if it emits any errors; however, I then realized that sparse doesn't
actually *do* this, which seems like a bug.

> +: ${SPARSE=../sparse}
> +: ${SPARSE_FLAGS=}

I don't think the test suite should allow setting sparse flags, partly since
changing the flags may well cause a test to pass or fail when it shouldn't,
and partly since at some point different parts of the test suite might need
different flags.

> +for t in $BAD_EXP_OK; do
> +	$SPARSE $SPARSE_FLAGS $t 2>test-err
> +	if test -s test-err; then
> +		echo "UNEXPECTED PASS: $t"
> +		unexp=1
> +	else
> +		echo "(known) FAIL: $t"
> +	fi
> +done
> +
> +for t in $OK_EXP_BAD; do
> +	$SPARSE $SPARSE_FLAGS $t 2>test-err
> +	if test -s test-err; then
> +		echo "(known) FAIL: $t"
> +	else
> +		echo "UNEXPECTED PASS: $t"
> +		unexp=1
> +	fi
> +done

At first I balked a bit at the idea of having the test suite succeed while
some tests don't actually pass, but more I thought about it, I started to
agree that it seems like the right approach.  It means that we can ensure that
the test suite passes in every single Git revision of sparse.  Once all the
tests run as expected, we can dump this part of the test suite code.

- Josh Triplett


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

Re: [PATCH] Add a testsuite, call it on "make check"

[Pavel Roskin]

On Mon, 2006-11-06 at 16:46 -0800, Josh Triplett wrote:
> You rock.  A few comments:
> 
> In the long run, I'd like to check that the expected output matches the actual
> output, rather than just checking for the presence of output; we could do this
> by storing expected warning/error output in a file matching the test but with
> a .err extension.  That won't stop me from committing this test suite
> implementation in the meantime, though.

Actually, the output could be stored in the test itself with a certain
prefix, e.g.:

//experr FILE:3:9: error: undefined identifier 'x'
//experr FILE:4:7: error: incompatible types for 'case' statement

The testsuite would grep for "//experr " and remove it.  Also, "FILE:"
will be substituted with the actual filename.

If no "experr" is found, the output is expected to be empty.  The
"experr" lines should be at the end to avoid the need to change the line
numbers when the error message changes.

> I don't know for sure, since many of the test suite files don't include
> documentation, but I believe that some of them (like the preprocessor tests)
> only run as far as sparse -E, and they don't actually compile as C.  Thus,
> they shouldn't get listed under BAD_EXP_OK.  For these, I'd eventually like to
> check both the preprocessed output and the warning/error output against the
> expected values.

I didn't even know about the "-E" switch.  This reminds us that the
documentation need to be written.

Maybe all the information about the test should be embedded into the
test?  That would be the flags and the expected result.  The only bit of
information stored in the testsuite would be whether the test is known
to pass, and I'm not even sure about that.

> Several tests appear miscategorized in this test suite.  For example,
> struct-ns2.c includes a comment saying "This is not in scope and should barf
> loudly.", and includes code which should indeed cause sparse to barf loudly,
> but sparse does not output anything when run on that file, even with -Wall.

Actually, my patch for -Wall support was never applied, so sparse just
ignores it.  I guess I should resend it.

> Conversely, on initializer-entry-defined-twice.c, sparse actually *should*
> report a problem here (two problems, in fact), and it does; it just shouldn't
> report a problem on the third.  Also, builtin_safe1.c falls in the category of
> "doesn't currently pass", as it warns in two places it shouldn't, in addition
> to the various places it should; however, this would require checking the
> actual warning/error output.  Figuring out which of these various tests should
> pass and which should fail will require a fair bit of work.

I agree.  That's why we want to find the best infrastructure for the
testsuite first.

> Ideally, I'd love to see the test suite find new tests automatically, rather
> than needing to list them explicitly; with a warning/error output file for
> each test, the only difference between an OK_EXP_OK and a BAD_EXP_BAD becomes
> "does the .err file have any content", so that just leaves the known failures,
> which we can list explicitly since we don't need to make it easy to add more.

I think the testsuite can find all files ending with ".c" in the
validation directory.  I guess the tests from phase2 and phase3 could be
moved up and given the ".c" extension as well.  But I have no idea why
those directories were created in the first place.

We also have test-*.c files in the main directory.  They should probably
be moved elsewhere, but I'm not sure if they are suitable for automated
testing.

> Pavel Roskin wrote:
> > --- /dev/null
> > +++ b/validation/testsuite
> > @@ -0,0 +1,118 @@
> > +#! /bin/sh
> > +
> > +set -e
> 
> At first, this seemed wrong, since sparse should exit with a non-zero exit
> code if it emits any errors; however, I then realized that sparse doesn't
> actually *do* this, which seems like a bug.

That's the first bug found by the testsuite :)

I think it would be reasonable to fix it and then rethink the testsuite.

> > +: ${SPARSE=../sparse}
> > +: ${SPARSE_FLAGS=}
> 
> I don't think the test suite should allow setting sparse flags, partly since
> changing the flags may well cause a test to pass or fail when it shouldn't,
> and partly since at some point different parts of the test suite might need
> different flags.

I agree.  However, we need to be able to specify flags for some tests.

> At first I balked a bit at the idea of having the test suite succeed while
> some tests don't actually pass, but more I thought about it, I started to
> agree that it seems like the right approach.  It means that we can ensure that
> the test suite passes in every single Git revision of sparse.  Once all the
> tests run as expected, we can dump this part of the test suite code.

The idea is that the testsuite fails if some tests don't behave as
expected.  This means that the behavior of sparse has changed and the
testsuite needs to be reexamined.  Even if a test that used to fail is
passing now, it needs attention to make sure that the test passes for
legitimate reasons.

It's also very important to catch the cases when sparse produces
different output on different machines.  It's a clear sign that there is
a major bug e.g. an endianess issue or an uninitialized variable.  This
should not be tolerated even temporarily.

I even tend to think that we want to encode the actual output even if
it's known to be wrong.  A flag should indicate that it's not what we
want sparse to do, but it's what it does.  It may be hard to predict the
output from the code that have not be written yet :)

-- 
Regards,
Pavel Roskin