diff mbox series

[v6] lib: add basic KUnit test for lib/math

Message ID 20210416180427.1545645-1-dlatypov@google.com
State New
Headers show
Series [v6] lib: add basic KUnit test for lib/math | expand

Commit Message

Daniel Latypov April 16, 2021, 6:04 p.m. UTC
Add basic test coverage for files that don't require any config options:
* part of math.h (what seem to be the most commonly used macros)
* 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
  * looking at code coverage, we hit all the branches in the .c files

Signed-off-by: Daniel Latypov <dlatypov@google.com>
Reviewed-by: David Gow <davidgow@google.com>
---
Changes since v5:
* add in test cases for roundup/rounddown
* address misc comments from David

Changes since v4:
* add in test cases for some math.h macros (abs, round_up/round_down,
  div_round_down/closest)
* use parameterized testing less to keep things terser

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.
---
 lib/math/Kconfig      |  12 ++
 lib/math/Makefile     |   2 +
 lib/math/math_kunit.c | 291 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 305 insertions(+)
 create mode 100644 lib/math/math_kunit.c


base-commit: 7e25f40eab52c57ff6772d27d2aef3640a3237d7

Comments

David Gow April 17, 2021, 4:16 a.m. UTC | #1
On Sat, Apr 17, 2021 at 2:04 AM Daniel Latypov <dlatypov@google.com> wrote:
>

> Add basic test coverage for files that don't require any config options:

> * part of math.h (what seem to be the most commonly used macros)

> * 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

>   * looking at code coverage, we hit all the branches in the .c files

>

> Signed-off-by: Daniel Latypov <dlatypov@google.com>

> Reviewed-by: David Gow <davidgow@google.com>

> ---


Thanks: I've tested this version, and am happy with it. A part of me
still kind-of would like there to be names for the parameters, but I
definitely understand that it doesn't really work well for the lcm and
gcd cases where we're doing both (a,b) and (b,a). So let's keep it
as-is.

Hopefully we can get these in for 5.13!

Cheers,
-- David
kernel test robot April 17, 2021, 5:58 a.m. UTC | #2
Hi Daniel,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on 7e25f40eab52c57ff6772d27d2aef3640a3237d7]

url:    https://github.com/0day-ci/linux/commits/Daniel-Latypov/lib-add-basic-KUnit-test-for-lib-math/20210417-020619
base:   7e25f40eab52c57ff6772d27d2aef3640a3237d7
config: powerpc-randconfig-c004-20210416 (attached as .config)
compiler: powerpc-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/0f1888ffeaa6baa1bc2a99eac8ba7d1df29c8450
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Daniel-Latypov/lib-add-basic-KUnit-test-for-lib-math/20210417-020619
        git checkout 0f1888ffeaa6baa1bc2a99eac8ba7d1df29c8450
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross W=1 ARCH=powerpc 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

   lib/math/math_kunit.c: In function 'abs_test':
>> lib/math/math_kunit.c:41:1: warning: the frame size of 1088 bytes is larger than 1024 bytes [-Wframe-larger-than=]

      41 | }
         | ^


vim +41 lib/math/math_kunit.c

    14	
    15	static void abs_test(struct kunit *test)
    16	{
    17		KUNIT_EXPECT_EQ(test, abs((char)0), (char)0);
    18		KUNIT_EXPECT_EQ(test, abs((char)42), (char)42);
    19		KUNIT_EXPECT_EQ(test, abs((char)-42), (char)42);
    20	
    21		/* The expression in the macro is actually promoted to an int. */
    22		KUNIT_EXPECT_EQ(test, abs((short)0),  0);
    23		KUNIT_EXPECT_EQ(test, abs((short)42),  42);
    24		KUNIT_EXPECT_EQ(test, abs((short)-42),  42);
    25	
    26		KUNIT_EXPECT_EQ(test, abs(0),  0);
    27		KUNIT_EXPECT_EQ(test, abs(42),  42);
    28		KUNIT_EXPECT_EQ(test, abs(-42),  42);
    29	
    30		KUNIT_EXPECT_EQ(test, abs(0L), 0L);
    31		KUNIT_EXPECT_EQ(test, abs(42L), 42L);
    32		KUNIT_EXPECT_EQ(test, abs(-42L), 42L);
    33	
    34		KUNIT_EXPECT_EQ(test, abs(0LL), 0LL);
    35		KUNIT_EXPECT_EQ(test, abs(42LL), 42LL);
    36		KUNIT_EXPECT_EQ(test, abs(-42LL), 42LL);
    37	
    38		/* Unsigned types get casted to signed. */
    39		KUNIT_EXPECT_EQ(test, abs(0ULL), 0LL);
    40		KUNIT_EXPECT_EQ(test, abs(42ULL), 42LL);
  > 41	}

    42	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
