Re: [PATCH v2 2/4] kunit: tool: Support skipped tests in kunit_tool

[Brendan Higgins <brendanhiggins@google.com>]

On Tue, Jun 1, 2021 at 8:46 AM Daniel Latypov <dlatypov@google.com> wrote:
>
> On Fri, May 28, 2021 at 12:59 AM David Gow <davidgow@google.com> wrote:
> >
> > Add support for the SKIP directive to kunit_tool's TAP parser.
> >
> > Skipped tests now show up as such in the printed summary. The number of
> > skipped tests is counted, and if all tests in a suite are skipped, the
> > suite is also marked as skipped. Otherwise, skipped tests do affect the
> > suite result.
> >
> > Example output:
> > [00:22:34] ======== [SKIPPED] example_skip ========
> > [00:22:34] [SKIPPED] example_skip_test # SKIP this test should be skipped
> > [00:22:34] [SKIPPED] example_mark_skipped_test # SKIP this test should be skipped
> > [00:22:34] ============================================================
> > [00:22:34] Testing complete. 2 tests run. 0 failed. 0 crashed. 2 skipped.
> >
> > Signed-off-by: David Gow <davidgow@google.com>
>
> Reviewed-by: Daniel Latypov <dlatypov@google.com>
>
> Some minor remarks, but this looks good to me.
>
> Though I'm surprised there has not been any bikeshedding done about
> the color of the SKIPPED output.
> So I'll throw an opinion out there.
> I think yellow is fine, but I did somewhat recently change another
> similar tool to go from yellow => cyan for SKIPPED. The motivation
> there was to have a color for "flaky" tests that stood out, and the
> most appropriate ANSI color seemed to be yellow (between green for
> PASSED and red for FAILED).
> And I don't know if KUnit tool will ever get to the point where we
> automatically rerun tests on failure, as I can see an argument for
> that logic living a layer above.

I do have some sympathy for using a different color for each type of
message. I am not arguing against cyan, but I am also OK with yellow.
However, if we get to the point where we support flaky warnings, what
if we used orange for flaky?