| Message ID | 20201112180732.75589-6-andriy.shevchenko@linux.intel.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [v4,1/6] lib/list_kunit: Follow new file name convention for KUnit tests | expand |
On Thu, 12 Nov 2020 20:07:32 +0200 Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote: > Test get_option() for a starter which is provided by cmdline.c. > > ... > > lib/cmdline_kunit.c | 98 +++++++++++++++++++++++++++++++++++++++++++++ The spectacular warning storm which this produces makes me wonder if you sent the correct version? I mean, the cmdline_test_values[] definition wasn't even close. This fixes it all for me: --- a/lib/cmdline_kunit.c~lib-cmdline_kunit-add-a-new-test-suite-for-cmdline-api-fix +++ a/lib/cmdline_kunit.c @@ -13,19 +13,19 @@ static const char *cmdline_test_strings[ "+," , "--", ",,", "''" , "\"\",", "\",\"", "-\"\"", "\"", }; -static const char *cmdline_test_values[] = { +static const char cmdline_test_values[] = { 1, 1, 1, 1, 2, 3, 2, 3, 1, 3, 2, 1, 1, 1, 3, 1, }; -static void cmdline_do_one_test(struct kunit *test, char *in, int rc, int offset) +static void cmdline_do_one_test(struct kunit *test, const char *in, int rc, int offset) { const char *fmt = "Pattern: %s"; - char *out = in; + const char *out = in; int dummy; int ret; - ret = get_option(&out, &dummy); + ret = get_option((char **)&out, &dummy); KUNIT_EXPECT_EQ_MSG(test, ret, rc, fmt, in); KUNIT_EXPECT_PTR_EQ_MSG(test, out, in + offset, fmt, in);
On Thu, Nov 12, 2020 at 07:10:25PM -0800, Andrew Morton wrote: > On Thu, 12 Nov 2020 20:07:32 +0200 Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote: > > > Test get_option() for a starter which is provided by cmdline.c. > > > > ... > > > > lib/cmdline_kunit.c | 98 +++++++++++++++++++++++++++++++++++++++++++++ First of all, thanks for taking care of this! > The spectacular warning storm which this produces makes me wonder if > you sent the correct version? I mean, the cmdline_test_values[] > definition wasn't even close. This now puzzles me, what happened. I will check my branches, because certainly I was not only compiled that, but actually run those test cases. There is warning about license, I'm going to address right now.
On Mon, Nov 16, 2020 at 11:53:09AM +0200, Andy Shevchenko wrote: > On Thu, Nov 12, 2020 at 07:10:25PM -0800, Andrew Morton wrote: > > On Thu, 12 Nov 2020 20:07:32 +0200 Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote: > > > > > Test get_option() for a starter which is provided by cmdline.c. > > > > > > ... > > > > > > lib/cmdline_kunit.c | 98 +++++++++++++++++++++++++++++++++++++++++++++ > > First of all, thanks for taking care of this! > > > The spectacular warning storm which this produces makes me wonder if > > you sent the correct version? I mean, the cmdline_test_values[] > > definition wasn't even close. > > This now puzzles me, what happened. I will check my branches, because certainly > I was not only compiled that, but actually run those test cases. David, Brendan, can we somehow make those warnings visible when run test suite via kunit Python wrapper? It seems to me that I run test cases only via wrapper and I have got no warnings which are ones that must be fixed before submission. It's actual disadvantage of the framework (use of it via wrapper) that makes me feel like I would rather not hurry up with KUnit.
On Mon, Nov 16, 2020 at 6:40 PM Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote: > > On Mon, Nov 16, 2020 at 11:53:09AM +0200, Andy Shevchenko wrote: > > On Thu, Nov 12, 2020 at 07:10:25PM -0800, Andrew Morton wrote: > > > On Thu, 12 Nov 2020 20:07:32 +0200 Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote: > > > > > > > Test get_option() for a starter which is provided by cmdline.c. > > > > > > > > ... > > > > > > > > lib/cmdline_kunit.c | 98 +++++++++++++++++++++++++++++++++++++++++++++ > > > > First of all, thanks for taking care of this! > > > > > The spectacular warning storm which this produces makes me wonder if > > > you sent the correct version? I mean, the cmdline_test_values[] > > > definition wasn't even close. > > > > This now puzzles me, what happened. I will check my branches, because certainly > > I was not only compiled that, but actually run those test cases. > > David, Brendan, can we somehow make those warnings visible when run test suite > via kunit Python wrapper? There's a patch in the kunit-fixes branch which should fix this this: https://git.kernel.org/pub/scm/linux/kernel/git/shuah/linux-kselftest.git/commit/?h=kunit-fixes&id=390881448b1ff1e9d82896abbbda7cdb8e0be27c > > It seems to me that I run test cases only via wrapper and I have got no > warnings which are ones that must be fixed before submission. It's actual > disadvantage of the framework (use of it via wrapper) that makes me feel > like I would rather not hurry up with KUnit. Yeah: it's a pain, and I've been hit by it a couple of times recently, too. If you're not able to pick up the changes in the kunit-fixes branch above, then it's definitely best to also manually build a kernel to get the build logs. In any case, that fix should be merged for 5.10. Sorry for the inconvenience! -- David
On Tue, Nov 17, 2020 at 07:43:28AM +0800, David Gow wrote: > On Mon, Nov 16, 2020 at 6:40 PM Andy Shevchenko > <andriy.shevchenko@linux.intel.com> wrote: > > > > On Mon, Nov 16, 2020 at 11:53:09AM +0200, Andy Shevchenko wrote: > > > On Thu, Nov 12, 2020 at 07:10:25PM -0800, Andrew Morton wrote: > > > > On Thu, 12 Nov 2020 20:07:32 +0200 Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote: > > > > > > > > > Test get_option() for a starter which is provided by cmdline.c. > > > > > > > > > > ... > > > > > > > > > > lib/cmdline_kunit.c | 98 +++++++++++++++++++++++++++++++++++++++++++++ > > > > > > First of all, thanks for taking care of this! > > > > > > > The spectacular warning storm which this produces makes me wonder if > > > > you sent the correct version? I mean, the cmdline_test_values[] > > > > definition wasn't even close. > > > > > > This now puzzles me, what happened. I will check my branches, because certainly > > > I was not only compiled that, but actually run those test cases. > > > > David, Brendan, can we somehow make those warnings visible when run test suite > > via kunit Python wrapper? > > There's a patch in the kunit-fixes branch which should fix this this: > https://git.kernel.org/pub/scm/linux/kernel/git/shuah/linux-kselftest.git/commit/?h=kunit-fixes&id=390881448b1ff1e9d82896abbbda7cdb8e0be27c Good we have a fix! > > It seems to me that I run test cases only via wrapper and I have got no > > warnings which are ones that must be fixed before submission. It's actual > > disadvantage of the framework (use of it via wrapper) that makes me feel > > like I would rather not hurry up with KUnit. > > Yeah: it's a pain, and I've been hit by it a couple of times recently, > too. If you're not able to pick up the changes in the kunit-fixes > branch above, then it's definitely best to also manually build a > kernel to get the build logs. > > In any case, that fix should be merged for 5.10. > > Sorry for the inconvenience! No problem, I will keep above in mind, thanks.
diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug index 6140413174be..b939740f93b7 100644 --- a/lib/Kconfig.debug +++ b/lib/Kconfig.debug @@ -2283,6 +2283,17 @@ config LINEAR_RANGES_TEST If unsure, say N. +config CMDLINE_KUNIT_TEST + tristate "KUnit test for cmdline API" + depends on KUNIT + help + This builds the cmdline API unit test. + Tests the logic of API provided by cmdline.c. + 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 BITS_TEST tristate "KUnit test for bits.h" depends on KUNIT diff --git a/lib/Makefile b/lib/Makefile index dc76e7d8a453..985c6a8909e0 100644 --- a/lib/Makefile +++ b/lib/Makefile @@ -350,5 +350,6 @@ obj-$(CONFIG_PLDMFW) += pldmfw/ # KUnit tests obj-$(CONFIG_BITFIELD_KUNIT) += bitfield_kunit.o obj-$(CONFIG_BITS_TEST) += bits_kunit.o +obj-$(CONFIG_CMDLINE_KUNIT_TEST) += cmdline_kunit.o obj-$(CONFIG_LINEAR_RANGES_TEST) += linear_ranges_kunit.o obj-$(CONFIG_LIST_KUNIT_TEST) += list_kunit.o diff --git a/lib/cmdline_kunit.c b/lib/cmdline_kunit.c new file mode 100644 index 000000000000..acad1386d54c --- /dev/null +++ b/lib/cmdline_kunit.c @@ -0,0 +1,98 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * Test cases for API provided by cmdline.c + */ + +#include <kunit/test.h> +#include <linux/kernel.h> +#include <linux/random.h> +#include <linux/string.h> + +static const char *cmdline_test_strings[] = { + "\"\"", "" , "=" , "\"-", "," , "-," , ",-" , "-" , + "+," , "--", ",,", "''" , "\"\",", "\",\"", "-\"\"", "\"", +}; + +static const char *cmdline_test_values[] = { + 1, 1, 1, 1, 2, 3, 2, 3, + 1, 3, 2, 1, 1, 1, 3, 1, +}; + +static void cmdline_do_one_test(struct kunit *test, char *in, int rc, int offset) +{ + const char *fmt = "Pattern: %s"; + char *out = in; + int dummy; + int ret; + + ret = get_option(&out, &dummy); + + KUNIT_EXPECT_EQ_MSG(test, ret, rc, fmt, in); + KUNIT_EXPECT_PTR_EQ_MSG(test, out, in + offset, fmt, in); +} + +static void cmdline_test_noint(struct kunit *test) +{ + unsigned int i = 0; + + do { + const char *str = cmdline_test_strings[i]; + int rc = 0; + int offset; + + /* Only first and leading '-' will advance the pointer */ + offset = !!(*str == '-'); + cmdline_do_one_test(test, str, rc, offset); + } while (++i < ARRAY_SIZE(cmdline_test_strings)); +} + +static void cmdline_test_lead_int(struct kunit *test) +{ + unsigned int i = 0; + char in[32]; + + do { + const char *str = cmdline_test_strings[i]; + int rc = cmdline_test_values[i]; + int offset; + + sprintf(in, "%u%s", get_random_int() % 256, str); + /* Only first '-' after the number will advance the pointer */ + offset = strlen(in) - strlen(str) + !!(rc == 2); + cmdline_do_one_test(test, in, rc, offset); + } while (++i < ARRAY_SIZE(cmdline_test_strings)); +} + +static void cmdline_test_tail_int(struct kunit *test) +{ + unsigned int i = 0; + char in[32]; + + do { + const char *str = cmdline_test_strings[i]; + /* When "" or "-" the result will be valid integer */ + int rc = strcmp(str, "") ? (strcmp(str, "-") ? 0 : 1) : 1; + int offset; + + sprintf(in, "%s%u", str, get_random_int() % 256); + /* + * Only first and leading '-' not followed by integer + * will advance the pointer. + */ + offset = rc ? strlen(in) : !!(*str == '-'); + cmdline_do_one_test(test, in, rc, offset); + } while (++i < ARRAY_SIZE(cmdline_test_strings)); +} + +static struct kunit_case cmdline_test_cases[] = { + KUNIT_CASE(cmdline_test_noint), + KUNIT_CASE(cmdline_test_lead_int), + KUNIT_CASE(cmdline_test_tail_int), + {} +}; + +static struct kunit_suite cmdline_test_suite = { + .name = "cmdline", + .test_cases = cmdline_test_cases, +}; +kunit_test_suite(cmdline_test_suite);
Test get_option() for a starter which is provided by cmdline.c. Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> --- v4: new patch lib/Kconfig.debug | 11 +++++ lib/Makefile | 1 + lib/cmdline_kunit.c | 98 +++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 110 insertions(+) create mode 100644 lib/cmdline_kunit.c