diff mbox

[kvm-unit-tests,1/6] lib/report: allow test skipping

Message ID 1450128261-21170-2-git-send-email-rkrcmar@redhat.com (mailing list archive)
State New, archived
Headers show

Commit Message

Radim Krčmář Dec. 14, 2015, 9:24 p.m. UTC
This patch allows us to explicitly mark a unit-test as skipped.
If all unit-tests were skipped, the whole test is reported as skipped as
well.  This also includes the case where no report()s were done, but
the test still ended with report_summary().

When the whole test is skipped, ./run_tests.sh prints "skip" instead of
green "PASS".

Return value of 77 is used to please Autotools.  I also renamed few
things in reporting code and chose to refactor a logic while at it.

Signed-off-by: Radim Kr?má? <rkrcmar@redhat.com>
---
 lib/libcflat.h |  1 +
 lib/report.c   | 43 +++++++++++++++++++++++++++++--------------
 run_tests.sh   | 13 ++++++++-----
 3 files changed, 38 insertions(+), 19 deletions(-)

Comments

Andrew Jones Dec. 14, 2015, 10 p.m. UTC | #1
On Mon, Dec 14, 2015 at 10:24:16PM +0100, Radim Kr?má? wrote:
> This patch allows us to explicitly mark a unit-test as skipped.
> If all unit-tests were skipped, the whole test is reported as skipped as
> well.  This also includes the case where no report()s were done, but
> the test still ended with report_summary().
> 
> When the whole test is skipped, ./run_tests.sh prints "skip" instead of
> green "PASS".
> 
> Return value of 77 is used to please Autotools.  I also renamed few
> things in reporting code and chose to refactor a logic while at it.
> 
> Signed-off-by: Radim Kr?má? <rkrcmar@redhat.com>
> ---
>  lib/libcflat.h |  1 +
>  lib/report.c   | 43 +++++++++++++++++++++++++++++--------------
>  run_tests.sh   | 13 ++++++++-----
>  3 files changed, 38 insertions(+), 19 deletions(-)
> 
> diff --git a/lib/libcflat.h b/lib/libcflat.h
> index 9747ccdbc9f1..070818354ee1 100644
> --- a/lib/libcflat.h
> +++ b/lib/libcflat.h
> @@ -61,6 +61,7 @@ void report_prefix_push(const char *prefix);
>  void report_prefix_pop(void);
>  void report(const char *msg_fmt, bool pass, ...);
>  void report_xfail(const char *msg_fmt, bool xfail, bool pass, ...);
> +void report_skip(const char *msg_fmt, ...);
>  int report_summary(void);
>  
>  #define ARRAY_SIZE(_a) (sizeof(_a)/sizeof((_a)[0]))
> diff --git a/lib/report.c b/lib/report.c
> index 35e664108a92..e07baa347298 100644
> --- a/lib/report.c
> +++ b/lib/report.c
> @@ -13,7 +13,7 @@
>  #include "libcflat.h"
>  #include "asm/spinlock.h"
>  
> -static unsigned int tests, failures, xfailures;
> +static unsigned int tests, failures, xfailures, skipped;
>  static char prefixes[256];
>  static struct spinlock lock;
>  
> @@ -43,25 +43,27 @@ void report_prefix_pop(void)
>  	spin_unlock(&lock);
>  }
>  
> -void va_report_xfail(const char *msg_fmt, bool xfail, bool cond, va_list va)
> +void va_report(const char *msg_fmt, bool pass, bool xfail, bool skip, va_list va)

Line greater than 80 char here. Yes, that was supposed to induce an eye
roll. But... this file doesn't have any "long" lines yet, so we could
continue avoiding them.

