From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Jones Subject: Re: [PATCH kvm-unit-tests 1/6] lib/report: allow test skipping Date: Mon, 14 Dec 2015 16:12:19 -0600 Message-ID: <20151214221219.GF3633@hawk.localdomain> References: <1450128261-21170-1-git-send-email-rkrcmar@redhat.com> <1450128261-21170-2-git-send-email-rkrcmar@redhat.com> <20151214220019.GB3633@hawk.localdomain> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: QUOTED-PRINTABLE Cc: kvm@vger.kernel.org, Paolo Bonzini To: Radim =?utf-8?B?S3LEjW3DocWZ?= Return-path: Received: from mx1.redhat.com ([209.132.183.28]:47050 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753585AbbLNWM0 (ORCPT ); Mon, 14 Dec 2015 17:12:26 -0500 Received: from int-mx11.intmail.prod.int.phx2.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24]) by mx1.redhat.com (Postfix) with ESMTPS id 359EEC0B9308 for ; Mon, 14 Dec 2015 22:12:26 +0000 (UTC) Content-Disposition: inline In-Reply-To: <20151214220019.GB3633@hawk.localdomain> Sender: kvm-owner@vger.kernel.org List-ID: 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=C4=8Dm=C3=A1=C5=99= 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 skipp= ed as > > well. This also includes the case where no report()s were done, bu= t > > the test still ended with report_summary(). > >=20 > > When the whole test is skipped, ./run_tests.sh prints "skip" instea= d of > > green "PASS". > >=20 > > 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. > >=20 > > Signed-off-by: Radim Kr=C4=8Dm=C3=A1=C5=99 > > --- > > lib/libcflat.h | 1 + > > lib/report.c | 43 +++++++++++++++++++++++++++++-------------- > > run_tests.sh | 13 ++++++++----- > > 3 files changed, 38 insertions(+), 19 deletions(-) > >=20 > > 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); > > =20 > > #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" > > =20 > > -static unsigned int tests, failures, xfailures; > > +static unsigned int tests, failures, xfailures, skipped; > > static char prefixes[256]; > > static struct spinlock lock; > > =20 > > @@ -43,25 +43,27 @@ void report_prefix_pop(void) > > spin_unlock(&lock); > > } > > =20 > > -void va_report_xfail(const char *msg_fmt, bool xfail, bool cond, v= a_list va) > > +void va_report(const char *msg_fmt, bool pass, bool xfail, bool sk= ip, va_list va) >=20 > Line greater than 80 char here. Yes, that was supposed to induce an e= ye > roll. But... this file doesn't have any "long" lines yet, so we could > continue avoiding them. >=20 > > { > > - char *pass =3D xfail ? "XPASS" : "PASS"; > > - char *fail =3D xfail ? "XFAIL" : "FAIL"; > > char buf[2000]; > > + char *prefix =3D skip ? "SKIP" > > + : xfail ? (pass ? "XPASS" : "XFAIL") > > + : (pass ? "PASS" : "FAIL"); > > =20 > > spin_lock(&lock); > > =20 > > 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++; > > =20 > > 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); > > } > > =20 > > @@ -79,7 +81,15 @@ void report_xfail(const char *msg_fmt, bool xfai= l, 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); > > } > > =20 > > @@ -89,9 +99,14 @@ int report_summary(void) > > =20 > > printf("\nSUMMARY: %d tests, %d unexpected failures", tests, fail= ures); > > 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 =3D=3D skipped) > > + return 77; /* blame AUTOTOOLS */ > > + > > return failures > 0 ? 1 : 0; > > =20 > > 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 n= eed 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" ;; >=20 > Why not "\e[31mSKIP\e[0m"? (and without those escape sequences echo d= oesn't > need -e) oops, copy+paste error, I meant to put use color code 33 (yellow). >=20 > > + *) echo -ne "\e[31mFAIL\e[0m" > > + esac > > =20 > > - if [ $? -le 1 ]; then > > - echo -e "\e[32mPASS\e[0m $1" > > - else > > - echo -e "\e[31mFAIL\e[0m $1" > > - fi > > + echo " $1" > > } > > =20 > > function usage() > > --=20 > > 2.6.4 > >=20 > > -- > > 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 >=20 > Thanks, > drew