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

[Andrew Jones <drjones@redhat.com>]

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