[PATCH 0/5] tests & testsuite small improvements

[PATCH 0/5] tests & testsuite small improvements

[Luc Van Oostenryck]

This serie aims to improve a bit the testsuite,
especially the exactitude about what is reported.

The patches are independent of each others.

Luc Van Oostenryck (5):
  testsuite: give a proper name to the 'binary-constant' test
  testsuite: make tests known to fail effectively fail
  testsuite: report as error tests known to fail but which succeed
  testsuite: simplify the ioc-typecheck case
  testsuite: add a simple test for -Wenum-mismatch

 validation/badtype1.c        |  4 ++++
 validation/binary-constant.c |  2 +-
 validation/enum-mismatch.c   | 19 +++++++++++++++++++
 validation/ioc-typecheck.c   | 12 +++---------
 validation/struct-ns2.c      |  6 ++++++
 validation/test-suite        | 17 ++++++++++++++---
 6 files changed, 47 insertions(+), 13 deletions(-)
 create mode 100644 validation/enum-mismatch.c

-- 
2.10.1

[PATCH 1/5] testsuite: give a proper name to the 'binary-constant' test

[Luc Van Oostenryck]

The template for this test was visibly copied from another one
and totally irrelevant to what is tested.

Give it another name, briefly describing the subject of the test.

Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
 validation/binary-constant.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/validation/binary-constant.c b/validation/binary-constant.c
index a589403a..c4ae0454 100644
--- a/validation/binary-constant.c
+++ b/validation/binary-constant.c
@@ -3,5 +3,5 @@ extern int x;
 int x = 0b11;
 
 /*
- * check-name: inline compound literals
+ * check-name: binary constant
  */
-- 
2.10.1

[PATCH 2/5] testsuite: make tests known to fail effectively fail

[Luc Van Oostenryck]

These tests must fail because some errors which should
be detected are not.
But the test suite lack to see that because those tests
don't have the check-error-start/end section.

Add such a section (with madeup error message) so that the
testsuite now properly see and report the failure.

Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
 validation/badtype1.c   | 4 ++++
 validation/struct-ns2.c | 6 ++++++
 2 files changed, 10 insertions(+)

diff --git a/validation/badtype1.c b/validation/badtype1.c
index ced7f9f1..b15cb50e 100644
--- a/validation/badtype1.c
+++ b/validation/badtype1.c
@@ -3,4 +3,8 @@ static void foo(enum bar baz);
 /*
  * check-name: enum not in scope
  * check-known-to-fail
+ *
+ * check-error-start
+badtype1.c:1:22: warning: bad scope for 'enum bar'
+ * check-error-end
  */
diff --git a/validation/struct-ns2.c b/validation/struct-ns2.c
index 4dd2c3bf..c5afbb71 100644
--- a/validation/struct-ns2.c
+++ b/validation/struct-ns2.c
@@ -16,4 +16,10 @@ h (void)
 /*
  * check-name: struct not in scope
  * check-known-to-fail
+ *
+ * check-error-start
+struct-ns2.c:2:11: warning: bad scope for 'struct Bar'
+struct-ns2.c:12:14: error: incomplete type/unknown size for 'y'
+struct-ns2.c:13:5: error: using member 'i' in incomplete 'struct Bar'
+ * check-error-end
  */
-- 
2.10.1

[PATCH 3/5] testsuite: report as error tests known to fail but which succeed

[Luc Van Oostenryck]

Such situation may simply show that what was tested is now fixed
and that it's juste the test annotation which need to be adapted,
but can be a sign that something else is broken.

Reporting the exact result (failure/success, known-to-fail/expect-to-succeed)
make the testsuite more useful and allow to use more efficiently
git-bisect or other automated testing tools.

Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
 validation/test-suite | 17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/validation/test-suite b/validation/test-suite
index df5a7c60..03421a24 100755
--- a/validation/test-suite
+++ b/validation/test-suite
@@ -161,15 +161,26 @@ do_test()
 		test_failed=1
 	fi
 
+	get_tag "check-known-to-fail" $file
+	if [ "$?" -eq "0" ]; then
+		must_fail=1
+		known_ko_tests=`expr $known_ko_tests + 1`
+	else
+		must_fail=0
+	fi
+
 	if [ "$test_failed" -eq "1" ]; then
 		ko_tests=`expr $ko_tests + 1`
-		get_tag "check-known-to-fail" $file
-		if [ "$?" -eq "0" ]; then
+		if [ "$must_fail" -eq "1" ]; then
 			echo "info: test '$file' is known to fail"
-			known_ko_tests=`expr $known_ko_tests + 1`
 		fi
 		return 1
 	else
+		if [ "$must_fail" -eq "1" ]; then
+			echo "info: test '$file' is known to fail but succeed!"
+			ko_tests=`expr $ko_tests + 1`
+			return 1
+		fi
 		ok_tests=`expr $ok_tests + 1`
 		return 0
 	fi
-- 
2.10.1

[PATCH 4/5] testsuite: simplify the ioc-typecheck case

[Luc Van Oostenryck]

The test was relatively convoluted and it wasn't very clear what
was the problem and waht was tested.