>  {
> -	char *pass = xfail ? "XPASS" : "PASS";
> -	char *fail = xfail ? "XFAIL" : "FAIL";
>  	char buf[2000];
> +	char *prefix = skip ? "SKIP"
> +	                    : xfail ? (pass ? "XPASS" : "XFAIL")
> +	                            : (pass ? "PASS"  : "FAIL");
>  
>  	spin_lock(&lock);
>  
>  	tests++;
> -	printf("%s: ", cond ? pass : fail);
> +	printf("%s: ", prefix);
>  	puts(prefixes);
>  	vsnprintf(buf, sizeof(buf), msg_fmt, va);
>  	puts(buf);
>  	puts("\n");
> -	if (xfail && cond)
> -		failures++;
> -	else if (xfail)
> +
> +	if (skip)
> +		skipped++;
> +	else if (xfail && !pass)
>  		xfailures++;
> -	else if (!cond)
> +	else if (xfail || !pass)
>  		failures++;
>  
>  	spin_unlock(&lock);
> @@ -71,7 +73,7 @@ void report(const char *msg_fmt, bool pass, ...)
>  {
>  	va_list va;
>  	va_start(va, pass);
> -	va_report_xfail(msg_fmt, false, pass, va);
> +	va_report(msg_fmt, pass, false, false, va);
>  	va_end(va);
>  }
>  
> @@ -79,7 +81,15 @@ void report_xfail(const char *msg_fmt, bool xfail, bool pass, ...)
>  {
>  	va_list va;
>  	va_start(va, pass);
> -	va_report_xfail(msg_fmt, xfail, pass, va);
> +	va_report(msg_fmt, pass, xfail, false, va);
> +	va_end(va);
> +}
> +
> +void report_skip(const char *msg_fmt, ...)
> +{
> +	va_list va;
> +	va_start(va, msg_fmt);
> +	va_report(msg_fmt, false, false, true, va);
>  	va_end(va);
>  }
>  
> @@ -89,9 +99,14 @@ int report_summary(void)
>  
>  	printf("\nSUMMARY: %d tests, %d unexpected failures", tests, failures);
>  	if (xfailures)
> -		printf(", %d expected failures\n", xfailures);
> -	else
> -		printf("\n");
> +		printf(", %d expected failures", xfailures);
> +	if (skipped)
> +		printf(", %d skipped", skipped);
> +	printf("\n");
> +
> +	if (tests == skipped)
> +		return 77; /* blame AUTOTOOLS */
> +
>  	return failures > 0 ? 1 : 0;
>  
>  	spin_unlock(&lock);
> diff --git a/run_tests.sh b/run_tests.sh
> index fad22a935b00..4d813b9a7084 100755
> --- a/run_tests.sh
> +++ b/run_tests.sh
> @@ -55,12 +55,15 @@ function run()
>      # extra_params in the config file may contain backticks that need to be
>      # expanded, so use eval to start qemu
>      eval $cmdline >> test.log
> +    # The first bit of return value is too hard to use, just skip it.
> +    # Unit-tests' return value is shifted by one.
> +    case $(($? >> 1)) in
> +    0)  echo -ne "\e[32mPASS\e[0m" ;;
> +    77) echo -ne "skip" ;;

Why not "\e[31mSKIP\e[0m"? (and without those escape sequences echo doesn't
need -e)

