Re: [PATCH v4] Documentation: kunit: add tips for running KUnit
On Wed, Apr 14, 2021 at 3:23 PM Daniel Latypov wrote: > > This is long overdue. > > There are several things that aren't nailed down (in-tree > .kunitconfig's), or partially broken (GCOV on UML), but having them > documented, warts and all, is better than having nothing. > > This covers a bunch of the more recent features > * kunit_filter_glob > * kunit.py run --kunitconfig > * slightly more detail on building tests as modules > * CONFIG_KUNIT_DEBUGFS > > By my count, the only headline features now not mentioned are the KASAN > integration and KernelCI json output support (kunit.py run --json). > > And then it also discusses how to get code coverage reports under UML > and non-UML since this is a question people have repeatedly asked. > > Non-UML coverage collection is no different from normal, but we should > probably explicitly call this out. > > As for UML, I was able to get it working again with two small hacks.* > E.g. with CONFIG_KUNIT=y && CONFIG_KUNIT_ALL_TESTS=y > Overall coverage rate: > lines..: 15.1% (18294 of 120776 lines) > functions..: 16.8% (1860 of 11050 functions) > > Note: this doesn't document --alltests since this is not stable yet. > Hopefully being run more frequently as part of KernelCI will help... > > *Using gcc/gcov-6 and not using uml_abort() in os_dump_core(). > I've documented these hacks in "Notes" but left TODOs for > brendanhigg...@google.com who tracked down the runtime issue in GCC. > To be clear: these are not issues specific to KUnit, but rather to UML. > > Signed-off-by: Daniel Latypov > Reviewed-by: David Gow Reviewed-by: Brendan Higgins
Re: [PATCH v4 1/3] kunit: make test->lock irq safe
On Tue, Apr 13, 2021 at 3:07 AM wrote: > > From: Vlastimil Babka > > The upcoming SLUB kunit test will be calling kunit_find_named_resource() from > a context with disabled interrupts. That means kunit's test->lock needs to be > IRQ safe to avoid potential deadlocks and lockdep splats. > > This patch therefore changes the test->lock usage to spin_lock_irqsave() > and spin_unlock_irqrestore(). > > Signed-off-by: Vlastimil Babka > Signed-off-by: Oliver Glitta Reviewed-by: Brendan Higgins Thanks!
Re: [PATCH] Documentation: dev-tools: Add Testing Overview
On Mon, Apr 12, 2021 at 3:43 AM Marco Elver wrote:
>
> On Sat, 10 Apr 2021 at 13:53, Daniel Latypov wrote:
> > On Sat, Apr 10, 2021 at 12:05 AM David Gow wrote:
> [...]
> > > +
> > > +
> > > +Sanitizers
> > > +==
> > > +
>
> The "sanitizers" have originally been a group of tools that relied on
> compiler instrumentation to perform various dynamic analysis
> (initially ASan, TSan, MSan for user space). The term "sanitizer" has
> since been broadened to include a few non-compiler based tools such as
> GWP-ASan in user space, of which KFENCE is its kernel cousin but it
> doesn't have "sanitizer" in its name (because we felt GWP-KASAN was
> pushing it with the acronyms ;-)). Also, these days we have HW_TAGS
> based KASAN, which doesn't rely on compiler instrumentation but
> instead on MTE in Arm64.
>
> Things like kmemleak have never really been called a sanitizer, but
> they _are_ dynamic analysis tools.
>
> So to avoid confusion, in particular avoid establishing "sanitizers"
> to be synonymous with "dynamic analysis" ("all sanitizers are dynamic
> analysis tools, but not all dynamic analysis tools are sanitizers"),
> the section here should not be called "Sanitizers" but "Dynamic
> Analysis Tools". We could have a subsection "Sanitizers", but I think
> it's not necessary.
>
> > > +The kernel also supports a number of sanitizers, which attempt to detect
> > > +classes of issues when the occur in a running kernel. These typically
> >
> > *they occur
> >
> > > +look for undefined behaviour of some kind, such as invalid memory
> > > accesses,
> > > +concurrency issues such as data races, or other undefined behaviour like
> > > +integer overflows.
> > > +
> > > +* :doc:`kmemleak` (Kmemleak) detects possible memory leaks.
> > > +* :doc:`kasan` detects invalid memory accesses such as out-of-bounds and
> > > + use-after-free errors.
> > > +* :doc:`ubsan` detects behaviour that is undefined by the C standard,
> > > like
> > > + integer overflows.
> > > +* :doc:`kcsan` detects data races.
> > > +* :doc:`kfence` is a low-overhead detector of memory issues, which is
> > > much
> > > + faster than KASAN and can be used in production.
> >
> > Hmm, it lives elsewhere, but would also calling out lockdep here be useful?
> > I've also not heard anyone call it a sanitizer before, but it fits the
> > definition you've given.
> >
> > Now that I think about it, I've never looked for documentation on it,
> > is this the best page?
> > https://www.kernel.org/doc/html/latest/locking/lockdep-design.html
>
> Not a "sanitizer" but our sanitizers are all dynamic analysis tools,
> and lockdep is also a dynamic analysis tool.
>
> If we want to be pedantic, the kernel has numerous options to add
> "instrumentation" (compiler based or explicit) that will detect some
> kind of error at runtime. Most of them live in lib/Kconfig.debug. I
> think mentioning something like that is in scope of this document, but
> we certainly can't mention all debug tools the kernel has to offer.
> Mentioning the big ones like above and then referring to
> lib/Kconfig.debug is probably fine.
>
> Dmitry recently gave an excellent talk on some of this:
> https://www.youtube.com/watch?v=ufcyOkgFZ2Q
Good point Marco, and we (KUnit - myself, Daniel, and David) gave a
talk on KUnit at LF. Also, I think Shuah is/has given one (soon)?
Might be a good idea to link those here?
Re: [PATCH] Documentation: dev-tools: Add Testing Overview
On Sat, Apr 10, 2021 at 12:05 AM David Gow wrote: > > The kernel now has a number of testing and debugging tools, and we've > seen a bit of confusion about what the differences between them are. > > Add a basic documentation outlining the testing tools, when to use each, > and how they interact. > > This is a pretty quick overview rather than the idealised "kernel > testing guide" that'd probably be optimal, but given the number of times > questions like "When do you use KUnit and when do you use Kselftest?" > are being asked, it seemed worth at least having something. Hopefully > this can form the basis for more detailed documentation later. > > Signed-off-by: David Gow With the exception of some minor nits, I think the below will make a great initial testing overview guide! Thanks for getting the ball rolling on this! > --- > Documentation/dev-tools/index.rst| 3 + > Documentation/dev-tools/testing-overview.rst | 102 +++ > 2 files changed, 105 insertions(+) > create mode 100644 Documentation/dev-tools/testing-overview.rst > > diff --git a/Documentation/dev-tools/index.rst > b/Documentation/dev-tools/index.rst > index 1b1cf4f5c9d9..f590e5860794 100644 > --- a/Documentation/dev-tools/index.rst > +++ b/Documentation/dev-tools/index.rst > @@ -7,6 +7,8 @@ be used to work on the kernel. For now, the documents have > been pulled > together without any significant effort to integrate them into a coherent > whole; patches welcome! > > +A brief overview of testing-specific tools can be found in > :doc:`testing-overview`. > + I think I would like to make this a little more apparent. This index here is a bit bare bones and I think this testing-overview could be a good: "I am lost where do I start?" sort of doc. That being said, I am not sure what the best way to emphasize this might be. Maybe just have an intro paragraph here with some callout text like in a `note` or something like that. > .. class:: toc-title > >Table of contents > @@ -14,6 +16,7 @@ whole; patches welcome! > .. toctree:: > :maxdepth: 2 > > + testing-overview > coccinelle > sparse > kcov > diff --git a/Documentation/dev-tools/testing-overview.rst > b/Documentation/dev-tools/testing-overview.rst > new file mode 100644 > index ..8452adcb8608 > --- /dev/null > +++ b/Documentation/dev-tools/testing-overview.rst > @@ -0,0 +1,102 @@ > +.. SPDX-License-Identifier: GPL-2.0 > + > + > +Kernel Testing Guide > + > + > + > +There are a number of different tools for testing the Linux kernel, so > knowing > +when to use each of them can be a challenge. This document provides a rough > +overview of their differences, and how they fit together. > + > + > +Writing and Running Tests > += > + > +The bulk of kernel tests are written using either the :doc:`kselftest > +` or :doc:`KUnit ` frameworks. These both provide > +infrastructure to help make running tests and groups of tests easier, as well > +as providing helpers to aid in writing new tests. > + > +If you're looking to verify the behaviour of the Kernel — particularly > specific > +parts of the kernel — then you'll want to use `KUnit` or `kselftest`. > + > + > +The Difference Between KUnit and kselftest > +-- > + > +:doc:`KUnit ` is an entirely in-kernel system for "white box" > +testing: because test code is part of the kernel, it can access internal > +structures and functions which aren't exposed to userspace. > + > +`KUnit` tests therefore are best written against small, self-contained parts > +of the kernel, which can be tested in isolation. This aligns well with the > +concept of Unit testing. I think I might have pushed the "unit testing" stuff too hard in the past, but I feel that if you are going to mention "the concept of unit testing" it might be a good idea to link to an authoritative source on what unit testing is. Maybe link to Martin Fowler or something like that? > +For example, a KUnit test might test an individual kernel function (or even a > +single codepath through a function, such as an error handling case), rather > +than a feature as a whole. > + > +There is a KUnit test style guide which may give further pointers I know you linked the index page for KUnit above, but I think you might want to link the KUnit style guide here since you mention it. > +:doc:`kselftest `, on the other hand, is largely implemented in > +userspace, and tests are normal userspace scripts or programs. > + > +This makes it easier to write more complicated tests, or tests which need to > +manipulate the overall system state more (e.g., spawning processes, etc.). > +However, it's not possible to call kernel functions directly unless they're > +exposed to userspace (by a syscall, device, filesystem, etc.) Some tests to > +also provide a kernel module which is loaded by the test, though for tests > +which run mostly or
Re: [PATCH] Documentation: kunit: add tips for running KUnit
On Mon, Apr 12, 2021 at 10:27 AM Daniel Latypov wrote:
>
> hOn Fri, Apr 9, 2021 at 9:10 PM David Gow wrote:
> >
> > Thanks for writing this: it's good to have these things documented at last!
> >
> > There are definitely a few things this document points out which still
> > need deciding, which does make this document lean a bit into "design
> > discussion" territory in a few of the notes. This doesn't bother me --
> > it's an accurate description of the state of things -- but I wouldn't
> > want this documentation held up too long because of these sorts of
> > TODOs (and can definitely see how having too many of them might
> > discourage KUnit use a bit). Particularly things like the
> > ".kunitconfig" fragment file feature stuff: I feel that's something
> > better discussed on patches adding/using the feature than in the
> > documentation / reviews of the documentation, so I'd rather drop or
> > simplify those '..note:'s than bokeshed about it here (something I'm a
> > little guilty of below).
>
> I don't think we'll actually make progress on any of those in the near
> future though.
> So I figured it'd be best to accurately represent the state of the
> world ~somewhere.
>
> But it did feel a bit strange to do it here, so I'm not against removing it.
I actually like the accurate and upfront way that you spelled these things out.
> > Otherwise, a few minor comments and nitpicks:
> >
> > -- David
> >
> > On Sat, Apr 10, 2021 at 2:01 AM Daniel Latypov wrote:
> > >
> > > This is long overdue.
> > >
> > > There are several things that aren't nailed down (in-tree
> > > .kunitconfig's), or partially broken (GCOV on UML), but having them
> > > documented, warts and all, is better than having nothing.
> > >
> > > This covers a bunch of the more recent features
> > > * kunit_filter_glob
> > > * kunit.py run --kunitconfig
> > > * kunit.py run --alltests
> > > * slightly more detail on building tests as modules
> > > * CONFIG_KUNIT_DEBUGFS
> > >
> > > By my count, the only headline features now not mentioned are the KASAN
> > > integration and KernelCI json output support (kunit.py run --json).
> > >
> > > And then it also discusses how to get code coverage reports under UML
> > > and non-UML since this is a question people have repeatedly asked.
> > >
> > > Non-UML coverage collection is no differnt from normal, but we should
> > > probably explicitly call thsi out.
> >
> > Nit: typos in 'different' and 'this'.
> Fixed.
> >
> > >
> > > As for UML, I was able to get it working again with two small hacks.*
> > > E.g. with CONFIG_KUNIT=y && CONFIG_KUNIT_ALL_TESTS=y
> > > Overall coverage rate:
> > > lines..: 15.1% (18294 of 120776 lines)
> > > functions..: 16.8% (1860 of 11050 functions)
> > >
> > > *Switching to use gcc/gcov-6 and not using uml_abort().
> > > I've documented these hacks in "Notes" but left TODOs for
> > > brendanhigg...@google.com who tracked down the runtime issue in GCC.
> > > To be clear: these are not issues specific to KUnit, but rather to UML.
> >
> > (We should probably note where uml_abort() needs to be replaced if
> > we're mentioning this, though doing so below in the more detailed
> > section may be more useful.)
>
> Updated to
> *Using gcc/gcov-6 and not using uml_abort() in os_dump_core().
>
> I figured we'd be more precise in the documentation itself.
>
> >
> > >
> > > Signed-off-by: Daniel Latypov
> > > ---
> > > Documentation/dev-tools/kunit/index.rst | 1 +
> > > .../dev-tools/kunit/running_tips.rst | 278 ++
> > > Documentation/dev-tools/kunit/start.rst | 2 +
> > > 3 files changed, 281 insertions(+)
> > > create mode 100644 Documentation/dev-tools/kunit/running_tips.rst
> > >
> > > diff --git a/Documentation/dev-tools/kunit/index.rst
> > > b/Documentation/dev-tools/kunit/index.rst
> > > index 848478838347..7f7cf8d2ab20 100644
> > > --- a/Documentation/dev-tools/kunit/index.rst
> > > +++ b/Documentation/dev-tools/kunit/index.rst
> > > @@ -14,6 +14,7 @@ KUnit - Unit Testing for the Linux Kernel
> > > style
> > > faq
> > > tips
> > > + running_tips
> > >
> > > What is KUnit?
> > > ==
> > > diff --git a/Documentation/dev-tools/kunit/running_tips.rst
> > > b/Documentation/dev-tools/kunit/running_tips.rst
> > > new file mode 100644
> > > index ..d38e665e530f
> > > --- /dev/null
> > > +++ b/Documentation/dev-tools/kunit/running_tips.rst
> > > @@ -0,0 +1,278 @@
> > > +.. SPDX-License-Identifier: GPL-2.0
> > > +
> > > +
> > > +Tips For Running KUnit Tests
> > > +
> > > +
> > > +Using ``kunit.py run`` ("kunit tool")
> > > +=
> > > +
> > > +Running from any directory
> > > +--
> > > +
> > > +It can be handy to create a bash function like:
> > > +
> > > +.. code-block:: bash
> > > +
> > > + function run_kunit() {
> > > + ( cd "$(git rev-parse
Re: [PATCH v3 1/2] kunit: add a KUnit test for SLUB debugging functionality
On Thu, Apr 8, 2021 at 10:19 AM Daniel Latypov wrote:
>
> On Thu, Apr 8, 2021 at 3:30 AM Marco Elver wrote:
> >
> > On Tue, 6 Apr 2021 at 12:57, Vlastimil Babka wrote:
> > >
> > >
> > > On 4/1/21 11:24 PM, Marco Elver wrote:
> > > > On Thu, 1 Apr 2021 at 21:04, Daniel Latypov wrote:
> > > >> > }
> > > >> > #else
> > > >> > static inline bool slab_add_kunit_errors(void) { return
> > > >> > false; }
> > > >> > #endif
> > > >> >
> > > >> > And anywhere you want to increase the error count, you'd call
> > > >> > slab_add_kunit_errors().
> > > >> >
> > > >> > Another benefit of this approach is that if KUnit is disabled, there
> > > >> > is
> > > >> > zero overhead and no additional code generated (vs. the current
> > > >> > approach).
> > > >>
> > > >> The resource approach looks really good, but...
> > > >> You'd be picking up a dependency on
> > > >> https://lore.kernel.org/linux-kselftest/20210311152314.3814916-2-dlaty...@google.com/
> > > >> current->kunit_test will always be NULL unless CONFIG_KASAN=y &&
> > > >> CONFIG_KUNIT=y at the moment.
> > > >> My patch drops the CONFIG_KASAN requirement and opens it up to all
> > > >> tests.
> > > >
> > > > Oh, that's a shame, but hopefully it'll be in -next soon.
> > > >
> > > >> At the moment, it's just waiting another look over from Brendan or
> > > >> David.
> > > >> Any ETA on that, folks? :)
> > > >>
> > > >> So if you don't want to get blocked on that for now, I think it's fine
> > > >> to add:
> > > >> #ifdef CONFIG_SLUB_KUNIT_TEST
> > > >> int errors;
> > > >> #endif
> > > >
> > > > Until kunit fixes setting current->kunit_test, a cleaner workaround
> > > > that would allow to do the patch with kunit_resource, is to just have
> > > > an .init/.exit function that sets it ("current->kunit_test = test;").
> > > > And then perhaps add a note ("FIXME: ...") to remove it once the above
> > > > patch has landed.
> > > >
> > > > At least that way we get the least intrusive change for mm/slub.c, and
> > > > the test is the only thing that needs a 2-line patch to clean up
> > > > later.
> > >
> > > So when testing internally Oliver's new version with your suggestions
> > > (thanks
> > > again for those), I got lockdep splats because slab_add_kunit_errors is
> > > called
> > > also from irq disabled contexts, and kunit_find_named_resource will call
> > > spin_lock(>lock) that's not irq safe. Can we make the lock irq
> > > safe? I
> > > tried the change below and it makde the problem go away. If you agree, the
> > > question is how to proceed - make it part of Oliver's patch series and let
> > > Andrew pick it all with eventually kunit team's acks on this patch, or
> > > whatnot.
> >
> > From what I can tell it should be fine to make it irq safe (ack for
> > your patch below). Regarding patch logistics, I'd probably add it to
> > the series. If that ends up not working, we'll find out sooner or
> > later.
> >
> > (FYI, the prerequisite patch for current->kunit_test is in -next now.)
>
> Yep.
> There's also two follow-up patches in
> https://git.kernel.org/pub/scm/linux/kernel/git/shuah/linux-kselftest.git/log/?h=kunit
>
> >
> > KUnit maintainers, do you have any preferences?
>
> Poked offline and Brendan and David seemed fine either way.
> So probably just include it in this patch series for convenience.
>
> Brendan also mentioned KUnit used to use spin_lock_irqsave/restore()
> but had been told to not use it until necessary.
> See
> https://lore.kernel.org/linux-kselftest/20181016235120.138227-3-brendanhigg...@google.com/
> So I think there's no objections to the patch itself either.
>
> But I'd wait for Brendan to chime in to confirm.
That's correct. Before KUnit was accepted upstream, early versions of
the patchset used the irqsave/restore versions. I was asked to remove
them until they were necessary, and it looks like that time is now :-)
So yes, I would be happy to see this patch go in. Looks good to me the
way you have it below. Send it out as its own patch in your series and
I will give it a Reviewed-by.
Thanks!
> > > 8<
> > >
> > > commit ab28505477892e9824c57ac338c88aec2ec0abce
> > > Author: Vlastimil Babka
> > > Date: Tue Apr 6 12:28:07 2021 +0200
> > >
> > > kunit: make test->lock irq safe
> > >
> > > diff --git a/include/kunit/test.h b/include/kunit/test.h
> > > index 49601c4b98b8..524d4789af22 100644
> > > --- a/include/kunit/test.h
> > > +++ b/include/kunit/test.h
> > > @@ -515,8 +515,9 @@ kunit_find_resource(struct kunit *test,
> > > void *match_data)
> > > {
> > > struct kunit_resource *res, *found = NULL;
> > > + unsigned long flags;
> > >
> > > - spin_lock(>lock);
> > > + spin_lock_irqsave(>lock, flags);
> > >
> > > list_for_each_entry_reverse(res, >resources, node) {
> > > if (match(test, res, (void *)match_data)) {
> > > @@ -526,7 +527,7 @@ kunit_find_resource(struct kunit *test,
> > > }
>
Re: [PATCH] Documentation: kunit: add tips for using current->kunit_test
On Tue, Apr 6, 2021 at 3:51 PM Daniel Latypov wrote:
>
> As of commit 359a376081d4 ("kunit: support failure from dynamic analysis
> tools"), we can use current->kunit_test to find the current kunit test.
>
> Mention this in tips.rst and give an example of how this can be used in
> conjunction with `test->priv` to pass around state and specifically
> implement something like mocking.
> There's a lot more we could go into on that topic, but given that
> example is already longer than every other "tip" on this page, we just
> point to the API docs and leave filling in the blanks as an exercise to
> the reader.
>
> Also give an example of kunit_fail_current_test().
>
> Signed-off-by: Daniel Latypov
Reviewed-by: Brendan Higgins
Re: [PATCH] kunit: fix -Wunused-function warning for __kunit_fail_current_test
On Tue, Apr 6, 2021 at 10:29 AM Daniel Latypov wrote:
>
> When CONFIG_KUNIT is not enabled, __kunit_fail_current_test() an empty
> static function.
>
> But GCC complains about unused static functions, *unless* they're static
> inline.
> So add inline to make GCC happy.
>
> Signed-off-by: Daniel Latypov
> Fixes: 359a376081d4 ("kunit: support failure from dynamic analysis tools")
Reviewed-by: Brendan Higgins
Re: [PATCH v3 1/2] kunit: add a KUnit test for SLUB debugging functionality
On Thu, Apr 1, 2021 at 12:04 PM 'Daniel Latypov' via KUnit Development
wrote:
>
> On Thu, Apr 1, 2021 at 2:16 AM 'Marco Elver' via KUnit Development
> wrote:
[...]
> > #else
> > static inline bool slab_add_kunit_errors(void) { return false; }
> > #endif
> >
> > And anywhere you want to increase the error count, you'd call
> > slab_add_kunit_errors().
> >
> > Another benefit of this approach is that if KUnit is disabled, there is
> > zero overhead and no additional code generated (vs. the current
> > approach).
>
> The resource approach looks really good, but...
> You'd be picking up a dependency on
> https://lore.kernel.org/linux-kselftest/20210311152314.3814916-2-dlaty...@google.com/
> current->kunit_test will always be NULL unless CONFIG_KASAN=y &&
> CONFIG_KUNIT=y at the moment.
> My patch drops the CONFIG_KASAN requirement and opens it up to all tests.
>
> At the moment, it's just waiting another look over from Brendan or David.
> Any ETA on that, folks? :)
I just gave a "Reviewed-by" and sent it off to Shuah. Should be
available in 5.13.
Cheers
Re: [PATCH] kunit: make KUNIT_EXPECT_STREQ() quote values, don't print literals
On Fri, Feb 5, 2021 at 2:18 PM Daniel Latypov wrote:
>
> Before:
> > Expected str == "world", but
> > str == hello
> > "world" == world
>
> After:
> > Expected str == "world", but
> > str == "hello"
>
>
> Note: like the literal ellision for integers, this doesn't handle the
> case of
> KUNIT_EXPECT_STREQ(test, "hello", "world")
> since we don't expect it to realistically happen in checked in tests.
> (If you really wanted a test to fail, KUNIT_FAIL("msg") exists)
>
> In that case, you'd get:
> > Expected "hello" == "world", but
>
>
> Signed-off-by: Daniel Latypov
Reviewed-by: Brendan Higgins
Re: [PATCH] kunit: tool: make --kunitconfig accept dirs, add lib/kunit fragment
> TL;DR > $ ./tools/testing/kunit/kunit.py run --kunitconfig=lib/kunit > > Per suggestion from Ted [1], we can reduce the amount of typing by > assuming a convention that these files are named '.kunitconfig'. > > In the case of [1], we now have > $ ./tools/testing/kunit/kunit.py run --kunitconfig=fs/ext4 > > Also add in such a fragment for kunit itself so we can give that as an > example more close to home (and thus less likely to be accidentally > broken). > > [1] https://lore.kernel.org/linux-ext4/ycnf4yp1db97z...@mit.edu/ > > Signed-off-by: Daniel Latypov Reviewed-by: Brendan Higgins
Re: [PATCH v4 1/2] kunit: support failure from dynamic analysis tools
On Thu, Mar 11, 2021 at 7:23 AM Daniel Latypov wrote:
>
> From: Uriel Guajardo
>
> Add a kunit_fail_current_test() function to fail the currently running
> test, if any, with an error message.
>
> This is largely intended for dynamic analysis tools like UBSAN and for
> fakes.
> E.g. say I had a fake ops struct for testing and I wanted my `free`
> function to complain if it was called with an invalid argument, or
> caught a double-free. Most return void and have no normal means of
> signalling failure (e.g. super_operations, iommu_ops, etc.).
>
> Key points:
> * Always update current->kunit_test so anyone can use it.
> * commit 83c4e7a0363b ("KUnit: KASAN Integration") only updated it for
> CONFIG_KASAN=y
>
> * Create a new header so non-test code doesn't have
> to include all of (e.g. lib/ubsan.c)
>
> * Forward the file and line number to make it easier to track down
> failures
>
> * Declare the helper function for nice __printf() warnings about mismatched
> format strings even when KUnit is not enabled.
>
> Example output from kunit_fail_current_test("message"):
> [15:19:34] [FAILED] example_simple_test
> [15:19:34] # example_simple_test: initializing
> [15:19:34] # example_simple_test: lib/kunit/kunit-example-test.c:24:
> message
> [15:19:34] not ok 1 - example_simple_test
>
> Co-developed-by: Daniel Latypov
> Signed-off-by: Daniel Latypov
> Signed-off-by: Uriel Guajardo
> Reviewed-by: Alan Maguire
Reviewed-by: Brendan Higgins
Re: [PATCH 4/6] um: split up CONFIG_GCOV
On Fri, Mar 12, 2021 at 1:56 AM Johannes Berg wrote: > > From: Johannes Berg > > It's not always desirable to collect coverage data for the > entire kernel, so split off CONFIG_GCOV_BASE. This option > only enables linking with coverage options, and compiles a > single file (reboot.c) with them as well to force gcov to > be linked into the kernel binary. That way, modules also > work. > > To use this new option properly, one needs to manually add > '-fprofile-arcs -ftest-coverage' to the compiler options > of some object(s) or subdir(s) to collect coverage data at > the desired places. > > Signed-off-by: Johannes Berg Hey, thanks for doing this! I was looking into this a few weeks ago and root caused part of the issue in GCC and in the kernel, but I did not have a fix put together. Anyway, most of the patches make sense to me, but I am not able to apply this patch on torvalds/master. Do you mind sending a rebase so I can test it? Thanks!
Re: [PATCH v2] kunit: fix checkpatch warning
On Tue, Mar 2, 2021 at 6:03 PM Lucas Stankus wrote:
>
> Tidy up code by fixing the following checkpatch warnings:
> CHECK: Alignment should match open parenthesis
> CHECK: Lines should not end with a '('
>
> Signed-off-by: Lucas Stankus
Did you change anything other than fixing the Signed-off-by that Shuah
requested?
Generally when you make a small change after receiving a Reviewed-by
(especially one so small as here), you are supposed to include the
Reviewed-by with the other git commit message footers directly below
the "Signed-off-by". Please remember to do so in the future.
Also, when you make a change to a patch and send out a subsequent
revision, it is best practice to make note explaining the changes you
made since the last revision in the "comment section" [1] of the
git-diff, right after the three dashes and before the change log as
you can see in this example [2] (note that everything after
"Signed-off-by: David Gow \n ---" and before
"tools/testing/kunit/configs/broken_on_uml.config | 2 ++" is discarded
by git am).
Anyway, aside from these minor points of LKML best practices, this
patch still looks good to me:
Reviewed-by: Brendan Higgins
[1]
https://stackoverflow.com/questions/18979120/is-it-possible-to-add-a-comment-to-a-diff-file-unified
[2] https://lore.kernel.org/lkml/20210209071034.3268897-1-david...@google.com/T/
Re: [PATCH] kunit: tool: Fix a python tuple typing error
On Mon, Feb 22, 2021 at 9:49 PM 'David Gow' via KUnit Development
wrote:
>
> The first argument to namedtuple() should match the name of the type,
> which wasn't the case for KconfigEntryBase.
>
> Fixing this is enough to make mypy show no python typing errors again.
>
> Fixes 97752c39bd ("kunit: kunit_tool: Allow .kunitconfig to disable config
> items")
> Signed-off-by: David Gow
Acked-by: Brendan Higgins
Re: [PATCH] kunit: fix checkpatch warning
On Fri, Feb 26, 2021 at 12:54 PM Lucas Pires Stankus
wrote:
>
> Tidy up code by fixing the following checkpatch warnings:
> CHECK: Alignment should match open parenthesis
> CHECK: Lines should not end with a '('
>
> Signed-off-by: Lucas Stankus
Reviewed-by: Brendan Higgins
Thanks!
Re: [PATCH] kunit: tool: Disable PAGE_POISONING under --alltests
On Mon, Feb 8, 2021 at 11:10 PM David Gow wrote:
>
> kunit_tool maintains a list of config options which are broken under
> UML, which we exclude from an otherwise 'make ARCH=um allyesconfig'
> build used to run all tests with the --alltests option.
>
> Something in UML allyesconfig is causing segfaults when page poisining
> is enabled (and is poisoning with a non-zero value). Previously, this
> didn't occur, as allyesconfig enabled the CONFIG_PAGE_POISONING_ZERO
> option, which worked around the problem by zeroing memory. This option
> has since been removed, and memory is now poisoned with 0xAA, which
> triggers segfaults in many different codepaths, preventing UML from
> booting.
>
> Note that we have to disable both CONFIG_PAGE_POISONING and
> CONFIG_DEBUG_PAGEALLOC, as the latter will 'select' the former on
> architectures (such as UML) which don't implement __kernel_map_pages().
>
> Ideally, we'd fix this properly by tracking down the real root cause,
> but since this is breaking KUnit's --alltests feature, it's worth
> disabling there in the meantime so the kernel can boot to the point
> where tests can actually run.
>
> Fixes: f289041ed4 ("mm, page_poison: remove CONFIG_PAGE_POISONING_ZERO")
> Signed-off-by: David Gow
Reviewed-by: Brendan Higgins
Re: [PATCH v3 1/2] kunit: support failure from dynamic analysis tools
On Thu, Feb 11, 2021 at 12:58 PM Daniel Latypov wrote:
>
> On Thu, Feb 11, 2021 at 7:40 AM Alan Maguire wrote:
> >
> > On Thu, 11 Feb 2021, David Gow wrote:
> >
> > > On Wed, Feb 10, 2021 at 6:14 AM Daniel Latypov
> > > wrote:
> > > >
> > > > From: Uriel Guajardo
> > > >
> > > > Add a kunit_fail_current_test() function to fail the currently running
> > > > test, if any, with an error message.
> > > >
> > > > This is largely intended for dynamic analysis tools like UBSAN and for
> > > > fakes.
> > > > E.g. say I had a fake ops struct for testing and I wanted my `free`
> > > > function to complain if it was called with an invalid argument, or
> > > > caught a double-free. Most return void and have no normal means of
> > > > signalling failure (e.g. super_operations, iommu_ops, etc.).
> > > >
> > > > Key points:
> > > > * Always update current->kunit_test so anyone can use it.
> > > > * commit 83c4e7a0363b ("KUnit: KASAN Integration") only updated it for
> > > > CONFIG_KASAN=y
> > > >
> > > > * Create a new header so non-test code doesn't have
> > > > to include all of (e.g. lib/ubsan.c)
> > > >
> > > > * Forward the file and line number to make it easier to track down
> > > > failures
> > > >
> > > > * Declare the helper function for nice __printf() warnings about
> > > > mismatched
> > > > format strings even when KUnit is not enabled.
> > > >
> > > > Example output from kunit_fail_current_test("message"):
> > > > [15:19:34] [FAILED] example_simple_test
> > > > [15:19:34] # example_simple_test: initializing
> > > > [15:19:34] # example_simple_test:
> > > > lib/kunit/kunit-example-test.c:24: message
> > > > [15:19:34] not ok 1 - example_simple_test
> > > >
> > > > Co-developed-by: Daniel Latypov
> > > > Signed-off-by: Uriel Guajardo
> > > > Signed-off-by: Daniel Latypov
> > > > ---
> > > > include/kunit/test-bug.h | 30 ++
> > > > lib/kunit/test.c | 37 +
> > > > 2 files changed, 63 insertions(+), 4 deletions(-)
> > > > create mode 100644 include/kunit/test-bug.h
> > > >
> > > > diff --git a/include/kunit/test-bug.h b/include/kunit/test-bug.h
> > > > new file mode 100644
> > > > index ..18b1034ec43a
> > > > --- /dev/null
> > > > +++ b/include/kunit/test-bug.h
> > > > @@ -0,0 +1,30 @@
> > > > +/* SPDX-License-Identifier: GPL-2.0 */
> > > > +/*
> > > > + * KUnit API allowing dynamic analysis tools to interact with KUnit
> > > > tests
> > > > + *
> > > > + * Copyright (C) 2020, Google LLC.
> > > > + * Author: Uriel Guajardo
> > > > + */
> > > > +
> > > > +#ifndef _KUNIT_TEST_BUG_H
> > > > +#define _KUNIT_TEST_BUG_H
> > > > +
> > > > +#define kunit_fail_current_test(fmt, ...) \
> > > > + __kunit_fail_current_test(__FILE__, __LINE__, fmt,
> > > > ##__VA_ARGS__)
> > > > +
> > > > +#if IS_ENABLED(CONFIG_KUNIT)
> > >
> > > As the kernel test robot has pointed out on the second patch, this
> > > probably should be IS_BUILTIN(), otherwise this won't build if KUnit
> > > is a module, and the code calling it isn't.
> > >
> > > This does mean that things like UBSAN integration won't work if KUnit
> > > is a module, which is a shame.
> > >
> > > (It's worth noting that the KASAN integration worked around this by
> > > only calling inline functions, which would therefore be built-in even
> > > if the rest of KUnit was built as a module. I don't think it's quite
> > > as convenient to do that here, though.)
> > >
> >
> > Right, static inline'ing __kunit_fail_current_test() seems problematic
> > because it calls other exported functions; more below
> >
> > > > +
> > > > +extern __printf(3, 4) void __kunit_fail_current_test(const char *file,
> > > > int line,
> > > > + const char *fmt,
> > > > ...);
> > > > +
> > > > +#else
> > > > +
> > > > +static __printf(3, 4) void __kunit_fail_current_test(const char *file,
> > > > int line,
> > > > + const char *fmt,
> > > > ...)
> > > > +{
> > > > +}
> > > > +
> > > > +#endif
> > > > +
> > > > +
> > > > +#endif /* _KUNIT_TEST_BUG_H */
> > > > diff --git a/lib/kunit/test.c b/lib/kunit/test.c
> > > > index ec9494e914ef..5794059505cf 100644
> > > > --- a/lib/kunit/test.c
> > > > +++ b/lib/kunit/test.c
> > > > @@ -7,6 +7,7 @@
> > > > */
> > > >
> > > > #include
> > > > +#include
> > > > #include
> > > > #include
> > > > #include
> > > > @@ -16,6 +17,38 @@
> > > > #include "string-stream.h"
> > > > #include "try-catch-impl.h"
> > > >
> > > > +/*
> > > > + * Fail the current test and print an error message to the log.
> > > > + */
> > > > +void __kunit_fail_current_test(const char *file, int line, const char
> > > > *fmt, ...)
> > > > +{
> > > > + va_list args;
> > > > + int len;
> > > > + char *buffer;
> > > > +
> > > > + if (!current->kunit_test)
> > > > + return;
> > > > +
> > > > +
Re: [PATCH v4 0/3] kunit: support running subsets of test suites from kunit.py
On Fri, Feb 5, 2021 at 4:09 PM Daniel Latypov wrote: > > When using `kunit.py run` to run tests, users must populate a > `kunitconfig` file to select the options the tests are hidden behind and > all their dependencies. > > The patch [1] to allow specifying a path to kunitconfig promises to make > this nicer as we can have checked in files corresponding to different > sets of tests. > > But it's still annoying > 1) when trying to run a subet of tests > 2) when you want to run tests that don't have such a pre-existing > kunitconfig and selecting all the necessary options is tricky. > > This patch series aims to alleviate both: > 1) `kunit.py run 'my-suite-*'` > I.e. use my current kunitconfig, but just run suites that match this glob > 2) `kunit.py run --alltests 'my-suite-*'` > I.e. use allyesconfig so I don't have to worry about writing a > kunitconfig at all. > > See the first commit message for more details and discussion about > future work. > > This patch series also includes a bugfix for a latent bug that can't be > triggered right now but has worse consequences as a result of the > changes needed to plumb in this suite name glob. > > [1] > https://lore.kernel.org/linux-kselftest/20210201205514.3943096-1-dlaty...@google.com/ Tested-by: Brendan Higgins
Re: [PATCH] Documentation: kunit: add tips.rst for small examples
On Mon, Jan 25, 2021 at 10:53 AM Daniel Latypov wrote: > > ./usage.rst contains fairly long examples and explanations of things > like how to fake a class and how to use parameterized tests (and how you > could do table-driven tests yourself). > > It's not exactly necessary information, so we add a new page with more > digestible tips like "use kunit_kzalloc() instead of kzalloc() so you > don't have to worry about calling kfree() yourself" and the like. > > Change start.rst to point users to this new page first and let them know > that usage.rst is more of optional further reading. > > Signed-off-by: Daniel Latypov Reviewed-by: Brendan Higgins
Re: [PATCH v2] kunit: make kunit_tool accept optional path to .kunitconfig fragment
On Mon, Feb 1, 2021 at 12:55 PM Daniel Latypov wrote: > > Currently running tests via KUnit tool means tweaking a .kunitconfig > file, which you'd keep around locally and never commit. > This changes makes it so users can pass in a path to a kunitconfig. > > One of the imagined use cases is having kunitconfig fragments in-tree > to formalize interesting sets of tests for features/subsystems, e.g. > $ ./tools/testing/kunit/kunit.py run --kunticonfig=fs/ext4/kunitconfig > > For now, this hypothetical fs/ext4/kunitconfig would contain > CONFIG_KUNIT=y > CONFIG_EXT4_FS=y > CONFIG_EXT4_KUNIT_TESTS=y > > At the moment, it's not hard to manually whip up this file, but as more > and more tests get added, this will get tedious. > > It also opens the door to documenting how to run all the tests relevant > to a specific subsystem or feature as a simple one-liner. > > This can be seen as an analogue to tools/testing/selftests/*/config > But in the case of KUnit, the tests live in the same directory as the > code-under-test, so it feels more natural to allow the kunitconfig > fragments to live anywhere. (Though, people could create a separate > directory if wanted; this patch imposes no restrictions on the path). > > Signed-off-by: Daniel Latypov Tested-by: Brendan Higgins
Re: [PATCH v3 0/3] kunit: support running subsets of test suites from kunit.py
On Thu, Feb 4, 2021 at 2:54 PM Daniel Latypov wrote: > > When using `kunit.py run` to run tests, users must populate a > `kunitconfig` file to select the options the tests are hidden behind and > all their dependencies. > > The patch [1] to allow specifying a path to kunitconfig promises to make > this nicer as we can have checked in files corresponding to different > sets of tests. > > But it's still annoying > 1) when trying to run a subet of tests > 2) when you want to run tests that don't have such a pre-existing > kunitconfig and selecting all the necessary options is tricky. > > This patch series aims to alleviate both: > 1) `kunit.py run 'my-suite-*'` > I.e. use my current kunitconfig, but just run suites that match this glob > 2) `kunit.py run --alltests 'my-suite-*'` > I.e. use allyesconfig so I don't have to worry about writing a > kunitconfig at all. > > See the first commit message for more details and discussion about > future work. > > This patch series also includes a bugfix for a latent bug that can't be > triggered right now but has worse consequences as a result of the > changes needed to plumb in this suite name glob. > > [1] > https://lore.kernel.org/linux-kselftest/20210201205514.3943096-1-dlaty...@google.com/ I tried applying this series on top of several of your other patches over the past cycle and running the kunit_tool unit tests (kunit_tool_test.py) and I got several failures: == FAIL: test_exec_builddir (__main__.KUnitMainTest) -- Traceback (most recent call last): File "tools/testing/kunit/kunit_tool_test.py", line 417, in test_exec_builddir self.linux_source_mock.run_kernel.assert_called_once_with(build_dir=build_dir, timeout=300) File "/usr/lib/python3.8/unittest/mock.py", line 925, in assert_called_once_with return self.assert_called_with(*args, **kwargs) File "/usr/lib/python3.8/unittest/mock.py", line 913, in assert_called_with raise AssertionError(_error_message()) from cause AssertionError: expected call not found. Expected: run_kernel(build_dir='.kunit', timeout=300) Actual: run_kernel(timeout=300, filter_glob='', build_dir='.kunit') == FAIL: test_exec_passes_args_pass (__main__.KUnitMainTest) -- Traceback (most recent call last): File "tools/testing/kunit/kunit_tool_test.py", line 337, in test_exec_passes_args_pass self.linux_source_mock.run_kernel.assert_called_once_with(build_dir='.kunit', timeout=300) File "/usr/lib/python3.8/unittest/mock.py", line 925, in assert_called_once_with return self.assert_called_with(*args, **kwargs) File "/usr/lib/python3.8/unittest/mock.py", line 913, in assert_called_with raise AssertionError(_error_message()) from cause AssertionError: expected call not found. Expected: run_kernel(build_dir='.kunit', timeout=300) Actual: run_kernel(timeout=300, filter_glob='', build_dir='.kunit') == FAIL: test_exec_timeout (__main__.KUnitMainTest) -- Traceback (most recent call last): File "tools/testing/kunit/kunit_tool_test.py", line 385, in test_exec_timeout self.linux_source_mock.run_kernel.assert_called_once_with(build_dir='.kunit', timeout=timeout) File "/usr/lib/python3.8/unittest/mock.py", line 925, in assert_called_once_with return self.assert_called_with(*args, **kwargs) File "/usr/lib/python3.8/unittest/mock.py", line 913, in assert_called_with raise AssertionError(_error_message()) from cause AssertionError: expected call not found. Expected: run_kernel(build_dir='.kunit', timeout=3453) Actual: run_kernel(timeout=3453, filter_glob='', build_dir='.kunit') == FAIL: test_run_builddir (__main__.KUnitMainTest) -- Traceback (most recent call last): File "tools/testing/kunit/kunit_tool_test.py", line 400, in test_run_builddir self.linux_source_mock.run_kernel.assert_called_once_with( File "/usr/lib/python3.8/unittest/mock.py", line 925, in assert_called_once_with return self.assert_called_with(*args, **kwargs) File "/usr/lib/python3.8/unittest/mock.py", line 913, in assert_called_with raise AssertionError(_error_message()) from cause AssertionError: expected call not found. Expected: run_kernel(build_dir='.kunit', timeout=300) Actual: run_kernel(timeout=300, filter_glob='', build_dir='.kunit') == FAIL: test_run_passes_args_pass (__main__.KUnitMainTest) -- Traceback (most recent call last): File
Re: [PATCH] KUnit: Docs: make start.rst example Kconfig follow style.rst
On Tue, Jan 19, 2021 at 3:52 PM Daniel Latypov wrote: > > The primary change is that we want to encourage people to respect > KUNIT_ALL_TESTS to make it easy to run all the relevant tests for a > given config. > > Signed-off-by: Daniel Latypov Reviewed-by: Brendan Higgins
Re: [PATCH] kunit: tool: simplify kconfig is_subset_of() logic
On Tue, Dec 8, 2020 at 3:21 PM Daniel Latypov wrote: > > Don't use an O(nm) algorithm* and make it more readable by using a dict. > > *Most obviously, it does a nested for-loop over the entire other config. > A bit more subtle, it calls .entries(), which constructs a set from the > list for _every_ outer iteration. > > Signed-off-by: Daniel Latypov Tested-by: Brendan Higgins Acked-by: Brendan Higgins
Re: [PATCH] kunit: Print test statistics on failure
On Thu, Dec 10, 2020 at 11:23 PM David Gow wrote:
>
> When a number of tests fail, it can be useful to get higher-level
> statistics of how many tests are failing (or how many parameters are
> failing in parameterised tests), and in what cases or suites. This is
> already done by some non-KUnit tests, so add support for automatically
> generating these for KUnit tests.
>
> This change adds a 'kunit_stats_enabled' switch which has three values:
> - 0: No stats are printed (current behaviour)
> - 1: Stats are printed only for tests/suites with more than one
> subtests, and at least one failure (new default)
> - 2: Always print test statistics
>
> For parameterised tests, the summary line looks as follows:
> "# inode_test_xtimestamp_decoding: 0 / 16 test parameters failed"
> For test suites, it looks like this:
> "# ext4_inode_test: (0 / 1) tests failed (0 / 16 test parameters)"
>
> kunit_tool is also updated to correctly ignore diagnostic lines, so that
> these statistics do not prevent the result from parsing.
>
> Signed-off-by: David Gow
Sorry, forgot about this. A couple of comments below:
> ---
>
> This is largely a follow-up to the discussion here:
>
> https://lore.kernel.org/linux-kselftest/CABVgOSmy4n_LGwDS7yWfoLftcQzxv6S+iXx9Y=opcgg2gu0...@mail.gmail.com/T/#t
>
> Does this seem like a sensible addition?
I am fine with it as long as other people want it.
> Cheers,
> -- David
>
> lib/kunit/test.c| 71 +
> tools/testing/kunit/kunit_parser.py | 2 +-
> 2 files changed, 72 insertions(+), 1 deletion(-)
>
> diff --git a/lib/kunit/test.c b/lib/kunit/test.c
> index ec9494e914ef..711e269366a7 100644
> --- a/lib/kunit/test.c
> +++ b/lib/kunit/test.c
> @@ -9,6 +9,7 @@
> #include
> #include
> #include
> +#include
> #include
> #include
>
> @@ -16,6 +17,40 @@
> #include "string-stream.h"
> #include "try-catch-impl.h"
>
> +/*
> + * KUnit statistic mode:
> + * 0 - disabled
> + * 1 - only when there is at least one failure, and more than one subtest
> + * 2 - enabled
> + */
> +static int kunit_stats_enabled = 1;
> +core_param(kunit_stats_enabled, kunit_stats_enabled, int, 0644);
I think this should be a module param.
Also, remember to document the param with: MODULE_PARM_DESC()
> +static bool kunit_should_print_stats(int num_failures, int num_subtests)
> +{
> + if (kunit_stats_enabled == 0)
> + return false;
> +
> + if (kunit_stats_enabled == 2)
> + return true;
> +
> + return (num_failures > 0 && num_subtests > 1);
> +}
> +
> +static void kunit_print_test_stats(struct kunit *test,
> + size_t num_failures, size_t num_subtests)
> +{
> + if (!kunit_should_print_stats(num_failures, num_subtests))
> + return;
> +
> + kunit_log(KERN_INFO, test,
> + KUNIT_SUBTEST_INDENT
> + "# %s: %lu / %lu test parameters failed",
> + test->name,
> + num_failures,
> + num_subtests);
> +}
> +
> /*
> * Append formatted message to log, size of which is limited to
> * KUNIT_LOG_SIZE bytes (including null terminating byte).
> @@ -346,15 +381,37 @@ static void kunit_run_case_catch_errors(struct
> kunit_suite *suite,
> test_case->success = test->success;
> }
>
> +static void kunit_print_suite_stats(struct kunit_suite *suite,
> + size_t num_failures,
> + size_t total_param_failures,
> + size_t total_params)
> +{
> + size_t num_cases = kunit_suite_num_test_cases(suite);
> +
> + if (!kunit_should_print_stats(num_failures, num_cases))
> + return;
> +
> + kunit_log(KERN_INFO, suite,
> + "# %s: (%lu / %lu) tests failed (%lu / %lu test
> parameters)",
> + suite->name,
> + num_failures,
> + num_cases,
> + total_param_failures,
> + total_params);
> +}
> +
> int kunit_run_tests(struct kunit_suite *suite)
> {
> char param_desc[KUNIT_PARAM_DESC_SIZE];
> struct kunit_case *test_case;
> + size_t num_suite_failures = 0;
> + size_t total_param_failures = 0, total_params = 0;
>
> kunit_print_subtest_start(suite);
>
> kunit_suite_for_each_test_case(suite, test_case) {
> struct kunit test = { .param_value = NULL, .param_index = 0 };
> + size_t num_params = 0, num_failures = 0;
> bool test_success = true;
>
> if (test_case->generate_params) {
> @@ -385,13 +442,27 @@ int kunit_run_tests(struct kunit_suite *suite)
> test.param_value =
> test_case->generate_params(test.param_value, param_desc);
> test.param_index++;
> }
> +
> + if (!test.success)
>
Re: [PATCH v2 3/3] kunit: tool: fix unintentional statefulness in run_kernel()
On Thu, Feb 4, 2021 at 9:31 AM Daniel Latypov wrote:
>
> This is a bug that has been present since the first version of this
> code.
> Using [] as a default parameter is dangerous, since it's mutable.
>
> Example using the REPL:
> >>> def bad(param = []):
> ... param.append(len(param))
> ... print(param)
> ...
> >>> bad()
> [0]
> >>> bad()
> [0, 1]
>
> This wasn't a concern in the past since it would just keep appending the
> same values to it.
>
> E.g. before, `args` would just grow in size like:
> [mem=1G', 'console=tty']
> [mem=1G', 'console=tty', mem=1G', 'console=tty']
>
> But with now filter_glob, this is more dangerous, e.g.
> run_kernel(filter_glob='my-test*') # default modified here
> run_kernel() # filter_glob still applies here!
> That earlier `filter_glob` will affect all subsequent calls that don't
> specify `args`.
>
> Note: currently the kunit tool only calls run_kernel() at most once, so
> it's not possible to trigger any negative side-effects right now.
>
> Fixes: 6ebf5866f2e8 ("kunit: tool: add Python wrappers for running KUnit
> tests")
> Signed-off-by: Daniel Latypov
Whoah, nice catch! I didn't even know that was a thing!
Reviewed-by: Brendan Higgins
Re: [PATCH v2 2/3] kunit: tool: add support for filtering suites by glob
On Thu, Feb 4, 2021 at 9:31 AM Daniel Latypov wrote: > > This allows running different subsets of tests, e.g. > > $ ./tools/testing/kunit/kunit.py build > $ ./tools/testing/kunit/kunit.py exec 'list*' > $ ./tools/testing/kunit/kunit.py exec 'kunit*' > > This passes the "kunit_filter.glob" commandline option to the UML > kernel, which currently only supports filtering by suite name. > > Signed-off-by: Daniel Latypov Reviewed-by: Brendan Higgins
Re: [PATCH v2 1/3] kunit: add kunit.filter_glob cmdline option to filter suites
On Thu, Feb 4, 2021 at 9:31 AM Daniel Latypov wrote: > > E.g. specifying this would run suites with "list" in their name. > kunit.filter_glob=list* > > Note: the executor prints out a TAP header that includes the number of > suites we intend to run. > So unless we want to report empty results for filtered-out suites, we > need to do the filtering here in the executor. > It's also probably better in the executor since we most likely don't > want any filtering to apply to tests built as modules. > > This code does add a CONFIG_GLOB=y dependency for CONFIG_KUNIT=y. > But the code seems light enough that it shouldn't be an issue. > > For now, we only filter on suite names so we don't have to create copies > of the suites themselves, just the array (of arrays) holding them. > > The name is rather generic since in the future, we could consider > extending it to a syntax like: > kunit.filter_glob=. > E.g. to run all the del list tests > kunit.filter_glob=list-kunit-test.*del* > > But at the moment, it's far easier to manually comment out test cases in > test files as opposed to messing with sets of Kconfig entries to select > specific suites. > So even just doing this makes using kunit far less annoying. > > Signed-off-by: Daniel Latypov One minor issue below, otherwise: Reviewed-by: Brendan Higgins > --- > lib/kunit/Kconfig| 1 + > lib/kunit/executor.c | 91 +++- > 2 files changed, 83 insertions(+), 9 deletions(-) > > diff --git a/lib/kunit/Kconfig b/lib/kunit/Kconfig > index 00909e6a2443..0b5dfb001bac 100644 > --- a/lib/kunit/Kconfig > +++ b/lib/kunit/Kconfig > @@ -4,6 +4,7 @@ > > menuconfig KUNIT > tristate "KUnit - Enable support for unit tests" > + select GLOB if KUNIT=y > help > Enables support for kernel unit tests (KUnit), a lightweight unit > testing and mocking framework for the Linux kernel. These tests are > diff --git a/lib/kunit/executor.c b/lib/kunit/executor.c > index a95742a4ece7..996efb80dba6 100644 > --- a/lib/kunit/executor.c > +++ b/lib/kunit/executor.c > @@ -1,6 +1,8 @@ > // SPDX-License-Identifier: GPL-2.0 > > #include > +#include > +#include > > /* > * These symbols point to the .kunit_test_suites section and are defined in > @@ -11,14 +13,79 @@ extern struct kunit_suite * const * const > __kunit_suites_end[]; > > #if IS_BUILTIN(CONFIG_KUNIT) > > -static void kunit_print_tap_header(void) > +static char *filter_glob; > +module_param(filter_glob, charp, 0); You should probably also use MODULE_PARM_DESC().
Re: [PATCH v2] kunit: make kunit_tool accept optional path to .kunitconfig fragment
On Mon, Feb 1, 2021 at 12:55 PM Daniel Latypov wrote: > > Currently running tests via KUnit tool means tweaking a .kunitconfig > file, which you'd keep around locally and never commit. > This changes makes it so users can pass in a path to a kunitconfig. > > One of the imagined use cases is having kunitconfig fragments in-tree > to formalize interesting sets of tests for features/subsystems, e.g. > $ ./tools/testing/kunit/kunit.py run --kunticonfig=fs/ext4/kunitconfig > > For now, this hypothetical fs/ext4/kunitconfig would contain > CONFIG_KUNIT=y > CONFIG_EXT4_FS=y > CONFIG_EXT4_KUNIT_TESTS=y > > At the moment, it's not hard to manually whip up this file, but as more > and more tests get added, this will get tedious. > > It also opens the door to documenting how to run all the tests relevant > to a specific subsystem or feature as a simple one-liner. > > This can be seen as an analogue to tools/testing/selftests/*/config > But in the case of KUnit, the tests live in the same directory as the > code-under-test, so it feels more natural to allow the kunitconfig > fragments to live anywhere. (Though, people could create a separate > directory if wanted; this patch imposes no restrictions on the path). > > Signed-off-by: Daniel Latypov Reviewed-by: Brendan Higgins
Re: [RFC 0/3] kunit vs structleak
On Wed, Jan 27, 2021 at 12:15 PM Kees Cook wrote: > > On Mon, Jan 25, 2021 at 01:45:25PM +0100, Arnd Bergmann wrote: > > From: Arnd Bergmann > > > > I ran into a couple of problems with kunit tests taking too much stack > > space, sometimes dangerously so. These the the three instances that > > cause an increase over the warning limit of some architectures: > > > > lib/bitfield_kunit.c:93:1: error: the frame size of 7440 bytes is larger > > than 2048 bytes [-Werror=frame-larger-than=] > > drivers/base/test/property-entry-test.c:481:1: error: the frame size of > > 2640 bytes is larger than 2048 bytes [-Werror=frame-larger-than=] > > drivers/thunderbolt/test.c:1529:1: error: the frame size of 1176 bytes is > > larger than 1024 bytes [-Werror=frame-larger-than=] > > > > Ideally there should be a way to rewrite the kunit infrastructure > > that avoids the explosion of stack data when the structleak plugin > > is used. > > > > A rather drastic measure would be to use Kconfig logic to make > > the two options mutually exclusive. This would clearly work, but > > is probably not needed. > > > > As a simpler workaround, this disables the plugin for the three > > files in which the excessive stack usage was observed. > > > > Arnd > > > > Arnd Bergmann (3): > > bitfield: build kunit tests without structleak plugin > > drivers/base: build kunit tests without structleak plugin > > thunderbolt: build kunit tests without structleak plugin > > > > drivers/base/test/Makefile | 1 + > > drivers/thunderbolt/Makefile | 1 + > > lib/Makefile | 1 + > > 3 files changed, 3 insertions(+) > > I think I'd prefer centralizing the disabling, as done with the other > plugins, instead of sprinkling "open coded" command-line options around > the kernel's Makefiles. :) > > For example: > > > diff --git a/scripts/Makefile.gcc-plugins b/scripts/Makefile.gcc-plugins > index 952e46876329..2d5009e3b593 100644 > --- a/scripts/Makefile.gcc-plugins > +++ b/scripts/Makefile.gcc-plugins > @@ -21,6 +21,10 @@ > gcc-plugin-cflags-$(CONFIG_GCC_PLUGIN_STRUCTLEAK_BYREF_ALL) \ > += -fplugin-arg-structleak_plugin-byref-all > gcc-plugin-cflags-$(CONFIG_GCC_PLUGIN_STRUCTLEAK) \ > += -DSTRUCTLEAK_PLUGIN > +ifdef CONFIG_GCC_PLUGIN_STRUCTLEAK > +DISABLE_STRUCTLEAK_PLUGIN += -fplugin-arg-structleak_plugin-disable > +endif > +export DISABLE_STRUCTLEAK_PLUGIN > > gcc-plugin-$(CONFIG_GCC_PLUGIN_RANDSTRUCT) += randomize_layout_plugin.so > gcc-plugin-cflags-$(CONFIG_GCC_PLUGIN_RANDSTRUCT) \ > > > And then use DISABLE_STRUCTLEAK_PLUGIN. This looks fine to me. Does somebody want me to send this out as a patch? Don't want to steal anyone's thunder :-)
Re: [PATCH] kunit: don't show `1 == 1` in failed assertion messages
On Thu, Jan 28, 2021 at 6:26 PM Daniel Latypov wrote: > > Currently, given something (fairly dystopian) like > > KUNIT_EXPECT_EQ(test, 2 + 2, 5) > > KUnit will prints a failure message like this. > > Expected 2 + 2 == 5, but > > 2 + 2 == 4 > > 5 == 5 > > With this patch, the output just becomes > > Expected 2 + 2 == 5, but > > 2 + 2 == 4 > > This patch is slightly hacky, but it's quite common* to compare an > expression to a literal integer value, so this can make KUnit less > chatty in many cases. (This patch also fixes variants like > KUNIT_EXPECT_GT, LE, et al.). > > It also allocates an additional string briefly, but given this only > happens on test failures, it doesn't seem too bad a tradeoff. > Also, in most cases it'll realize the lengths are unequal and bail out > before the allocation. > > We could save the result of the formatted string to avoid wasting this > extra work, but it felt cleaner to leave it as-is. > > Edge case: for something silly and unrealistic like > > KUNIT_EXPECT_EQ(test, 4, 5); > > It'll generate this message with a trailing "but" > > Expected 2 + 2 == 5, but > > > > It didn't feel worth adding a check up-front to see if both sides are > literals to handle this better. > > *A quick grep suggests 100+ comparisons to an integer literal as the > right hand side. > > Signed-off-by: Daniel Latypov I don't feel very strongly about this either way. In any case: Reviewed-by: Brendan Higgins
Re: [PATCH v4 1/3] kunit: tool: surface and address more typing issues
On Thu, Jan 14, 2021 at 4:39 PM Daniel Latypov wrote: > > The authors of this tool were more familiar with a different > type-checker, https://github.com/google/pytype. > > That's open source, but mypy seems more prevalent (and runs faster). > And unlike pytype, mypy doesn't try to infer types so it doesn't check > unanotated functions. > > So annotate ~all functions in kunit tool to increase type-checking > coverage. > Note: per https://www.python.org/dev/peps/pep-0484/, `__init__()` should > be annotated as `-> None`. > > Doing so makes mypy discover a number of new violations. > Exclude main() since we reuse `request` for the different types of > requests, which mypy isn't happy about. > > This commit fixes all but one error, where `TestSuite.status` might be > None. > > Signed-off-by: Daniel Latypov > Reviewed-by: David Gow Tested-by: Brendan Higgins Acked-by: Brendan Higgins
Re: [PATCH v4 2/3] kunit: tool: fix minor typing issue with None status
On Thu, Jan 14, 2021 at 4:39 PM Daniel Latypov wrote:
>
> The code to handle aggregating statuses didn't check that the status
> actually got set to some non-None value.
> Default the value to SUCCESS instead of adding a bunch of `is None`
> checks.
>
> This sorta follows the precedent in commit 3fc48259d525 ("kunit: Don't
> fail test suites if one of them is empty").
>
> Also slightly simplify the code and add type annotations.
>
> Signed-off-by: Daniel Latypov
> Reviewed-by: David Gow
Reviewed-by: Brendan Higgins
Tested-by: Brendan Higgins
Re: [PATCH v4 3/3] kunit: tool: move kunitconfig parsing into __init__, make it optional
On Thu, Jan 14, 2021 at 4:39 PM Daniel Latypov wrote: > > LinuxSourceTree will unceremoniously crash if the user doesn't call > read_kunitconfig() first in a number of functions. > > And currently every place we create an instance, the caller also calls > create_kunitconfig() and read_kunitconfig(). > Move these instead into __init__() so they can't be forgotten and to > reduce copy-paste. > > The https://github.com/google/pytype type-checker complained that > _config wasn't initialized. With this, kunit_tool now type checks > under both pytype and mypy. > > Add an optional boolean that can be used to disable this for use cases > in the future where we might not need/want to load the config. > > Signed-off-by: Daniel Latypov > Reviewed-by: Brendan Higgins Tested-by: Brendan Higgins
Re: [PATCH v3 1/3] kunit: tool: surface and address more typing issues
On Thu, Jan 7, 2021 at 3:48 PM Daniel Latypov wrote: > > The authors of this tool were more familiar with a different > type-checker, https://github.com/google/pytype. > > That's open source, but mypy seems more prevalent (and runs faster). > And unlike pytype, mypy doesn't try to infer types so it doesn't check > unanotated functions. > > So annotate ~all functions in kunit tool to increase type-checking > coverage. > Note: per https://www.python.org/dev/peps/pep-0484/, `__init__()` should > be annotated as `-> None`. > > Doing so makes mypy discover a number of new violations. > Exclude main() since we reuse `request` for the different types of > requests, which mypy isn't happy about. > > This commit fixes all but one error, where `TestSuite.status` might be > None. > > Signed-off-by: Daniel Latypov > Reviewed-by: David Gow Hey, I just tried applying this series as a pre-check before sending it off to Shuah and it no longer applies to torvalds/master. Please fix and resend.
Re: [PATCH] kunit: tool: Fix spelling of "diagnostic" in kunit_parser
On Fri, Dec 11, 2020 at 2:32 PM David Gow wrote: > > Various helper functions were misspelling "diagnostic" in their names. > It finally got annoying, so fix it. > > Signed-off-by: David Gow Tested-by: Brendan Higgins
Re: [PATCH] kunit: tool: Fix spelling of "diagnostic" in kunit_parser
On Fri, Dec 11, 2020 at 2:32 PM David Gow wrote: > > Various helper functions were misspelling "diagnostic" in their names. > It finally got annoying, so fix it. > > Signed-off-by: David Gow Reviewed-by: Brendan Higgins
Re: [PATCH] Documentation: kunit: include example of a parameterized test
On Tue, Dec 15, 2020 at 4:23 PM Daniel Latypov wrote:
>
> Commit fadb08e7c750 ("kunit: Support for Parameterized Testing")
> introduced support but lacks documentation for how to use it.
>
> This patch builds on commit 1f0e943df68a ("Documentation: kunit: provide
> guidance for testing many inputs") to show a minimal example of the new
> feature.
>
> Signed-off-by: Daniel Latypov
Tested-by: Brendan Higgins
Acked-by: Brendan Higgins
Re: [PATCH v2 1/4] kunit: tool: fix unit test cleanup handling
On Wed, Dec 2, 2020 at 11:09 AM Daniel Latypov wrote: > > * Stop leaking file objects. > * Use self.addCleanup() to ensure we call cleanup functions even if > setUp() fails. > * use mock.patch.stopall instead of more error-prone manual approach > > Signed-off-by: Daniel Latypov Tested-by: Brendan Higgins Acked-by: Brendan Higgins
Re: [PATCH v2 2/4] kunit: tool: stop using bare asserts in unit test
On Wed, Dec 2, 2020 at 11:09 AM Daniel Latypov wrote: > > Use self.assertEqual/assertNotEqual() instead. > Besides being more appropriate in a unit test, it'll also give a better > error message by show the unexpected values. > > Also > * Delete redundant check of exception types. self.assertRaises does this. > * s/kall/call. There's no reason to name it this way. > * This is probably a misunderstanding from the docs which uses it > since `mock.call` is in scope as `call`. > > Signed-off-by: Daniel Latypov Tested-by: Brendan Higgins Acked-by: Brendan Higgins
Re: [PATCH v2 3/4] kunit: tool: use `with open()` in unit test
On Wed, Dec 2, 2020 at 11:09 AM Daniel Latypov wrote: > > The use of manual open() and .close() calls seems to be an attempt to > keep the contents in scope. > But Python doesn't restrict variables like that, so we can introduce new > variables inside of a `with` and use them outside. > > Do so to make the code more Pythonic. > > Signed-off-by: Daniel Latypov Tested-by: Brendan Higgins Acked-by: Brendan Higgins
Re: [PATCH v2 4/4] minor: kunit: tool: fix unit test so it can run from non-root dir
On Wed, Dec 2, 2020 at 11:09 AM Daniel Latypov wrote:
>
> Also take this time to rename get_absolute_path() to test_data_path().
>
> 1. the name is currently a lie. It gives relative paths, e.g. if I run
> from the same dir as the test file, it gives './test_data/'
>
> See https://docs.python.org/3/reference/import.html#__file__, which
> doesn't stipulate that implementations provide absolute paths.
>
> 2. it's only used for generating paths to tools/testing/kunit/test_data/
> So we can tersen things by making it less general.
>
> Cache the absolute path to the test data files per suggestion from [1].
> Using relative paths, the tests break because of this code in kunit.py
> if get_kernel_root_path():
> os.chdir(get_kernel_root_path())
>
> [1]
> https://lore.kernel.org/linux-kselftest/cabvgosnh0gz7z5jhrcgyg1wg0zddbtlosucob-gwmexlgvt...@mail.gmail.com/
>
> Fixes: 5578d008d9e0 ("kunit: tool: fix running kunit_tool from outside kernel
> tree")
> Signed-off-by: Daniel Latypov
Tested-by: Brendan Higgins
Acked-by: Brendan Higgins
Re: [PATCH v3 3/3] kunit: tool: move kunitconfig parsing into __init__, make it optional
On Thu, Jan 7, 2021 at 3:48 PM Daniel Latypov wrote: > > LinuxSourceTree will unceremoniously crash if the user doesn't call > read_kunitconfig() first in a number of functions. > > And currently every place we create an instance, the caller also calls > create_kunitconfig() and read_kunitconfig(). > Move these instead into __init__() so they can't be forgotten and to > reduce copy-paste. > > The https://github.com/google/pytype type-checker complained that > _config wasn't initialized. With this, kunit_tool now type checks > under both pytype and mypy. > > Add an optional boolean that can be used to disable this for use cases > in the future where we might not need/want to load the config. > > Signed-off-by: Daniel Latypov Very nice! This makes the code much more readable! Reviewed-by: Brendan Higgins
Re: [PATCH v3 2/3] kunit: tool: fix minor typing issue with None status
On Thu, Jan 7, 2021 at 3:48 PM Daniel Latypov wrote:
>
> The code to handle aggregating statuses didn't check that the status
> actually got set to some non-None value.
> Default the value to SUCCESS instead of adding a bunch of `is None`
> checks.
>
> This sorta follows the precedent in commit 3fc48259d525 ("kunit: Don't
> fail test suites if one of them is empty").
>
> Also slightly simplify the code and add type annotations.
>
> Signed-off-by: Daniel Latypov
> Reviewed-by: David Gow
Reviewed-by: Brendan Higgins
Re: [PATCH v3 1/3] kunit: tool: surface and address more typing issues
On Thu, Jan 7, 2021 at 3:48 PM Daniel Latypov wrote: > > The authors of this tool were more familiar with a different > type-checker, https://github.com/google/pytype. > > That's open source, but mypy seems more prevalent (and runs faster). > And unlike pytype, mypy doesn't try to infer types so it doesn't check > unanotated functions. > > So annotate ~all functions in kunit tool to increase type-checking > coverage. > Note: per https://www.python.org/dev/peps/pep-0484/, `__init__()` should > be annotated as `-> None`. > > Doing so makes mypy discover a number of new violations. > Exclude main() since we reuse `request` for the different types of > requests, which mypy isn't happy about. > > This commit fixes all but one error, where `TestSuite.status` might be > None. > > Signed-off-by: Daniel Latypov > Reviewed-by: David Gow Acked-by: Brendan Higgins
Re: [PATCH] kunit: tool: Force the use of the 'tty' console for UML
On Mon, Dec 21, 2020 at 11:39 PM David Gow wrote:
>
> kunit_tool relies on the UML console outputting printk() output to the
> tty in order to get results. Since the default console driver could
> change, pass 'console=tty' to the kernel.
>
> This is triggered by a change[1] to use ttynull as a fallback console
> driver which -- by chance or by design -- seems to have changed the
> default console output on UML, breaking kunit_tool. While this may be
> fixed, we should be less fragile to such changes in the default.
>
> [1]:
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=757055ae8dedf5333af17b3b5b4b70ba9bc9da4e
>
> Signed-off-by: David Gow
> Fixes: 757055ae8ded ("init/console: Use ttynull as a fallback when there is
> no console")
Acked-by: Brendan Higgins
Thanks for taking care of this!
Re: [PATCH v8 1/2] kunit: Support for Parameterized Testing
On Sun, Nov 15, 2020 at 10:58 AM Arpitha Raghunandan <98.a...@gmail.com> wrote: > > Implementation of support for parameterized testing in KUnit. This > approach requires the creation of a test case using the > KUNIT_CASE_PARAM() macro that accepts a generator function as input. > > This generator function should return the next parameter given the > previous parameter in parameterized tests. It also provides a macro to > generate common-case generators based on arrays. Generators may also > optionally provide a human-readable description of parameters, which is > displayed where available. > > Note, currently the result of each parameter run is displayed in > diagnostic lines, and only the overall test case output summarizes > TAP-compliant success or failure of all parameter runs. In future, when > supported by kunit-tool, these can be turned into subsubtest outputs. > > Signed-off-by: Arpitha Raghunandan <98.a...@gmail.com> > Co-developed-by: Marco Elver > Signed-off-by: Marco Elver Acked-by: Brendan Higgins
Re: [PATCH v9 1/2] kunit: Support for Parameterized Testing
On Mon, Nov 23, 2020 at 11:25 PM David Gow wrote: > > On Mon, Nov 23, 2020 at 9:08 PM Marco Elver wrote: > > > > On Tue, 17 Nov 2020 at 08:21, David Gow wrote: > > > On Mon, Nov 16, 2020 at 1:41 PM Arpitha Raghunandan <98.a...@gmail.com> > > > wrote: > > > > > > > > Implementation of support for parameterized testing in KUnit. This > > > > approach requires the creation of a test case using the > > > > KUNIT_CASE_PARAM() macro that accepts a generator function as input. > > > > > > > > This generator function should return the next parameter given the > > > > previous parameter in parameterized tests. It also provides a macro to > > > > generate common-case generators based on arrays. Generators may also > > > > optionally provide a human-readable description of parameters, which is > > > > displayed where available. > > > > > > > > Note, currently the result of each parameter run is displayed in > > > > diagnostic lines, and only the overall test case output summarizes > > > > TAP-compliant success or failure of all parameter runs. In future, when > > > > supported by kunit-tool, these can be turned into subsubtest outputs. > > > > > > > > Signed-off-by: Arpitha Raghunandan <98.a...@gmail.com> > > > > Co-developed-by: Marco Elver > > > > Signed-off-by: Marco Elver > > > > --- > > > [Resending this because my email client re-defaulted to HTML! Aarrgh!] > > > > > > This looks good to me! I tested it in UML and x86-64 w/ KASAN, and > > > both worked fine. > > > > > > Reviewed-by: David Gow > > > Tested-by: David Gow > > > > Thank you! > > > > > Thanks for sticking with this! > > > > Will these patches be landing in 5.11 or 5.12? > > > > I can't think of any reason not to have these in 5.11. We haven't > started staging things in the kselftest/kunit branch for 5.11 yet, > though. > > Patch 2 will probably need to be acked by Ted for ext4 first. > > Brendan, Shuah: can you make sure this doesn't get lost in patchwork? Looks good to me. I would definitely like to pick this up. But yeah, in order to pick up 2/2 we will need an ack from either Ted or Iurii. Ted seems to be busy right now, so I think I will just ask Shuah to go ahead and pick this patch up by itself and we or Ted can pick up patch 2/2 later. Cheers
Re: [PATCH v2] Documentation: kunit: provide guidance for testing many inputs
On Mon, Nov 23, 2020 at 2:59 PM Daniel Latypov wrote: > > usage.rst goes into a detailed section about faking out classes, but > currently lacks wording about how one might idiomatically test a range > of inputs. > > Add a new chapter for "Common Patterns" and group "Isolating behvaior" > and this new section under there. > > Give an example of how one might test a hash function via macros/helper > funcs and a table-driven test and very briefly discuss pros and cons. > > Also highlight the KUNIT_EXPECT_*_MSG() variants (that aren't mentioned > elsewhere [1]) which are particularly useful in these situations. > > It is also criminally underused at the moment, only appearing in 2 > tests (both written by people involved in KUnit). > > [1] not even on > https://www.kernel.org/doc/html/latest/dev-tools/kunit/api/test.html > > Signed-off-by: Daniel Latypov Reviewed-by: Brendan Higgins
Re: [PATCH] kunit: kunit_tool: Correctly parse diagnostic messages
On Mon, Nov 9, 2020 at 11:29 PM David Gow wrote: > > Currently, kunit_tool expects all diagnostic lines in test results to > contain ": " somewhere, as both the subtest header and the crash report > do. Fix this to accept any line starting with (minus indent) "# " as > being a valid diagnostic line. > > This matches what the TAP spec[1] and the draft KTAP spec[2] are > expecting. > > [1]: http://testanything.org/tap-specification.html > [2]: > https://lore.kernel.org/linux-kselftest/cy4pr13mb1175b804e31e502221bc8163fd...@cy4pr13mb1175.namprd13.prod.outlook.com/T/ > > Signed-off-by: David Gow Reviewed-by: Brendan Higgins
Re: [PATCH] Documentation: kunit: provide guidance for testing many inputs
On Mon, Nov 2, 2020 at 1:37 PM Daniel Latypov wrote:
>
> usage.rst goes into a detailed about faking out classes, but currently
> lacks wording about how one might idiomatically test a range of inputs.
>
> Give an example of how one might test a hash function via macros/helper
> funcs and a table-driven test and very briefly discuss pros and cons.
>
> Also highlight the KUNIT_EXPECT_*_MSG() variants (that aren't mentioned
> elsewhere [1]) which are particularly useful in these situations.
>
> It is also criminally underused at the moment, only appearing in 2
> tests (both written by people involved in KUnit).
>
> [1] not even on
> https://www.kernel.org/doc/html/latest/dev-tools/kunit/api/test.html
>
> Signed-off-by: Daniel Latypov
Aside from the minor comment I made below, I like the patch; it is a
definite improvement, but I think the test you wrote that ultimately
led to this documentation fix had more information in it than this
documentation. I think it only contains the pattern that you outlined
here, but I think it does include some other best practices. Maybe we
should add some more documentation patches with more code examples in
the future?
Anyway, like I said, I think this patch in and of itself looks pretty good.
Reviewed-by: Brendan Higgins
> ---
> Documentation/dev-tools/kunit/usage.rst | 66 +
> 1 file changed, 66 insertions(+)
>
> diff --git a/Documentation/dev-tools/kunit/usage.rst
> b/Documentation/dev-tools/kunit/usage.rst
> index 62142a47488c..317390df2b96 100644
> --- a/Documentation/dev-tools/kunit/usage.rst
> +++ b/Documentation/dev-tools/kunit/usage.rst
> @@ -451,6 +451,72 @@ We can now use it to test ``struct eeprom_buffer``:
> destroy_eeprom_buffer(ctx->eeprom_buffer);
> }
>
> +Testing various inputs
> +--
Since this, by my count, the second test pattern that we are
introducing here, could we maybe call that out with a subheading or a
new section or something? It would be nice if we could sort of build
up a cookbook of testing patterns.
> +Testing just a few inputs might not be enough to have confidence that the
> code
> +works correctly, e.g. for a hash function.
> +
> +In such cases, it can be helpful to have a helper macro or function, e.g.
> this
> +fictitious example for ``md5sum(1)``
> +
> +.. code-block:: c
> +
> + /* Note: the cast is to satisfy overly strict type-checking. */
> + #define TEST_MD5(in, want) \
> + md5sum(in, out); \
> + KUNIT_EXPECT_STREQ_MSG(test, (char *)out, want, "md5sum(%s)",
> in);
> +
> + char out[16];
> + TEST_MD5("hello world", "5eb63bbbe01eeed093cb22bb8f5acdc3");
> + TEST_MD5("hello world!", "fc3ff98e8c6a0d3087d515c0473f8677");
> +
> +Note the use of ``KUNIT_EXPECT_STREQ_MSG`` to give more context when it fails
> +and make it easier to track down. (Yes, in this example, ``want`` is likely
> +going to be unique enough on its own).
> +
> +The ``_MSG`` variants are even more useful when the same expectation is
> called
> +multiple times (in a loop or helper function) and thus the line number isn't
> +enough to identify what failed, like below.
> +
> +In some cases, it can be helpful to write a *table-driven test* instead, e.g.
> +
> +.. code-block:: c
> +
> + int i;
> + char out[16];
> +
> + struct md5_test_case {
> + const char *str;
> + const char *md5;
> + };
> +
> + struct md5_test_case cases[] = {
> + {
> + .str = "hello world",
> + .md5 = "5eb63bbbe01eeed093cb22bb8f5acdc3",
> + },
> + {
> + .str = "hello world!",
> + .md5 = "fc3ff98e8c6a0d3087d515c0473f8677",
> + },
> + };
> + for (i = 0; i < ARRAY_SIZE(cases); ++i) {
> + md5sum(cases[i].str, out);
> + KUNIT_EXPECT_STREQ_MSG(test, (char *)out, cases[i].md5,
> + "md5sum(%s)", cases[i].str);
> + }
> +
> +
> +There's more boilerplate involved, but it can:
> +
> +* be more readable when there are multiple inputs/outputs thanks to field
> names,
> +
> + * E.g. see ``fs/ext4/inode-test.c`` for an example of both.
> +* reduce duplication if test cases can be shared across multiple tests.
> +
> + * E.g. if we had a magical ``undo_md5sum`` function, we could reuse
> ``cases``.
> +
> .. _kunit-on-non-uml:
>
> KUnit on non-UML architectures
>
> base-commit: 77c8473edf7f7664137f555cfcdc8c460bbd947d
> --
> 2.29.1.341.ge80a0c044ae-goog
>
Re: [RFC PATCH] bpf: preload: Fix build error when O= is set
On Thu, Nov 19, 2020 at 12:50 AM David Gow wrote: > > If BPF_PRELOAD is enabled, and an out-of-tree build is requested with > make O=, compilation seems to fail with: > > tools/scripts/Makefile.include:4: *** O=.kunit does not exist. Stop. > make[4]: *** [../kernel/bpf/preload/Makefile:8: kernel/bpf/preload/libbpf.a] > Error 2 > make[3]: *** [../scripts/Makefile.build:500: kernel/bpf/preload] Error 2 > make[2]: *** [../scripts/Makefile.build:500: kernel/bpf] Error 2 > make[2]: *** Waiting for unfinished jobs > make[1]: *** [.../Makefile:1799: kernel] Error 2 > make[1]: *** Waiting for unfinished jobs > make: *** [Makefile:185: __sub-make] Error 2 > > By the looks of things, this is because the (relative path) O= passed on > the command line is being passed to the libbpf Makefile, which then > can't find the directory. Given OUTPUT= is being passed anyway, we can > work around this by explicitly setting an empty O=, which will be > ignored in favour of OUTPUT= in tools/scripts/Makefile.include. > > Signed-off-by: David Gow Seems sensible to me. I have no strong feeling as to whether we just turn this off on UML or whether we do the fix you proposed here though. Nevertheless, I would like to see *some* fix go in before v5.10 is released. Reviewed-by: Brendan Higgins
[PATCH v1] kunit: tool: unmark test_data as binary blobs
The tools/testing/kunit/test_data/ directory was marked as binary
because some of the test_data files cause checkpatch warnings. Fix this
by dropping the .gitattributes file.
Fixes: afc63da64f1e ("kunit: kunit_parser: make parser more robust")
Signed-off-by: Brendan Higgins
---
tools/testing/kunit/.gitattributes | 1 -
1 file changed, 1 deletion(-)
delete mode 100644 tools/testing/kunit/.gitattributes
diff --git a/tools/testing/kunit/.gitattributes
b/tools/testing/kunit/.gitattributes
deleted file mode 100644
index 5b7da1fc3b8f1..0
--- a/tools/testing/kunit/.gitattributes
+++ /dev/null
@@ -1 +0,0 @@
-test_data/* binary
base-commit: 4ef8451b332662d004df269d4cdeb7d9f31419b5
--
2.29.1.341.ge80a0c044ae-goog
Re: [PATCH] kunit: fix display of failed expectations for strings
On Mon, Nov 2, 2020 at 3:23 PM Daniel Latypov wrote: > > Currently the following expectation > KUNIT_EXPECT_STREQ(test, "hi", "bye"); > will produce: > Expected "hi" == "bye", but > "hi" == 1625079497 > "bye" == 1625079500 > > After this patch: > Expected "hi" == "bye", but > "hi" == hi > "bye" == bye > > KUNIT_INIT_BINARY_STR_ASSERT_STRUCT() was written but just mistakenly > not actually used by KUNIT_EXPECT_STREQ() and friends. > > Signed-off-by: Daniel Latypov Reviewed-by: Brendan Higgins Tested-by: Brendan Higgins
Re: [PATCH v3] lib: Convert test_printf.c to KUnit
On Tue, Nov 3, 2020 at 8:22 AM Greg KH wrote:
>
> On Tue, Nov 03, 2020 at 05:11:47PM +0100, Petr Mladek wrote:
> > On Tue 2020-11-03 12:52:23, Greg KH wrote:
> > > On Tue, Nov 03, 2020 at 01:33:53PM +0200, Andy Shevchenko wrote:
> > > > On Tue, Nov 03, 2020 at 04:40:49PM +0530, Arpitha Raghunandan wrote:
> > > > > Convert test lib/test_printf.c to KUnit. More information about
> > > > > KUnit can be found at:
> > > > > https://www.kernel.org/doc/html/latest/dev-tools/kunit/index.html.
> > > > > KUnit provides a common framework for unit tests in the kernel.
> > > > > KUnit and kselftest are standardizing around KTAP, converting this
> > > > > test to KUnit makes this test output in KTAP which we are trying to
> > > > > make the standard test result format for the kernel. More about
> > > > > the KTAP format can be found at:
> > > > > https://lore.kernel.org/linux-kselftest/cy4pr13mb1175b804e31e502221bc8163fd...@cy4pr13mb1175.namprd13.prod.outlook.com/.
> > > > > I ran both the original and converted tests as is to produce the
> > > > > output for success of the test in the two cases. I also ran these
> > > > > tests with a small modification to show the difference in the output
> > > > > for failure of the test in both cases. The modification I made is:
> > > > > - test("127.000.000.001|127.0.0.1", "%pi4|%pI4", _addr,
> > > > > _addr);
> > > > > + test("127-000.000.001|127.0.0.1", "%pi4|%pI4", _addr,
> > > > > _addr);
> > > > >
> > > > > Original test success:
> > > > > [0.540860] test_printf: loaded.
> > > > > [0.540863] test_printf: random seed = 0x5c46c33837bc0619
> > > > > [0.541022] test_printf: all 388 tests passed
> > > > >
> > > > > Original test failure:
> > > > > [0.537980] test_printf: loaded.
> > > > > [0.537983] test_printf: random seed = 0x1bc1efd881954afb
> > > > > [0.538029] test_printf: vsnprintf(buf, 256, "%pi4|%pI4", ...)
> > > > > wrote '127.000.000.001|127.0.0.1', expected
> > > > > '127-000.000.001|127.0.0.1'
> > > > > [0.538030] test_printf: kvasprintf(..., "%pi4|%pI4", ...)
> > > > > returned '127.000.000.001|127.0.0.1', expected
> > > > > '127-000.000.001|127.0.0.1'
> > > > > [0.538124] test_printf: failed 2 out of 388 tests
> > > > > [0.538125] test_printf: random seed used was 0x1bc1efd881954afb
> > > > >
> > > > > Converted test success:
> > > > > # Subtest: printf
> > > > > 1..25
> > > > > ok 1 - test_basic
> > > > > ok 2 - test_number
> > > > > ok 3 - test_string
> > > > > ok 4 - plain
> > > > > ok 5 - null_pointer
> > > > > ok 6 - error_pointer
> > > > > ok 7 - invalid_pointer
> > > > > ok 8 - symbol_ptr
> > > > > ok 9 - kernel_ptr
> > > > > ok 10 - struct_resource
> > > > > ok 11 - addr
> > > > > ok 12 - escaped_str
> > > > > ok 13 - hex_string
> > > > > ok 14 - mac
> > > > > ok 15 - ip
> > > > > ok 16 - uuid
> > > > > ok 17 - dentry
> > > > > ok 18 - struct_va_format
> > > > > ok 19 - time_and_date
> > > > > ok 20 - struct_clk
> > > > > ok 21 - bitmap
> > > > > ok 22 - netdev_features
> > > > > ok 23 - flags
> > > > > ok 24 - errptr
> > > > > ok 25 - fwnode_pointer
> > > > > ok 1 - printf
> > > > >
> > > > > Converted test failure:
> > > > > # Subtest: printf
> > > > > 1..25
> > > > > ok 1 - test_basic
> > > > > ok 2 - test_number
> > > > > ok 3 - test_string
> > > > > ok 4 - plain
> > > > > ok 5 - null_pointer
> > > > > ok 6 - error_pointer
> > > > > ok 7 - invalid_pointer
> > > > > ok 8 - symbol_ptr
> > > > > ok 9 - kernel_ptr
> > > > > ok 10 - struct_resource
> > > > > ok 11 - addr
> > > > > ok 12 - escaped_str
> > > > > ok 13 - hex_string
> > > > > ok 14 - mac
> > > > > # ip: EXPECTATION FAILED at lib/printf_kunit.c:82
> > > > > vsnprintf(buf, 256, "%pi4|%pI4", ...) wrote
> > > > > '127.000.000.001|127.0.0.1', expected '127-000.000.001|127.0.0.1'
> > > > > # ip: EXPECTATION FAILED at lib/printf_kunit.c:124
> > > > > kvasprintf(..., "%pi4|%pI4", ...) returned
> > > > > '127.000.000.001|127.0.0.1', expected '127-000.000.001|127.0.0.1'
> > > > > not ok 15 - ip
> > > > > ok 16 - uuid
> > > > > ok 17 - dentry
> > > > > ok 18 - struct_va_format
> > > > > ok 19 - time_and_date
> > > > > ok 20 - struct_clk
> > > > > ok 21 - bitmap
> > > > > ok 22 - netdev_features
> > > > > ok 23 - flags
> > > > > ok 24 - errptr
> > > > > ok 25 - fwnode_pointer
> > > > > not ok 1 - printf
> > > >
> > > > Better, indeed.
> > > >
> > > > But can be this improved to have a cumulative statistics, like showing
> > > > only
> > > > number of total, succeeded, failed with details of the latter ones?
> > >
> > > Is that the proper test output format? We have a standard...
> >
> > What is the standard, please?
>
> The TAP format should be the standard, no reason the kernel can not spit
> out the same test message format that the
Re: [RFC v2 00/12] kunit: introduce class mocking support.
+Joel Stanley +Daniel Vetter
If I remember correctly, both of you said you were interested in
mocking on KUnit. This RFC only has some of the mocking features that
I mentioned previously, but I would still like to get your thoughts.
On Mon, Oct 12, 2020 at 3:21 PM Daniel Latypov wrote:
>
> # Background
> KUnit currently lacks any first-class support for mocking.
> For an overview and discussion on the pros and cons, see
> https://martinfowler.com/articles/mocksArentStubs.html
>
> This patch set introduces the basic machinery needed for mocking:
> setting and validating expectations, setting default actions, etc.
>
> Using that basic infrastructure, we add macros for "class mocking", as
> it's probably the easiest type of mocking to start with.
>
> ## Class mocking
>
> By "class mocking", we're referring mocking out function pointers stored
> in structs like:
> struct sender {
> int (*send)(struct sender *sender, int data);
> };
> or in ops structs
> struct sender {
> struct send_ops *ops; // contains `send`
> };
>
> After the necessary DEFINE_* macros, we can then write code like
> struct MOCK(sender) mock_sender = CONSTRUCT_MOCK(sender, test);
>
> /* Fake an error for a specific input. */
> handle = KUNIT_EXPECT_CALL(send(, kunit_int_eq(42)));
> handle->action = kunit_int_return(test, -EINVAL);
>
> /* Pass the mocked object to some code under test. */
> KUNIT_EXPECT_EQ(test, -EINVAL, send_message(...));
>
> I.e. the goal is to make it easier to test
> 1) with less dependencies (we don't need to setup a real `sender`)
> 2) unusual/error conditions more easily.
>
> In the future, we hope to build upon this to support mocking in more
> contexts, e.g. standalone funcs, etc.
>
> # TODOs
>
> ## Naming
> This introduces a number of new macros for dealing with mocks,
> e.g:
> DEFINE_STRUCT_CLASS_MOCK(METHOD(foo), CLASS(example),
>RETURNS(int),
>PARAMS(struct example *, int));
> ...
> KUNIT_EXPECT_CALL(foo(mock_get_ctrl(mock_example), ...);
> For consistency, we could prefix everything with KUNIT, e.g.
> `KUNIT_DEFINE_STRUCT_CLASS_MOCK` and `kunit_mock_get_ctrl`, but it feels
> like the names might be long enough that they would hinder readability.
>
> ## Usage
> For now the only use of class mocking is in kunit-example-test.c
> As part of changing this from an RFC to a real patch set, we're hoping
> to include at least one example.
>
> Pointers to bits of code where this would be useful that aren't too
> hairy would be appreciated.
> E.g. could easily add a test for tools/perf/ui/progress.h, e.g. that
> ui_progress__init() calls ui_progress_ops.init(), but that likely isn't
> useful to anyone.
>
> ---
> v2:
> * Pass `struct kunit *` to mock init's to allow allocating ops structs.
> * Update kunit-example-test.cc to do so as a more realistic example.
> v1:
> https://lore.kernel.org/linux-kselftest/20200918183114.2571146-1-dlaty...@google.com/
> ---
>
> Brendan Higgins (9):
> kunit: test: add kunit_stream a std::stream like logger
> kunit: test: add concept of post conditions
> checkpatch: add support for struct MOCK(foo) syntax
> kunit: mock: add parameter list manipulation macros
> kunit: mock: add internal mock infrastructure
> kunit: mock: add basic matchers and actions
> kunit: mock: add class mocking support
> kunit: mock: add struct param matcher
> kunit: mock: implement nice, strict and naggy mock distinctions
>
> Daniel Latypov (2):
> Revert "kunit: move string-stream.h to lib/kunit"
> kunit: expose kunit_set_failure() for use by mocking
>
> Marcelo Schmitt (1):
> kunit: mock: add macro machinery to pick correct format args
>
> include/kunit/assert.h | 3 +-
> include/kunit/kunit-stream.h | 94 +++
> include/kunit/mock.h | 902 +
> include/kunit/params.h | 305 +
> {lib => include}/kunit/string-stream.h | 2 +
> include/kunit/test.h | 9 +
> lib/kunit/Makefile | 9 +-
> lib/kunit/assert.c | 2 -
> lib/kunit/common-mocks.c | 409 +++
> lib/kunit/kunit-example-test.c | 98 +++
> lib/kunit/kunit-stream.c | 110 +++
> lib/kunit/mock-macro-test.c| 241 +++
> lib/kunit/mock-test.c | 531 +++
> lib/kunit/mock.c | 370 ++
> lib/kunit/string-stream-test.c | 3 +-
> lib/kunit/string-stream.c | 5 +-
> lib/kunit/test.c
Re: [PATCH] kunit: Fix kunit.py parse subcommand (use null build_dir)
On Wed, Oct 21, 2020 at 12:16 AM David Gow wrote:
>
> When JSON support was added in [1], the KunitParseRequest tuple was
> updated to contain a 'build_dir' field, but kunit.py parse doesn't
> accept --build_dir as an option. The code nevertheless tried to access
> it, resulting in this error:
>
> AttributeError: 'Namespace' object has no attribute 'build_dir'
>
> Given that the parser only uses the build_dir variable to set the
> 'build_environment' json field, we set it to None (which gives the JSON
> 'null') for now. Ultimately, we probably do want to be able to set this,
> but since it's new functionality which (for the parse subcommand) never
> worked, this is the quickest way of getting it back up and running.
>
> [1]:
> https://git.kernel.org/pub/scm/linux/kernel/git/shuah/linux-kselftest.git/commit/?h=kunit-fixes=21a6d1780d5bbfca0ce9b8104ca6233502fcbf86
>
> Fixes: 21a6d1780d5bbfca0ce9b8104ca6233502fcbf86 ("kunit: tool: allow
> generating test results in JSON")
> Signed-off-by: David Gow
Reviewed-by: Brendan Higgins
Tested-by: Brendan Higgins
Re: [PATCH 2/2] kunit: tool: Mark 'kunittest_config' as constant again
On Sun, Oct 25, 2020 at 5:45 AM wrote:
>
> On Thu, Oct 22, 2020 at 08:35:26AM +0200, SeongJae Park wrote:
> > On Wed, 21 Oct 2020 14:32:52 -0700 Brendan Higgins
> > wrote:
> >
> > > On Mon, Oct 12, 2020 at 3:27 AM SeongJae Park wrote:
> > > >
> > > > From: SeongJae Park
> > > >
> > > > 'kunit_kernel.kunittest_config' was constant at first, and therefore it
> > > > used UPPER_SNAKE_CASE naming convention that usually means it is
> > > > constant in Python world. But, commit e3212513a8f0 ("kunit: Create
> > > > default config in '--build_dir'") made it modifiable to fix a use case
> > > > of the tool and thus the naming also changed to lower_snake_case.
> > > > However, this resulted in a confusion. As a result, some successing
> > > > changes made the tool unittest fail, and a fix[1] of it again incurred
> > > > the '--build_dir' use case failure.
> > > >
> > > > As the previous commit fixed the '--build_dir' use case without
> > > > modifying the variable again, this commit marks the variable as constant
> > > > again with UPPER_SNAKE_CASE, to reduce future confusions.
> > > >
> > > > [1] Commit d43c7fb05765 ("kunit: tool: fix improper treatment of file
> > > > location")
> > > >
> > > > Signed-off-by: SeongJae Park
> > >
> > > Reviewed-by: Brendan Higgins
> >
> > Thanks :)
> >
> > >
> > > Thanks for this! This is something I meant to fix a while ago and forgot
> > > about.
> > >
> > > One minor issue, this patch does not apply on torvalds/master right
> > > now. Could you please rebase this?
> >
> > Surely of course, I will send next version soon.
>
> May I ask what happened to [1]?
> I mean it seems these two are goind to collide.
>
> Brendan?
>
> [1]:
> https://lore.kernel.org/linux-kselftest/20201015152348.65147-1-andriy.shevche...@linux.intel.com/
Sorry for the confusion here. After an initial glance at your patches
(before I did the review end of last week) I thought only the first
patch from SeongJae would potentially conflict with yours (Andy's)
(hence why I hadn't reviewed it yet, I was waiting until after I
looked at yours).
I noticed on Friday that SeongJae's changes were actually fully
encompassed by Andy's, so I am taking Andy's not SongJae's. Sorry, I
was going to notify SongJae today, but you beat me to it.
Sorry everyone.
Re: [PATCH] ext: EXT4_KUNIT_TESTS should depend on EXT4_FS instead of selecting it
On Wed, Oct 21, 2020 at 3:36 PM Theodore Y. Ts'o wrote:
>
> On Wed, Oct 21, 2020 at 02:16:56PM -0700, Randy Dunlap wrote:
> > On 10/21/20 2:15 PM, Brendan Higgins wrote:
> > > On Tue, Oct 20, 2020 at 12:37 AM Geert Uytterhoeven
> > > wrote:
> > >>
> > >> EXT4_KUNIT_TESTS selects EXT4_FS, thus enabling an optional feature the
> > >> user may not want to enable. Fix this by making the test depend on
> > >> EXT4_FS instead.
> > >>
> > >> Fixes: 1cbeab1b242d16fd ("ext4: add kunit test for decoding extended
> > >> timestamps")
> > >> Signed-off-by: Geert Uytterhoeven
> > >
> > > If I remember correctly, having EXT4_KUNIT_TESTS select EXT4_FS was
> > > something that Ted specifically requested, but I don't have any strong
> > > feelings on it either way.
> >
> > omg, please No. depends on is the right fix here.
>
> So my requirement which led to that particular request is to keep what
> needs to be placed in .kunitconfig to a small and reasonable set.
>
> Per Documentation/dev-tools/kunit, we start by:
>
> cd $PATH_TO_LINUX_REPO
> cp arch/um/configs/kunit_defconfig .kunitconfig
>
> we're then supposed to add whatever Kunit tests we want to enable, to wit:
>
> CONFIG_EXT4_KUNIT_TESTS=y
>
> so that .kunitconfig would look like this:
>
> CONFIG_KUNIT=y
> CONFIG_KUNIT_TEST=y
> CONFIG_KUNIT_EXAMPLE_TEST=y
> CONFIG_EXT4_KUNIT_TESTS=y
>
> ... and then you should be able to run:
>
> ./tools/testing/kunit/kunit.py run
>
> ... and have the kunit tests run. I would *not* like to have to put a
> huge long list of CONFIG_* dependencies into the .kunitconfig file.
>
> I'm don't particularly care how this gets achieved, but please think
> about how to make it easy for a kernel developer to run a specific set
> of subsystem unit tests. (In fact, being able to do something like
> "kunit.py run fs/ext4 fs/jbd2" or maybe "kunit.py run fs/..." would be
> *great*. No need to fuss with hand editing the .kunitconfig file at
> all would be **wonderful**.
So you, me, Luis, David, and a whole bunch of other people have been
thinking about this problem for a while. What if we just put
kunitconfig fragments in directories along side the test files they
enable?
For example, we could add a file to fs/ext4/kunitconfig which contains:
CONFIG_EXT4_FS=y
CONFIG_EXT4_KUNIT_TESTS=y
We could do something similar in fs/jdb2, etc.
Obviously some logically separate KUnit tests (different maintainers,
different Kconfig symbols, etc) reside in the same directory, for
these we could name the kunitconfig file something like
lib/list-test.kunitconfig (not a great example because lists are
always built into Linux), but you get the idea.
Then like Ted suggested, if you call kunit.py run foo/bar, then
if bar is a directory, then kunit.py will look for foo/bar/kunitconfig
if bar is a file ending with .kunitconfig like foo/bar.kunitconfig,
then it will use that kunitconfig
if bar is '...' (foo/...) then kunit.py will look for all kunitconfigs
underneath foo.
Once all the kunitconfigs have been resolved, they will be merged into
the .kunitconfig. If they can be successfully merged together, the new
.kunitconfig will then continue to function as it currently does.
What do people think about this?
Re: [PATCH] lib: add basic KUnit test for lib/math
On Thu, Oct 22, 2020 at 12:04 PM Andy Shevchenko wrote: > > On Thu, Oct 22, 2020 at 11:53:50AM -0700, Brendan Higgins wrote: > > On Thu, Oct 22, 2020 at 8:06 AM Andy Shevchenko > > wrote: > > > On Wed, Oct 21, 2020 at 10:47:50AM -0700, Daniel Latypov wrote: > > ... > > > > You need to put detailed comments in the code to have it as real example > > > how to > > > create the KUnit test. But hey, it will mean that documentation sucks. So, > > > please update documentation to cover issues that you found and which > > > motivated > > > you to create these test cases. > > > > I don't entirely disagree; leaning too heavily on code examples can be > > detrimental to docs. That being said, when I use other people's code, > > I often don't even look at the docs. So, I think the ideal is to have > > both. > > Why do we have docs in the first place? > For test cases I think it's a crucial part, because tests many time are > written > by newbies, who would like to understand all under-the-hood stuff. You imply Good point. Yeah, we don't really have any documentation that explains the internals at all. I agree: we need to fix that. > that they need to drop themselves into the code directly. It's very harsh to > begin with Linux kernel development, really. No, I was not trying to imply that everyone should just jump in the code and ignore the docs. I was just trying to point out that some people - myself included - rather see code than docs. Clearly some people prefer docs over code as well. Thus, I was trying to argue that both are appropriate. Nevertheless, based on the other comments you sent, I don't think that's what we are talking about: sounds like you just want us to improve the docs first to make sure we do it. That's fine with me. > > > Summarize this, please create usable documentation first. > > So, no go for this w/o documentation being up-to-date. Or be honest to > everybody, it's sucks it needs to be removed. Then I will get your point. > > -- > With Best Regards, > Andy Shevchenko > >
Re: [PATCH] lib: add basic KUnit test for lib/math
On Thu, Oct 22, 2020 at 8:06 AM Andy Shevchenko wrote: > > On Wed, Oct 21, 2020 at 10:47:50AM -0700, Daniel Latypov wrote: > > On Tue, Oct 20, 2020 at 8:40 PM David Gow wrote: > > > On Tue, Oct 20, 2020 at 6:46 AM Daniel Latypov > > > wrote: > > > > > > > > Add basic test coverage for files that don't require any config options: > > > > * gcd.c > > > > * lcm.c > > > > * int_sqrt.c > > > > * reciprocal_div.c > > > > (Ignored int_pow.c since it's a simple textbook algorithm.) > > > > > > > I don't see a particular reason why int_pow.c being a simple algorithm > > > means it shouldn't be tested. I'm not saying it has to be tested by > > > this particular change -- and I doubt the test would be > > > earth-shatteringly interesting -- but there's no real reason against > > > testing it. > > > > Agreed on principle, but int_pow() feels like a special case. > > I've written it the exact same way (modulo variable names+types) > > several times in personal projects. > > Even the spacing matched exactly in a few of those... > > But if you would like to *teach* somebody by this exemplary piece of code, you > better do it close to ideal. > > > > > These tests aren't particularly interesting, but > > > > * they're chosen as easy to understand examples of how to write tests > > > > * provides a place to add tests for any new files in this dir > > > > * written so adding new test cases to cover edge cases should be easy > > > > > > I think these tests can stand on their own merits, rather than just as > > > examples (though I do think they do make good additional examples for > > > how to test these sorts of functions). > > > So, I'd treat this as an actual test of the maths functions (and > > > you've got what seems to me a decent set of test cases for that, > > > though there are a couple of comments below) first, and any use it > > > gains as an example is sort-of secondary to that (anything that makes > > > it a better example is likely to make it a better test anyway). > > > > > > In any case, modulo the comments below, this seems good to me. > > > > Ack. > > I'll wait on Andy's input before deciding whether or not to push out a > > v2 with the changes. > > You need to put detailed comments in the code to have it as real example how > to > create the KUnit test. But hey, it will mean that documentation sucks. So, > please update documentation to cover issues that you found and which motivated > you to create these test cases. I don't entirely disagree; leaning too heavily on code examples can be detrimental to docs. That being said, when I use other people's code, I often don't even look at the docs. So, I think the ideal is to have both. > Summarize this, please create usable documentation first.
Re: [PATCH] lib: add basic KUnit test for lib/math
On Thu, Oct 22, 2020 at 9:26 AM Daniel Latypov wrote: > > On Thu, Oct 22, 2020 at 8:06 AM Andy Shevchenko > wrote: > > > > On Wed, Oct 21, 2020 at 10:47:50AM -0700, Daniel Latypov wrote: > > > On Tue, Oct 20, 2020 at 8:40 PM David Gow wrote: > > > > On Tue, Oct 20, 2020 at 6:46 AM Daniel Latypov > > > > wrote: > > > > > > > > > > Add basic test coverage for files that don't require any config > > > > > options: > > > > > * gcd.c > > > > > * lcm.c > > > > > * int_sqrt.c > > > > > * reciprocal_div.c > > > > > (Ignored int_pow.c since it's a simple textbook algorithm.) > > > > > > > > > I don't see a particular reason why int_pow.c being a simple algorithm > > > > means it shouldn't be tested. I'm not saying it has to be tested by > > > > this particular change -- and I doubt the test would be > > > > earth-shatteringly interesting -- but there's no real reason against > > > > testing it. > > > > > > Agreed on principle, but int_pow() feels like a special case. > > > I've written it the exact same way (modulo variable names+types) > > > several times in personal projects. > > > Even the spacing matched exactly in a few of those... > > > > But if you would like to *teach* somebody by this exemplary piece of code, > > you > > better do it close to ideal. > > > > > > > These tests aren't particularly interesting, but > > > > > * they're chosen as easy to understand examples of how to write tests > > > > > * provides a place to add tests for any new files in this dir > > > > > * written so adding new test cases to cover edge cases should be easy > > > > > > > > I think these tests can stand on their own merits, rather than just as > > > > examples (though I do think they do make good additional examples for > > > > how to test these sorts of functions). > > > > So, I'd treat this as an actual test of the maths functions (and > > > > you've got what seems to me a decent set of test cases for that, > > > > though there are a couple of comments below) first, and any use it > > > > gains as an example is sort-of secondary to that (anything that makes > > > > it a better example is likely to make it a better test anyway). > > > > > > > > In any case, modulo the comments below, this seems good to me. > > > > > > Ack. > > > I'll wait on Andy's input before deciding whether or not to push out a > > > v2 with the changes. > > > > You need to put detailed comments in the code to have it as real example > > how to > > create the KUnit test. But hey, it will mean that documentation sucks. So, > > Out of curiosity > * By "it will mean that documentation sucks," do you mean that the > documentation will rot faster if people are using the existing in-tree > tests as their entrypoint? > * What level of detailed comments? On the level of kunit-example-test.c? > * More concretely, then we'd have a comment block linking to the > example and then describing table driven unit testing? > * And then maybe another block about invariants instead of the > perhaps too-terse /* gcd(a,b) == gcd(b,a) */ ? > > > please update documentation to cover issues that you found and which > > motivated > > you to create these test cases. > > > > Summarize this, please create usable documentation first. > > Sounds good. > I'm generally wary people not reading the docs, and of documentation > examples becoming bitrotted faster than actual code. > But so far KUnit seems to be doing relatively well on both fronts. > > usage.rst is currently the best place for this, but it felt like it > would quickly become a dumping ground for miscellaneous tips and > tricks. Yeah, I think it has already started to become a dumping ground for everything; it already has everything except: getting started, FAQ, API reference, and some minor details about the command line tool. > I'll spend some time thinking if we want a new file or not, based on > how much I want to write about the things this test demonstrated > (EXPECT_*MSG, table driven tests, testing invariants, etc). > > In offline discussions with David, the idea had come up with having a > set of (relatively) simple tests in tree that the documentation could > point to as examples of these things. That would keep the line count > in usage.rst down a bit. > (But then it would necessitate more tests like this math_test.c) I do like the idea of having as much example code as possible in a place where it actually gets compiled occasionally. One of the biggest bitrot issues we have had in KUnit documentation so far is example code falling out of date; that's less likely to happen if the examples get compiled. It would be super cool if there was some way to have example code in the documentation that also gets compiled and run, but I don't believe that that would be a feasible thing to do right now. In any case, not to sound hypocritical or lazy, but when I use other people's code I tend to trust the code a lot more than I trust the docs. Even if we do an absolutely stellar job with our docs, I suspect
Re: [PATCH] kunit: Fix kunit.py --raw_output option
On Wed, Oct 21, 2020 at 8:05 PM David Gow wrote:
>
> Due to the raw_output() function on kunit_parser.py actually being a
> generator, it only runs if something reads the lines it returns. Since
> we no-longer do that (parsing doesn't actually happen if raw_output is
> enabled), it was not printing anything.
>
> Fixes: 45ba7a893ad89114e773b3dc32f6431354c465d6 ("kunit: kunit_tool:
> Separate out config/build/exec/parse")
> Signed-off-by: David Gow
Thanks for fixing this!
Reviewed-by: Brendan Higgins
Tested-by: Brendan Higgins
Re: [PATCH 2/2] kunit: tool: Mark 'kunittest_config' as constant again
On Mon, Oct 12, 2020 at 3:27 AM SeongJae Park wrote:
>
> From: SeongJae Park
>
> 'kunit_kernel.kunittest_config' was constant at first, and therefore it
> used UPPER_SNAKE_CASE naming convention that usually means it is
> constant in Python world. But, commit e3212513a8f0 ("kunit: Create
> default config in '--build_dir'") made it modifiable to fix a use case
> of the tool and thus the naming also changed to lower_snake_case.
> However, this resulted in a confusion. As a result, some successing
> changes made the tool unittest fail, and a fix[1] of it again incurred
> the '--build_dir' use case failure.
>
> As the previous commit fixed the '--build_dir' use case without
> modifying the variable again, this commit marks the variable as constant
> again with UPPER_SNAKE_CASE, to reduce future confusions.
>
> [1] Commit d43c7fb05765 ("kunit: tool: fix improper treatment of file
> location")
>
> Signed-off-by: SeongJae Park
Reviewed-by: Brendan Higgins
Thanks for this! This is something I meant to fix a while ago and forgot about.
One minor issue, this patch does not apply on torvalds/master right
now. Could you please rebase this?
Re: [PATCH] ext: EXT4_KUNIT_TESTS should depend on EXT4_FS instead of selecting it
On Tue, Oct 20, 2020 at 12:37 AM Geert Uytterhoeven
wrote:
>
> EXT4_KUNIT_TESTS selects EXT4_FS, thus enabling an optional feature the
> user may not want to enable. Fix this by making the test depend on
> EXT4_FS instead.
>
> Fixes: 1cbeab1b242d16fd ("ext4: add kunit test for decoding extended
> timestamps")
> Signed-off-by: Geert Uytterhoeven
If I remember correctly, having EXT4_KUNIT_TESTS select EXT4_FS was
something that Ted specifically requested, but I don't have any strong
feelings on it either way.
Re: [PATCH v3 5/6] kunit: test: fix remaining kernel-doc warnings
On Wed, Oct 21, 2020 at 5:17 AM Mauro Carvalho Chehab
wrote:
>
> test.h still produce three warnings:
>
> include/kunit/test.h:282: warning: Function parameter or member
> '__suites' not described in 'kunit_test_suites_for_module'
> include/kunit/test.h:282: warning: Excess function parameter
> 'suites_list' description in 'kunit_test_suites_for_module'
> include/kunit/test.h:314: warning: Excess function parameter 'suites'
> description in 'kunit_test_suites'
>
> They're all due to errors at kernel-doc markups. Update them.
>
> It should be noticed that this patch moved a kernel-doc
> markup that were located at the wrong place, and using a wrong
> name. Kernel-doc only supports kaving the markup just before the
> function/macro declaration. Placing it elsewhere will make it do
> wrong assumptions.
>
> Fixes: aac35468ca20 ("kunit: test: create a single centralized executor for
> all tests")
> Signed-off-by: Mauro Carvalho Chehab
Reviewed-by: Brendan Higgins
Tested-by: Brendan Higgins
Re: [PATCH] Documentation: kunit: Update Kconfig parts for KUNIT's module support
On Wed, Oct 21, 2020 at 12:32 PM SeongJae Park wrote:
>
> From: SeongJae Park
>
> If 'CONFIG_KUNIT=m', letting kunit tests that do not support loadable
> module build depends on 'KUNIT' instead of 'KUNIT=y' result in compile
> errors. This commit updates the document for this.
>
> Fixes: 9fe124bf1b77 ("kunit: allow kunit to be loaded as a module")
> Signed-off-by: SeongJae Park
> Reviewed-by: David Gow
Reviewed-by: Brendan Higgins
[PATCH v1] kunit: tools: fix kunit_tool tests for parsing test plans
Some tests logs for kunit_tool tests are missing their test plans
causing their tests to fail; fix this by adding the test plans.
Fixes: 45dcbb6f5ef7 ("kunit: test: add test plan to KUnit TAP format")
Signed-off-by: Brendan Higgins
---
tools/testing/kunit/kunit_tool_test.py| 32 ++
.../test_data/test_config_printk_time.log | Bin 1584 -> 1605 bytes
.../test_data/test_interrupted_tap_output.log | Bin 1982 -> 2003 bytes
.../test_data/test_kernel_panic_interrupt.log | Bin 1321 -> 1342 bytes
.../test_data/test_multiple_prefixes.log | Bin 1832 -> 1861 bytes
.../kunit/test_data/test_pound_no_prefix.log | Bin 1193 -> 1200 bytes
.../kunit/test_data/test_pound_sign.log | Bin 1656 -> 1676 bytes
7 files changed, 25 insertions(+), 7 deletions(-)
diff --git a/tools/testing/kunit/kunit_tool_test.py
b/tools/testing/kunit/kunit_tool_test.py
index 99c3c5671ea48..0b60855fb8198 100755
--- a/tools/testing/kunit/kunit_tool_test.py
+++ b/tools/testing/kunit/kunit_tool_test.py
@@ -179,7 +179,7 @@ class KUnitParserTest(unittest.TestCase):
print_mock = mock.patch('builtins.print').start()
result = kunit_parser.parse_run_tests(
kunit_parser.isolate_kunit_output(file.readlines()))
- print_mock.assert_any_call(StrContains("no kunit output
detected"))
+ print_mock.assert_any_call(StrContains('no tests run!'))
print_mock.stop()
file.close()
@@ -198,39 +198,57 @@ class KUnitParserTest(unittest.TestCase):
'test_data/test_config_printk_time.log')
with open(prefix_log) as file:
result = kunit_parser.parse_run_tests(file.readlines())
- self.assertEqual('kunit-resource-test', result.suites[0].name)
+ self.assertEqual(
+ kunit_parser.TestStatus.SUCCESS,
+ result.status)
+ self.assertEqual('kunit-resource-test',
result.suites[0].name)
def test_ignores_multiple_prefixes(self):
prefix_log = get_absolute_path(
'test_data/test_multiple_prefixes.log')
with open(prefix_log) as file:
result = kunit_parser.parse_run_tests(file.readlines())
- self.assertEqual('kunit-resource-test', result.suites[0].name)
+ self.assertEqual(
+ kunit_parser.TestStatus.SUCCESS,
+ result.status)
+ self.assertEqual('kunit-resource-test',
result.suites[0].name)
def test_prefix_mixed_kernel_output(self):
mixed_prefix_log = get_absolute_path(
'test_data/test_interrupted_tap_output.log')
with open(mixed_prefix_log) as file:
result = kunit_parser.parse_run_tests(file.readlines())
- self.assertEqual('kunit-resource-test', result.suites[0].name)
+ self.assertEqual(
+ kunit_parser.TestStatus.SUCCESS,
+ result.status)
+ self.assertEqual('kunit-resource-test',
result.suites[0].name)
def test_prefix_poundsign(self):
pound_log = get_absolute_path('test_data/test_pound_sign.log')
with open(pound_log) as file:
result = kunit_parser.parse_run_tests(file.readlines())
- self.assertEqual('kunit-resource-test', result.suites[0].name)
+ self.assertEqual(
+ kunit_parser.TestStatus.SUCCESS,
+ result.status)
+ self.assertEqual('kunit-resource-test',
result.suites[0].name)
def test_kernel_panic_end(self):
panic_log =
get_absolute_path('test_data/test_kernel_panic_interrupt.log')
with open(panic_log) as file:
result = kunit_parser.parse_run_tests(file.readlines())
- self.assertEqual('kunit-resource-test', result.suites[0].name)
+ self.assertEqual(
+ kunit_parser.TestStatus.TEST_CRASHED,
+ result.status)
+ self.assertEqual('kunit-resource-test',
result.suites[0].name)
def test_pound_no_prefix(self):
pound_log =
get_absolute_path('test_data/test_pound_no_prefix.log')
with open(pound_log) as file:
result = kunit_parser.parse_run_tests(file.readlines())
- self.assertEqual('kunit-resource-test', result.suites[0].name)
+ self.assertEqual(
+ kunit_parser.TestStatus.SUCCESS,
+
[PATCH v1] i2c: aspeed: add KUnit tests for clock parameters
Add KUnit tests for Aspeed I2C driver to test setting clock divider
registers given an input clock speed.
I wrote this test a while ago and it found a bug in the Aspeed I2C
driver a couple years ago[1].
Link[1]: https://lore.kernel.org/patchwork/patch/989312/
Signed-off-by: Brendan Higgins
---
MAINTAINERS | 1 +
drivers/i2c/busses/Kconfig | 18 ++-
drivers/i2c/busses/i2c-aspeed-test.c | 218 +++
drivers/i2c/busses/i2c-aspeed.c | 4 +
4 files changed, 240 insertions(+), 1 deletion(-)
create mode 100644 drivers/i2c/busses/i2c-aspeed-test.c
diff --git a/MAINTAINERS b/MAINTAINERS
index deaafb617361c..683382df2434a 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1653,6 +1653,7 @@ L:open...@lists.ozlabs.org (moderated for
non-subscribers)
S: Maintained
F: Documentation/devicetree/bindings/i2c/i2c-aspeed.txt
F:
Documentation/devicetree/bindings/interrupt-controller/aspeed,ast2400-i2c-ic.txt
+F: drivers/i2c/busses/i2c-aspeed-test.c
F: drivers/i2c/busses/i2c-aspeed.c
F: drivers/irqchip/irq-aspeed-i2c-ic.c
diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
index 293e7a0760e77..0f12090f9fe26 100644
--- a/drivers/i2c/busses/Kconfig
+++ b/drivers/i2c/busses/Kconfig
@@ -379,7 +379,7 @@ config I2C_ALTERA
config I2C_ASPEED
tristate "Aspeed I2C Controller"
- depends on ARCH_ASPEED || COMPILE_TEST
+ depends on ARCH_ASPEED || COMPILE_TEST || KUNIT=y
help
If you say yes to this option, support will be included for the
Aspeed I2C controller.
@@ -387,6 +387,22 @@ config I2C_ASPEED
This driver can also be built as a module. If so, the module
will be called i2c-aspeed.
+config I2C_ASPEED_KUNIT_TEST
+ bool "Aspeed I2C Controller KUnit test"
+ depends on I2C_ASPEED=y
+ help
+ This builds the Aspeed I2C KUnit tests.
+
+ KUnit tests run during boot and output the results to the debug log
+ in TAP format (https://testanything.org/). Only useful for kernel devs
+ running KUnit test harness and are not for inclusion into a
+ production build.
+
+ For more information on KUnit and unit tests in general please refer
+ to the KUnit documentation in Documentation/dev-tools/kunit/.
+
+ If unsure, say N.
+
config I2C_AT91
tristate "Atmel AT91 I2C Two-Wire interface (TWI)"
depends on ARCH_AT91 || COMPILE_TEST
diff --git a/drivers/i2c/busses/i2c-aspeed-test.c
b/drivers/i2c/busses/i2c-aspeed-test.c
new file mode 100644
index 0..93e73af95b645
--- /dev/null
+++ b/drivers/i2c/busses/i2c-aspeed-test.c
@@ -0,0 +1,218 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Aspeed 24XX/25XX I2C Controller KUnit tests.
+ *
+ * Copyright (C) 2020 Google LLC.
+ */
+
+#include
+
+#define ASPEED_I2C_MAX_BASE_DIVISOR(1 <<
ASPEED_I2CD_TIME_BASE_DIVISOR_MASK)
+#define ASPEED_I2C_24XX_CLK_HIGH_LOW_MASK GENMASK(2, 0)
+#define ASPEED_I2C_24XX_CLK_HIGH_LOW_MAX
((ASPEED_I2C_24XX_CLK_HIGH_LOW_MASK + 1) * 2)
+#define ASPEED_I2C_24XX_MAX_DIVISOR\
+ (ASPEED_I2C_MAX_BASE_DIVISOR * ASPEED_I2C_24XX_CLK_HIGH_LOW_MAX)
+#define ASPEED_I2C_25XX_CLK_HIGH_LOW_MASK GENMASK(3, 0)
+#define ASPEED_I2C_25XX_CLK_HIGH_LOW_MAX
((ASPEED_I2C_25XX_CLK_HIGH_LOW_MASK + 1) * 2)
+#define ASPEED_I2C_25XX_MAX_DIVISOR\
+ (ASPEED_I2C_MAX_BASE_DIVISOR * ASPEED_I2C_25XX_CLK_HIGH_LOW_MAX)
+
+static u32 aspeed_i2c_get_base_clk(u32 reg_val)
+{
+ return reg_val & ASPEED_I2CD_TIME_BASE_DIVISOR_MASK;
+}
+
+static u32 aspeed_i2c_get_clk_high(u32 reg_val)
+{
+ return (reg_val & ASPEED_I2CD_TIME_SCL_HIGH_MASK) >>
+ ASPEED_I2CD_TIME_SCL_HIGH_SHIFT;
+}
+
+static u32 aspeed_i2c_get_clk_low(u32 reg_val)
+{
+ return (reg_val & ASPEED_I2CD_TIME_SCL_LOW_MASK) >>
+ ASPEED_I2CD_TIME_SCL_LOW_SHIFT;
+}
+
+static void aspeed_i2c_get_clk_reg_val_params_test(struct kunit *test,
+ u32
(*get_clk_reg_val)(struct device *, u32),
+ u32 divisor,
+ u32 base_clk,
+ u32 clk_high,
+ u32 clk_low)
+{
+ u32 reg_val;
+
+ reg_val = get_clk_reg_val(NULL, divisor);
+ KUNIT_ASSERT_EQ(test,
+ (u32)(reg_val & ~(ASPEED_I2CD_TIME_SCL_HIGH_MASK |
+ ASPEED_I2CD_TIME_SCL_LOW_MASK |
+ ASPEED_I2CD_TIME_BASE_DIVISOR_MASK)),
+ (u32)0);
+ KUNIT_EXPECT_EQ(test, aspeed_i2c_get_base_clk(reg_val), base_clk);
+ KUNIT_EXPECT_EQ(test, aspe
Re: [PATCH] lib: kunit: Fix compilation test when using TEST_BIT_FIELD_COMPILE
On Thu, Oct 15, 2020 at 5:08 AM Vitor Massaru Iha wrote:
>
> A build condition was missing around a compilation test, this compilation
> test comes from the original test_bitfield code.
>
> And removed unnecessary code for this test.
>
> Fixes: d2585f5164c2 ("lib: kunit: add bitfield test conversion to KUnit")
> Signed-off-by: Vitor Massaru Iha
> Link:
> https://lore.kernel.org/linux-next/20201015163056.56fcc...@canb.auug.org.au/
Reviewed-by: Brendan Higgins
Thanks for taking care of this so quickly!
Re: [PATCH] lib: kunit: add test_min_heap test conversion to KUnit
On Tue, Aug 4, 2020 at 9:22 AM Vitor Massaru Iha wrote: > > Hi Peter, > > On Tue, Aug 4, 2020 at 11:23 AM wrote: > > > > On Tue, Aug 04, 2020 at 10:46:21AM -0300, Vitor Massaru Iha wrote: > > > On Tue, Aug 4, 2020 at 10:25 AM wrote: > > > > On Wed, Jul 29, 2020 at 06:57:17PM -0300, Vitor Massaru Iha wrote: > > > > > > > > > The results can be seen this way: > > > > > > > > > > This is an excerpt from the test.log with the result in TAP format: > > > > > [snip] > > > > > ok 5 - example > > > > > # Subtest: min-heap > > > > > 1..6 > > > > > ok 1 - test_heapify_all_true > > > > > ok 2 - test_heapify_all_false > > > > > ok 3 - test_heap_push_true > > > > > ok 4 - test_heap_push_false > > > > > ok 5 - test_heap_pop_push_true > > > > > ok 6 - test_heap_pop_push_false > > > > > [snip] > > > > So ^ is TAP format? > > Yep, you can see the spec here: > https://testanything.org/tap-specification.html > > > > > > > I don't care or care to use either; what does dmesg do? It used to be > > > > that just building the self-tests was sufficient and any error would > > > > show in dmesg when you boot the machine. > > > > > > > > But if I now have to use some damn tool, this is a regression. > > > > > > If you don't want to, you don't need to use the kunit-tool. If you > > > compile the tests as builtin and run the Kernel on your machine > > > the test result will be shown in dmesg in TAP format. > > > > That's seems a lot more verbose than it is now. I've recently even done > > a bunch of tests that don't print anything on success, dmesg is clutter > > enough already. > > What tests do you refer to? > > Running the test_min_heap.c, I got this from dmesg: > > min_heap_test: test passed > > And running min_heap_kunit.c: > > ok 1 - min-heap Gentle poke. I think Vitor was looking for a response. My guess is that Ian was waiting for a follow up patch.
Re: [PATCH] lib: Convert test_printf.c to KUnit
On Mon, Oct 12, 2020 at 1:13 PM Brendan Higgins wrote: > > On Fri, Aug 21, 2020 at 5:19 AM Rasmus Villemoes > wrote: > > Sorry about the late reply. I saw activity on this before and thought > it was under control. I only saw the unresolved state now looking > through patchwork. > > > On 21/08/2020 13.37, Petr Mladek wrote: > > > On Mon 2020-08-17 09:06:32, Rasmus Villemoes wrote: > > >> On 17/08/2020 06.30, Arpitha Raghunandan wrote: > > >>> Converts test lib/test_printf.c to KUnit. > > >>> More information about KUnit can be found at > > >>> https://www.kernel.org/doc/html/latest/dev-tools/kunit/index.html. > > >>> KUnit provides a common framework for unit tests in the kernel. > > >> > > >> So I can continue to build a kernel with some appropriate CONFIG set to > > >> y, boot it under virt-me, run dmesg and see if I broke printf? That's > > >> what I do now, and I don't want to have to start using some enterprisy > > >> framework. > > > > > > I had the same concern. I have tried it. > > > > Thanks for doing that and reporting the results. > > > > > #> modprobe printf_kunit > > > > > > produced the following in dmesg: > > > > > > [ 60.931175] printf_kunit: module verification failed: signature and/or > > > required key missing - tainting kernel > > > [ 60.942209] TAP version 14 > > > [ 60.945197] # Subtest: printf-kunit-test > > > [ 60.945200] 1..1 > > > [ 60.951092] ok 1 - selftest > > > [ 60.953414] ok 1 - printf-kunit-test > > > > > > I could live with the above. Then I tried to break a test by the > > > following change: > > > > > > > > > diff --git a/lib/printf_kunit.c b/lib/printf_kunit.c > > > index 68ac5f9b8d28..1689dadd70a3 100644 > > > --- a/lib/printf_kunit.c > > > +++ b/lib/printf_kunit.c > > > @@ -395,7 +395,7 @@ ip4(struct kunit *kunittest) > > > sa.sin_port = cpu_to_be16(12345); > > > sa.sin_addr.s_addr = cpu_to_be32(0x7f01); > > > > > > - test(kunittest, "127.000.000.001|127.0.0.1", "%pi4|%pI4", > > > _addr, _addr); > > > + test(kunittest, "127-000.000.001|127.0.0.1", "%pi4|%pI4", > > > _addr, _addr); > > > test(kunittest, "127.000.000.001|127.0.0.1", "%piS|%pIS", , > > > ); > > > sa.sin_addr.s_addr = cpu_to_be32(0x01020304); > > > test(kunittest, "001.002.003.004:12345|1.2.3.4:12345", > > > "%piSp|%pISp", , ); > > > > > > > > > It produced:: > > > > > > [ 56.786858] TAP version 14 > > > [ 56.787493] # Subtest: printf-kunit-test > > > [ 56.787494] 1..1 > > > [ 56.788612] # selftest: EXPECTATION FAILED at lib/printf_kunit.c:76 > > >Expected memcmp(test_buffer, expect, written) == 0, but > > >memcmp(test_buffer, expect, written) == 1 > > >0 == 0 > > >vsnprintf(buf, 256, "%pi4|%pI4", ...) wrote > > > '127.000.000.001|127.0.0.1', expected '127-000.000.001|127.0.0.1' > > > [ 56.795433] # selftest: EXPECTATION FAILED at lib/printf_kunit.c:76 > > >Expected memcmp(test_buffer, expect, written) == 0, but > > >memcmp(test_buffer, expect, written) == 1 > > >0 == 0 > > >vsnprintf(buf, 20, "%pi4|%pI4", ...) wrote > > > '127.000.000.001|127', expected '127-000.000.001|127' > > > [ 56.800909] # selftest: EXPECTATION FAILED at > > > lib/printf_kunit.c:108 > > >Expected memcmp(p, expect, elen+1) == 0, but > > >memcmp(p, expect, elen+1) == 1 > > >0 == 0 > > >kvasprintf(..., "%pi4|%pI4", ...) returned > > > '127.000.000.001|127.0.0.1', expected '127-000.000.001|127.0.0.1' > > > [ 56.806497] not ok 1 - selftest > > > [ 56.806497] not ok 1 - printf-kunit-test > > > > > > while the original code would have written the following error messages: > > > > > > [ 95.859225] test_printf: loaded. > > > [ 95.860031] test_printf: vsnprintf(buf, 256, "%pi4|%pI4", ...) wrote > > > '127.000.000.001|127.0.0.1', expected '
Re: [PATCH] lib: Convert test_printf.c to KUnit
On Fri, Aug 21, 2020 at 03:28:49PM +0300, Andy Shevchenko wrote: > On Fri, Aug 21, 2020 at 01:37:10PM +0200, Petr Mladek wrote: > > On Mon 2020-08-17 09:06:32, Rasmus Villemoes wrote: > > > On 17/08/2020 06.30, Arpitha Raghunandan wrote: > > > > Converts test lib/test_printf.c to KUnit. > > > > More information about KUnit can be found at > > > > https://www.kernel.org/doc/html/latest/dev-tools/kunit/index.html. > > > > KUnit provides a common framework for unit tests in the kernel. > > > > > > So I can continue to build a kernel with some appropriate CONFIG set to > > > y, boot it under virt-me, run dmesg and see if I broke printf? That's > > > what I do now, and I don't want to have to start using some enterprisy > > > framework. > > > > I had the same concern. I have tried it. Sorry you feel that way. Do you have any suggestions on how we can make it seem less enterprisy? Seems like there are people here who are not a fan of the output format, so of which we can fix here, some of which is part of KTAP[1]. > Which raises an obvious question: did the people who convert this test this > themselves? Looks like a janitor work in the area without understanding the > area good enough. Looks to me like Arpitha ran it, but you are right, we don't have a lot of familiarity with this area; we were treating it as "janitor work" as you say. Our intention was just to take some existing tests and as non-invasively as possible, get them to report using a common format, and maybe even get some of the tests to follow a common pattern. > Probably I will NAK all those patches from now on, until it will be good > commit > messages and cover of risen aspects, including reference to before and after > outcome for passed and failed test cases. Fair enough, hopefully we can address these issues in the next revision. One issue though, with the "before and after outcome" you are referencing; are you referring to the issue that Petr pointed out in how they are inconsistent: + original code: vsnprintf(buf, 6, "%pi4|%pI4", ...) wrote '127.0', expected '127-0' + kunit code: vsnprintf(buf, 20, "%pi4|%pI4", ...) wrote '127.000.000.001|127', expected '127-000.000.001|127' (I think Rasmus addressed this.) Or are your referring to something else? > Brendan, I guess the ball now on your side to prove this is good activity. And I see that we are off to a great start! :-) In all seriousness, I am really sorry about this. I kind of bungled this up trying to go after too many of these conversions at once. Arpitha, can you get this follow up patch out? [1] https://lore.kernel.org/linux-kselftest/cy4pr13mb1175b804e31e502221bc8163fd...@cy4pr13mb1175.namprd13.prod.outlook.com/
Re: [PATCH] lib: Convert test_printf.c to KUnit
On Fri, Aug 21, 2020 at 5:19 AM Rasmus Villemoes wrote: Sorry about the late reply. I saw activity on this before and thought it was under control. I only saw the unresolved state now looking through patchwork. > On 21/08/2020 13.37, Petr Mladek wrote: > > On Mon 2020-08-17 09:06:32, Rasmus Villemoes wrote: > >> On 17/08/2020 06.30, Arpitha Raghunandan wrote: > >>> Converts test lib/test_printf.c to KUnit. > >>> More information about KUnit can be found at > >>> https://www.kernel.org/doc/html/latest/dev-tools/kunit/index.html. > >>> KUnit provides a common framework for unit tests in the kernel. > >> > >> So I can continue to build a kernel with some appropriate CONFIG set to > >> y, boot it under virt-me, run dmesg and see if I broke printf? That's > >> what I do now, and I don't want to have to start using some enterprisy > >> framework. > > > > I had the same concern. I have tried it. > > Thanks for doing that and reporting the results. > > > #> modprobe printf_kunit > > > > produced the following in dmesg: > > > > [ 60.931175] printf_kunit: module verification failed: signature and/or > > required key missing - tainting kernel > > [ 60.942209] TAP version 14 > > [ 60.945197] # Subtest: printf-kunit-test > > [ 60.945200] 1..1 > > [ 60.951092] ok 1 - selftest > > [ 60.953414] ok 1 - printf-kunit-test > > > > I could live with the above. Then I tried to break a test by the following > > change: > > > > > > diff --git a/lib/printf_kunit.c b/lib/printf_kunit.c > > index 68ac5f9b8d28..1689dadd70a3 100644 > > --- a/lib/printf_kunit.c > > +++ b/lib/printf_kunit.c > > @@ -395,7 +395,7 @@ ip4(struct kunit *kunittest) > > sa.sin_port = cpu_to_be16(12345); > > sa.sin_addr.s_addr = cpu_to_be32(0x7f01); > > > > - test(kunittest, "127.000.000.001|127.0.0.1", "%pi4|%pI4", > > _addr, _addr); > > + test(kunittest, "127-000.000.001|127.0.0.1", "%pi4|%pI4", > > _addr, _addr); > > test(kunittest, "127.000.000.001|127.0.0.1", "%piS|%pIS", , ); > > sa.sin_addr.s_addr = cpu_to_be32(0x01020304); > > test(kunittest, "001.002.003.004:12345|1.2.3.4:12345", > > "%piSp|%pISp", , ); > > > > > > It produced:: > > > > [ 56.786858] TAP version 14 > > [ 56.787493] # Subtest: printf-kunit-test > > [ 56.787494] 1..1 > > [ 56.788612] # selftest: EXPECTATION FAILED at lib/printf_kunit.c:76 > >Expected memcmp(test_buffer, expect, written) == 0, but > >memcmp(test_buffer, expect, written) == 1 > >0 == 0 > >vsnprintf(buf, 256, "%pi4|%pI4", ...) wrote > > '127.000.000.001|127.0.0.1', expected '127-000.000.001|127.0.0.1' > > [ 56.795433] # selftest: EXPECTATION FAILED at lib/printf_kunit.c:76 > >Expected memcmp(test_buffer, expect, written) == 0, but > >memcmp(test_buffer, expect, written) == 1 > >0 == 0 > >vsnprintf(buf, 20, "%pi4|%pI4", ...) wrote > > '127.000.000.001|127', expected '127-000.000.001|127' > > [ 56.800909] # selftest: EXPECTATION FAILED at lib/printf_kunit.c:108 > >Expected memcmp(p, expect, elen+1) == 0, but > >memcmp(p, expect, elen+1) == 1 > >0 == 0 > >kvasprintf(..., "%pi4|%pI4", ...) returned > > '127.000.000.001|127.0.0.1', expected '127-000.000.001|127.0.0.1' > > [ 56.806497] not ok 1 - selftest > > [ 56.806497] not ok 1 - printf-kunit-test > > > > while the original code would have written the following error messages: > > > > [ 95.859225] test_printf: loaded. > > [ 95.860031] test_printf: vsnprintf(buf, 256, "%pi4|%pI4", ...) wrote > > '127.000.000.001|127.0.0.1', expected '127-000.000.001|127.0.0.1' > > [ 95.862630] test_printf: vsnprintf(buf, 6, "%pi4|%pI4", ...) wrote > > '127.0', expected '127-0' > > [ 95.864118] test_printf: kvasprintf(..., "%pi4|%pI4", ...) returned > > '127.000.000.001|127.0.0.1', expected '127-000.000.001|127.0.0.1' > > [ 95.866589] test_printf: failed 3 out of 388 tests > > > > > > Even the error output is acceptable for me. > > Urgh. Yeah, perhaps, but the original is much more readable; it really > doesn't matter that a memcmp() fails to compare equal to 0, that's > merely how we figured out that the output was wrong. We can go back to the original error reporting format, just as long as you don't mind the "ok" lines interspersed throughout (this is part of an attempt to standardize around the KTAP reporting format[1]. > I am just curious why > > the 2nd failure is different: > > > >+ original code: vsnprintf(buf, 6, "%pi4|%pI4", ...) wrote '127.0', > > expected '127-0' > >+ kunit code: vsnprintf(buf, 20, "%pi4|%pI4", ...) wrote > > '127.000.000.001|127', expected '127-000.000.001|127' > > That's by design. If you read the code, there's a comment that says we > do every test case four
Re: [PATCH] lib: kunit: add bitfield test conversion to KUnit
On Thu, Sep 17, 2020 at 3:00 AM Johannes Berg wrote: > > On Wed, 2020-08-19 at 14:10 -0700, Brendan Higgins wrote: > > On Wed, Jul 29, 2020 at 10:58 AM Vitor Massaru Iha > > wrote: > > > This adds the conversion of the runtime tests of test_bitfield, > > > from `lib/test_bitfield.c` to KUnit tests. > > > > > > Please apply this commit first (linux-kselftest/kunit-fixes): > > > 3f37d14b8a3152441f36b6bc74000996679f0998 kunit: kunit_config: Fix parsing > > > of CONFIG options with space > > > > > > Code Style Documentation: [0] > > > > > > Signed-off-by: Vitor Massaru Iha > > > Link: [0] > > > https://lore.kernel.org/linux-kselftest/20200620054944.167330-1-david...@google.com/T/#u > > > > Reviewed-by: Brendan Higgins > > > > Probably still want a review from Johannes though. > > Huh, sorry, this slipped through the cracks. > > Yeah, don't really care, looks fine to me? I'm not familiar with the > kunit infrastructure much yet. Cool, well in that case I will apply it. > Not sure I see much value in converting TEST_BITFIELD_COMPILE to a > KUNIT_CASE though, because anyway it will not compile if you enable > that? IOW, just leaving the function there without any KUNIT_CASE() > reference to it should be fine and saves you an ifdef ... Well I think it is also the case that we only want to count the test case if it actually has everything to run; that is a point that is somewhat up in the air. David is exploring adding the concept of "skipped" tests to KUnit, but we don't have that yet. Cheers!
Re: [PATCH] kunit: tool: fix display of make errors
On Wed, Sep 30, 2020 at 11:32 AM Daniel Latypov wrote: > > CalledProcessError stores the output of the failed process as `bytes`, > not a `str`. > > So when we log it on build error, the make output is all crammed into > one line with "\n" instead of actually printing new lines. > > After this change, we get readable output with new lines, e.g. > > CC lib/kunit/kunit-example-test.o > > In file included from ../lib/kunit/test.c:9: > > ../include/kunit/test.h:22:1: error: unknown type name > > ‘invalid_type_that_causes_compile’ > >22 | invalid_type_that_causes_compile errors; > > | ^~~~ > > make[3]: *** [../scripts/Makefile.build:283: lib/kunit/test.o] Error 1 > > Secondly, trying to concat exceptions to strings will fail with > > TypeError: can only concatenate str (not "OSError") to str > so fix this with an explicit cast to str. > > Signed-off-by: Daniel Latypov Reviewed-by: Brendan Higgins Tested-by: Brendan Higgins Cheers!
Re: [RFC v1 00/12] kunit: introduce class mocking support.
On Tue, Sep 22, 2020 at 5:24 PM Daniel Latypov wrote:
>
> On Fri, Sep 18, 2020 at 11:31 AM Daniel Latypov wrote:
> >
> > # Background
> > KUnit currently lacks any first-class support for mocking.
> > For an overview and discussion on the pros and cons, see
> > https://martinfowler.com/articles/mocksArentStubs.html
> >
> > This patch set introduces the basic machinery needed for mocking:
> > setting and validating expectations, setting default actions, etc.
> >
> > Using that basic infrastructure, we add macros for "class mocking", as
> > it's probably the easiest type of mocking to start with.
> >
> > ## Class mocking
> >
> > By "class mocking", we're referring mocking out function pointers stored
> > in structs like:
> > struct sender {
> > int (*send)(struct sender *sender, int data);
> > };
>
> Discussed this offline a bit with Brendan and David.
> The requirement that the first argument is a `sender *` means this
> doesn't work for a few common cases.
>
> E.g. ops structs don't usually have funcs that take `XXX_ops *`
> `pci_ops` all take a `struct pci_bus *`, which at least contains a
> `struct pci_ops*`.
>
> Why does this matter?
> We need to stash a `struct mock` somewhere to store expectations.
> For this version of class mocking (in patch 10), we've assumed we could have
>
> struct MOCK(sender) {
> struct mock ctrl;
> struct sender trgt; // assumed to be the first param
> }
>
> Then we can always use `container_of(sender)` to get the MOCK(sender)
> which holds `ctrl`.
> But if the first parameter isn't a `struct sender*`, this trick
> obviously doesn't work.
>
> So to support something like pci_ops, we'd need another approach to
> stash `ctrl`.
>
> After thinking a bit more, we could perhaps decouple the first param
> from the mocked struct.
> Using pci_ops as the example:
>
> struct MOCK(pci_ops) {
> struct mock ctrl;
> struct pci_bus *self; // this is the first param. Using
> python terminology here.
> struct pci_ops trgt; // continue to store this, as this holds
> the function pointers
> }
>
> // Name of this func is currently based on the class we're mocking
> static inline struct mock *from_pci_ops_to_mock(
> const struct pci_bus *self)
> {
> return mock_get_ctrl(container_of(self, struct MOCK(pci_ops),
> self));
> }
>
> // then in tests, we'd need something like
> struct pci_bus bus;
> bus.ops = mock_get_trgt(mock_ops);
> mock_ops.self =
>
> Do others have thoughts/opinions?
In the above example, wouldn't we actually want to mock struct
pci_bus, not struct pci_ops?
I think pci_bus is what actually gets implemented. Consider the
Rockchip PCIe host controller:
Rockchip provides it's own pci_ops struct:
https://elixir.bootlin.com/linux/latest/source/drivers/pci/controller/pcie-rockchip-host.c#L256
If you look at one of the implemented methods, say
rockchip_pcie_rd_conf(), you will see:
...
struct rockchip_pcie *rockchip = bus->sysdata;
...
(This function signature is:
int (*read)(struct pci_bus *bus, unsigned int devfn, int where, int
size, u32 *val);
).
Thus, conceptually struct pci_bus is what is being manipulated; it
best fits the candidate for the *self pointer.
In fact - if I am not mistaken - I think if you were to mock pci_bus
and just pretend that the methods were on pci_bus, not pci_ops, I
think it might just work.
The bigger problem is that it seems pci_bus does not want the user to
allocate it: in the case of rockchip_pcie, it is allocated by
devm_pci_alloc_host_bridge(). Thus, embedding pci_bus inside of a
MOCK(pci_bus) would probably not work, so you would need to have
different tooling around that and would then need to provide a custom
definition of from_pci_bus_to_mock() (generated by
DECLARE_STRUCT_CLASS_MOCK_CONVERTER()).
> After grepping around for examples, I'm struck by how the number of
> outliers there are.
> E.g. most take a `struct thing *` as the first param, but cfspi_ifc in
> caif_spi.h opts to take that as the last parameter.
That's not a problem, just use the
DECLARE_STRUCT_CLASS_MOCK_HANDLE_INDEX() variant to create your mock
(as opposed to DECLARE_STRUCT_CLASS_MOCK()).
For example let's say you have the following struct that you want to mock:
struct example {
...
int (*foo)(int bar, char baz, struct example *self);
};
You could mock the function with:
DECLARE_STRUCT_CLASS_MOCK_HANDLE_INDEX(
METHOD(foo), CLASS(example),
2, /* The "handle" is in position 2. */
RETURNS(int),
PARAMS(int, char, struct example *));
> And others take a different mix of structs for each function.
>
> But it feels like a decoupled self / struct-holding-func-pointers
> approach should be generic enough, as far as I can tell.
> The more exotic types will probably have to remain unsupported.
I think the code is pretty much here to handle any situation in which
one of the parameters input to the function can be used to look up a
mock object; we just need to expose the
[PATCH v1] kunit: tool: handle when .kunit exists but .kunitconfig does not
Right now .kunitconfig and the build dir are automatically created if
the build dir does not exists; however, if the build dir is present and
.kunitconfig is not, kunit_tool will crash.
Fix this by checking for both the build dir as well as the .kunitconfig.
NOTE: This depends on commit 5578d008d9e0 ("kunit: tool: fix running
kunit_tool from outside kernel tree")
Link:
https://git.kernel.org/pub/scm/linux/kernel/git/shuah/linux-kselftest.git/commit/?id=5578d008d9e06bb531fb3e62dd17096d9fd9c853
Signed-off-by: Brendan Higgins
---
tools/testing/kunit/kunit.py | 12
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/tools/testing/kunit/kunit.py b/tools/testing/kunit/kunit.py
index e2caf4e24ecb2..8ab17e21a3578 100755
--- a/tools/testing/kunit/kunit.py
+++ b/tools/testing/kunit/kunit.py
@@ -243,6 +243,8 @@ def main(argv, linux=None):
if cli_args.subcommand == 'run':
if not os.path.exists(cli_args.build_dir):
os.mkdir(cli_args.build_dir)
+
+ if not os.path.exists(kunit_kernel.kunitconfig_path):
create_default_kunitconfig()
if not linux:
@@ -258,10 +260,12 @@ def main(argv, linux=None):
if result.status != KunitStatus.SUCCESS:
sys.exit(1)
elif cli_args.subcommand == 'config':
- if cli_args.build_dir:
- if not os.path.exists(cli_args.build_dir):
- os.mkdir(cli_args.build_dir)
- create_default_kunitconfig()
+ if cli_args.build_dir and (
+ not os.path.exists(cli_args.build_dir)):
+ os.mkdir(cli_args.build_dir)
+
+ if not os.path.exists(kunit_kernel.kunitconfig_path):
+ create_default_kunitconfig()
if not linux:
linux = kunit_kernel.LinuxSourceTree()
base-commit: d96fe1a5485fa978a6e3690adc4dbe4d20b5baa4
--
2.28.0.681.g6f77f65b4e-goog
[PATCH v1] kunit: tool: fix --alltests flag
Alltests flag evidently stopped working when run from outside of the root of the source tree, so fix that. Also add an additional broken config to the broken_on_uml config. Signed-off-by: Brendan Higgins --- tools/testing/kunit/configs/broken_on_uml.config | 1 + tools/testing/kunit/kunit_kernel.py | 15 ++- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/tools/testing/kunit/configs/broken_on_uml.config b/tools/testing/kunit/configs/broken_on_uml.config index 239b9f03da2c..a7f0603d33f6 100644 --- a/tools/testing/kunit/configs/broken_on_uml.config +++ b/tools/testing/kunit/configs/broken_on_uml.config @@ -39,3 +39,4 @@ # CONFIG_QCOM_CPR is not set # CONFIG_RESET_BRCMSTB_RESCAL is not set # CONFIG_RESET_INTEL_GW is not set +# CONFIG_ADI_AXI_ADC is not set diff --git a/tools/testing/kunit/kunit_kernel.py b/tools/testing/kunit/kunit_kernel.py index e20e2056cb38..1b1826500f61 100644 --- a/tools/testing/kunit/kunit_kernel.py +++ b/tools/testing/kunit/kunit_kernel.py @@ -53,18 +53,23 @@ class LinuxSourceTreeOperations(object): except subprocess.CalledProcessError as e: raise ConfigError(e.output) - def make_allyesconfig(self): + def make_allyesconfig(self, build_dir, make_options): kunit_parser.print_with_timestamp( 'Enabling all CONFIGs for UML...') + command = ['make', 'ARCH=um', 'allyesconfig'] + if make_options: + command.extend(make_options) + if build_dir: + command += ['O=' + build_dir] process = subprocess.Popen( - ['make', 'ARCH=um', 'allyesconfig'], + command, stdout=subprocess.DEVNULL, stderr=subprocess.STDOUT) process.wait() kunit_parser.print_with_timestamp( 'Disabling broken configs to run KUnit tests...') with ExitStack() as es: - config = open(KCONFIG_PATH, 'a') + config = open(get_kconfig_path(build_dir), 'a') disable = open(BROKEN_ALLCONFIG_PATH, 'r').read() config.write(disable) kunit_parser.print_with_timestamp( @@ -161,9 +166,9 @@ class LinuxSourceTree(object): return self.build_config(build_dir, make_options) def build_um_kernel(self, alltests, jobs, build_dir, make_options): - if alltests: - self._ops.make_allyesconfig() try: + if alltests: + self._ops.make_allyesconfig(build_dir, make_options) self._ops.make_olddefconfig(build_dir, make_options) self._ops.make(jobs, build_dir, make_options) except (ConfigError, BuildError) as e: base-commit: 92a2b470086f68bf35eb9f94b6cb5ebdfac41b25 -- 2.28.0.681.g6f77f65b4e-goog
Re: linux-next: Tree for Sep 2 (lib/ubsan.c)
On Tue, Sep 8, 2020 at 5:55 PM Shuah Khan wrote:
>
> On 9/8/20 6:49 PM, Randy Dunlap wrote:
> > On 9/8/20 5:46 PM, Stephen Rothwell wrote:
> >> Hi Randy,
> >>
> >> On Tue, 8 Sep 2020 07:38:31 -0700 Randy Dunlap
> >> wrote:
> >>>
> >>> On 9/4/20 12:59 AM, Brendan Higgins wrote:
> >>>> On Thu, Sep 3, 2020 at 11:12 PM Randy Dunlap
> >>>> wrote:
> >>>>>
> >>>>> On 9/2/20 8:44 AM, Randy Dunlap wrote:
> >>>>>> On 9/2/20 1:09 AM, Stephen Rothwell wrote:
> >>>>>>> Hi all,
> >>>>>>>
> >>>>>>> Changes since 20200828:
> >>>>>>>
> >>>>>>
> >>>>>>
> >>>>>> on i386:
> >>>>>>
> >>>>>> ../lib/ubsan.c: In function ‘ubsan_prologue’:
> >>>>>> ../lib/ubsan.c:141:2: error: implicit declaration of function
> >>>>>> ‘kunit_fail_current_test’; did you mean ‘kunit_init_test’?
> >>>>>> [-Werror=implicit-function-declaration]
> >>>>>>kunit_fail_current_test();
> >>>>>>
> >>>>>>
> >>>>>> Full randconfig file is attached.
> >>>>>>
> >>>>>
> >>>>> Hi Brendan,
> >>>>>
> >>>>> Do you know anything about this build error?
> >>>>>
> >>>>> I can't find kunit_fail_current_test() anywhere.
> >>>>
> >>>> Yeah, this got applied for some reason without the prerequisite
> >>>> patches. It is from a two patch series, the other being here:
> >>>>
> >>>> https://lore.kernel.org/linux-kselftest/20200813205722.1384108-1-urielguajard...@gmail.com/
> >>>>
> >>>> which in turn depends on another patchset which didn't make it into 5.9.
> >>>>
> >>>> Again, I don't know why this was applied without it's prereqs. Sorry
> >>>> about that.
> >>>>
> >>>
> >>> Well. Who is responsible for this small mess?
> >>> It is still killing linux-next builds for me (2020-0908).
> >>
> >> It came in via the kunit-next tree (Shuah cc'd). I will revert commit
> >> abe83f7621ee ("kunit: ubsan integration") today.
> >
> >
>
> Sorry about that. I picked this up for 5.10 since it had the reviewed
> by tags from Brendan.
>
> I will drop this from kselftest kunit
Thanks for taking care of this!
Re: [PATCH v3 3/5] i2c: aspeed: Mask IRQ status to relevant bits
On Wed, Sep 9, 2020 at 1:31 PM Eddie James wrote: > > Mask the IRQ status to only the bits that the driver checks. This > prevents excessive driver warnings when operating in slave mode > when additional bits are set that the driver doesn't handle. > > Signed-off-by: Eddie James > Reviewed-by: Tao Ren Sorry, looks like I didn't get my comment in in time. Looks good in principle. One minor comment below: > --- > drivers/i2c/busses/i2c-aspeed.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/i2c/busses/i2c-aspeed.c b/drivers/i2c/busses/i2c-aspeed.c > index 31268074c422..724bf30600d6 100644 > --- a/drivers/i2c/busses/i2c-aspeed.c > +++ b/drivers/i2c/busses/i2c-aspeed.c > @@ -69,6 +69,7 @@ > * These share bit definitions, so use the same values for the enable & > * status bits. > */ > +#define ASPEED_I2CD_INTR_RECV_MASK 0xf000 Could we define ASPEED_I2CD_INTR_RECV_MASK to be ASPEED_I2CD_INTR_ALL ? > #define ASPEED_I2CD_INTR_SDA_DL_TIMEOUTBIT(14) > #define ASPEED_I2CD_INTR_BUS_RECOVER_DONE BIT(13) > #define ASPEED_I2CD_INTR_SLAVE_MATCH BIT(7) > @@ -604,6 +605,7 @@ static irqreturn_t aspeed_i2c_bus_irq(int irq, void > *dev_id) > writel(irq_received & ~ASPEED_I2CD_INTR_RX_DONE, >bus->base + ASPEED_I2C_INTR_STS_REG); > readl(bus->base + ASPEED_I2C_INTR_STS_REG); > + irq_received &= ASPEED_I2CD_INTR_RECV_MASK; > irq_remaining = irq_received; > > #if IS_ENABLED(CONFIG_I2C_SLAVE) > -- > 2.26.2 >
Re: linux-next: Tree for Sep 2 (lib/ubsan.c)
On Thu, Sep 3, 2020 at 11:12 PM Randy Dunlap wrote: > > On 9/2/20 8:44 AM, Randy Dunlap wrote: > > On 9/2/20 1:09 AM, Stephen Rothwell wrote: > >> Hi all, > >> > >> Changes since 20200828: > >> > > > > > > on i386: > > > > ../lib/ubsan.c: In function ‘ubsan_prologue’: > > ../lib/ubsan.c:141:2: error: implicit declaration of function > > ‘kunit_fail_current_test’; did you mean ‘kunit_init_test’? > > [-Werror=implicit-function-declaration] > > kunit_fail_current_test(); > > > > > > Full randconfig file is attached. > > > > Hi Brendan, > > Do you know anything about this build error? > > I can't find kunit_fail_current_test() anywhere. Yeah, this got applied for some reason without the prerequisite patches. It is from a two patch series, the other being here: https://lore.kernel.org/linux-kselftest/20200813205722.1384108-1-urielguajard...@gmail.com/ which in turn depends on another patchset which didn't make it into 5.9. Again, I don't know why this was applied without it's prereqs. Sorry about that.
Re: [PATCH] Documentation: kunit: Add naming guidelines
On Thu, Aug 27, 2020 at 11:28 AM Marco Elver wrote: > > On Thu, 27 Aug 2020 at 18:17, David Gow wrote: > [...] > > > First of all, thanks for the talk yesterday! I only looked at this > > > because somebody pasted the LKML link. :-) > > > > No worries! Clearly this document needed linking -- even I was > > starting to suspect the reason no-one was complaining about this was > > that no-one had read it. :-) > [...] > > > > > > While I guess this ship has sailed, and *_kunit.c is the naming > > > convention now, I hope this is still just a recommendation and names of > > > the form *-test.c are not banned! > > > > The ship hasn't technically sailed until this patch is actually > > accepted. Thus far, we hadn't had any real complaints about the > > _kunit.c idea, though that may have been due to this email not > > reaching enough people. If you haven't read the discussion in > > https://lore.kernel.org/linux-kselftest/202006141005.BA19A9D3@keescook/t/#u > > it's worthwhile: the _kunit.c name is discussed, and Kees lays out > > some more detailed rationale for it as well. > > Thanks, I can see the rationale. AFAIK the main concern was "it does > not distinguish it from other tests". That was my understanding as well. > > > $> git grep 'KUNIT.*-test.o' > > > drivers/base/power/Makefile:obj-$(CONFIG_PM_QOS_KUNIT_TEST) += > > > qos-test.o > > > drivers/base/test/Makefile:obj-$(CONFIG_KUNIT_DRIVER_PE_TEST) += > > > property-entry-test.o > > > fs/ext4/Makefile:obj-$(CONFIG_EXT4_KUNIT_TESTS) += > > > ext4-inode-test.o > > > kernel/Makefile:obj-$(CONFIG_SYSCTL_KUNIT_TEST) += sysctl-test.o > > > lib/Makefile:obj-$(CONFIG_LIST_KUNIT_TEST) += list-test.o > > > lib/kunit/Makefile:obj-$(CONFIG_KUNIT_TEST) += > > > kunit-test.o > > > lib/kunit/Makefile:obj-$(CONFIG_KUNIT_TEST) += > > > string-stream-test.o > > > lib/kunit/Makefile:obj-$(CONFIG_KUNIT_EXAMPLE_TEST) += > > > kunit-example-test.o > > > > > > $> git grep 'KUNIT.*_kunit.o' > > > # Returns nothing > > > > > > > This was definitely something we noted. Part of the goal of having > > _kunit.c as a filename suffix (and, perhaps more importantly, the > > _kunit module name suffix) was to have a way of both distinguishing > > KUnit tests from non-KUnit ones (of which there are quite a few > > already with -test names), and to have a way of quickly determining > > what modules contain KUnit tests. That really only works if everyone > > is using it, so the plan was to push this as much as possible. This'd > > probably include renaming most of the existing test files to match, > > which is a bit of a pain (particularly when converting non-KUnit tests > > to KUnit and similar), but is a one-time thing. > > Feel free to ignore the below, but here might be one concern: > > I think the problem of distinguishing KUnit tests from non-KUnit tests > is a technical problem (in fact, the Kconfig entries have all the info > we need), but a solution that sacrifices readability might cause > unnecessary friction. > > The main issue I foresee is that the _kunit.c name is opaque as to > what the file does, but merely indicates one of its dependencies. Most > of us clearly know that KUnit is a test framework, but it's a level of > indirection nevertheless. (But _kunit_test.c is also bad, because it's > unnecessarily long.) For a dozen tests, that's probably still fine. > But now imagine 100s of tests, people will quickly wonder "what's this > _kunit thing?". And if KUnit tests are eventually the dominant tests, > does it still matter? > > I worry that because of the difficulty of enforcing the name, choosing > something unintuitive will also achieve the opposite result: > proliferation of even more diverse names. A generic convention like > "*-test.c" will be close enough to what's intuitive for most people, > and we might actually have a chance of getting everyone to stick to > it. > > The problem of identifying all KUnit tests can be solved with a script. Fair point. However, converting all non-kselftests to kselftest or KUnit will likely take time, and kselftest will likely be with us forever. In short, other tests are likely to co-exist with KUnit for a long time if not in perpetuity; thus, I think Kees' point stands in this case. > > > Just an idea: Maybe the names are also an opportunity to distinguish > > > real _unit_ style tests and then the rarer integration-style tests. I > > > personally prefer using the more generic *-test.c, at least for the > > > integration-style tests I've been working on (KUnit is still incredibly > > > valuable for integration-style tests, because otherwise I'd have to roll > > > my own poor-man's version of KUnit, so thank you!). Using *_kunit.c for > > > such tests is unintuitive, because the word "unit" hints at "unit tests" > > > -- and having descriptive (and not misleading) filenames is still > > > important. So I hope you won't mind if
Re: [PATCH] lib: kunit: add list_sort test conversion to KUnit
On Wed, Jul 29, 2020 at 12:24 PM Vitor Massaru Iha wrote: > > This adds the conversion of the runtime tests of test_list_sort, > from `lib/test_list_sort.c` to KUnit tests. > > Please apply this commit first (linux-kselftest/kunit-fixes): > 3f37d14b8a3152441f36b6bc74000996679f0998 kunit: kunit_config: Fix parsing of > CONFIG options with space > > Code Style Documentation: [0] > > Signed-off-by: Vitor Massaru Iha > Link: [0] > https://lore.kernel.org/linux-kselftest/20200620054944.167330-1-david...@google.com/T/#u Reviewed-by: Brendan Higgins It would still be good to get some of the original authors/reviewers to review this.
Re: [PATCH] lib: kunit: add bitfield test conversion to KUnit
On Wed, Jul 29, 2020 at 10:58 AM Vitor Massaru Iha wrote: > > This adds the conversion of the runtime tests of test_bitfield, > from `lib/test_bitfield.c` to KUnit tests. > > Please apply this commit first (linux-kselftest/kunit-fixes): > 3f37d14b8a3152441f36b6bc74000996679f0998 kunit: kunit_config: Fix parsing of > CONFIG options with space > > Code Style Documentation: [0] > > Signed-off-by: Vitor Massaru Iha > Link: [0] > https://lore.kernel.org/linux-kselftest/20200620054944.167330-1-david...@google.com/T/#u Reviewed-by: Brendan Higgins Probably still want a review from Johannes though.
Re: [PATCH v2] lib: Convert test_user_copy to KUnit test
On Fri, Jul 17, 2020 at 5:51 PM Vitor Massaru Iha wrote: > > This adds the conversion of the runtime tests of test_user_copy fuctions, > from `lib/test_user_copy.c` to KUnit tests. > > Signed-off-by: Vitor Massaru Iha Reviewed-by: Brendan Higgins Thanks!
Re: [PATCH v2] lib: kunit: Provides a userspace memory context when tests are compiled as module
On Fri, Jul 17, 2020 at 5:50 PM Vitor Massaru Iha wrote:
>
> KUnit test cases run on kthreads, and kthreads don't have an
> adddress space (current->mm is NULL), but processes have mm.
>
> The purpose of this patch is to allow to borrow mm to KUnit kthread
> after userspace is brought up, because we know that there are processes
> running, at least the process that loaded the module to borrow mm.
>
> This allows, for example, tests such as user_copy_kunit, which uses
> vm_mmap, which needs current->mm.
>
> Signed-off-by: Vitor Massaru Iha
Can you put these together in the same patch series as you had before?
When I asked you to split the patch up, I was just asking about that
specific patch within the series. I still think all the patches go
together.
As for this specific patch, I see one minor issue below; other than
that, this looks good to me, so:
Reviewed-by: Brendan Higgins
> ---
> v2:
> * splitted patch in 3:
> - Allows to install and load modules in root filesystem;
> - Provides an userspace memory context when tests are compiled
> as module;
> - Convert test_user_copy to KUnit test;
> * added documentation;
> * added more explanation;
> * tested a pointer;
> * released mput();
> ---
> Documentation/dev-tools/kunit/usage.rst | 14 ++
> include/kunit/test.h| 12
> lib/kunit/try-catch.c | 15 ++-
> 3 files changed, 40 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/dev-tools/kunit/usage.rst
> b/Documentation/dev-tools/kunit/usage.rst
> index 3c3fe8b5fecc..9f909157be34 100644
> --- a/Documentation/dev-tools/kunit/usage.rst
> +++ b/Documentation/dev-tools/kunit/usage.rst
> @@ -448,6 +448,20 @@ We can now use it to test ``struct eeprom_buffer``:
>
> .. _kunit-on-non-uml:
>
> +User-space context
> +--
> +
> +I case you need a user-space context, for now this is only possible through
> +tests compiled as a module. And it will be necessary to use a root filesystem
> +and uml_utilities.
> +
> +Example:
> +
> +.. code-block:: bash
> +
> + ./tools/testing/kunit/kunit.py run --timeout=60
> --uml_rootfs_dir=.uml_rootfs
> +
> +
> KUnit on non-UML architectures
> ==
I think the above documentation change belongs in the other related
patch where you introduce the --uml_rootfs_dir flag.
> diff --git a/include/kunit/test.h b/include/kunit/test.h
> index 59f3144f009a..ae3337139c65 100644
> --- a/include/kunit/test.h
> +++ b/include/kunit/test.h
> @@ -222,6 +222,18 @@ struct kunit {
> * protect it with some type of lock.
> */
> struct list_head resources; /* Protected by lock. */
> + /*
> +* KUnit test cases run on kthreads, and kthreads don't have an
> +* adddress space (current->mm is NULL), but processes have mm.
> +*
> +* The purpose of this mm_struct is to allow to borrow mm to KUnit
> kthread
> +* after userspace is brought up, because we know that there are
> processes
> +* running, at least the process that loaded the module to borrow mm.
> +*
> +* This allows, for example, tests such as user_copy_kunit, which uses
> +* vm_mmap, which needs current->mm.
> +*/
> + struct mm_struct *mm;
> };
>
> void kunit_init_test(struct kunit *test, const char *name, char *log);
> diff --git a/lib/kunit/try-catch.c b/lib/kunit/try-catch.c
> index 0dd434e40487..d03e2093985b 100644
> --- a/lib/kunit/try-catch.c
> +++ b/lib/kunit/try-catch.c
> @@ -11,7 +11,8 @@
> #include
> #include
> #include
> -
> +#include
> +#include
> #include "try-catch-impl.h"
>
> void __noreturn kunit_try_catch_throw(struct kunit_try_catch *try_catch)
> @@ -24,8 +25,17 @@ EXPORT_SYMBOL_GPL(kunit_try_catch_throw);
> static int kunit_generic_run_threadfn_adapter(void *data)
> {
> struct kunit_try_catch *try_catch = data;
> + struct kunit *test = try_catch->test;
> +
> + if (test != NULL && test->mm != NULL)
> + kthread_use_mm(test->mm);
>
> try_catch->try(try_catch->context);
> + if (test != NULL && test->mm != NULL) {
> + kthread_unuse_mm(test->mm);
> + mmput(test->mm);
> + test->mm = NULL;
> + }
>
> complete_and_exit(try_catch->try_completion, 0);
> }
> @@ -65,6 +75,9 @@ void kunit_try_catch_run(struct kunit_try_catch *try_catch,
> void *context)
> try_catch->conte
Re: [RFC v7 03/10] mm/damon-test: Add more unit tests for 'init_regions'
On Tue, Aug 18, 2020 at 12:26 AM SeongJae Park wrote: > > From: SeongJae Park > > This commit adds more test cases for the new feature, 'init_regions'. > > Signed-off-by: SeongJae Park Reviewed-by: Brendan Higgins
[PATCH v2 1/2] kunit: tool: fix running kunit_tool from outside kernel tree
Currently kunit_tool does not work correctly when executed from a path outside of the kernel tree, so make sure that the current working directory is correct and the kunit_dir is properly initialized before running. Signed-off-by: Brendan Higgins --- tools/testing/kunit/kunit.py | 13 + 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/tools/testing/kunit/kunit.py b/tools/testing/kunit/kunit.py index 425ef40067e7e..e2caf4e24ecb2 100755 --- a/tools/testing/kunit/kunit.py +++ b/tools/testing/kunit/kunit.py @@ -237,9 +237,13 @@ def main(argv, linux=None): cli_args = parser.parse_args(argv) + if get_kernel_root_path(): + os.chdir(get_kernel_root_path()) + if cli_args.subcommand == 'run': if not os.path.exists(cli_args.build_dir): os.mkdir(cli_args.build_dir) + create_default_kunitconfig() if not linux: linux = kunit_kernel.LinuxSourceTree() @@ -257,6 +261,7 @@ def main(argv, linux=None): if cli_args.build_dir: if not os.path.exists(cli_args.build_dir): os.mkdir(cli_args.build_dir) + create_default_kunitconfig() if not linux: linux = kunit_kernel.LinuxSourceTree() @@ -270,10 +275,6 @@ def main(argv, linux=None): if result.status != KunitStatus.SUCCESS: sys.exit(1) elif cli_args.subcommand == 'build': - if cli_args.build_dir: - if not os.path.exists(cli_args.build_dir): - os.mkdir(cli_args.build_dir) - if not linux: linux = kunit_kernel.LinuxSourceTree() @@ -288,10 +289,6 @@ def main(argv, linux=None): if result.status != KunitStatus.SUCCESS: sys.exit(1) elif cli_args.subcommand == 'exec': - if cli_args.build_dir: - if not os.path.exists(cli_args.build_dir): - os.mkdir(cli_args.build_dir) - if not linux: linux = kunit_kernel.LinuxSourceTree() base-commit: 30185b69a2d533c4ba6ca926b8390ce7de495e29 -- 2.28.0.236.gb10cc79966-goog
[PATCH v2 2/2] kunit: tool: allow generating test results in JSON
From: Heidi Fahim
Add a --json flag, which when specified generates JSON formatted test
results conforming to the KernelCI API test_group spec[1]. The user can
use the new flag to specify a filename to print the json formatted
results to.
Link[1]: https://api.kernelci.org/schema-test-group.html#post
Signed-off-by: Heidi Fahim
Signed-off-by: Brendan Higgins
---
tools/testing/kunit/kunit.py | 35 +++---
tools/testing/kunit/kunit_json.py | 63 ++
tools/testing/kunit/kunit_tool_test.py | 33 ++
3 files changed, 125 insertions(+), 6 deletions(-)
create mode 100644 tools/testing/kunit/kunit_json.py
diff --git a/tools/testing/kunit/kunit.py b/tools/testing/kunit/kunit.py
index e2caf4e24ecb2..3c95a0eb0d048 100755
--- a/tools/testing/kunit/kunit.py
+++ b/tools/testing/kunit/kunit.py
@@ -17,6 +17,7 @@ from collections import namedtuple
from enum import Enum, auto
import kunit_config
+import kunit_json
import kunit_kernel
import kunit_parser
@@ -30,9 +31,9 @@ KunitBuildRequest = namedtuple('KunitBuildRequest',
KunitExecRequest = namedtuple('KunitExecRequest',
['timeout', 'build_dir', 'alltests'])
KunitParseRequest = namedtuple('KunitParseRequest',
- ['raw_output', 'input_data'])
+ ['raw_output', 'input_data', 'build_dir',
'json'])
KunitRequest = namedtuple('KunitRequest', ['raw_output','timeout', 'jobs',
- 'build_dir', 'alltests',
+ 'build_dir', 'alltests', 'json',
'make_options'])
KernelDirectoryPath = sys.argv[0].split('tools/testing/kunit/')[0]
@@ -113,12 +114,22 @@ def parse_tests(request: KunitParseRequest) ->
KunitResult:
test_result = kunit_parser.TestResult(kunit_parser.TestStatus.SUCCESS,
[],
'Tests not Parsed.')
+
if request.raw_output:
kunit_parser.raw_output(request.input_data)
else:
test_result = kunit_parser.parse_run_tests(request.input_data)
parse_end = time.time()
+ if request.json:
+ json_obj = kunit_json.get_json_result(
+ test_result=test_result,
+ def_config='kunit_defconfig',
+ build_dir=request.build_dir,
+ json_path=request.json)
+ if request.json == 'stdout':
+ print(json_obj)
+
if test_result.status != kunit_parser.TestStatus.SUCCESS:
return KunitResult(KunitStatus.TEST_FAILURE, test_result,
parse_end - parse_start)
@@ -151,7 +162,9 @@ def run_tests(linux: kunit_kernel.LinuxSourceTree,
return exec_result
parse_request = KunitParseRequest(request.raw_output,
- exec_result.result)
+ exec_result.result,
+ request.build_dir,
+ request.json)
parse_result = parse_tests(parse_request)
run_end = time.time()
@@ -195,7 +208,12 @@ def add_exec_opts(parser):
def add_parse_opts(parser):
parser.add_argument('--raw_output', help='don\'t format output from
kernel',
action='store_true')
-
+ parser.add_argument('--json',
+ nargs='?',
+ help='Stores test results in a JSON, and either '
+ 'prints to stdout or saves to file if a '
+ 'filename is specified',
+ type=str, const='stdout', default=None)
def main(argv, linux=None):
parser = argparse.ArgumentParser(
@@ -253,6 +271,7 @@ def main(argv, linux=None):
cli_args.jobs,
cli_args.build_dir,
cli_args.alltests,
+ cli_args.json,
cli_args.make_options)
result = run_tests(linux, request)
if result.status != KunitStatus.SUCCESS:
@@ -297,7 +316,9 @@ def main(argv, linux=None):
cli_args.alltests)
exec_result = exec_tests(linux, exec_request)
parse_request = KunitParseRequest(cli_args.raw_output,
- exec_result.result)
+ exec_result.result,
+ cli_args.build_dir,
+ cli_args.j
Re: [PATCH v1 2/2] kunit: tool: allow generating test results in JSON
On Tue, Aug 11, 2020 at 12:56 PM Bird, Tim wrote: > > > > > -Original Message- > > From: Brendan Higgins > > Sent: Friday, August 7, 2020 7:17 PM > > > > From: Heidi Fahim > > > > Add a --json flag, which when specified when kunit_tool is run, > > generates JSON formatted test results conforming to the KernelCI API > > test_group spec[1]. The user can the new flag to specify a filename as > > Seems to be missing a word. "The user can ? the new flag" Whoops, good catch. I will fix it in the next revision. > > the value to json in order to store the JSON results under linux/.
Re: [PATCH 2/2] kunit: ubsan integration
On Thu, Aug 6, 2020 at 10:43 AM Uriel Guajardo
wrote:
>
> Integrates UBSAN into the KUnit testing framework. It fails KUnit tests
> whenever it reports undefined behavior.
>
> Signed-off-by: Uriel Guajardo
You should resend this to the UBSAN maintainers as well; they will
need to sign off on this.
In the future, make sure to run get_maintainers.pl
Reviewed-by: Brendan Higgins
> ---
> lib/ubsan.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/lib/ubsan.c b/lib/ubsan.c
> index cb9af3f6b77e..1460e2c828c8 100644
> --- a/lib/ubsan.c
> +++ b/lib/ubsan.c
> @@ -14,6 +14,7 @@
> #include
> #include
> #include
> +#include
>
> #include "ubsan.h"
>
> @@ -137,6 +138,7 @@ static void ubsan_prologue(struct source_location *loc,
> const char *reason)
> {
> current->in_ubsan++;
>
> + kunit_fail_current_test();
> pr_err(""
> "\n");
> pr_err("UBSAN: %s in %s:%d:%d\n", reason, loc->file_name,
> --
> 2.28.0.163.g6104cc2f0b6-goog
>
Re: [PATCH 1/2] kunit: support failure from dynamic analysis tools
On Thu, Aug 6, 2020 at 10:43 AM Uriel Guajardo wrote: > > Adds an API to allow dynamic analysis tools to fail the currently > running KUnit test case. > > - Always places the kunit test in the task_struct to allow other tools > to access the currently running KUnit test. > > - Creates a new header file to avoid circular dependencies that could be > created from the test.h file. > > Requires KASAN-KUnit integration patch to access the kunit test from > task_struct: > https://lore.kernel.org/linux-kselftest/20200606040349.246780-2-david...@google.com/ > > Signed-off-by: Uriel Guajardo Reviewed-by: Brendan Higgins
Re: [PATCH] kunit: added lockdep support
On Thu, Aug 6, 2020 at 1:43 PM Uriel Guajardo wrote:
>
> On Thu, Aug 6, 2020 at 3:37 PM Uriel Guajardo
> wrote:
> >
> > From: Uriel Guajardo
> >
> > KUnit tests will now fail if lockdep detects an error during a test
> > case.
> >
> > The idea comes from how lib/locking-selftest [1] checks for lock errors: we
> > first if lock debugging is turned on. If not, an error must have
> > occurred, so we fail the test and restart lockdep for the next test case.
> >
> > Like the locking selftests, we also fix possible preemption count
> > corruption from lock bugs.
Sorry, just noticed: You probably want to send this to some of the
lockdep maintainers or the maintainers of the kselftest for lockdep.
> > Depends on kunit: support failure from dynamic analysis tools [2]
> >
> > [1]
> > https://elixir.bootlin.com/linux/v5.7.12/source/lib/locking-selftest.c#L1137
> >
> > [2]
> > https://lore.kernel.org/linux-kselftest/20200806174326.3577537-1-urielguajard...@gmail.com/
> >
> > Signed-off-by: Uriel Guajardo
> > ---
> > lib/kunit/test.c | 26 +-
> > 1 file changed, 25 insertions(+), 1 deletion(-)
> >
> > diff --git a/lib/kunit/test.c b/lib/kunit/test.c
> > index d8189d827368..0838ececa005 100644
> > --- a/lib/kunit/test.c
> > +++ b/lib/kunit/test.c
> > @@ -11,6 +11,8 @@
> > #include
> > #include
> > #include
> > +#include
> > +#include
> >
> > #include "debugfs.h"
> > #include "string-stream.h"
> > @@ -22,6 +24,26 @@ void kunit_fail_current_test(void)
> > kunit_set_failure(current->kunit_test);
> > }
> >
> > +static inline void kunit_check_locking_bugs(struct kunit *test,
> > + unsigned long
> > saved_preempt_count)
> > +{
> > + preempt_count_set(saved_preempt_count);
> > +#ifdef CONFIG_TRACE_IRQFLAGS
> > + if (softirq_count())
> > + current->softirqs_enabled = 0;
> > + else
> > + current->softirqs_enabled = 1;
> > +#endif
>
> I am not entirely sure why lib/locking-selftests enables/disables
> softirqs, but I suspect it has to do with the fact that preempt_count
> became corrupted, and somehow softirqs became incorrectly
> enabled/disabled as a result. The resetting of the preemption count
> will undo the enabling/disabling accordingly. Any insight on this
> would be appreciated!
>
> > +#if IS_ENABLED(CONFIG_LOCKDEP)
> > + local_irq_disable();
> > + if (!debug_locks) {
> > + kunit_set_failure(test);
> > + lockdep_reset();
> > + }
> > + local_irq_enable();
> > +#endif
> > +}
> > +
> > static void kunit_print_tap_version(void)
> > {
> > static bool kunit_has_printed_tap_version;
> > @@ -289,6 +311,7 @@ static void kunit_try_run_case(void *data)
> > struct kunit *test = ctx->test;
> > struct kunit_suite *suite = ctx->suite;
> > struct kunit_case *test_case = ctx->test_case;
> > + unsigned long saved_preempt_count = preempt_count();
> >
> > current->kunit_test = test;
> >
> > @@ -298,7 +321,8 @@ static void kunit_try_run_case(void *data)
> > * thread will resume control and handle any necessary clean up.
> > */
> > kunit_run_case_internal(test, suite, test_case);
> > - /* This line may never be reached. */
> > + /* These lines may never be reached. */
> > + kunit_check_locking_bugs(test, saved_preempt_count);
> > kunit_run_case_cleanup(test, suite);
> > }
> >
> > --
> > 2.28.0.236.gb10cc79966-goog
> >
Re: [PATCH] kunit: added lockdep support
On Thu, Aug 6, 2020 at 1:37 PM Uriel Guajardo wrote: > > From: Uriel Guajardo > > KUnit tests will now fail if lockdep detects an error during a test > case. > > The idea comes from how lib/locking-selftest [1] checks for lock errors: we > first if lock debugging is turned on. If not, an error must have > occurred, so we fail the test and restart lockdep for the next test case. > > Like the locking selftests, we also fix possible preemption count > corruption from lock bugs. > > Depends on kunit: support failure from dynamic analysis tools [2] > > [1] > https://elixir.bootlin.com/linux/v5.7.12/source/lib/locking-selftest.c#L1137 > > [2] > https://lore.kernel.org/linux-kselftest/20200806174326.3577537-1-urielguajard...@gmail.com/ > > Signed-off-by: Uriel Guajardo Reviewed-by: Brendan Higgins
Re: [PATCH v1 1/2] kunit: tool: fix running kunit_tool from outside kernel tree
On Fri, Aug 7, 2020 at 10:45 PM David Gow wrote:
>
> On Sat, Aug 8, 2020 at 9:17 AM Brendan Higgins
> wrote:
> >
> > Currently kunit_tool does not work correctly when executed from a path
> > outside of the kernel tree, so make sure that the current working
> > directory is correct and the kunit_dir is properly initialized before
> > running.
> >
> > Signed-off-by: Brendan Higgins
> > ---
> > tools/testing/kunit/kunit.py | 8
> > 1 file changed, 8 insertions(+)
> >
> > diff --git a/tools/testing/kunit/kunit.py b/tools/testing/kunit/kunit.py
> > index 425ef40067e7..96344a11ff1f 100755
> > --- a/tools/testing/kunit/kunit.py
> > +++ b/tools/testing/kunit/kunit.py
> > @@ -237,9 +237,14 @@ def main(argv, linux=None):
> >
> > cli_args = parser.parse_args(argv)
> >
> > + if get_kernel_root_path():
> > + print('cd ' + get_kernel_root_path())
> Do we want to print this, or is it a leftover debug statement?
Whoops, I was supposed to delete that. That's embarrassing... ^_^;
> > + os.chdir(get_kernel_root_path())
> > +
> > if cli_args.subcommand == 'run':
> > if not os.path.exists(cli_args.build_dir):
> > os.mkdir(cli_args.build_dir)
> > + create_default_kunitconfig()
> Why are we adding this everywhere when it's already in config_tests,
> which should already be called in all of the places where a
> kunitconfig is required?
Ah yes, .kunitconfig needs to be created before config_tests() can be
called because the LinuxSourceTree constructor needs .kunitconfig to
exist.
> Is the goal to always copy the default kunitconfig when creating a new
> build_dir? While I can sort-of see why we might want to do that, if
> the build dir doesn't exist, most of the subcommands will fail anyway
> (maybe we should only create the build-dir for 'config' and 'run'?)
I just did it because we were getting a failure in a constructor so we
couldn't do much. Ideally we would check that the current state allows
for the command that the user intended to run, but I think that's
beyond the scope of this change.
So I guess the real question is: Is it okay for it to crash in the
constructor with a cryptic error message for now, or do we want to let
it fail with a slightly less cryptic message later?
> > if not linux:
> > linux = kunit_kernel.LinuxSourceTree()
> > @@ -257,6 +262,7 @@ def main(argv, linux=None):
> > if cli_args.build_dir:
> > if not os.path.exists(cli_args.build_dir):
> > os.mkdir(cli_args.build_dir)
> > + create_default_kunitconfig()
> >
> > if not linux:
> > linux = kunit_kernel.LinuxSourceTree()
> > @@ -273,6 +279,7 @@ def main(argv, linux=None):
> > if cli_args.build_dir:
> > if not os.path.exists(cli_args.build_dir):
> > os.mkdir(cli_args.build_dir)
> > + create_default_kunitconfig()
> >
> > if not linux:
> > linux = kunit_kernel.LinuxSourceTree()
> > @@ -291,6 +298,7 @@ def main(argv, linux=None):
> > if cli_args.build_dir:
> > if not os.path.exists(cli_args.build_dir):
> > os.mkdir(cli_args.build_dir)
> > + create_default_kunitconfig()
> >
> > if not linux:
> > linux = kunit_kernel.LinuxSourceTree()
> >
> > base-commit: 30185b69a2d533c4ba6ca926b8390ce7de495e29
> > --
> > 2.28.0.236.gb10cc79966-goog
> >
[PATCH v1 2/2] kunit: tool: allow generating test results in JSON
From: Heidi Fahim
Add a --json flag, which when specified when kunit_tool is run,
generates JSON formatted test results conforming to the KernelCI API
test_group spec[1]. The user can the new flag to specify a filename as
the value to json in order to store the JSON results under linux/.
Link[1]: https://api.kernelci.org/schema-test-group.html#post
Signed-off-by: Heidi Fahim
Signed-off-by: Brendan Higgins
---
tools/testing/kunit/kunit.py | 35 +++---
tools/testing/kunit/kunit_json.py | 63 ++
tools/testing/kunit/kunit_tool_test.py | 33 ++
3 files changed, 125 insertions(+), 6 deletions(-)
create mode 100644 tools/testing/kunit/kunit_json.py
diff --git a/tools/testing/kunit/kunit.py b/tools/testing/kunit/kunit.py
index 96344a11ff1f..485b7c63b967 100755
--- a/tools/testing/kunit/kunit.py
+++ b/tools/testing/kunit/kunit.py
@@ -17,6 +17,7 @@ from collections import namedtuple
from enum import Enum, auto
import kunit_config
+import kunit_json
import kunit_kernel
import kunit_parser
@@ -30,9 +31,9 @@ KunitBuildRequest = namedtuple('KunitBuildRequest',
KunitExecRequest = namedtuple('KunitExecRequest',
['timeout', 'build_dir', 'alltests'])
KunitParseRequest = namedtuple('KunitParseRequest',
- ['raw_output', 'input_data'])
+ ['raw_output', 'input_data', 'build_dir',
'json'])
KunitRequest = namedtuple('KunitRequest', ['raw_output','timeout', 'jobs',
- 'build_dir', 'alltests',
+ 'build_dir', 'alltests', 'json',
'make_options'])
KernelDirectoryPath = sys.argv[0].split('tools/testing/kunit/')[0]
@@ -113,12 +114,22 @@ def parse_tests(request: KunitParseRequest) ->
KunitResult:
test_result = kunit_parser.TestResult(kunit_parser.TestStatus.SUCCESS,
[],
'Tests not Parsed.')
+
if request.raw_output:
kunit_parser.raw_output(request.input_data)
else:
test_result = kunit_parser.parse_run_tests(request.input_data)
parse_end = time.time()
+ if request.json:
+ json_obj = kunit_json.get_json_result(
+ test_result=test_result,
+ def_config='kunit_defconfig',
+ build_dir=request.build_dir,
+ json_path=request.json)
+ if request.json == 'stdout':
+ print(json_obj)
+
if test_result.status != kunit_parser.TestStatus.SUCCESS:
return KunitResult(KunitStatus.TEST_FAILURE, test_result,
parse_end - parse_start)
@@ -151,7 +162,9 @@ def run_tests(linux: kunit_kernel.LinuxSourceTree,
return exec_result
parse_request = KunitParseRequest(request.raw_output,
- exec_result.result)
+ exec_result.result,
+ request.build_dir,
+ request.json)
parse_result = parse_tests(parse_request)
run_end = time.time()
@@ -195,7 +208,12 @@ def add_exec_opts(parser):
def add_parse_opts(parser):
parser.add_argument('--raw_output', help='don\'t format output from
kernel',
action='store_true')
-
+ parser.add_argument('--json',
+ nargs='?',
+ help='Stores test results in a JSON, and either '
+ 'prints to stdout or saves to file if a '
+ 'filename is specified',
+ type=str, const='stdout', default=None)
def main(argv, linux=None):
parser = argparse.ArgumentParser(
@@ -254,6 +272,7 @@ def main(argv, linux=None):
cli_args.jobs,
cli_args.build_dir,
cli_args.alltests,
+ cli_args.json,
cli_args.make_options)
result = run_tests(linux, request)
if result.status != KunitStatus.SUCCESS:
@@ -308,7 +327,9 @@ def main(argv, linux=None):
cli_args.alltests)
exec_result = exec_tests(linux, exec_request)
parse_request = KunitParseRequest(cli_args.raw_output,
- exec_result.result)
+ exec_result.result,
+ cli_args.build_
[PATCH v1 1/2] kunit: tool: fix running kunit_tool from outside kernel tree
Currently kunit_tool does not work correctly when executed from a path
outside of the kernel tree, so make sure that the current working
directory is correct and the kunit_dir is properly initialized before
running.
Signed-off-by: Brendan Higgins
---
tools/testing/kunit/kunit.py | 8
1 file changed, 8 insertions(+)
diff --git a/tools/testing/kunit/kunit.py b/tools/testing/kunit/kunit.py
index 425ef40067e7..96344a11ff1f 100755
--- a/tools/testing/kunit/kunit.py
+++ b/tools/testing/kunit/kunit.py
@@ -237,9 +237,14 @@ def main(argv, linux=None):
cli_args = parser.parse_args(argv)
+ if get_kernel_root_path():
+ print('cd ' + get_kernel_root_path())
+ os.chdir(get_kernel_root_path())
+
if cli_args.subcommand == 'run':
if not os.path.exists(cli_args.build_dir):
os.mkdir(cli_args.build_dir)
+ create_default_kunitconfig()
if not linux:
linux = kunit_kernel.LinuxSourceTree()
@@ -257,6 +262,7 @@ def main(argv, linux=None):
if cli_args.build_dir:
if not os.path.exists(cli_args.build_dir):
os.mkdir(cli_args.build_dir)
+ create_default_kunitconfig()
if not linux:
linux = kunit_kernel.LinuxSourceTree()
@@ -273,6 +279,7 @@ def main(argv, linux=None):
if cli_args.build_dir:
if not os.path.exists(cli_args.build_dir):
os.mkdir(cli_args.build_dir)
+ create_default_kunitconfig()
if not linux:
linux = kunit_kernel.LinuxSourceTree()
@@ -291,6 +298,7 @@ def main(argv, linux=None):
if cli_args.build_dir:
if not os.path.exists(cli_args.build_dir):
os.mkdir(cli_args.build_dir)
+ create_default_kunitconfig()
if not linux:
linux = kunit_kernel.LinuxSourceTree()
base-commit: 30185b69a2d533c4ba6ca926b8390ce7de495e29
--
2.28.0.236.gb10cc79966-goog
[PATCH v6 4/5] kunit: test: add test plan to KUnit TAP format
TAP 14 allows an optional test plan to be emitted before the start of
the start of testing[1]; this is valuable because it makes it possible
for a test harness to detect whether the number of tests run matches the
number of tests expected to be run, ensuring that no tests silently
failed.
Link[1]:
https://github.com/isaacs/testanything.github.io/blob/tap14/tap-version-14-specification.md#the-plan
Signed-off-by: Brendan Higgins
Reviewed-by: Stephen Boyd
---
lib/kunit/executor.c | 17
lib/kunit/test.c | 11 ---
tools/testing/kunit/kunit_parser.py | 76 ++
.../test_is_test_passed-all_passed.log| Bin 1562 -> 1567 bytes
.../test_data/test_is_test_passed-crash.log | Bin 3016 -> 3021 bytes
.../test_data/test_is_test_passed-failure.log | Bin 1700 -> 1705 bytes
6 files changed, 79 insertions(+), 25 deletions(-)
diff --git a/lib/kunit/executor.c b/lib/kunit/executor.c
index 4aab7f70a88c3..a95742a4ece73 100644
--- a/lib/kunit/executor.c
+++ b/lib/kunit/executor.c
@@ -11,10 +11,27 @@ extern struct kunit_suite * const * const
__kunit_suites_end[];
#if IS_BUILTIN(CONFIG_KUNIT)
+static void kunit_print_tap_header(void)
+{
+ struct kunit_suite * const * const *suites, * const *subsuite;
+ int num_of_suites = 0;
+
+ for (suites = __kunit_suites_start;
+suites < __kunit_suites_end;
+suites++)
+ for (subsuite = *suites; *subsuite != NULL; subsuite++)
+ num_of_suites++;
+
+ pr_info("TAP version 14\n");
+ pr_info("1..%d\n", num_of_suites);
+}
+
int kunit_run_all_tests(void)
{
struct kunit_suite * const * const *suites;
+ kunit_print_tap_header();
+
for (suites = __kunit_suites_start;
suites < __kunit_suites_end;
suites++)
diff --git a/lib/kunit/test.c b/lib/kunit/test.c
index 918dff400a9d7..b1835ccb3fce2 100644
--- a/lib/kunit/test.c
+++ b/lib/kunit/test.c
@@ -19,16 +19,6 @@ static void kunit_set_failure(struct kunit *test)
WRITE_ONCE(test->success, false);
}
-static void kunit_print_tap_version(void)
-{
- static bool kunit_has_printed_tap_version;
-
- if (!kunit_has_printed_tap_version) {
- pr_info("TAP version 14\n");
- kunit_has_printed_tap_version = true;
- }
-}
-
/*
* Append formatted message to log, size of which is limited to
* KUNIT_LOG_SIZE bytes (including null terminating byte).
@@ -68,7 +58,6 @@ EXPORT_SYMBOL_GPL(kunit_suite_num_test_cases);
static void kunit_print_subtest_start(struct kunit_suite *suite)
{
- kunit_print_tap_version();
kunit_log(KERN_INFO, suite, KUNIT_SUBTEST_INDENT "# Subtest: %s",
suite->name);
kunit_log(KERN_INFO, suite, KUNIT_SUBTEST_INDENT "1..%zd",
diff --git a/tools/testing/kunit/kunit_parser.py
b/tools/testing/kunit/kunit_parser.py
index f13e0c0d66639..8019e3dd4c32f 100644
--- a/tools/testing/kunit/kunit_parser.py
+++ b/tools/testing/kunit/kunit_parser.py
@@ -45,10 +45,11 @@ class TestStatus(Enum):
FAILURE = auto()
TEST_CRASHED = auto()
NO_TESTS = auto()
+ FAILURE_TO_PARSE_TESTS = auto()
kunit_start_re = re.compile(r'TAP version [0-9]+$')
kunit_end_re = re.compile('(List of all partitions:|'
- 'Kernel panic - not syncing: VFS:|reboot: System
halted)')
+ 'Kernel panic - not syncing: VFS:)')
def isolate_kunit_output(kernel_output):
started = False
@@ -109,7 +110,7 @@ OkNotOkResult = namedtuple('OkNotOkResult',
['is_ok','description', 'text'])
OK_NOT_OK_SUBTEST = re.compile(r'^[\s]+(ok|not ok) [0-9]+ - (.*)$')
-OK_NOT_OK_MODULE = re.compile(r'^(ok|not ok) [0-9]+ - (.*)$')
+OK_NOT_OK_MODULE = re.compile(r'^(ok|not ok) ([0-9]+) - (.*)$')
def parse_ok_not_ok_test_case(lines: List[str], test_case: TestCase) -> bool:
save_non_diagnositic(lines, test_case)
@@ -197,7 +198,9 @@ def max_status(left: TestStatus, right: TestStatus) ->
TestStatus:
else:
return TestStatus.SUCCESS
-def parse_ok_not_ok_test_suite(lines: List[str], test_suite: TestSuite) ->
bool:
+def parse_ok_not_ok_test_suite(lines: List[str],
+ test_suite: TestSuite,
+ expected_suite_index: int) -> bool:
consume_non_diagnositic(lines)
if not lines:
test_suite.status = TestStatus.TEST_CRASHED
@@ -210,6 +213,12 @@ def parse_ok_not_ok_test_suite(lines: List[str],
test_suite: TestSuite) -> bool:
test_suite.status = TestStatus.SUCCESS
else:
test_suite.status = TestStatus.FAILURE
+ suite_index = int(match.group(2))
+ if suite_index != expected_suite_index:
+ print_w
[PATCH v6 1/5] vmlinux.lds.h: add linker section for KUnit test suites
Add a linker section where KUnit can put references to its test suites. This patch is the first step in transitioning to dispatching all KUnit tests from a centralized executor rather than having each as its own separate late_initcall. Co-developed-by: Iurii Zaikin Signed-off-by: Iurii Zaikin Signed-off-by: Brendan Higgins Reviewed-by: Stephen Boyd --- include/asm-generic/vmlinux.lds.h | 10 +- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h index 052e0f05a9841..75130a4d92ef0 100644 --- a/include/asm-generic/vmlinux.lds.h +++ b/include/asm-generic/vmlinux.lds.h @@ -692,7 +692,8 @@ THERMAL_TABLE(governor) \ EARLYCON_TABLE()\ LSM_TABLE() \ - EARLY_LSM_TABLE() + EARLY_LSM_TABLE() \ + KUNIT_TABLE() #define INIT_TEXT \ *(.init.text .init.text.*) \ @@ -884,6 +885,13 @@ KEEP(*(.con_initcall.init)) \ __con_initcall_end = .; +/* Alignment must be consistent with (kunit_suite *) in include/kunit/test.h */ +#define KUNIT_TABLE() \ + . = ALIGN(8); \ + __kunit_suites_start = .; \ + KEEP(*(.kunit_test_suites)) \ + __kunit_suites_end = .; + #ifdef CONFIG_BLK_DEV_INITRD #define INIT_RAM_FS\ . = ALIGN(4); \ -- 2.28.0.163.g6104cc2f0b6-goog
