diff mbox series

ext4: add .kunitconfig fragment to enable ext4-specific tests

Message ID 20210210013206.136227-1-dlatypov@google.com
State Awaiting Upstream
Headers show
Series ext4: add .kunitconfig fragment to enable ext4-specific tests | expand

Commit Message

Daniel Latypov Feb. 10, 2021, 1:32 a.m. UTC
As of [1], we no longer want EXT4_KUNIT_TESTS and others to `select`
their deps. This means it can get harder to get all the right things
selected as we gain more tests w/ more deps over time.

This patch (and [2]) proposes we store kunitconfig fragments in-tree to
represent sets of tests. (N.B. right now we only have one ext4 test).

There's still a discussion to be had about how to have a hierarchy of
these files (e.g. if one wanted to test all of fs/, not just fs/ext4).

But this fragment would likely be a leaf node and isn't blocked on
deciding if we want `import` statements and the like.

Usage
=====

Before [2] (on its way to being merged):
  $ cp fs/ext4/.kunitconfig .kunit/
  $ ./tools/testing/kunit.py run

After [2]:
  $ ./tools/testing/kunit.py run --kunitconfig=fs/ext4/.kunitconfig

".kunitconfig" vs "kunitconfig"
===============================

See also: commit 14ee5cfd4512 ("kunit: Rename 'kunitconfig' to '.kunitconfig'").
* The bit about .gitignore exluding it by default is now a con, however.
* But there are a lot of directories with files that begin with "k" and
  so this could cause some annoyance w/ tab completion*
* This is the name kunit.py expects right now, so some people are used
  to .kunitconfig over "kunitconfig"

[1] https://lore.kernel.org/linux-ext4/20210122110234.2825685-1-geert@linux-m68k.org/
[2] https://git.kernel.org/pub/scm/linux/kernel/git/shuah/linux-kselftest.git/commit/?h=kunit&id=243180f5924ed27ea417db39feb7f9691777688e

* 372/5556 directories isn't too much, but still not a small number:
$ find -type f -name 'k*' | xargs dirname | sort -u | wc -l
372

Signed-off-by: Daniel Latypov <dlatypov@google.com>
---
 fs/ext4/.kunitconfig | 3 +++
 1 file changed, 3 insertions(+)
 create mode 100644 fs/ext4/.kunitconfig


base-commit: 88bb507a74ea7d75fa49edd421eaa710a7d80598

Comments

Theodore Ts'o Feb. 10, 2021, 2:33 a.m. UTC | #1
On Tue, Feb 09, 2021 at 05:32:06PM -0800, Daniel Latypov wrote:
> 
> After [2]:
>   $ ./tools/testing/kunit.py run --kunitconfig=fs/ext4/.kunitconfig

Any chance that in the future this might become:

$ ./tools/testing/kunit.py run --kunitconfig=fs/ext4

Or better yet, syntactic sugar like:

$ ./tools/testing/kunit.py test fs/ext4

would be really nice.

						- Ted
Daniel Latypov Feb. 10, 2021, 5:44 a.m. UTC | #2
On Tue, Feb 9, 2021 at 6:33 PM Theodore Ts'o <tytso@mit.edu> wrote:
>
> On Tue, Feb 09, 2021 at 05:32:06PM -0800, Daniel Latypov wrote:
> >
> > After [2]:
> >   $ ./tools/testing/kunit.py run --kunitconfig=fs/ext4/.kunitconfig
>
> Any chance that in the future this might become:
>
> $ ./tools/testing/kunit.py run --kunitconfig=fs/ext4

I've been in favor of something like that for a while, but haven't
gotten folks to agree on the details.

Using bazel-like syntax for a bit, I'd really like it if we had some
easy way to do
$ kunit test //fs/...  # run all fs tests across all subdirs

But since there's the possibility of having tests w/ incompatible
requirements, I don't know that kunit.py can support it.
(Tbh, I think just concatenating fragments would probably just work
99% of the time so kunit.py could get away with doing that).

