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

Radim Krčmář
2015-12-18T11:18:22Z

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 :)