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

[Andrew Jones <drjones@redhat.com>]

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