| Message ID | 20210625111603.358518-1-davidgow@google.com |
|---|---|
| State | New |
| Headers | show |
| Series | kunit: Fix merge issue in suite filtering test | expand |
On 6/25/21 5:16 AM, David Gow wrote: > There were a couple of errors introuced when > "kunit: add unit test for filtering suites by names"[1] was merged in > c9d80ffc5a. > > An erroneous '+' was introduced in executor.c, and the executor_test.c > file went missing. This causes the kernel to fail to compile if > CONFIG_KUNIT is enabled, as reported in [2,3]. > > As with the original, I've tested by running just the new tests using > itself: > $ ./tools/testing/kunit/kunit.py run '*exec*' > > [1]: https://lore.kernel.org/linux-kselftest/20210421020427.2384721-1-dlatypov@google.com/ > [2]: https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org/thread/6IKQX5JXZF7I3NFH4IAWUMHXEQSCPNDP/ > [3]: https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org/thread/EKY7ZH5YDCCTSJF2G7XFPMGIXQSUVD3Y/ > > Fixes: c9d80ffc5a ("kunit: add unit test for filtering suites by names") > Reported-by: kernel test robot <lkp@intel.com> > Signed-off-by: David Gow <davidgow@google.com> > --- > > This is another fix for the kunit-fixes branch, where there seems to > have been an issue merging the "kunit: add unit test for filtering > suites by names" patch here: > https://git.kernel.org/pub/scm/linux/kernel/git/shuah/linux-kselftest.git/commit/?h=kunit-fixes&id=c9d80ffc5a0a30955de0b8c5c46a05906d417800 > > Again, feel free to squash this into the original patch if that works > better. > Thank you. My bad. Applied to kunit-fixes now. thanks, -- Shuah
On 6/25/21 11:08 AM, Daniel Latypov wrote: > On Fri, Jun 25, 2021 at 9:11 AM Shuah Khan <skhan@linuxfoundation.org> wrote: >> >> On 6/25/21 5:16 AM, David Gow wrote: >>> There were a couple of errors introuced when >>> "kunit: add unit test for filtering suites by names"[1] was merged in >>> c9d80ffc5a. >>> >>> An erroneous '+' was introduced in executor.c, and the executor_test.c >>> file went missing. This causes the kernel to fail to compile if >>> CONFIG_KUNIT is enabled, as reported in [2,3]. >>> >>> As with the original, I've tested by running just the new tests using >>> itself: >>> $ ./tools/testing/kunit/kunit.py run '*exec*' >>> >>> [1]: https://lore.kernel.org/linux-kselftest/20210421020427.2384721-1-dlatypov@google.com/ >>> [2]: https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org/thread/6IKQX5JXZF7I3NFH4IAWUMHXEQSCPNDP/ >>> [3]: https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org/thread/EKY7ZH5YDCCTSJF2G7XFPMGIXQSUVD3Y/ >>> >>> Fixes: c9d80ffc5a ("kunit: add unit test for filtering suites by names") >>> Reported-by: kernel test robot <lkp@intel.com> >>> Signed-off-by: David Gow <davidgow@google.com> >>> --- >>> >>> This is another fix for the kunit-fixes branch, where there seems to >>> have been an issue merging the "kunit: add unit test for filtering >>> suites by names" patch here: >>> https://git.kernel.org/pub/scm/linux/kernel/git/shuah/linux-kselftest.git/commit/?h=kunit-fixes&id=c9d80ffc5a0a30955de0b8c5c46a05906d417800 >>> >>> Again, feel free to squash this into the original patch if that works >>> better. >>> >> >> Thank you. My bad. Applied to kunit-fixes now. > > Hmm, it looks like executor_test.c might not have made it into kunit-fixes. > I believe this is the applied version of this patch: > > $ git show d833ce7480864d4d7eb2dbb04320858be3578b2a --stat > commit d833ce7480864d4d7eb2dbb04320858be3578b2a > Author: David Gow <davidgow@google.com> > Date: Fri Jun 25 04:16:03 2021 -0700 > > kunit: Fix merge issue in suite filtering test > ... > lib/kunit/executor.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > The result looks like this: > $ ./tools/testing/kunit/kunit.py run > ... > $ make ARCH=um --jobs=8 O=.kunit > ERROR:root:../lib/kunit/executor.c:140:10: fatal error: > executor_test.c: No such file or directory > 140 | #include "executor_test.c" > | ^~~~~~~~~~~~~~~~~ > > > I just `git am` or something just really doesn't like executor_test.c :) > > My mistake it looks like in merging the patch. I had to fix merge conflicts and made a mistake. I will fix it now. Odd that my local compile didn't catch the problem. I used the tools/testing/kunit/kunit.py build thanks, -- Shuah
diff --git a/lib/kunit/executor.c b/lib/kunit/executor.c index 672f91486d23..acd1de436f59 100644 --- a/lib/kunit/executor.c +++ b/lib/kunit/executor.c @@ -23,7 +23,7 @@ static char *kunit_shutdown; core_param(kunit_shutdown, kunit_shutdown, charp, 0644); static struct kunit_suite * const * -+kunit_filter_subsuite(struct kunit_suite * const * const subsuite, +kunit_filter_subsuite(struct kunit_suite * const * const subsuite, const char *filter_glob) { int i, n = 0; diff --git a/lib/kunit/executor_test.c b/lib/kunit/executor_test.c new file mode 100644 index 000000000000..cdbe54b16501 --- /dev/null +++ b/lib/kunit/executor_test.c @@ -0,0 +1,133 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * KUnit test for the KUnit executor. + * + * Copyright (C) 2021, Google LLC. + * Author: Daniel Latypov <dlatypov@google.com> + */ + +#include <kunit/test.h> + +static void kfree_at_end(struct kunit *test, const void *to_free); +static struct kunit_suite *alloc_fake_suite(struct kunit *test, + const char *suite_name); + +static void filter_subsuite_test(struct kunit *test) +{ + struct kunit_suite *subsuite[3] = {NULL, NULL, NULL}; + struct kunit_suite * const *filtered; + + subsuite[0] = alloc_fake_suite(test, "suite1"); + subsuite[1] = alloc_fake_suite(test, "suite2"); + + /* Want: suite1, suite2, NULL -> suite2, NULL */ + filtered = kunit_filter_subsuite(subsuite, "suite2*"); + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, filtered); + kfree_at_end(test, filtered); + + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, filtered[0]); + KUNIT_EXPECT_STREQ(test, (const char *)filtered[0]->name, "suite2"); + + KUNIT_EXPECT_FALSE(test, filtered[1]); +} + +static void filter_subsuite_to_empty_test(struct kunit *test) +{ + struct kunit_suite *subsuite[3] = {NULL, NULL, NULL}; + struct kunit_suite * const *filtered; + + subsuite[0] = alloc_fake_suite(test, "suite1"); + subsuite[1] = alloc_fake_suite(test, "suite2"); + + filtered = kunit_filter_subsuite(subsuite, "not_found"); + kfree_at_end(test, filtered); /* just in case */ + + KUNIT_EXPECT_FALSE_MSG(test, filtered, + "should be NULL to indicate no match"); +} + +static void kfree_subsuites_at_end(struct kunit *test, struct suite_set *suite_set) +{ + struct kunit_suite * const * const *suites; + + kfree_at_end(test, suite_set->start); + for (suites = suite_set->start; suites < suite_set->end; suites++) + kfree_at_end(test, *suites); +} + +static void filter_suites_test(struct kunit *test) +{ + /* Suites per-file are stored as a NULL terminated array */ + struct kunit_suite *subsuites[2][2] = { + {NULL, NULL}, + {NULL, NULL}, + }; + /* Match the memory layout of suite_set */ + struct kunit_suite * const * const suites[2] = { + subsuites[0], subsuites[1], + }; + + const struct suite_set suite_set = { + .start = suites, + .end = suites + 2, + }; + struct suite_set filtered = {.start = NULL, .end = NULL}; + + /* Emulate two files, each having one suite */ + subsuites[0][0] = alloc_fake_suite(test, "suite0"); + subsuites[1][0] = alloc_fake_suite(test, "suite1"); + + /* Filter out suite1 */ + filtered = kunit_filter_suites(&suite_set, "suite0"); + kfree_subsuites_at_end(test, &filtered); /* let us use ASSERTs without leaking */ + KUNIT_ASSERT_EQ(test, filtered.end - filtered.start, (ptrdiff_t)1); + + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, filtered.start); + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, filtered.start[0]); + KUNIT_EXPECT_STREQ(test, (const char *)filtered.start[0][0]->name, "suite0"); +} + +static struct kunit_case executor_test_cases[] = { + KUNIT_CASE(filter_subsuite_test), + KUNIT_CASE(filter_subsuite_to_empty_test), + KUNIT_CASE(filter_suites_test), + {} +}; + +static struct kunit_suite executor_test_suite = { + .name = "kunit_executor_test", + .test_cases = executor_test_cases, +}; + +kunit_test_suites(&executor_test_suite); + +/* Test helpers */ + +static void kfree_res_free(struct kunit_resource *res) +{ + kfree(res->data); +} + +/* Use the resource API to register a call to kfree(to_free). + * Since we never actually use the resource, it's safe to use on const data. + */ +static void kfree_at_end(struct kunit *test, const void *to_free) +{ + /* kfree() handles NULL already, but avoid allocating a no-op cleanup. */ + if (IS_ERR_OR_NULL(to_free)) + return; + kunit_alloc_and_get_resource(test, NULL, kfree_res_free, GFP_KERNEL, + (void *)to_free); +} + +static struct kunit_suite *alloc_fake_suite(struct kunit *test, + const char *suite_name) +{ + struct kunit_suite *suite; + + /* We normally never expect to allocate suites, hence the non-const cast. */ + suite = kunit_kzalloc(test, sizeof(*suite), GFP_KERNEL); + strncpy((char *)suite->name, suite_name, sizeof(suite->name) - 1); + + return suite; +}
There were a couple of errors introuced when "kunit: add unit test for filtering suites by names"[1] was merged in c9d80ffc5a. An erroneous '+' was introduced in executor.c, and the executor_test.c file went missing. This causes the kernel to fail to compile if CONFIG_KUNIT is enabled, as reported in [2,3]. As with the original, I've tested by running just the new tests using itself: $ ./tools/testing/kunit/kunit.py run '*exec*' [1]: https://lore.kernel.org/linux-kselftest/20210421020427.2384721-1-dlatypov@google.com/ [2]: https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org/thread/6IKQX5JXZF7I3NFH4IAWUMHXEQSCPNDP/ [3]: https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org/thread/EKY7ZH5YDCCTSJF2G7XFPMGIXQSUVD3Y/ Fixes: c9d80ffc5a ("kunit: add unit test for filtering suites by names") Reported-by: kernel test robot <lkp@intel.com> Signed-off-by: David Gow <davidgow@google.com> --- This is another fix for the kunit-fixes branch, where there seems to have been an issue merging the "kunit: add unit test for filtering suites by names" patch here: https://git.kernel.org/pub/scm/linux/kernel/git/shuah/linux-kselftest.git/commit/?h=kunit-fixes&id=c9d80ffc5a0a30955de0b8c5c46a05906d417800 Again, feel free to squash this into the original patch if that works better. Cheers, -- David lib/kunit/executor.c | 2 +- lib/kunit/executor_test.c | 133 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 134 insertions(+), 1 deletion(-) create mode 100644 lib/kunit/executor_test.c