| Message ID | 1450374823-7648-6-git-send-email-rkrcmar@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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 -- 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
On Thu, Dec 17, 2015 at 01:30:23PM -0600, Andrew Jones wrote: > 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); > } Hmm, while I still think we should avoid using static, to allow new wrappers, the wrapper I wrote here as an example wouldn't be necessary if report_skip's inputs were instead void report_skip(const char *msg_fmt, bool pass, bool skip, ...) Why not do that? > > > { > > - 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 -- 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
2015-12-17 13:37-0600, Andrew Jones: > On Thu, Dec 17, 2015 at 01:30:23PM -0600, Andrew Jones wrote: >> 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> >> > --- >> > diff --git a/lib/report.c b/lib/report.c >> > @@ -43,25 +43,28 @@ void report_prefix_pop(void) >> > -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); >> } > > Hmm, while I still think we should avoid using static, to allow new wrappers, > the wrapper I wrote here as an example wouldn't be necessary if report_skip's > inputs were instead That breaks encapsulation -- if we ever want to change va_report(), we've just made our lives harder. > void report_skip(const char *msg_fmt, bool pass, bool skip, ...) > > Why not do that? Yeah, some cases want to unconditionally skip, so we'd want to have both. I'll think of naming during lunch :) -- 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
On Fri, Dec 18, 2015 at 12:18:19PM +0100, Radim Kr?má? wrote: > 2015-12-17 13:37-0600, Andrew Jones: > > On Thu, Dec 17, 2015 at 01:30:23PM -0600, Andrew Jones wrote: > >> 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> > >> > --- > >> > diff --git a/lib/report.c b/lib/report.c > >> > @@ -43,25 +43,28 @@ void report_prefix_pop(void) > >> > -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); > >> } > > > > Hmm, while I still think we should avoid using static, to allow new wrappers, > > the wrapper I wrote here as an example wouldn't be necessary if report_skip's > > inputs were instead > > That breaks encapsulation -- if we ever want to change va_report(), > we've just made our lives harder. OK, let's make it static and extend the API. > > > void report_skip(const char *msg_fmt, bool pass, bool skip, ...) > > > > Why not do that? > > Yeah, some cases want to unconditionally skip, so we'd want to have > both. I'll think of naming during lunch :) > -- > 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 -- 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 --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) { - 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" return $ret }
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(-)