So --kunitconfig=<path> is currently a compromise to give us a less
controversial way of providing one-liners for testing a whole
subdirectory.

I don't think there'd be too much opposition for --kunitconfig to
automatically append ".kunitconfig" when passed a directory.
But there might be some, since a reader might think --kunitconfig=dir/
means it's recursing over all subdirs.

>
> Or better yet, syntactic sugar like:
>
> $ ./tools/testing/kunit.py test fs/ext4

The positional argument for run/exec is probably going to be taken by:
https://lore.kernel.org/linux-kselftest/20210206000854.2037923-1-dlatypov@google.com/
https://git.kernel.org/pub/scm/linux/kernel/git/shuah/linux-kselftest.git/commit/?h=kunit&id=5d31f71efcb6bce56ca3ab92eed0c8f2dbcc6f9a

So we'd see something like:
$ ./tools/testing/kunit.py run --kunitconfig=fs/ext4 '*inode*'

Or if we set and followed naming conventions:
$ ./tools/testing/kunit.py run --alltests "ext4-*"
(this would take a lot longer to build however...)

Filtering could also let us curate only a few, less granular
.kunitconfig fragments (at the cost of higher build time).
E.g.
$ ./tools/testing/kunit.py run --kunitconfig=fs/ "ext4-*"

>
> would be really nice.
>
>                                                 - Ted
Theodore Ts'o Feb. 12, 2021, 4:22 a.m. UTC | #3
On Tue, Feb 09, 2021 at 05:32:06PM -0800, Daniel Latypov wrote:
> As of [1], we no longer want EXT4_KUNIT_TESTS and others to `select`
> their deps. This means it can get harder to get all the right things
> selected as we gain more tests w/ more deps over time.
> 
> This patch (and [2]) proposes we store kunitconfig fragments in-tree to
> represent sets of tests. (N.B. right now we only have one ext4 test).
> 
> There's still a discussion to be had about how to have a hierarchy of
> these files (e.g. if one wanted to test all of fs/, not just fs/ext4).
> 
> But this fragment would likely be a leaf node and isn't blocked on
> deciding if we want `import` statements and the like.
> 
> Usage
> =====
> 
> Before [2] (on its way to being merged):
>   $ cp fs/ext4/.kunitconfig .kunit/
>   $ ./tools/testing/kunit.py run
> 
> After [2]:
>   $ ./tools/testing/kunit.py run --kunitconfig=fs/ext4/.kunitconfig

Thanks, applied, with one minor fixup.  The path to kunit.py is

./tools/testing/kunit/kunit.py

						- Ted
Daniel Latypov Feb. 23, 2021, 12:23 a.m. UTC | #4
On Tue, Feb 9, 2021 at 6:33 PM Theodore Ts'o <tytso@mit.edu> wrote:
>
> On Tue, Feb 09, 2021 at 05:32:06PM -0800, Daniel Latypov wrote:
> >
> > After [2]:
> >   $ ./tools/testing/kunit.py run --kunitconfig=fs/ext4/.kunitconfig
>
> Any chance that in the future this might become:
>
> $ ./tools/testing/kunit.py run --kunitconfig=fs/ext4

For future reference, this patch would make ^ work.
https://lore.kernel.org/linux-kselftest/20210222225241.201145-1-dlatypov@google.com

(This time w/o the typo in the path to kunit.py :)

>
> Or better yet, syntactic sugar like:
>
> $ ./tools/testing/kunit.py test fs/ext4
>
> would be really nice.
>
>                                                 - Ted
diff mbox series

Patch

diff --git a/fs/ext4/.kunitconfig b/fs/ext4/.kunitconfig
new file mode 100644
index 000000000000..bf51da7cd9fc
--- /dev/null
+++ b/fs/ext4/.kunitconfig
@@ -0,0 +1,3 @@ 
+CONFIG_KUNIT=y
+CONFIG_EXT4_FS=y
+CONFIG_EXT4_KUNIT_TESTS=y