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

Andrew Jones
2015-12-18T18:55:25Z

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