Devarsh Thakkar May 16, 2024, 10:19 a.m. UTC | #3
Hi Daniel, Andy,

On 16/04/21 23:34, Daniel Latypov wrote:
> Add basic test coverage for files that don't require any config options:
> * part of math.h (what seem to be the most commonly used macros)
> * 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
>   * looking at code coverage, we hit all the branches in the .c files
> 
> Signed-off-by: Daniel Latypov <dlatypov@google.com>
> Reviewed-by: David Gow <davidgow@google.com>

Just checking if something else was pending on this patch-set for this not
getting merged?

I needed this patch-set for adding tests for new macros I am adding in math.h
as suggested in this thread [1], so wanted to pull this in my series and add
changes on top of that for new macros.

Kindly let me know your thoughts on this.

[1]: https://lore.kernel.org/all/ZkIG0-01pz632l4R@smile.fi.intel.com/#t

Regards
Devarsh
> ---
> Changes since v5:
> * add in test cases for roundup/rounddown
> * address misc comments from David
> 
> Changes since v4:
> * add in test cases for some math.h macros (abs, round_up/round_down,
>   div_round_down/closest)
> * use parameterized testing less to keep things terser
> 
> 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.
> ---
>  lib/math/Kconfig      |  12 ++
>  lib/math/Makefile     |   2 +
>  lib/math/math_kunit.c | 291 ++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 305 insertions(+)
>  create mode 100644 lib/math/math_kunit.c
> 
> diff --git a/lib/math/Kconfig b/lib/math/Kconfig
> index f19bc9734fa7..a974d4db0f9c 100644
> --- a/lib/math/Kconfig
> +++ b/lib/math/Kconfig
> @@ -15,3 +15,15 @@ config PRIME_NUMBERS
>  
>  config RATIONAL
>  	bool
> +
> +config MATH_KUNIT_TEST
> +	tristate "KUnit test for lib/math and math.h" if !KUNIT_ALL_TESTS
> +	depends on KUNIT
> +	default KUNIT_ALL_TESTS
> +	help
> +		This builds unit tests for lib/math and math.h.
> +
> +		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.
> 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..556c23b17c3c
> --- /dev/null
> +++ b/lib/math/math_kunit.c
> @@ -0,0 +1,291 @@
> +// 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>
> +
> +static void abs_test(struct kunit *test)
> +{
> +	KUNIT_EXPECT_EQ(test, abs((char)0), (char)0);
> +	KUNIT_EXPECT_EQ(test, abs((char)42), (char)42);
> +	KUNIT_EXPECT_EQ(test, abs((char)-42), (char)42);
> +
> +	/* The expression in the macro is actually promoted to an int. */
> +	KUNIT_EXPECT_EQ(test, abs((short)0),  0);
> +	KUNIT_EXPECT_EQ(test, abs((short)42),  42);
> +	KUNIT_EXPECT_EQ(test, abs((short)-42),  42);
> +
> +	KUNIT_EXPECT_EQ(test, abs(0),  0);
> +	KUNIT_EXPECT_EQ(test, abs(42),  42);
> +	KUNIT_EXPECT_EQ(test, abs(-42),  42);
> +
> +	KUNIT_EXPECT_EQ(test, abs(0L), 0L);
> +	KUNIT_EXPECT_EQ(test, abs(42L), 42L);
> +	KUNIT_EXPECT_EQ(test, abs(-42L), 42L);
> +
> +	KUNIT_EXPECT_EQ(test, abs(0LL), 0LL);
> +	KUNIT_EXPECT_EQ(test, abs(42LL), 42LL);
> +	KUNIT_EXPECT_EQ(test, abs(-42LL), 42LL);
> +
> +	/* Unsigned types get casted to signed. */
> +	KUNIT_EXPECT_EQ(test, abs(0ULL), 0LL);
> +	KUNIT_EXPECT_EQ(test, abs(42ULL), 42LL);
> +}
> +
> +static void int_sqrt_test(struct kunit *test)
> +{
> +	KUNIT_EXPECT_EQ(test, int_sqrt(0UL), 0UL);
> +	KUNIT_EXPECT_EQ(test, int_sqrt(1UL), 1UL);
> +	KUNIT_EXPECT_EQ(test, int_sqrt(4UL), 2UL);
> +	KUNIT_EXPECT_EQ(test, int_sqrt(5UL), 2UL);
> +	KUNIT_EXPECT_EQ(test, int_sqrt(8UL), 2UL);
> +	KUNIT_EXPECT_EQ(test, int_sqrt(1UL << 30), 1UL << 15);
> +}
> +
> +static void round_up_test(struct kunit *test)
> +{
> +	KUNIT_EXPECT_EQ(test, round_up(0, 1), 0);
> +	KUNIT_EXPECT_EQ(test, round_up(1, 2), 2);
> +	KUNIT_EXPECT_EQ(test, round_up(3, 2), 4);
> +	KUNIT_EXPECT_EQ(test, round_up((1 << 30) - 1, 2), 1 << 30);
> +	KUNIT_EXPECT_EQ(test, round_up((1 << 30) - 1, 1 << 29), 1 << 30);
> +}
> +
> +static void round_down_test(struct kunit *test)
> +{
> +	KUNIT_EXPECT_EQ(test, round_down(0, 1), 0);
> +	KUNIT_EXPECT_EQ(test, round_down(1, 2), 0);
> +	KUNIT_EXPECT_EQ(test, round_down(3, 2), 2);
> +	KUNIT_EXPECT_EQ(test, round_down((1 << 30) - 1, 2), (1 << 30) - 2);
> +	KUNIT_EXPECT_EQ(test, round_down((1 << 30) - 1, 1 << 29), 1 << 29);
> +}
> +
> +/* These versions can round to numbers that aren't a power of two */
> +static void roundup_test(struct kunit *test)
> +{
> +	KUNIT_EXPECT_EQ(test, roundup(0, 1), 0);
> +	KUNIT_EXPECT_EQ(test, roundup(1, 2), 2);
> +	KUNIT_EXPECT_EQ(test, roundup(3, 2), 4);
> +	KUNIT_EXPECT_EQ(test, roundup((1 << 30) - 1, 2), 1 << 30);
> +	KUNIT_EXPECT_EQ(test, roundup((1 << 30) - 1, 1 << 29), 1 << 30);
> +
> +	KUNIT_EXPECT_EQ(test, roundup(3, 2), 4);
> +	KUNIT_EXPECT_EQ(test, roundup(4, 3), 6);
> +}
> +
> +static void rounddown_test(struct kunit *test)
> +{
> +	KUNIT_EXPECT_EQ(test, rounddown(0, 1), 0);
> +	KUNIT_EXPECT_EQ(test, rounddown(1, 2), 0);
> +	KUNIT_EXPECT_EQ(test, rounddown(3, 2), 2);
> +	KUNIT_EXPECT_EQ(test, rounddown((1 << 30) - 1, 2), (1 << 30) - 2);
> +	KUNIT_EXPECT_EQ(test, rounddown((1 << 30) - 1, 1 << 29), 1 << 29);
> +
> +	KUNIT_EXPECT_EQ(test, rounddown(3, 2), 2);
> +	KUNIT_EXPECT_EQ(test, rounddown(4, 3), 3);
> +}
> +
> +static void div_round_up_test(struct kunit *test)
> +{
> +	KUNIT_EXPECT_EQ(test, DIV_ROUND_UP(0, 1), 0);
> +	KUNIT_EXPECT_EQ(test, DIV_ROUND_UP(20, 10), 2);
> +	KUNIT_EXPECT_EQ(test, DIV_ROUND_UP(21, 10), 3);
> +	KUNIT_EXPECT_EQ(test, DIV_ROUND_UP(21, 20), 2);
> +	KUNIT_EXPECT_EQ(test, DIV_ROUND_UP(21, 99), 1);
> +}
> +
> +static void div_round_closest_test(struct kunit *test)
> +{
> +	KUNIT_EXPECT_EQ(test, DIV_ROUND_CLOSEST(0, 1), 0);
> +	KUNIT_EXPECT_EQ(test, DIV_ROUND_CLOSEST(20, 10), 2);
> +	KUNIT_EXPECT_EQ(test, DIV_ROUND_CLOSEST(21, 10), 2);
> +	KUNIT_EXPECT_EQ(test, DIV_ROUND_CLOSEST(25, 10), 3);
> +}
> +
> +/* 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);
> +}
> +
> +struct u32_test_case {
> +	u32 a, b;
> +	u32 result;
> +};
> +
> +static struct u32_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 u32_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 void reciprocal_scale_test(struct kunit *test)
> +{
> +	KUNIT_EXPECT_EQ(test, reciprocal_scale(0u, 100), 0u);
> +	KUNIT_EXPECT_EQ(test, reciprocal_scale(1u, 100), 0u);
> +	KUNIT_EXPECT_EQ(test, reciprocal_scale(1u << 4, 1 << 28), 1u);
> +	KUNIT_EXPECT_EQ(test, reciprocal_scale(1u << 16, 1 << 28), 1u << 12);
> +	KUNIT_EXPECT_EQ(test, reciprocal_scale(~0u, 1 << 28), (1u << 28) - 1);
> +}
> +
> +static struct kunit_case math_test_cases[] = {
> +	KUNIT_CASE(abs_test),
> +	KUNIT_CASE(int_sqrt_test),
> +	KUNIT_CASE(round_up_test),
> +	KUNIT_CASE(round_down_test),
> +	KUNIT_CASE(roundup_test),
> +	KUNIT_CASE(rounddown_test),
> +	KUNIT_CASE(div_round_up_test),
> +	KUNIT_CASE(div_round_closest_test),
> +	KUNIT_CASE_PARAM(gcd_test, gcd_gen_params),
> +	KUNIT_CASE_PARAM(lcm_test, lcm_gen_params),
> +	KUNIT_CASE_PARAM(reciprocal_div_test, reciprocal_div_gen_params),
> +	KUNIT_CASE(reciprocal_scale_test),
> +	{}
> +};
> +
> +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: 7e25f40eab52c57ff6772d27d2aef3640a3237d7
Andy Shevchenko May 16, 2024, 10:26 a.m. UTC | #4
On Thu, May 16, 2024 at 03:49:44PM +0530, Devarsh Thakkar wrote:
> Hi Daniel, Andy,
> 
> On 16/04/21 23:34, Daniel Latypov wrote:
> > Add basic test coverage for files that don't require any config options:
> > * part of math.h (what seem to be the most commonly used macros)
> > * 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
> >   * looking at code coverage, we hit all the branches in the .c files
> > 
> > Signed-off-by: Daniel Latypov <dlatypov@google.com>
> > Reviewed-by: David Gow <davidgow@google.com>
> 
> Just checking if something else was pending on this patch-set for this not
> getting merged?
> 
> I needed this patch-set for adding tests for new macros I am adding in math.h
> as suggested in this thread [1], so wanted to pull this in my series and add
> changes on top of that for new macros.
> 
> Kindly let me know your thoughts on this.

Wow, blast from the past!
But good finding, it would be good to have more math.h related test cases.
Daniel Latypov May 16, 2024, 3:16 p.m. UTC | #5
On Thu, May 16, 2024 at 3:19 AM Devarsh Thakkar <devarsht@ti.com> wrote:
>
> Hi Daniel, Andy,
>
> On 16/04/21 23:34, Daniel Latypov wrote:
> > Add basic test coverage for files that don't require any config options:
> > * part of math.h (what seem to be the most commonly used macros)
> > * 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
> >   * looking at code coverage, we hit all the branches in the .c files
> >
> > Signed-off-by: Daniel Latypov <dlatypov@google.com>
> > Reviewed-by: David Gow <davidgow@google.com>
>
> Just checking if something else was pending on this patch-set for this not
> getting merged?
>
> I needed this patch-set for adding tests for new macros I am adding in math.h
> as suggested in this thread [1], so wanted to pull this in my series and add
> changes on top of that for new macros.
>
> Kindly let me know your thoughts on this.

This patch just fell through the cracks for me.
I had (wrongly) inferred that Andy might have had some lingering
reservations about this patch (that it was too contrived, might not be
useful to have tests for stuff like abs(), etc.).

Feel free to pull this into your series.

Looking over the code itself, I think this still looks valid and
stylistically correct with regard to KUnit.
I haven't gone and validated that it still compiles and runs just yet, though.
But if you do run into any problems, let me know and I can help send
you a fixed version.

Thanks for picking this up,
Daniel
diff mbox series

Patch

diff --git a/lib/math/Kconfig b/lib/math/Kconfig
index f19bc9734fa7..a974d4db0f9c 100644
--- a/lib/math/Kconfig
+++ b/lib/math/Kconfig
@@ -15,3 +15,15 @@  config PRIME_NUMBERS
 
 config RATIONAL
 	bool
+
+config MATH_KUNIT_TEST
+	tristate "KUnit test for lib/math and math.h" if !KUNIT_ALL_TESTS
+	depends on KUNIT
+	default KUNIT_ALL_TESTS
+	help
+		This builds unit tests for lib/math and math.h.
+
+		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.
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..556c23b17c3c
--- /dev/null
+++ b/lib/math/math_kunit.c
@@ -0,0 +1,291 @@ 
+// 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>
+
+static void abs_test(struct kunit *test)
+{
+	KUNIT_EXPECT_EQ(test, abs((char)0), (char)0);
+	KUNIT_EXPECT_EQ(test, abs((char)42), (char)42);
+	KUNIT_EXPECT_EQ(test, abs((char)-42), (char)42);
+
+	/* The expression in the macro is actually promoted to an int. */
+	KUNIT_EXPECT_EQ(test, abs((short)0),  0);
+	KUNIT_EXPECT_EQ(test, abs((short)42),  42);
+	KUNIT_EXPECT_EQ(test, abs((short)-42),  42);
+
+	KUNIT_EXPECT_EQ(test, abs(0),  0);
+	KUNIT_EXPECT_EQ(test, abs(42),  42);
+	KUNIT_EXPECT_EQ(test, abs(-42),  42);
+
+	KUNIT_EXPECT_EQ(test, abs(0L), 0L);
+	KUNIT_EXPECT_EQ(test, abs(42L), 42L);
+	KUNIT_EXPECT_EQ(test, abs(-42L), 42L);
+
+	KUNIT_EXPECT_EQ(test, abs(0LL), 0LL);
+	KUNIT_EXPECT_EQ(test, abs(42LL), 42LL);
+	KUNIT_EXPECT_EQ(test, abs(-42LL), 42LL);
+
+	/* Unsigned types get casted to signed. */
+	KUNIT_EXPECT_EQ(test, abs(0ULL), 0LL);
+	KUNIT_EXPECT_EQ(test, abs(42ULL), 42LL);
+}
+
+static void int_sqrt_test(struct kunit *test)
+{
+	KUNIT_EXPECT_EQ(test, int_sqrt(0UL), 0UL);
+	KUNIT_EXPECT_EQ(test, int_sqrt(1UL), 1UL);
+	KUNIT_EXPECT_EQ(test, int_sqrt(4UL), 2UL);
+	KUNIT_EXPECT_EQ(test, int_sqrt(5UL), 2UL);
+	KUNIT_EXPECT_EQ(test, int_sqrt(8UL), 2UL);
+	KUNIT_EXPECT_EQ(test, int_sqrt(1UL << 30), 1UL << 15);
+}
+
+static void round_up_test(struct kunit *test)
+{
+	KUNIT_EXPECT_EQ(test, round_up(0, 1), 0);
+	KUNIT_EXPECT_EQ(test, round_up(1, 2), 2);
+	KUNIT_EXPECT_EQ(test, round_up(3, 2), 4);
+	KUNIT_EXPECT_EQ(test, round_up((1 << 30) - 1, 2), 1 << 30);
+	KUNIT_EXPECT_EQ(test, round_up((1 << 30) - 1, 1 << 29), 1 << 30);
+}
+
+static void round_down_test(struct kunit *test)
+{
+	KUNIT_EXPECT_EQ(test, round_down(0, 1), 0);
+	KUNIT_EXPECT_EQ(test, round_down(1, 2), 0);
+	KUNIT_EXPECT_EQ(test, round_down(3, 2), 2);
+	KUNIT_EXPECT_EQ(test, round_down((1 << 30) - 1, 2), (1 << 30) - 2);
+	KUNIT_EXPECT_EQ(test, round_down((1 << 30) - 1, 1 << 29), 1 << 29);
+}
+
+/* These versions can round to numbers that aren't a power of two */
+static void roundup_test(struct kunit *test)
+{
+	KUNIT_EXPECT_EQ(test, roundup(0, 1), 0);
+	KUNIT_EXPECT_EQ(test, roundup(1, 2), 2);
+	KUNIT_EXPECT_EQ(test, roundup(3, 2), 4);
+	KUNIT_EXPECT_EQ(test, roundup((1 << 30) - 1, 2), 1 << 30);
+	KUNIT_EXPECT_EQ(test, roundup((1 << 30) - 1, 1 << 29), 1 << 30);
+
+	KUNIT_EXPECT_EQ(test, roundup(3, 2), 4);
+	KUNIT_EXPECT_EQ(test, roundup(4, 3), 6);
+}
+
+static void rounddown_test(struct kunit *test)
+{
+	KUNIT_EXPECT_EQ(test, rounddown(0, 1), 0);
+	KUNIT_EXPECT_EQ(test, rounddown(1, 2), 0);
+	KUNIT_EXPECT_EQ(test, rounddown(3, 2), 2);
+	KUNIT_EXPECT_EQ(test, rounddown((1 << 30) - 1, 2), (1 << 30) - 2);
+	KUNIT_EXPECT_EQ(test, rounddown((1 << 30) - 1, 1 << 29), 1 << 29);
+
+	KUNIT_EXPECT_EQ(test, rounddown(3, 2), 2);
+	KUNIT_EXPECT_EQ(test, rounddown(4, 3), 3);
+}
+
+static void div_round_up_test(struct kunit *test)
+{
+	KUNIT_EXPECT_EQ(test, DIV_ROUND_UP(0, 1), 0);
+	KUNIT_EXPECT_EQ(test, DIV_ROUND_UP(20, 10), 2);
+	KUNIT_EXPECT_EQ(test, DIV_ROUND_UP(21, 10), 3);
+	KUNIT_EXPECT_EQ(test, DIV_ROUND_UP(21, 20), 2);
+	KUNIT_EXPECT_EQ(test, DIV_ROUND_UP(21, 99), 1);
+}
+
+static void div_round_closest_test(struct kunit *test)
+{
+	KUNIT_EXPECT_EQ(test, DIV_ROUND_CLOSEST(0, 1), 0);
+	KUNIT_EXPECT_EQ(test, DIV_ROUND_CLOSEST(20, 10), 2);
+	KUNIT_EXPECT_EQ(test, DIV_ROUND_CLOSEST(21, 10), 2);
+	KUNIT_EXPECT_EQ(test, DIV_ROUND_CLOSEST(25, 10), 3);
+}
+
+/* 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);
+}
+
+struct u32_test_case {
+	u32 a, b;
+	u32 result;
+};
+
+static struct u32_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 u32_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 void reciprocal_scale_test(struct kunit *test)
+{
+	KUNIT_EXPECT_EQ(test, reciprocal_scale(0u, 100), 0u);
+	KUNIT_EXPECT_EQ(test, reciprocal_scale(1u, 100), 0u);
+	KUNIT_EXPECT_EQ(test, reciprocal_scale(1u << 4, 1 << 28), 1u);
+	KUNIT_EXPECT_EQ(test, reciprocal_scale(1u << 16, 1 << 28), 1u << 12);
+	KUNIT_EXPECT_EQ(test, reciprocal_scale(~0u, 1 << 28), (1u << 28) - 1);
+}
+
+static struct kunit_case math_test_cases[] = {
+	KUNIT_CASE(abs_test),
+	KUNIT_CASE(int_sqrt_test),
+	KUNIT_CASE(round_up_test),
+	KUNIT_CASE(round_down_test),
+	KUNIT_CASE(roundup_test),
+	KUNIT_CASE(rounddown_test),
+	KUNIT_CASE(div_round_up_test),
+	KUNIT_CASE(div_round_closest_test),
+	KUNIT_CASE_PARAM(gcd_test, gcd_gen_params),
+	KUNIT_CASE_PARAM(lcm_test, lcm_gen_params),
+	KUNIT_CASE_PARAM(reciprocal_div_test, reciprocal_div_gen_params),
+	KUNIT_CASE(reciprocal_scale_test),
+	{}
+};
+
+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");