> +    *)  echo -ne "\e[31mFAIL\e[0m"
> +    esac
>  
> -    if [ $? -le 1 ]; then
> -        echo -e "\e[32mPASS\e[0m $1"
> -    else
> -        echo -e "\e[31mFAIL\e[0m $1"
> -    fi
> +    echo " $1"
>  }
>  
>  function usage()
> -- 
> 2.6.4
> 
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Thanks,
drew
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Andrew Jones Dec. 14, 2015, 10:12 p.m. UTC | #2
On Mon, Dec 14, 2015 at 04:00:19PM -0600, Andrew Jones wrote:
> On Mon, Dec 14, 2015 at 10:24:16PM +0100, Radim Kr?má? wrote:
> > This patch allows us to explicitly mark a unit-test as skipped.
> > If all unit-tests were skipped, the whole test is reported as skipped as
> > well.  This also includes the case where no report()s were done, but
> > the test still ended with report_summary().
> > 
> > When the whole test is skipped, ./run_tests.sh prints "skip" instead of
> > green "PASS".
> > 
> > Return value of 77 is used to please Autotools.  I also renamed few
> > things in reporting code and chose to refactor a logic while at it.
> > 
> > Signed-off-by: Radim Kr?má? <rkrcmar@redhat.com>
> > ---
> >  lib/libcflat.h |  1 +
> >  lib/report.c   | 43 +++++++++++++++++++++++++++++--------------
> >  run_tests.sh   | 13 ++++++++-----
> >  3 files changed, 38 insertions(+), 19 deletions(-)
> > 
> > diff --git a/lib/libcflat.h b/lib/libcflat.h
> > index 9747ccdbc9f1..070818354ee1 100644
> > --- a/lib/libcflat.h
> > +++ b/lib/libcflat.h
> > @@ -61,6 +61,7 @@ void report_prefix_push(const char *prefix);
> >  void report_prefix_pop(void);
> >  void report(const char *msg_fmt, bool pass, ...);
> >  void report_xfail(const char *msg_fmt, bool xfail, bool pass, ...);
> > +void report_skip(const char *msg_fmt, ...);
> >  int report_summary(void);
> >  
> >  #define ARRAY_SIZE(_a) (sizeof(_a)/sizeof((_a)[0]))
> > diff --git a/lib/report.c b/lib/report.c
> > index 35e664108a92..e07baa347298 100644
> > --- a/lib/report.c
> > +++ b/lib/report.c
> > @@ -13,7 +13,7 @@
> >  #include "libcflat.h"
> >  #include "asm/spinlock.h"
> >  
> > -static unsigned int tests, failures, xfailures;
> > +static unsigned int tests, failures, xfailures, skipped;
> >  static char prefixes[256];
> >  static struct spinlock lock;
> >  
> > @@ -43,25 +43,27 @@ void report_prefix_pop(void)
> >  	spin_unlock(&lock);
> >  }
> >  
> > -void va_report_xfail(const char *msg_fmt, bool xfail, bool cond, va_list va)
> > +void va_report(const char *msg_fmt, bool pass, bool xfail, bool skip, va_list va)
> 
> Line greater than 80 char here. Yes, that was supposed to induce an eye
> roll. But... this file doesn't have any "long" lines yet, so we could
> continue avoiding them.
> 
> >  {
> > -	char *pass = xfail ? "XPASS" : "PASS";
> > -	char *fail = xfail ? "XFAIL" : "FAIL";
> >  	char buf[2000];
> > +	char *prefix = skip ? "SKIP"
> > +	                    : xfail ? (pass ? "XPASS" : "XFAIL")
> > +	                            : (pass ? "PASS"  : "FAIL");
> >  
> >  	spin_lock(&lock);
> >  
> >  	tests++;
> > -	printf("%s: ", cond ? pass : fail);
> > +	printf("%s: ", prefix);
> >  	puts(prefixes);
> >  	vsnprintf(buf, sizeof(buf), msg_fmt, va);
> >  	puts(buf);
> >  	puts("\n");
> > -	if (xfail && cond)
> > -		failures++;
> > -	else if (xfail)
> > +
> > +	if (skip)
> > +		skipped++;
> > +	else if (xfail && !pass)
> >  		xfailures++;
> > -	else if (!cond)
> > +	else if (xfail || !pass)
> >  		failures++;
> >  
> >  	spin_unlock(&lock);
> > @@ -71,7 +73,7 @@ void report(const char *msg_fmt, bool pass, ...)
> >  {
> >  	va_list va;
> >  	va_start(va, pass);
> > -	va_report_xfail(msg_fmt, false, pass, va);
> > +	va_report(msg_fmt, pass, false, false, va);
> >  	va_end(va);
> >  }
> >  
> > @@ -79,7 +81,15 @@ void report_xfail(const char *msg_fmt, bool xfail, bool pass, ...)
> >  {
> >  	va_list va;
> >  	va_start(va, pass);
> > -	va_report_xfail(msg_fmt, xfail, pass, va);
> > +	va_report(msg_fmt, pass, xfail, false, va);
> > +	va_end(va);
> > +}
> > +
> > +void report_skip(const char *msg_fmt, ...)
> > +{
> > +	va_list va;
> > +	va_start(va, msg_fmt);
> > +	va_report(msg_fmt, false, false, true, va);
> >  	va_end(va);
> >  }
> >  
> > @@ -89,9 +99,14 @@ int report_summary(void)
> >  
> >  	printf("\nSUMMARY: %d tests, %d unexpected failures", tests, failures);
> >  	if (xfailures)
> > -		printf(", %d expected failures\n", xfailures);
> > -	else
> > -		printf("\n");
> > +		printf(", %d expected failures", xfailures);
> > +	if (skipped)
> > +		printf(", %d skipped", skipped);
> > +	printf("\n");
> > +
> > +	if (tests == skipped)
> > +		return 77; /* blame AUTOTOOLS */
> > +
> >  	return failures > 0 ? 1 : 0;
> >  
> >  	spin_unlock(&lock);
> > diff --git a/run_tests.sh b/run_tests.sh
> > index fad22a935b00..4d813b9a7084 100755
> > --- a/run_tests.sh
> > +++ b/run_tests.sh
> > @@ -55,12 +55,15 @@ function run()
> >      # extra_params in the config file may contain backticks that need to be
> >      # expanded, so use eval to start qemu
> >      eval $cmdline >> test.log
> > +    # The first bit of return value is too hard to use, just skip it.
> > +    # Unit-tests' return value is shifted by one.
> > +    case $(($? >> 1)) in
> > +    0)  echo -ne "\e[32mPASS\e[0m" ;;
> > +    77) echo -ne "skip" ;;
> 
> Why not "\e[31mSKIP\e[0m"? (and without those escape sequences echo doesn't
> need -e)

