[PATCH v4 1/6] lib/list_kunit: Follow new file name convention for KUnit tests

[PATCH v4 1/6] lib/list_kunit: Follow new file name convention for KUnit tests

[Andy Shevchenko]

Follow new file name convention for the KUnit tests.
Since we have lib/*test*.c in a few variations,
use 'kunit' suffix to distinguish usual test cases
with KUnit-based ones.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Reviewed-by: David Gow <davidgow@google.com>
Acked-by: Brendan Higgins <brendanhiggins@google.com>
---
v4: added tag (Brendan)
 MAINTAINERS                       | 2 +-
 lib/Makefile                      | 2 +-
 lib/{list-test.c => list_kunit.c} | 0
 3 files changed, 2 insertions(+), 2 deletions(-)
 rename lib/{list-test.c => list_kunit.c} (100%)

diff --git a/MAINTAINERS b/MAINTAINERS
index f1f088a29bc2..5901b817d0bc 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -10222,7 +10222,7 @@ M:	David Gow <davidgow@google.com>
 L:	linux-kselftest@vger.kernel.org
 L:	kunit-dev@googlegroups.com
 S:	Maintained
-F:	lib/list-test.c
+F:	lib/list_kunit.c
 
 LITEX PLATFORM
 M:	Karol Gugala <kgugala@antmicro.com>
diff --git a/lib/Makefile b/lib/Makefile
index ce45af50983a..9d78ef73d6ed 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -349,6 +349,6 @@ obj-$(CONFIG_PLDMFW) += pldmfw/
 
 # KUnit tests
 obj-$(CONFIG_BITFIELD_KUNIT) += bitfield_kunit.o
-obj-$(CONFIG_LIST_KUNIT_TEST) += list-test.o
+obj-$(CONFIG_LIST_KUNIT_TEST) += list_kunit.o
 obj-$(CONFIG_LINEAR_RANGES_TEST) += test_linear_ranges.o
 obj-$(CONFIG_BITS_TEST) += test_bits.o
diff --git a/lib/list-test.c b/lib/list_kunit.c
similarity index 100%
rename from lib/list-test.c
rename to lib/list_kunit.c
-- 
2.28.0

[PATCH v4 2/6] lib/linear_ranges_kunit: Follow new file name convention for KUnit tests

[Andy Shevchenko]

Follow new file name convention for the KUnit tests.
Since we have lib/*test*.c in a few variations,
use 'kunit' suffix to distinguish usual test cases
with KUnit-based ones.

Cc: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Reviewed-by: David Gow <davidgow@google.com>
Acked-by: Mark Brown <broonie@kernel.org>
---
v4: added tag (Mark)

 MAINTAINERS                                         | 3 +--
 lib/Makefile                                        | 2 +-
 lib/{test_linear_ranges.c => linear_ranges_kunit.c} | 0
 3 files changed, 2 insertions(+), 3 deletions(-)
 rename lib/{test_linear_ranges.c => linear_ranges_kunit.c} (100%)

diff --git a/MAINTAINERS b/MAINTAINERS
index 5901b817d0bc..9a314e618705 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -10103,8 +10103,7 @@ F:	include/uapi/linux/lightnvm.h
 LINEAR RANGES HELPERS
 M:	Mark Brown <broonie@kernel.org>
 R:	Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
-F:	lib/linear_ranges.c
-F:	lib/test_linear_ranges.c
+F:	lib/linear_ranges*.c
 F:	include/linux/linear_range.h
 
 LINUX FOR POWER MACINTOSH
diff --git a/lib/Makefile b/lib/Makefile
index 9d78ef73d6ed..ef25cd94f34d 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -349,6 +349,6 @@ obj-$(CONFIG_PLDMFW) += pldmfw/
 
 # KUnit tests
 obj-$(CONFIG_BITFIELD_KUNIT) += bitfield_kunit.o
+obj-$(CONFIG_LINEAR_RANGES_TEST) += linear_ranges_kunit.o
 obj-$(CONFIG_LIST_KUNIT_TEST) += list_kunit.o
-obj-$(CONFIG_LINEAR_RANGES_TEST) += test_linear_ranges.o
 obj-$(CONFIG_BITS_TEST) += test_bits.o
diff --git a/lib/test_linear_ranges.c b/lib/linear_ranges_kunit.c
similarity index 100%
rename from lib/test_linear_ranges.c
rename to lib/linear_ranges_kunit.c
-- 
2.28.0

[PATCH v4 3/6] lib/bits_kunit: Follow new file name convention for KUnit tests

[Andy Shevchenko]

Follow new file name convention for the KUnit tests.
Since we have lib/*test*.c in a few variations,
use 'kunit' suffix to distinguish usual test cases
with KUnit-based ones.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Reviewed-by: David Gow <davidgow@google.com>
Acked-by: Brendan Higgins <brendanhiggins@google.com>
---
v4: added tag (Brendan)

 lib/Makefile                      | 2 +-
 lib/{test_bits.c => bits_kunit.c} | 0
 2 files changed, 1 insertion(+), 1 deletion(-)
 rename lib/{test_bits.c => bits_kunit.c} (100%)

diff --git a/lib/Makefile b/lib/Makefile
index ef25cd94f34d..dc76e7d8a453 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -349,6 +349,6 @@ obj-$(CONFIG_PLDMFW) += pldmfw/
 
 # KUnit tests
 obj-$(CONFIG_BITFIELD_KUNIT) += bitfield_kunit.o
+obj-$(CONFIG_BITS_TEST) += bits_kunit.o
 obj-$(CONFIG_LINEAR_RANGES_TEST) += linear_ranges_kunit.o
 obj-$(CONFIG_LIST_KUNIT_TEST) += list_kunit.o
-obj-$(CONFIG_BITS_TEST) += test_bits.o
diff --git a/lib/test_bits.c b/lib/bits_kunit.c
similarity index 100%
rename from lib/test_bits.c
rename to lib/bits_kunit.c
-- 
2.28.0

[PATCH v4 4/6] lib/cmdline: Fix get_option() for strings starting with hyphen

[Andy Shevchenko]

When string doesn't have an integer and starts from hyphen get_option()
may return interesting results. Fix it to return 0.

The simple_strtoull() is used due to absence of simple_strtoul()
in a boot code on some architectures.

Note, the Fixes tag below is rather for anthropological curiosity.

Fixes: f68565831e72 ("Import 2.4.0-test2pre3")
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
v4: new patch
 lib/cmdline.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/lib/cmdline.c b/lib/cmdline.c
index fbb9981a04a4..ca89846ee0bb 100644
--- a/lib/cmdline.c
+++ b/lib/cmdline.c
@@ -45,6 +45,9 @@ static int get_range(char **str, int *pint, int n)
  *	1 - int found, no subsequent comma
  *	2 - int found including a subsequent comma
  *	3 - hyphen found to denote a range
+ *
+ *	Leading hyphen without integer is no integer case, but we consume it
+ *	for the sake of simplification.
  */
 
 int get_option(char **str, int *pint)
