Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20210528075932.347154-1-davidgow@google.com> <20210528075932.347154-2-davidgow@google.com>
 <CAGS_qxpg7PdGPiP5kmzBthh=eHd+SYmyvUitQV40Weej3wD4QA@mail.gmail.com>
In-Reply-To: <CAGS_qxpg7PdGPiP5kmzBthh=eHd+SYmyvUitQV40Weej3wD4QA@mail.gmail.com>
From:   Brendan Higgins <brendanhiggins@google.com>
Date:   Fri, 4 Jun 2021 14:30:13 -0700
Message-ID: <CAFd5g44VS==1yULMRS-JMxrArj9GFJRkuDCxoxnZHcj3PVbFHg@mail.gmail.com>
Subject: Re: [PATCH v2 2/4] kunit: tool: Support skipped tests in kunit_tool
To:     Daniel Latypov <dlatypov@google.com>
Cc:     David Gow <davidgow@google.com>,
        Alan Maguire <alan.maguire@oracle.com>,
        Shuah Khan <skhan@linuxfoundation.org>,
        Marco Elver <elver@google.com>,
        KUnit Development <kunit-dev@googlegroups.com>,
        "open list:KERNEL SELFTEST FRAMEWORK" 
        <linux-kselftest@vger.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

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?