On 10/16/19 4:18 PM, Brendan Higgins wrote:
On Fri, Oct 11, 2019 at 6:19 AM Theodore Y. Ts'o <tytso@xxxxxxx> wrote:
On Fri, Oct 11, 2019 at 03:05:43AM -0700, Brendan Higgins wrote:
That's an interesting point. Should we try to establish a pattern for
how tests should be configured? My *very long term* goal is to
eventually have tests able to be built and run without any kind of
kernel of any kind, but I don't think that having a single config for
all tests in a subsystem gets in the way of that, so I don't think I
have a strong preference in terms of what I want to do.
Nevertheless, I think establishing patterns is good. Do we want to try
to follow Ted's preference as a general rule from now on?
As I suggested on another thread (started on kunit-dev, but Brendan
has cc'ed in linux-kselftest), I think it might really work well if
For reference, that thread can be found here:
https://lore.kernel.org/linux-kselftest/CAFd5g46+OMmP8mYsH8vcpMpdOeYryp=1Lsab4Hy6pAhWjX5-4Q@xxxxxxxxxxxxxx/
"make kunit" runs all of the kunit tests automatically. As we add
more kunit tests, finding all of the CONFIG options so they can be
added to the kunitconfig file is going to be hard, so kunit.py really
needs an --allconfig which does this automatically.
Along these lines, perhaps we should state that as a general rule the
CONFIG option for Kunit tests should only depend on KUINIT, and use
select to enable other dependencies. i.e., for the ext4 kunit tests,
I support this. Although I think that we will eventually find
ourselves in a position where it is not possible to satisfy all
dependencies for all KUnit tests, this may get us far enough along
that the problem may be easier, or may work well enough for a long
time. It's hard to say. In anycase, I think it makes sense for a unit
test config to select its dependencies. I also think it makes sense to
make each subsystem have a master config for all KUnit tests.
it should look like this:
config EXT4_KUNIT_TESTS
bool "KUnit test for ext4 inode"
select EXT4_FS
depends on KUNIT
...
In the current patch, we use "depends on EXT4_FS", which meant that
when I first added "CONFIG_EXT4_KUNIT_TESTS=y" to the kunitconfig
file, I got the following confusing error message:
% ./tools/testing/kunit/kunit.py run
Regenerating .config ...
ERROR:root:Provided Kconfig is not contained in validated .config!
Using "select EXT4_FS" makes it much easier to enable the ext4 kunit
tests in kunitconfig. At the moment requiring that we two lines to
kunitconfig to enable ext4 isn't _that_ bad:
CONFIG_EXT4_FS=y
CONFIG_EXT4_KUNIT_TESTS=y
but over time, if many subsystems start adding unit tests, the
overhead of managing the kunitconfig file is going to get unwieldy.
Agreed.
Hence my suggestion that we just make all Kunit CONFIG options depend
only on CONFIG_KUNIT.
Sounds good to me. I am a bit behind in reviews. I will review v5.
That makes sense for now. I think we will eventually reach a point
where that may not be enough or that we may have KUnit configs which
are mutually exclusive; nevertheless, I imagine that this may be a
good short term solution for a decent amount of time.
Shuah suggested an alternative in the form of config fragments. I
think Ted's solution is going to be easier to maintain in the short
term. Any other thoughts?
I don't recall commenting on config fragments per say. I think I was
asking if we can make the test data dynamic as opposed to static.
Lurii said it might be difficult to do it that way since we are doing
this at boot time + we are testing extfs. If not for this test, for
others, it would good to explore option to make test data dynamic,
so it would be easier to use custom test data.
I don't really buy the argument that unit tests should be deterministic
Possibly, but I would opt for having the ability to feed test data.
thanks,
-- Shuah