@@ -53,7 +56,10 @@ int get_option(char **str, int *pint)
 
 	if (!cur || !(*cur))
 		return 0;
-	*pint = simple_strtol(cur, str, 0);
+	if (*cur == '-')
+		*pint = -simple_strtoull(++cur, str, 0);
+	else
+		*pint = simple_strtoull(cur, str, 0);
 	if (cur == *str)
 		return 0;
 	if (**str == ',') {
-- 
2.28.0

[PATCH v4 5/6] lib/cmdline: Allow NULL to be an output for get_option()

[Andy Shevchenko]

In the future we would like to use get_option() to only validate
the string and parse it separately. To achieve this, allow
NULL to be an output for get_option().

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
v4: new patch
 lib/cmdline.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/lib/cmdline.c b/lib/cmdline.c
index ca89846ee0bb..9e186234edc0 100644
--- a/lib/cmdline.c
+++ b/lib/cmdline.c
@@ -35,11 +35,14 @@ static int get_range(char **str, int *pint, int n)
 /**
  *	get_option - Parse integer from an option string
  *	@str: option string
- *	@pint: (output) integer value parsed from @str
+ *	@pint: (optional output) integer value parsed from @str
  *
  *	Read an int from an option string; if available accept a subsequent
  *	comma as well.
  *
+ *	When @pint is NULL the function can be used as a validator of
+ *	the current option in the string.
+ *
  *	Return values:
  *	0 - no int in string
  *	1 - int found, no subsequent comma
@@ -53,13 +56,16 @@ static int get_range(char **str, int *pint, int n)
 int get_option(char **str, int *pint)
 {
 	char *cur = *str;
+	int value;
 
 	if (!cur || !(*cur))
 		return 0;
 	if (*cur == '-')
-		*pint = -simple_strtoull(++cur, str, 0);
+		value = -simple_strtoull(++cur, str, 0);
 	else
-		*pint = simple_strtoull(cur, str, 0);
+		value = simple_strtoull(cur, str, 0);
+	if (pint)
+		*pint = value;
 	if (cur == *str)
 		return 0;
 	if (**str == ',') {
-- 
2.28.0

[PATCH v4 6/6] lib/cmdline_kunit: Add a new test suite for cmdline API

[Andy Shevchenko]

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

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);
-- 
2.28.0

Re: [PATCH v4 6/6] lib/cmdline_kunit: Add a new test suite for cmdline API

[Andrew Morton]

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);
_

but it sucks that get_option()'s first arg isn't `const char **str'.  I
was too lazy to look at addressing that.



lib/cmdline_kunit.c:17:2: warning: initialization makes pointer from integer without a cast [-Wint-conversion]
  1, 1, 1, 1, 2, 3, 2, 3,
  ^
lib/cmdline_kunit.c:17:2: note: (near initialization for ‘cmdline_test_values[0]’)
lib/cmdline_kunit.c:17:5: warning: initialization makes pointer from integer without a cast [-Wint-conversion]
  1, 1, 1, 1, 2, 3, 2, 3,
     ^
lib/cmdline_kunit.c:17:5: note: (near initialization for ‘cmdline_test_values[1]’)
lib/cmdline_kunit.c:17:8: warning: initialization makes pointer from integer without a cast [-Wint-conversion]
  1, 1, 1, 1, 2, 3, 2, 3,
        ^
lib/cmdline_kunit.c:17:8: note: (near initialization for ‘cmdline_test_values[2]’)
lib/cmdline_kunit.c:17:11: warning: initialization makes pointer from integer without a cast [-Wint-conversion]
  1, 1, 1, 1, 2, 3, 2, 3,
           ^
lib/cmdline_kunit.c:17:11: note: (near initialization for ‘cmdline_test_values[3]’)
lib/cmdline_kunit.c:17:14: warning: initialization makes pointer from integer without a cast [-Wint-conversion]
  1, 1, 1, 1, 2, 3, 2, 3,
              ^
lib/cmdline_kunit.c:17:14: note: (near initialization for ‘cmdline_test_values[4]’)
lib/cmdline_kunit.c:17:17: warning: initialization makes pointer from integer without a cast [-Wint-conversion]
  1, 1, 1, 1, 2, 3, 2, 3,
                 ^
lib/cmdline_kunit.c:17:17: note: (near initialization for ‘cmdline_test_values[5]’)
lib/cmdline_kunit.c:17:20: warning: initialization makes pointer from integer without a cast [-Wint-conversion]
  1, 1, 1, 1, 2, 3, 2, 3,
                    ^
lib/cmdline_kunit.c:17:20: note: (near initialization for ‘cmdline_test_values[6]’)
lib/cmdline_kunit.c:17:23: warning: initialization makes pointer from integer without a cast [-Wint-conversion]
  1, 1, 1, 1, 2, 3, 2, 3,
                       ^
lib/cmdline_kunit.c:17:23: note: (near initialization for ‘cmdline_test_values[7]’)
lib/cmdline_kunit.c:18:2: warning: initialization makes pointer from integer without a cast [-Wint-conversion]
  1, 3, 2, 1, 1, 1, 3, 1,
  ^
lib/cmdline_kunit.c:18:2: note: (near initialization for ‘cmdline_test_values[8]’)
lib/cmdline_kunit.c:18:5: warning: initialization makes pointer from integer without a cast [-Wint-conversion]
  1, 3, 2, 1, 1, 1, 3, 1,
     ^
lib/cmdline_kunit.c:18:5: note: (near initialization for ‘cmdline_test_values[9]’)
lib/cmdline_kunit.c:18:8: warning: initialization makes pointer from integer without a cast [-Wint-conversion]
  1, 3, 2, 1, 1, 1, 3, 1,
        ^
lib/cmdline_kunit.c:18:8: note: (near initialization for ‘cmdline_test_values[10]’)
lib/cmdline_kunit.c:18:11: warning: initialization makes pointer from integer without a cast [-Wint-conversion]
  1, 3, 2, 1, 1, 1, 3, 1,
           ^
lib/cmdline_kunit.c:18:11: note: (near initialization for ‘cmdline_test_values[11]’)
lib/cmdline_kunit.c:18:14: warning: initialization makes pointer from integer without a cast [-Wint-conversion]
  1, 3, 2, 1, 1, 1, 3, 1,
              ^
lib/cmdline_kunit.c:18:14: note: (near initialization for ‘cmdline_test_values[12]’)
lib/cmdline_kunit.c:18:17: warning: initialization makes pointer from integer without a cast [-Wint-conversion]
  1, 3, 2, 1, 1, 1, 3, 1,
                 ^
lib/cmdline_kunit.c:18:17: note: (near initialization for ‘cmdline_test_values[13]’)
lib/cmdline_kunit.c:18:20: warning: initialization makes pointer from integer without a cast [-Wint-conversion]
  1, 3, 2, 1, 1, 1, 3, 1,
                    ^
lib/cmdline_kunit.c:18:20: note: (near initialization for ‘cmdline_test_values[14]’)
lib/cmdline_kunit.c:18:23: warning: initialization makes pointer from integer without a cast [-Wint-conversion]
  1, 3, 2, 1, 1, 1, 3, 1,
                       ^
lib/cmdline_kunit.c:18:23: note: (near initialization for ‘cmdline_test_values[15]’)
lib/cmdline_kunit.c: In function ‘cmdline_test_noint’:
lib/cmdline_kunit.c:45:29: warning: passing argument 2 of ‘cmdline_do_one_test’ discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers]
   cmdline_do_one_test(test, str, rc, offset);
                             ^~~
lib/cmdline_kunit.c:21:13: note: expected ‘char *’ but argument is of type ‘const char *’
 static void cmdline_do_one_test(struct kunit *test, char *in, int rc, int offset)
             ^~~~~~~~~~~~~~~~~~~
lib/cmdline_kunit.c: In function ‘cmdline_test_lead_int’:
lib/cmdline_kunit.c:56:12: warning: initialization makes integer from pointer without a cast [-Wint-conversion]
   int rc = cmdline_test_values[i];
            ^~~~~~~~~~~~~~~~~~~

Re: [PATCH v4 2/6] lib/linear_ranges_kunit: Follow new file name convention for KUnit tests

[Vaittinen, Matti]

Hello,

On Thu, 2020-11-12 at 20:07 +0200, Andy Shevchenko wrote:
> Follow new file name convention for the KUnit tests.
> Since we have lib/*test*.c in a few variations,
> use 'kunit' suffix to distinguish usual test cases
> with KUnit-based ones.
> 
> Cc: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Reviewed-by: David Gow <davidgow@google.com>
> Acked-by: Mark Brown <broonie@kernel.org>
> ---
> v4: added tag (Mark)
> 
>  MAINTAINERS                                         | 3 +--
>  lib/Makefile                                        | 2 +-
>  lib/{test_linear_ranges.c => linear_ranges_kunit.c} | 0
>  3 files changed, 2 insertions(+), 3 deletions(-)
>  rename lib/{test_linear_ranges.c => linear_ranges_kunit.c} (100%)

Sorry for late reaction. I need to adjust my mail filters. The KUnit
mails evaded my inbox even when I was in CC. I have own folder for
KUnit - which I do not look daily. (And no Brendan, the folder is not
called "trash", its's "kunit" and I do occasionally skim through the
mails there).

I see this was already applied so it is late - but I wanted to say
thank you for taking care of this Andy. It looks good to me.

--Matti

--
Matti Vaittinen, Linux device drivers
ROHM Semiconductors, Finland
SWDC
Kiviharjunlenkki 1E
90220 OULU
FINLAND

~~~ "I don't think so," said Rene Descartes. Just then he vanished ~~~

Simon says - in Latin please.
"non cogito me" dixit Rene Descarte, deinde evanescavit

(Thanks for the translation Simon)

Re: [PATCH v4 6/6] lib/cmdline_kunit: Add a new test suite for cmdline API

[Andy Shevchenko]

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.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v4 6/6] lib/cmdline_kunit: Add a new test suite for cmdline API

[Andy Shevchenko]

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.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v4 2/6] lib/linear_ranges_kunit: Follow new file name convention for KUnit tests

[andriy.shevchenko]

On Fri, Nov 13, 2020 at 06:22:08AM +0000, Vaittinen, Matti wrote:
> On Thu, 2020-11-12 at 20:07 +0200, Andy Shevchenko wrote:
> > Follow new file name convention for the KUnit tests.
> > Since we have lib/*test*.c in a few variations,
> > use 'kunit' suffix to distinguish usual test cases
> > with KUnit-based ones.

> Sorry for late reaction. I need to adjust my mail filters. The KUnit
> mails evaded my inbox even when I was in CC. I have own folder for
> KUnit - which I do not look daily. (And no Brendan, the folder is not
> called "trash", its's "kunit" and I do occasionally skim through the
> mails there).
> 
> I see this was already applied so it is late - but I wanted to say
> thank you for taking care of this Andy. It looks good to me.

You are welcome!  And thanks for review.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v4 6/6] lib/cmdline_kunit: Add a new test suite for cmdline API

[David Gow]

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

Re: [PATCH v4 6/6] lib/cmdline_kunit: Add a new test suite for cmdline API

[Andy Shevchenko]

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.

-- 
With Best Regards,
Andy Shevchenko