oops, copy+paste error, I meant to put use color code 33 (yellow).

> 
> > +    *)  echo -ne "\e[31mFAIL\e[0m"
> > +    esac
> >  
> > -    if [ $? -le 1 ]; then
> > -        echo -e "\e[32mPASS\e[0m $1"
> > -    else
> > -        echo -e "\e[31mFAIL\e[0m $1"
> > -    fi
> > +    echo " $1"
> >  }
> >  
> >  function usage()
> > -- 
> > 2.6.4
> > 
> > --
> > To unsubscribe from this list: send the line "unsubscribe kvm" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> Thanks,
> drew
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Radim Krčmář Dec. 15, 2015, 12:54 p.m. UTC | #3
2015-12-14 16:12-0600, Andrew Jones:
> On Mon, Dec 14, 2015 at 04:00:19PM -0600, Andrew Jones wrote:
>> On Mon, Dec 14, 2015 at 10:24:16PM +0100, Radim Kr?má? wrote:
>> > ---
>> > diff --git a/lib/libcflat.h b/lib/libcflat.h
>> > -void va_report_xfail(const char *msg_fmt, bool xfail, bool cond, va_list va)
>> > +void va_report(const char *msg_fmt, bool pass, bool xfail, bool skip, va_list va)
>> 
>> Line greater than 80 char here. Yes, that was supposed to induce an eye
>> roll. But... this file doesn't have any "long" lines yet, so we could
>> continue avoiding them.

Ah, yeah, young'uns these days ...

>> > diff --git a/run_tests.sh b/run_tests.sh
>> > index fad22a935b00..4d813b9a7084 100755
>> > --- a/run_tests.sh
>> > +++ b/run_tests.sh
>> > @@ -55,12 +55,15 @@ function run()
>> >      # extra_params in the config file may contain backticks that need to be
>> >      # expanded, so use eval to start qemu
>> >      eval $cmdline >> test.log
>> > +    # The first bit of return value is too hard to use, just skip it.
>> > +    # Unit-tests' return value is shifted by one.
>> > +    case $(($? >> 1)) in
>> > +    0)  echo -ne "\e[32mPASS\e[0m" ;;
>> > +    77) echo -ne "skip" ;;
>> 
>> Why not "\e[31mSKIP\e[0m"? (and without those escape sequences echo doesn't
>> need -e)
> 
> oops, copy+paste error, I meant to put use color code 33 (yellow).

