On Thu, 2 Apr 2020, Greg KH wrote:
On Thu, Apr 02, 2020 at 04:27:35PM +0100, Alan Maguire wrote:
On Wed, 1 Apr 2020, shuah wrote:
Hi Alan,
On 3/26/20 8:25 AM, Alan Maguire wrote:
add debugfs support for displaying kunit test suite results; this is especially useful for module-loaded tests to allow disentangling of test result display from other dmesg events. debugfs support is provided if CONFIG_KUNIT_DEBUGFS=y.
As well as printk()ing messages, we append them to a per-test log.
Signed-off-by: Alan Maguire alan.maguire@oracle.com Reviewed-by: Brendan Higgins brendanhiggins@google.com Reviewed-by: Frank Rowand frank.rowand@sony.com
include/kunit/test.h | 54 +++++++++++++++--- lib/kunit/Kconfig | 8 +++
Missing defaults for config options?
lib/kunit/Makefile | 4 ++ lib/kunit/debugfs.c | 116 ++++++++++++++++++++++++++++++++++++++ lib/kunit/debugfs.h | 30 ++++++++++ lib/kunit/kunit-test.c | 4 +- lib/kunit/test.c | 147 ++++++++++++++++++++++++++++++++++++++----------- 7 files changed, 322 insertions(+), 41 deletions(-) create mode 100644 lib/kunit/debugfs.c create mode 100644 lib/kunit/debugfs.h
snip
diff --git a/lib/kunit/Kconfig b/lib/kunit/Kconfig index 065aa16..95d12e3 100644 --- a/lib/kunit/Kconfig +++ b/lib/kunit/Kconfig @@ -14,6 +14,14 @@ menuconfig KUNIT if KUNIT +config KUNIT_DEBUGFS
- bool "KUnit - Enable /sys/kernel/debug/kunit debugfs representation"
- help
Enable debugfs representation for kunit. Currently this consistsof /sys/kernel/debug/kunit/<test_suite>/results files for eachtest suite, which allow users to see results of the last test suiterun that occurred.Any reason why there is default for this option?
Same for all other options. I am sending pull request for now without defaults. Would like to see this fixed for rc2.
Sure, I'll send a patch shortly. Just wanted to get opinions on what those defaults should be; my working assumption is
- CONFIG_KUNIT should be default n;
No default means 'n', so there's no need to say that at all.
Great!
- CONFIG_KUNIT_DEBUGFS should be default y (enabled by default if CONFIG_KUNIT is set);
Why? If it's is 'y', then don't even make it an option at all, just always have it :)
'y' is almost always reserved for "your machine will not function properly without this enabled".
Okay, so in this case not having a default makes sense I think (we added the option as saying 'n' avoids allocating per-test loggging memory which might cause problems on low-memory systems).
- CONFIG_KUNIT_TEST, CONFIG_KUNIT_EXAMPLE_TEST should be default n
So no need to specify anything, 'n' is the default.
Excellent! So based on the above seems like we don't need to add any defaults here I think. Shuah/Brendan, let me know if I'm missing anything here. Thanks!
Alan
thanks,
greg k-h