Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:18 as permitted sender) client-ip=2620:137:e000::1:18;
MIME-Version: 1.0
References: <20220430045639.839186-1-davidgow@google.com>
In-Reply-To: <20220430045639.839186-1-davidgow@google.com>
From:   Daniel Latypov <dlatypov@google.com>
Date:   Mon, 2 May 2022 17:37:13 -0500
Message-ID: <CAGS_qxrVnDnUD=0f8vX5C2SWG2mxxXdAWU9A+P7h-K2Y1nm88A@mail.gmail.com>
Subject: Re: [PATCH] kunit: tool: Add list of all valid test configs on UML
To:     David Gow <davidgow@google.com>
Cc:     Brendan Higgins <brendanhiggins@google.com>,
        Shuah Khan <skhan@linuxfoundation.org>,
        kunit-dev@googlegroups.com, linux-kselftest@vger.kernel.org,
        linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Fri, Apr 29, 2022 at 11:56 PM David Gow <davidgow@google.com> wrote:
>
> It's often desirable (particularly in test automation) to run as many
> tests as possible. This config enables all the tests which work as
> builtins under UML at present, increasing the total tests run from 156
> to 342 (not counting 36 'skipped' tests).

Just to clear up potential confusion for others, I'll note that these
aren't counting test cases.
This is from kunit.py's output, so it counts each parameter from
parameterized tests as "subtests."

Copying my command from
https://kunit-review.googlesource.com/c/linux/+/5249, one can use this
to count the # of test cases.
$ ./tools/testing/kunit/kunit.py run --kunitconfig=...
--raw_output=kunit --kernel_args=kunit.action=list | egrep
'^[a-z0-9_-]+\.[a-z0-9_-]+'

I see this enabling a total of 260 test _cases_ (including skipped).

The default (basically just CONFIG_KUNIT_ALL_TESTS=y) gives 192
(including skipped).

>
> They can be run with:
> ./tools/testing/kunit/kunit.py run
> --kunitconfig=./tools/testing/kunit/configs/all_tests_uml.config
>
> This acts as an in-between point between the KUNIT_ALL_TESTS config
> (which enables only tests whose dependencies are already enabled), and
> the kunit_tool --alltests option, which tries to use allyesconfig,
> taking a very long time to build and breaking very often.
>
> Signed-off-by: David Gow <davidgow@google.com>

Tested-by: Daniel Latypov <dlatypov@google.com>

Looks good to me, some small comments below.

> ---
>  .../kunit/configs/all_tests_uml.config        | 37 +++++++++++++++++++
>  1 file changed, 37 insertions(+)
>  create mode 100644 tools/testing/kunit/configs/all_tests_uml.config
>
> diff --git a/tools/testing/kunit/configs/all_tests_uml.config b/tools/testing/kunit/configs/all_tests_uml.config
> new file mode 100644
> index 000000000000..bdee36bef4a3
> --- /dev/null
> +++ b/tools/testing/kunit/configs/all_tests_uml.config
> @@ -0,0 +1,37 @@
> +# This config enables as many tests as possible under UML.
> +# It is intended for use in continuous integration systems and similar for
> +# automated testing of as much as possible.
> +# The config is manually maintained, though it uses KUNIT_ALL_TESTS=y to enable
> +# any tests whose dependencies are already satisfied. Please feel free to add
> +# more options if they any new tests.

missing: "enable"?
"if they enable any new tests"

Hmm, should we state a preference for how heavy (time or
resource-wise) tests should be?
Because the comment says it's meant for automation, but I can imagine
humans wanting to run it.
(I'm completely fine with us not stating one, just throwing the idea
out there for discussion)

Currently, I get this with an incremental rebuild:
Elapsed time: 141.627s total, 1.384s configuring, 136.175s building,
3.970s running

But we do have tests on other arches that take ~30s to run (kfence),
for example.
Would such tests be candidates for inclusion in this file?
Or is it only problematic when they start taking a couple minutes each?