Re: [PATCH kvm-unit-tests v2 05/12] lib/report: allow test skipping

[Andrew Jones <drjones@redhat.com>]

On Thu, Dec 17, 2015 at 06:53:36PM +0100, Radim Krčmář wrote:
> We can now 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 tests were run, but still
> ended with report_summary().
> 
> When the whole test is skipped, ./run_tests.sh prints yellow "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>
> ---
>  v2:
>  - turned skip into yellow SKIP [Drew]
>  - wrapped line at 80 characters [Drew]
>  - added static to va_report
>  
>  lib/libcflat.h   |  1 +
>  lib/report.c     | 44 ++++++++++++++++++++++++++++++--------------
>  scripts/run.bash | 12 +++++++-----
>  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..a47f2e00b529 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,28 @@ void report_prefix_pop(void)
>  	spin_unlock(&lock);
>  }
>  
> -void va_report_xfail(const char *msg_fmt, bool xfail, bool cond, va_list va)
> +static void va_report(const char *msg_fmt, bool pass, bool xfail, bool skip,
> +		va_list va)

Making this static disallows unit test writers to create their own
variable arg report() wrapper functions. Perhaps to determine whether
or not a skip is in order, e.g.

 xyz_report(msg, pass, ...)
 {
    va_list va;
    va_start(va, pass);
    if (xyz)
       va_report(msg, pass, false, false, va);
    else
       va_report(msg, false, false, true, va);
    va_end(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 +74,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 +82,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 +100,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/scripts/run.bash b/scripts/run.bash
> index 243586c6d2fc..b92611c29fbb 100644
> --- a/scripts/run.bash
> +++ b/scripts/run.bash
> @@ -46,11 +46,13 @@ function run()
>      # Unit-tests' return value is shifted by one.
>      ret=$(($? >> 1))
>  
> -    if [ $ret -eq 0 ]; then
> -        echo -e "\e[32mPASS\e[0m $1"
> -    else
> -        echo -e "\e[31mFAIL\e[0m $1"
> -    fi
> +    case $ret in
> +    0)  echo -ne "\e[32mPASS\e[0m" ;;
> +    77) echo -ne "\e[33mSKIP\e[0m" ;;
> +    *)  echo -ne "\e[31mFAIL\e[0m"
> +    esac
> +
> +    echo " $1"

While touching this, please change this to $testname, and anything
else in run() that still uses $N instead of its local var name.

>  
>      return $ret
>  }
> -- 
> 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