Will do.  (I started with yellow SKIP, but then kept white skip as I
didn't research the reason why it was chosen.)
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/lib/libcflat.h b/lib/libcflat.h
index 9747ccdbc9f1..070818354ee1 100644
--- a/lib/libcflat.h
+++ b/lib/libcflat.h
@@ -61,6 +61,7 @@  void report_prefix_push(const char *prefix);
 void report_prefix_pop(void);
 void report(const char *msg_fmt, bool pass, ...);
 void report_xfail(const char *msg_fmt, bool xfail, bool pass, ...);
+void report_skip(const char *msg_fmt, ...);
 int report_summary(void);
 
 #define ARRAY_SIZE(_a) (sizeof(_a)/sizeof((_a)[0]))
diff --git a/lib/report.c b/lib/report.c
index 35e664108a92..e07baa347298 100644
--- a/lib/report.c
+++ b/lib/report.c
@@ -13,7 +13,7 @@ 
 #include "libcflat.h"
 #include "asm/spinlock.h"
 
-static unsigned int tests, failures, xfailures;
+static unsigned int tests, failures, xfailures, skipped;
 static char prefixes[256];
 static struct spinlock lock;
 
@@ -43,25 +43,27 @@  void report_prefix_pop(void)
 	spin_unlock(&lock);
 }
 
-void va_report_xfail(const char *msg_fmt, bool xfail, bool cond, va_list va)
+void va_report(const char *msg_fmt, bool pass, bool xfail, bool skip, va_list va)
 {
-	char *pass = xfail ? "XPASS" : "PASS";
-	char *fail = xfail ? "XFAIL" : "FAIL";
 	char buf[2000];
+	char *prefix = skip ? "SKIP"
+	                    : xfail ? (pass ? "XPASS" : "XFAIL")
+	                            : (pass ? "PASS"  : "FAIL");
 
 	spin_lock(&lock);
 
 	tests++;
-	printf("%s: ", cond ? pass : fail);
+	printf("%s: ", prefix);
 	puts(prefixes);
 	vsnprintf(buf, sizeof(buf), msg_fmt, va);
 	puts(buf);
 	puts("\n");
-	if (xfail && cond)
-		failures++;
-	else if (xfail)
+
+	if (skip)
+		skipped++;
+	else if (xfail && !pass)
 		xfailures++;
-	else if (!cond)
+	else if (xfail || !pass)
 		failures++;
 
 	spin_unlock(&lock);
@@ -71,7 +73,7 @@  void report(const char *msg_fmt, bool pass, ...)
 {
 	va_list va;
 	va_start(va, pass);
-	va_report_xfail(msg_fmt, false, pass, va);
+	va_report(msg_fmt, pass, false, false, va);
 	va_end(va);
 }
 
@@ -79,7 +81,15 @@  void report_xfail(const char *msg_fmt, bool xfail, bool pass, ...)
 {
 	va_list va;
 	va_start(va, pass);
-	va_report_xfail(msg_fmt, xfail, pass, va);
+	va_report(msg_fmt, pass, xfail, false, va);
+	va_end(va);
+}
+
+void report_skip(const char *msg_fmt, ...)
+{
+	va_list va;
+	va_start(va, msg_fmt);
+	va_report(msg_fmt, false, false, true, va);
 	va_end(va);
 }
 
@@ -89,9 +99,14 @@  int report_summary(void)
 
 	printf("\nSUMMARY: %d tests, %d unexpected failures", tests, failures);
 	if (xfailures)
-		printf(", %d expected failures\n", xfailures);
-	else
-		printf("\n");
+		printf(", %d expected failures", xfailures);
+	if (skipped)
+		printf(", %d skipped", skipped);
+	printf("\n");
+
+	if (tests == skipped)
+		return 77; /* blame AUTOTOOLS */
+
 	return failures > 0 ? 1 : 0;
 
 	spin_unlock(&lock);
diff --git a/run_tests.sh b/run_tests.sh
index fad22a935b00..4d813b9a7084 100755
--- a/run_tests.sh
+++ b/run_tests.sh
@@ -55,12 +55,15 @@  function run()
     # extra_params in the config file may contain backticks that need to be
     # expanded, so use eval to start qemu
     eval $cmdline >> test.log
+    # The first bit of return value is too hard to use, just skip it.
+    # Unit-tests' return value is shifted by one.
+    case $(($? >> 1)) in
+    0)  echo -ne "\e[32mPASS\e[0m" ;;
+    77) echo -ne "skip" ;;
+    *)  echo -ne "\e[31mFAIL\e[0m"
+    esac
 
-    if [ $? -le 1 ]; then
-        echo -e "\e[32mPASS\e[0m $1"
-    else
-        echo -e "\e[31mFAIL\e[0m $1"
-    fi
+    echo " $1"
 }
 
 function usage()