From: John Hubbard <jhubbard@nvidia.com>
To: Marco Elver <elver@google.com>, Kees Cook <kees@kernel.org>
Cc: David Gow <davidgow@google.com>,
Brendan Higgins <brendan.higgins@linux.dev>,
Rae Moar <rmoar@google.com>, Jonathan Corbet <corbet@lwn.net>,
Linus Torvalds <torvalds@linux-foundation.org>,
<linux-kselftest@vger.kernel.org>, <kunit-dev@googlegroups.com>,
<linux-doc@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
<linux-hardening@vger.kernel.org>
Subject: Re: [PATCH v2] Documentation: KUnit: Update filename best practices
Date: Mon, 22 Jul 2024 16:49:35 -0700 [thread overview]
Message-ID: <d91ed522-9df6-4a83-9cc4-9f71f160f3e4@nvidia.com> (raw)
In-Reply-To: <Zp4spjsaqQ85fVuk@elver.google.com>
On 7/22/24 2:55 AM, Marco Elver wrote:
> On Sat, Jul 20, 2024 at 09:54AM -0700, Kees Cook wrote:
...
> I'm more confused now. This is just moving tests further away from what
> they are testing for no good reason. If there's a directory "foo", then
> moving things to "tests/foo" is unclear. It's unclear if "tests" is
> inside parent of "foo" or actually a subdir of "foo". Per the paragraph
> above, I inferred it's "foo/tests/foo/...", which is horrible. If it's
> "../tests/foo/..." it's also bad because it's just moving tests further
> away from what they are testing.
>
> And keeping tests close to the source files under test is generally
> considered good practice, as it avoids the friction required to discover
> where tests live. Moving tests to "../tests" or "../../*/tests" in the
> majority of cases is counterproductive.
>
> It is more important for people to quickly discover tests nearby and
> actually run them, vs. having them stashed away somewhere so they don't
> bother us.
>
> While we can apply common sense, all too often someone follows these
> rules blindly and we end up with a mess.
>
Here, you've actually made a good argument for "blindly" following the
new naming/location conventions: it's easier to find things if a
standard naming and location convention is in place. Especially if
we document it. Now if only someone would post a patch with such
documentation... :)
I would add that the "_kunit" part of the name is especially helpful,
because (as I mentioned earlier) these tests really are different enough
that it's worth calling out. You can run them simply by loading the
kernel module.
So if I want to quickly run kunit tests, searching for "*_kunit.c" does
help with that.
thanks,
--
John Hubbard
NVIDIA
next prev parent reply other threads:[~2024-07-22 23:49 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-07-20 16:54 [PATCH v2] Documentation: KUnit: Update filename best practices Kees Cook
2024-07-20 17:59 ` John Hubbard
2024-07-20 18:12 ` Kees Cook
2024-07-22 9:55 ` Marco Elver
2024-07-22 23:49 ` John Hubbard [this message]
2024-07-23 6:25 ` Marco Elver
2024-07-24 5:04 ` David Gow
2024-07-24 5:06 ` David Gow
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=d91ed522-9df6-4a83-9cc4-9f71f160f3e4@nvidia.com \
--to=jhubbard@nvidia.com \
--cc=brendan.higgins@linux.dev \
--cc=corbet@lwn.net \
--cc=davidgow@google.com \
--cc=elver@google.com \
--cc=kees@kernel.org \
--cc=kunit-dev@googlegroups.com \
--cc=linux-doc@vger.kernel.org \
--cc=linux-hardening@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-kselftest@vger.kernel.org \
--cc=rmoar@google.com \
--cc=torvalds@linux-foundation.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox