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: <20210930222048.1692635-1-dlatypov@google.com> <20210930222048.1692635-5-dlatypov@google.com>
 <CABVgOS=SVHSrnTaa0hYsyn8KV_A7FCTaAmDx5ST6Jb2k4TXWSg@mail.gmail.com> <CAFd5g45qD0H1sO2n-NcgpVKm-QRRBVSHXLyMRr9mmJxKDgpWMw@mail.gmail.com>
In-Reply-To: <CAFd5g45qD0H1sO2n-NcgpVKm-QRRBVSHXLyMRr9mmJxKDgpWMw@mail.gmail.com>
From:   Daniel Latypov <dlatypov@google.com>
Date:   Wed, 6 Oct 2021 10:26:25 -0700
Message-ID: <CAGS_qxqoGN50GL+TRV0rBUth+EEeS4USfH5AergZKBBV+oCEoA@mail.gmail.com>
Subject: Re: [PATCH v4 4/4] kunit: tool: support running each suite/test separately
To:     Brendan Higgins <brendanhiggins@google.com>
Cc:     David Gow <davidgow@google.com>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        KUnit Development <kunit-dev@googlegroups.com>,
        "open list:KERNEL SELFTEST FRAMEWORK" 
        <linux-kselftest@vger.kernel.org>,
        Shuah Khan <skhan@linuxfoundation.org>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Wed, Oct 6, 2021 at 10:23 AM 'Brendan Higgins' via KUnit
Development <kunit-dev@googlegroups.com> wrote:
>
> On Thu, Sep 30, 2021 at 4:05 PM David Gow <davidgow@google.com> wrote:
> >
> > On Fri, Oct 1, 2021 at 6:21 AM Daniel Latypov <dlatypov@google.com> wrote:
> > >
> > > The new --run_isolated flag makes the tool boot the kernel once per
> > > suite or test, preventing leftover state from one suite to impact the
> > > other. This can be useful as a starting point to debugging test
> > > hermeticity issues.
> > >
> > > Note: it takes a lot longer, so people should not use it normally.
> > >
> > > Consider the following very simplified example:
> > >
> > >   bool disable_something_for_test = false;
> > >   void function_being_tested() {
> > >     ...
> > >     if (disable_something_for_test) return;
> > >     ...
> > >   }
> > >
> > >   static void test_before(struct kunit *test)
> > >   {
> > >     disable_something_for_test = true;
> > >     function_being_tested();
> > >     /* oops, we forgot to reset it back to false */
> > >   }
> > >
> > >   static void test_after(struct kunit *test)
> > >   {
> > >     /* oops, now "fixing" test_before can cause test_after to fail! */
> > >     function_being_tested();
> > >   }
> > >
> > > Presented like this, the issues are obvious, but it gets a lot more
> > > complicated to track down as the amount of test setup and helper
> > > functions increases.
> > >
> > > Another use case is memory corruption. It might not be surfaced as a
> > > failure/crash in the test case or suite that caused it. I've noticed in
> > > kunit's own unit tests, the 3rd suite after might be the one to finally
> > > crash after an out-of-bounds write, for example.
> > >
> > > Example usage:
> > >
> > > Per suite:
> > > $ ./tools/testing/kunit/kunit.py run --kunitconfig=lib/kunit --run_isolated=suite
> > > ...
> > > Starting KUnit Kernel (1/7)...
> > > ============================================================
> > > ======== [PASSED] kunit_executor_test ========
> > > ....
> > > Testing complete. 5 tests run. 0 failed. 0 crashed. 0 skipped.
> > > Starting KUnit Kernel (2/7)...
> > > ============================================================
> > > ======== [PASSED] kunit-try-catch-test ========
> > > ...
> > >
> > > Per test:
> > > $ ./tools/testing/kunit/kunit.py run --kunitconfig=lib/kunit --run_isolated=test
> > > Starting KUnit Kernel (1/23)...
> > > ============================================================
> > > ======== [PASSED] kunit_executor_test ========
> > > [PASSED] parse_filter_test
> > > ============================================================
> > > Testing complete. 1 tests run. 0 failed. 0 crashed. 0 skipped.
> > > Starting KUnit Kernel (2/23)...
> > > ============================================================
> > > ======== [PASSED] kunit_executor_test ========
> > > [PASSED] filter_subsuite_test
> > > ...
> > >
> > > It works with filters as well:
> > > $ ./tools/testing/kunit/kunit.py run --kunitconfig=lib/kunit --run_isolated=suite example
> > > ...
> > > Starting KUnit Kernel (1/1)...
> > > ============================================================
> > > ======== [PASSED] example ========
> > > ...
> > >
> > > It also handles test filters, '*.*skip*' runs these 3 tests:
> > >   kunit_status.kunit_status_mark_skipped_test
> > >   example.example_skip_test
> > >   example.example_mark_skipped_test
> > >
> > > Signed-off-by: Daniel Latypov <dlatypov@google.com>
> > > Reviewed-by: David Gow <davidgow@google.com>
> > > ---
> >
> > Thanks -- this version works for me.
> >
> > I think the requirement that test and suite names contain neither full
> > stops nor spaces is a reasonable one. There aren't any current tests
> > which would break it, as far as I know.
>
> I agree. Is this currently codified in the test naming conventions document?

https://www.kernel.org/doc/html/latest/dev-tools/kunit/style.html

We have
  Test suites are named after the subsystem they are part of. If a
subsystem contains several suites, the specific area under test should
be appended to the subsystem name, separated by an underscore.
  As tests are C functions, they should be named and written in
accordance with the kernel coding style.

So nothing is explicitly stated for test suites.
Just a preference for underscores, sorta implying you shouldn't use
spaces or other delimiters like periods.

>
> --
> You received this message because you are subscribed to the Google Groups "KUnit Development" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to kunit-dev+unsubscribe@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/kunit-dev/CAFd5g45qD0H1sO2n-NcgpVKm-QRRBVSHXLyMRr9mmJxKDgpWMw%40mail.gmail.com.