The real problem is simply a problem about a non-constant
expression in an array designator.

Simplify the test to clearly show that.

Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
CC: Hans Verkuil <hans.verkuil@cisco.com>
---
 validation/ioc-typecheck.c | 12 +++---------
 1 file changed, 3 insertions(+), 9 deletions(-)

diff --git a/validation/ioc-typecheck.c b/validation/ioc-typecheck.c
index aa060f7b..34b37d31 100644
--- a/validation/ioc-typecheck.c
+++ b/validation/ioc-typecheck.c
@@ -1,16 +1,10 @@
 extern unsigned int __invalid_size_argument_for_IOC;
-#define _IOC_TYPECHECK(t) \
-                ((sizeof(t) == sizeof(t[1]) && \
-                    sizeof(t) < (1 << 14)) ? \
-                   sizeof(t) : __invalid_size_argument_for_IOC)
-
-#define TEST_IOCTL (50 | (_IOC_TYPECHECK(unsigned) << 8))

[PATCH 5/5] testsuite: add a simple test for -Wenum-mismatch

[Luc Van Oostenryck]

A nice feature
No tests existed for it
Create one now.

Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
 validation/enum-mismatch.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)
 create mode 100644 validation/enum-mismatch.c

diff --git a/validation/enum-mismatch.c b/validation/enum-mismatch.c
new file mode 100644
index 00000000..9a929d24
--- /dev/null
+++ b/validation/enum-mismatch.c
@@ -0,0 +1,19 @@
+enum ea { A = 0, };
+enum eb { B = 1, };
+
+
+static enum eb foo(enum ea a)
+{
+	return a;
+}
+
+/*
+ * check-name: enum-mismatch
+ * check-command: sparse -Wenum-mismatch $file
+ *
+ * check-error-start
+enum-mismatch.c:7:16: warning: mixing different enum types
+enum-mismatch.c:7:16:     int enum ea  versus
+enum-mismatch.c:7:16:     int enum eb 
+ * check-error-end
+ */
-- 
2.10.1

Re: [PATCH 1/5] testsuite: give a proper name to the 'binary-constant' test

[Christopher Li]

On Thu, Nov 3, 2016 at 5:45 AM, Luc Van Oostenryck
<luc.vanoostenryck@gmail.com> wrote:
> The template for this test was visibly copied from another one
> and totally irrelevant to what is tested.
>
> Give it another name, briefly describing the subject of the test.

Applied.

Chris

Re: [PATCH 2/5] testsuite: make tests known to fail effectively fail

[Christopher Li]

On Thu, Nov 3, 2016 at 5:45 AM, Luc Van Oostenryck
<luc.vanoostenryck@gmail.com> wrote:
> These tests must fail because some errors which should
> be detected are not.
> But the test suite lack to see that because those tests
> don't have the check-error-start/end section.
>
> Add such a section (with madeup error message) so that the
> testsuite now properly see and report the failure.

Applied.

Chris

Re: [PATCH 3/5] testsuite: report as error tests known to fail but which succeed

[Christopher Li]

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

On Thu, Nov 3, 2016 at 5:45 AM, Luc Van Oostenryck
<luc.vanoostenryck@gmail.com> wrote:
> Such situation may simply show that what was tested is now fixed
> and that it's juste the test annotation which need to be adapted,
> but can be a sign that something else is broken.
>
> Reporting the exact result (failure/success, known-to-fail/expect-to-succeed)
> make the testsuite more useful and allow to use more efficiently
> git-bisect or other automated testing tools.

I like what you are doing as a result.

But I think the implementation has room to improve.
I don't like deep and messy "else" statement.

I attach a patch which I modify base on yours. I hope it is
easier to read. Care to review it?

Chris

[-- Attachment #2: known-to-fail.patch --]
[-- Type: text/x-patch, Size: 977 bytes --]

--- .pc/9409919-3-5-testsuite-report-as-error-tests-known-to-fail-but-which-succeed.patch/validation/test-suite	2016-10-24 17:37:29.303846000 +0800
+++ validation/test-suite	2016-11-17 23:56:35.404392454 +0800
@@ -161,18 +161,25 @@ do_test()
 		test_failed=1
 	fi
 
-	if [ "$test_failed" -eq "1" ]; then
-		ko_tests=`expr $ko_tests + 1`
-		get_tag "check-known-to-fail" $file
-		if [ "$?" -eq "0" ]; then
+	get_tag "check-known-to-fail" $file
+	must_fail=`expr "$?" = 0`
+	known_ko_tests=`expr $known_ko_tests + $must_fail`
+
+	if [ "$must_fail" -eq "1" ]; then
+		if [ "$test_failed" -eq "1" ]; then
 			echo "info: test '$file' is known to fail"
-			known_ko_tests=`expr $known_ko_tests + 1`
+		else
+			echo "info: test '$file' is known to fail but succeed!"
+			test_failed=1
 		fi
-		return 1
+	fi
+
+	if [ "$test_failed" -eq "1" ]; then
+		ko_tests=`expr $ko_tests + 1`
 	else
 		ok_tests=`expr $ok_tests + 1`
-		return 0
 	fi
+	return $test_failed
 }
 
 do_test_suite()

