linux-kselftest.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v4] lib: add basic KUnit test for lib/math
@ 2021-04-09  1:40 Daniel Latypov
  2021-04-09 15:30 ` Andy Shevchenko
  0 siblings, 1 reply; 3+ messages in thread
From: Daniel Latypov @ 2021-04-09  1:40 UTC (permalink / raw)
  To: andriy.shevchenko
  Cc: brendanhiggins, davidgow, linux-kernel, kunit-dev,
	linux-kselftest, skhan, Daniel Latypov

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.)

These tests aren't particularly interesting, but they
* provide short and simple examples of parameterized tests
* provide a place to add tests for any new files in this dir
* are written so adding new test cases to cover edge cases should be easy

Signed-off-by: Daniel Latypov <dlatypov@google.com>
---
Changes since v3: 
* fix `checkpatch.pl --strict` warnings
* add test cases for gcd(0,0) and lcm(0,0)
* minor: don't test both gcd(a,b) and gcd(b,a) when a == b

Changes since v2: mv math_test.c => math_kunit.c

Changes since v1:
* Rebase and rewrite to use the new parameterized testing support.
* misc: fix overflow in literal and inline int_sqrt format string.
* related: commit 1f0e943df68a ("Documentation: kunit: provide guidance
for testing many inputs") was merged explaining the patterns shown here.
  * there's an in-flight patch to update it for parameterized testing.

v1: https://lore.kernel.org/lkml/20201019224556.3536790-1-dlatypov@google.com/
---
 lib/math/Kconfig      |   5 +
 lib/math/Makefile     |   2 +
 lib/math/math_kunit.c | 214 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 221 insertions(+)
 create mode 100644 lib/math/math_kunit.c

diff --git a/lib/math/Kconfig b/lib/math/Kconfig
index f19bc9734fa7..6ba8680439c1 100644
--- a/lib/math/Kconfig
+++ b/lib/math/Kconfig
@@ -15,3 +15,8 @@ config PRIME_NUMBERS
 
 config RATIONAL
 	bool
+
+config MATH_KUNIT_TEST
+	tristate "KUnit test for lib/math" if !KUNIT_ALL_TESTS
+	default KUNIT_ALL_TESTS
+	depends on KUNIT
diff --git a/lib/math/Makefile b/lib/math/Makefile
index be6909e943bd..30abb7a8d564 100644
--- a/lib/math/Makefile
+++ b/lib/math/Makefile
@@ -4,3 +4,5 @@ obj-y += div64.o gcd.o lcm.o int_pow.o int_sqrt.o reciprocal_div.o
 obj-$(CONFIG_CORDIC)		+= cordic.o
 obj-$(CONFIG_PRIME_NUMBERS)	+= prime_numbers.o
 obj-$(CONFIG_RATIONAL)		+= rational.o
+
+obj-$(CONFIG_MATH_KUNIT_TEST)	+= math_kunit.o
diff --git a/lib/math/math_kunit.c b/lib/math/math_kunit.c
new file mode 100644
index 000000000000..fed15ade8fb2
--- /dev/null
+++ b/lib/math/math_kunit.c
@@ -0,0 +1,214 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Simple KUnit suite for math helper funcs that are always enabled.
+ *
+ * Copyright (C) 2020, Google LLC.
+ * Author: Daniel Latypov <dlatypov@google.com>
+ */
+
+#include <kunit/test.h>
+#include <linux/gcd.h>
+#include <linux/kernel.h>
+#include <linux/lcm.h>
+#include <linux/reciprocal_div.h>
+
+/* Generic test case for unsigned long inputs. */
+struct test_case {
+	unsigned long a, b;
+	unsigned long result;
+};
+
+static struct test_case gcd_cases[] = {
+	{
+		.a = 0, .b = 0,
+		.result = 0,
+	},
+	{
+		.a = 0, .b = 1,
+		.result = 1,
+	},
+	{
+		.a = 2, .b = 2,
+		.result = 2,
+	},
+	{
+		.a = 2, .b = 4,
+		.result = 2,
+	},
+	{
+		.a = 3, .b = 5,
+		.result = 1,
+	},
+	{
+		.a = 3 * 9, .b = 3 * 5,
+		.result = 3,
+	},
+	{
+		.a = 3 * 5 * 7, .b = 3 * 5 * 11,
+		.result = 15,
+	},
+	{
+		.a = 1 << 21,
+		.b = (1 << 21) - 1,
+		.result = 1,
+	},
+};
+
+KUNIT_ARRAY_PARAM(gcd, gcd_cases, NULL);
+
+static void gcd_test(struct kunit *test)
+{
+	const char *message_fmt = "gcd(%lu, %lu)";
+	const struct test_case *test_param = test->param_value;
+
+	KUNIT_EXPECT_EQ_MSG(test, test_param->result,
+			    gcd(test_param->a, test_param->b),
+			    message_fmt, test_param->a,
+			    test_param->b);
+
+	if (test_param->a == test_param->b)
+		return;
+
+	/* gcd(a,b) == gcd(b,a) */
+	KUNIT_EXPECT_EQ_MSG(test, test_param->result,
+			    gcd(test_param->b, test_param->a),
+			    message_fmt, test_param->b,
+			    test_param->a);
+}
+
+static struct test_case lcm_cases[] = {
+	{
+		.a = 0, .b = 0,
+		.result = 0,
+	},
+	{
+		.a = 0, .b = 1,
+		.result = 0,
+	},
+	{
+		.a = 1, .b = 2,
+		.result = 2,
+	},
+	{
+		.a = 2, .b = 2,
+		.result = 2,
+	},
+	{
+		.a = 3 * 5, .b = 3 * 7,
+		.result = 3 * 5 * 7,
+	},
+};
+
+KUNIT_ARRAY_PARAM(lcm, lcm_cases, NULL);
+
+static void lcm_test(struct kunit *test)
+{
+	const char *message_fmt = "lcm(%lu, %lu)";
+	const struct test_case *test_param = test->param_value;
+
+	KUNIT_EXPECT_EQ_MSG(test, test_param->result,
+			    lcm(test_param->a, test_param->b),
+			    message_fmt, test_param->a,
+			    test_param->b);
+
+	if (test_param->a == test_param->b)
+		return;
+
+	/* lcm(a,b) == lcm(b,a) */
+	KUNIT_EXPECT_EQ_MSG(test, test_param->result,
+			    lcm(test_param->b, test_param->a),
+			    message_fmt, test_param->b,
+			    test_param->a);
+}
+
+static struct test_case int_sqrt_cases[] = {
+	{
+		.a = 0,
+		.result = 0,
+	},
+	{
+		.a = 1,
+		.result = 1,
+	},
+	{
+		.a = 4,
+		.result = 2,
+	},
+	{
+		.a = 5,
+		.result = 2,
+	},
+	{
+		.a = 8,
+		.result = 2,
+	},
+	{
+		.a = 1UL << 30,
+		.result = 1UL << 15,
+	},
+};
+
+KUNIT_ARRAY_PARAM(int_sqrt, int_sqrt_cases, NULL);
+
+static void int_sqrt_test(struct kunit *test)
+{
+	const struct test_case *test_param = test->param_value;
+
+	KUNIT_EXPECT_EQ_MSG(test, int_sqrt(test_param->a),
+			    test_param->result, "sqrt(%lu)",
+			    test_param->a);
+}
+
+struct reciprocal_test_case {
+	u32 a, b;
+	u32 result;
+};
+
+static struct reciprocal_test_case reciprocal_div_cases[] = {
+	{
+		.a = 0, .b = 1,
+		.result = 0,
+	},
+	{
+		.a = 42, .b = 20,
+		.result = 2,
+	},
+	{
+		.a = 42, .b = 9999,
+		.result = 0,
+	},
+	{
+		.a = (1 << 16), .b = (1 << 14),
+		.result = 1 << 2,
+	},
+};
+
+KUNIT_ARRAY_PARAM(reciprocal_div, reciprocal_div_cases, NULL);
+
+static void reciprocal_div_test(struct kunit *test)
+{
+	const struct reciprocal_test_case *test_param = test->param_value;
+	struct reciprocal_value rv = reciprocal_value(test_param->b);
+
+	KUNIT_EXPECT_EQ_MSG(test, test_param->result,
+			    reciprocal_divide(test_param->a, rv),
+			    "reciprocal_divide(%u, %u)",
+			    test_param->a, test_param->b);
+}
+
+static struct kunit_case math_test_cases[] = {
+	KUNIT_CASE_PARAM(gcd_test, gcd_gen_params),
+	KUNIT_CASE_PARAM(lcm_test, lcm_gen_params),
+	KUNIT_CASE_PARAM(int_sqrt_test, int_sqrt_gen_params),
+	KUNIT_CASE_PARAM(reciprocal_div_test, reciprocal_div_gen_params),
+	{}
+};
+
+static struct kunit_suite math_test_suite = {
+	.name = "lib-math",
+	.test_cases = math_test_cases,
+};
+
+kunit_test_suites(&math_test_suite);
+
+MODULE_LICENSE("GPL v2");

base-commit: 4fa56ad0d12e24df768c98bffe9039f915d1bc02
-- 
2.31.1.295.g9ea45b61b8-goog


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH v4] lib: add basic KUnit test for lib/math
  2021-04-09  1:40 [PATCH v4] lib: add basic KUnit test for lib/math Daniel Latypov
@ 2021-04-09 15:30 ` Andy Shevchenko
  2021-04-09 16:29   ` Daniel Latypov
  0 siblings, 1 reply; 3+ messages in thread
From: Andy Shevchenko @ 2021-04-09 15:30 UTC (permalink / raw)
  To: Daniel Latypov
  Cc: brendanhiggins, davidgow, linux-kernel, kunit-dev,
	linux-kselftest, skhan

On Thu, Apr 08, 2021 at 06:40:01PM -0700, 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.)

What about adding math.h test cases?

We have some macros there and it might be a good idea to test them, for example
that round_up() and roundup() produces the same output for the same (power of
two divisor) input.

> These tests aren't particularly interesting, but they
> * provide short and simple examples of parameterized tests
> * provide a place to add tests for any new files in this dir
> * are written so adding new test cases to cover edge cases should be easy

Yes, that's why I think macros also can be a good example how to test *macro*.

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH v4] lib: add basic KUnit test for lib/math
  2021-04-09 15:30 ` Andy Shevchenko
@ 2021-04-09 16:29   ` Daniel Latypov
  0 siblings, 0 replies; 3+ messages in thread
From: Daniel Latypov @ 2021-04-09 16:29 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Brendan Higgins, David Gow, Linux Kernel Mailing List,
	KUnit Development, open list:KERNEL SELFTEST FRAMEWORK,
	Shuah Khan

On Fri, Apr 9, 2021 at 8:30 AM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> On Thu, Apr 08, 2021 at 06:40:01PM -0700, 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.)
>
> What about adding math.h test cases?
>
> We have some macros there and it might be a good idea to test them, for example
> that round_up() and roundup() produces the same output for the same (power of
> two divisor) input.

I completely overlooked the macros in math.h, sounds like a good idea.

Grepping around, seems like abs() and DIV_ROUND_UP/CLOSEST() are among
the more popular macros:
$ ag -s '\bDIV_ROUND_UP\(' | wc -l
2946
$ ag -s '\babs\(' | wc -l
923
$ ag -s '\bDIV_ROUND_CLOSEST\(' | wc -l
864
$ ag -s '\bround_up\(' | wc -l
727
$ ag -s '\broundup\(' | wc -l
620
$ ag -s '\bround_down\(' | wc -l
371
$ ag -s 'rounddown\(' | wc -l
131


>
> > These tests aren't particularly interesting, but they
> > * provide short and simple examples of parameterized tests
> > * provide a place to add tests for any new files in this dir
> > * are written so adding new test cases to cover edge cases should be easy
>
> Yes, that's why I think macros also can be a good example how to test *macro*.

Yeah, there's more to cover there since they have a range of types
they can work on.

On another note, the parameterized test arrays all use unsigned long,
so abs() sticks out even more.
I'm thinking of something like

static void test_abs(struct kunit *test)
{
  KUNIT_EXPECT_EQ(test, abs('a'), 'a');
  KUNIT_EXPECT_EQ(test, abs(-'a'), 'a');
  ...
}

and then maybe use parameters for the other macros but also throw in
an additional test case like

static void test_div_round_up_diff_types(struct kunit *test)
{
  KUNIT_EXPECT_EQ(test, DIV_ROUND_UP((char) 42, (char) 10), (char) 4);
  KUNIT_EXPECT_EQ(test, DIV_ROUND_UP((int) 42, (int) 10), (int) 4);
  KUNIT_EXPECT_EQ(test, DIV_ROUND_UP((long) 42, (long) 10), (long) 4);
   ...
}

>
> --
> With Best Regards,
> Andy Shevchenko
>
>
> --
> You received this message because you are subscribed to the Google Groups "KUnit Development" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to kunit-dev+unsubscribe@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/kunit-dev/YHBzA7SwH194ywRv%40smile.fi.intel.com.

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2021-04-09 16:29 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-09  1:40 [PATCH v4] lib: add basic KUnit test for lib/math Daniel Latypov
2021-04-09 15:30 ` Andy Shevchenko
2021-04-09 16:29   ` Daniel Latypov

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).