Re: [PATCH 4/5] testsuite: simplify the ioc-typecheck case

[Christopher Li]

On Thu, Nov 3, 2016 at 5:45 AM, Luc Van Oostenryck
<luc.vanoostenryck@gmail.com> wrote:
> The test was relatively convoluted and it wasn't very clear what
> was the problem and waht was tested.
>
> The real problem is simply a problem about a non-constant
> expression in an array designator.
>
> Simplify the test to clearly show that.

Applied.

Chris

Re: [PATCH 5/5] testsuite: add a simple test for -Wenum-mismatch

[Christopher Li]

On Thu, Nov 3, 2016 at 5:45 AM, Luc Van Oostenryck
<luc.vanoostenryck@gmail.com> wrote:
> A nice feature
> No tests existed for it
> Create one now.

Applied.

Chris

Re: [PATCH 3/5] testsuite: report as error tests known to fail but which succeed

[Luc Van Oostenryck]

On Fri, Nov 18, 2016 at 12:13:58AM +0800, Christopher Li wrote:
> On Thu, Nov 3, 2016 at 5:45 AM, Luc Van Oostenryck
> <luc.vanoostenryck@gmail.com> wrote:
> > Such situation may simply show that what was tested is now fixed
> > and that it's juste the test annotation which need to be adapted,
> > but can be a sign that something else is broken.
> >
> > Reporting the exact result (failure/success, known-to-fail/expect-to-succeed)
> > make the testsuite more useful and allow to use more efficiently
> > git-bisect or other automated testing tools.
> 
> I like what you are doing as a result.
> 
> But I think the implementation has room to improve.
> I don't like deep and messy "else" statement.
> 
> I attach a patch which I modify base on yours. I hope it is
> easier to read. Care to review it?

Yes, I much prefer like this. Thanks.

But there is something I would like to check but I can't do that now.
I'll come back on it later.

Luc

Re: [PATCH 3/5] testsuite: report as error tests known to fail but which succeed

[Luc Van Oostenryck]

On Fri, Nov 18, 2016 at 12:13:58AM +0800, Christopher Li wrote:
> On Thu, Nov 3, 2016 at 5:45 AM, Luc Van Oostenryck
> <luc.vanoostenryck@gmail.com> wrote:
> > Such situation may simply show that what was tested is now fixed
> > and that it's juste the test annotation which need to be adapted,
> > but can be a sign that something else is broken.
> >
> > Reporting the exact result (failure/success, known-to-fail/expect-to-succeed)
> > make the testsuite more useful and allow to use more efficiently
> > git-bisect or other automated testing tools.
> 
> I like what you are doing as a result.
> 
> But I think the implementation has room to improve.
> I don't like deep and messy "else" statement.
> 
> I attach a patch which I modify base on yours. I hope it is
> easier to read. Care to review it?
> 
> Chris

Like I said I prefer your version, but could you add the following patch
on top of yours?

---
From: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
Date: Sat, 19 Nov 2016 01:28:31 +0100
Subject: [PATCH] testsuite: let the testsuite fail if we got any unexpected
 result

With this patch a failed test but known to fail will still be reported
as failed but won't make the whole testsuite to fail.
But a test known to fail but which succeed will make the testsuite
to fail. Normal tests, the ones not marked as known-to-fail, will
of course the testsuite to fail if one of them fail.

To put things simply: the testsuite will return a failure
if at least one test gave an unexpected result.

Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
 validation/test-suite | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/validation/test-suite b/validation/test-suite
index 4eff0150..ee6a90a5 100755
--- a/validation/test-suite
+++ b/validation/test-suite
@@ -11,6 +11,10 @@ if [ ! -x "$default_path/sparse-llvm" ]; then
 	disabled_cmds="sparsec sparsei sparse-llvm"
 fi
 
+# flags:
+#	- some tests gave an unexpected result
+failed=0
+
 # counts:
 #	- tests that have not been converted to test-suite format
 #	- tests that are disabled
@@ -165,6 +169,8 @@ do_test()
 	must_fail=`expr "$?" = 0`
 	known_ko_tests=`expr $known_ko_tests + $must_fail`
 
+	[ "$test_failed" -eq "$must_fail" ] || failed=1
+
 	if [ "$must_fail" -eq "1" ]; then
 		if [ "$test_failed" -eq "1" ]; then
 			echo "info: test '$file' is known to fail"
@@ -278,5 +284,5 @@ case "$1" in
 		;;
 esac
 
-exit 0
+exit $failed
 
-- 
2.10.2

Re: [PATCH 3/5] testsuite: report as error tests known to fail but which succeed

[Christopher Li]

On Sat, Nov 19, 2016 at 8:44 AM, Luc Van Oostenryck
<luc.vanoostenryck@gmail.com> wrote:
> Like I said I prefer your version, but could you add the following patch
> on top of yours?

So you want a return code from test-suit if the test-suit does not
run as expected.

Applied and squashed.

Chris