diff mbox series

Input: Add KUnit tests for some of the input core helper functions

Message ID 20230329092332.2143623-1-javierm@redhat.com
State Accepted
Commit fdefcbdd6f3618410a0afb2ac0071c04036f9602
Headers show
Series Input: Add KUnit tests for some of the input core helper functions | expand

Commit Message

Javier Martinez Canillas March 29, 2023, 9:23 a.m. UTC 
The input subsystem doesn't currently have any unit tests, let's add a
CONFIG_INPUT_KUNIT_TEST option that builds a test suite to be executed
with the KUnit test infrastructure.

For now, only three tests were added for some of the input core helper
functions that are trivial to test:

  * input_test_polling: set/get poll interval and set-up a poll handler.

  * input_test_timestamp: set/get input event timestamps.

  * input_test_match_device_id: match a device by bus, vendor, product
                                and events that is capable of handling.

But having the minimal KUnit support allows to add more tests and suites
as follow-up changes. The tests can be run with the following command:

  $ ./tools/testing/kunit/kunit.py run \
    --kunitconfig=drivers/input/tests/.kunitconfig

Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
---

 drivers/input/Kconfig            |  12 +++
 drivers/input/Makefile           |   1 +
 drivers/input/tests/Makefile     |   3 +
 drivers/input/tests/input_test.c | 144 +++++++++++++++++++++++++++++++
 4 files changed, 160 insertions(+)
 create mode 100644 drivers/input/tests/Makefile
 create mode 100644 drivers/input/tests/input_test.c


base-commit: 3a93e40326c8f470e71d20b4c42d36767450f38f

Comments

Javier Martinez Canillas March 30, 2023, 8:09 a.m. UTC | #1 
Daniel Latypov <dlatypov@google.com> writes:

Hello Daniel,

Thanks a lot for your feedback!

> On Wed, Mar 29, 2023 at 2:23 AM Javier Martinez Canillas
> <javierm@redhat.com> wrote:

[...]

>>
>>   $ ./tools/testing/kunit/kunit.py run \
>>     --kunitconfig=drivers/input/tests/.kunitconfig
>
> Nice!
> A few small suggestions below as someone who has worked on KUnit.
>
> FYI, to save a few keystrokes, you can omit the "/.kunitconfig" and
> just pass the dir, i.e.
>   --kunitconfig=drivers/input/tests
>

Ah, cool. I didn't know that.

[...]

>>  drivers/input/tests/input_test.c | 144 +++++++++++++++++++++++++++++++
>
> I don't see the .kunitconfig in the diff.
> Was it accidentally forgotten or does this patch apply to a tree that
> already has the file?
>
> (it's easy to forget since git will still ignore it by default, IIRC)
>

I did indeed forgot because as you mentioned git add complained and I
missed that needed to force to add it.

[...]

>> +         Say Y here if you want to build the KUnit tests for the input
>> +         subsystem. For more information about KUnit and unit tests in
>> +         general, please refer to the KUnit documentation in
>> +         Documentation/dev-tools/kunit/.
>> +
>> +         If in doubt, say "N".
>
> FYI, I know this is in the style guide, but I'd personally feel free
> to leave out this paragraph.
>
> Having such "advertising" about what KUnit is made more sense when
> less people knew about it.
> It's not known by everyone in the community yet, but we might be
> getting to a point where this turns into repetitive bloat.
>

Ok, I'll drop these.

[...]

>> +
>> +       ret = input_register_device(input_dev);
>> +       KUNIT_ASSERT_EQ(test, ret, 0);
>
> (very unlikely that this matters, but...)
> Hmm, should we call input_free_device() if this fails?
> i.e. something like
>
> ret = ...;
> if (ret) {
>   input_free_device(input_dev);
>   KUNIT_ASSERT_FAILURE(test, "failed to register device: %d", ret);
> }
>

Indeed. I'll do this too.

[...]

>> +
>> +       ret = input_get_poll_interval(input_dev);
>> +       KUNIT_ASSERT_EQ(test, ret, -EINVAL);
>
> minor suggestion: can we inline these? E.g.
>   KUNIT_ASSERT_EQ(test, -EINVAL, input_get_poll_interval(input_dev));
> This way on failure, KUnit can print the function call instead of just `ret`.
>
> Users could always find out what failed by the line #, but including
> it in the output would be a bit nicer.
>
> E.g. w/ KUNIT_EXPECT_EQ(test, 0, ...)
>
>     # example_simple_test: EXPECTATION FAILED at
> lib/kunit/kunit-example-test.c:29
>     Expected 0 == input_get_poll_interval(input_dev), but
>         input_get_poll_interval(input_dev) == 42 (0x2a)
>
> verus
>
>     # example_simple_test: EXPECTATION FAILED at
> lib/kunit/kunit-example-test.c:28
>     Expected ret == 0, but
>         ret == 42 (0x2a)
>

Great suggestion. I'll change too, it would also get rid of the ret variable.
diff mbox series

Patch

diff --git a/drivers/input/Kconfig b/drivers/input/Kconfig
index e2752f7364bc..e094e5bbaa0c 100644
--- a/drivers/input/Kconfig
+++ b/drivers/input/Kconfig
@@ -166,6 +166,18 @@  config INPUT_EVBUG
 	  To compile this driver as a module, choose M here: the
 	  module will be called evbug.
 
+config INPUT_KUNIT_TEST
+	tristate "KUnit tests for Input" if !KUNIT_ALL_TESTS
+	depends on INPUT && KUNIT=y
+	default KUNIT_ALL_TESTS
+	help
+	  Say Y here if you want to build the KUnit tests for the input
+	  subsystem. For more information about KUnit and unit tests in
+	  general, please refer to the KUnit documentation in
+	  Documentation/dev-tools/kunit/.
+
+	  If in doubt, say "N".
+
 config INPUT_APMPOWER
 	tristate "Input Power Event -> APM Bridge" if EXPERT
 	depends on INPUT && APM_EMULATION
diff --git a/drivers/input/Makefile b/drivers/input/Makefile
index 2266c7d010ef..c78753274921 100644
--- a/drivers/input/Makefile
+++ b/drivers/input/Makefile
@@ -26,6 +26,7 @@  obj-$(CONFIG_INPUT_JOYSTICK)	+= joystick/
 obj-$(CONFIG_INPUT_TABLET)	+= tablet/
 obj-$(CONFIG_INPUT_TOUCHSCREEN)	+= touchscreen/
 obj-$(CONFIG_INPUT_MISC)	+= misc/
+obj-$(CONFIG_INPUT_KUNIT_TEST)	+= tests/
 
 obj-$(CONFIG_INPUT_APMPOWER)	+= apm-power.o
 
diff --git a/drivers/input/tests/Makefile b/drivers/input/tests/Makefile
new file mode 100644
index 000000000000..90cf954181bc
--- /dev/null
+++ b/drivers/input/tests/Makefile
@@ -0,0 +1,3 @@ 
+# SPDX-License-Identifier: GPL-2.0
+
+obj-$(CONFIG_INPUT_KUNIT_TEST) += input_test.o
diff --git a/drivers/input/tests/input_test.c b/drivers/input/tests/input_test.c
new file mode 100644
index 000000000000..25bbf51b5c87
--- /dev/null
+++ b/drivers/input/tests/input_test.c
@@ -0,0 +1,144 @@ 
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * KUnit test for the input core.
+ *
+ * Copyright (c) 2023 Red Hat Inc
+ */
+
+#include <linux/delay.h>
+#include <linux/input.h>
+
+#include <kunit/test.h>
+
+#define POLL_INTERVAL 100
+
+static int input_test_init(struct kunit *test)
+{
+	struct input_dev *input_dev;
+	int ret;
+
+	input_dev = input_allocate_device();
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, input_dev);
+
+	input_dev->name = "Test input device";
+	input_dev->id.bustype = BUS_VIRTUAL;
+	input_dev->id.vendor = 1;
+	input_dev->id.product = 1;
+	input_dev->id.version = 1;
+	input_set_capability(input_dev, EV_KEY, BTN_LEFT);
+	input_set_capability(input_dev, EV_KEY, BTN_RIGHT);
+
+	ret = input_register_device(input_dev);
+	KUNIT_ASSERT_EQ(test, ret, 0);
+
+	test->priv = input_dev;
+
+	return 0;
+}
+
+static void input_test_exit(struct kunit *test)
+{
+	struct input_dev *input_dev = test->priv;
+
+	input_unregister_device(input_dev);
+}
+
+static void input_test_poll(struct input_dev *input) { }
+
+static void input_test_polling(struct kunit *test)
+{
+	struct input_dev *input_dev = test->priv;
+	int ret;
+
+	ret = input_get_poll_interval(input_dev);
+	KUNIT_ASSERT_EQ(test, ret, -EINVAL);
+
+	ret = input_setup_polling(input_dev, input_test_poll);
+	KUNIT_ASSERT_EQ(test, ret, 0);
+
+	input_set_poll_interval(input_dev, POLL_INTERVAL);
+
+	ret = input_get_poll_interval(input_dev);
+	KUNIT_ASSERT_EQ(test, ret, POLL_INTERVAL);
+}
+
+static void input_test_timestamp(struct kunit *test)
+{
+	const ktime_t invalid_timestamp = ktime_set(0, 0);
+	struct input_dev *input_dev = test->priv;
+	ktime_t *timestamp, time;
+	int ret;
+
+	timestamp = input_get_timestamp(input_dev);
+	time = timestamp[INPUT_CLK_MONO];
+
+	ret = ktime_compare(time, invalid_timestamp);
+	KUNIT_ASSERT_EQ(test, ret, 1);
+
+	time = ktime_get();
+	input_set_timestamp(input_dev, time);
+
+	timestamp = input_get_timestamp(input_dev);
+	KUNIT_ASSERT_EQ(test, ktime_compare(timestamp[INPUT_CLK_MONO],
+					    time), 0);
+}
+
+static void input_test_match_device_id(struct kunit *test)
+{
+	struct input_dev *input_dev = test->priv;
+	struct input_device_id id;
+
+	id.flags = INPUT_DEVICE_ID_MATCH_BUS;
+	id.bustype = BUS_VIRTUAL;
+	KUNIT_ASSERT_TRUE(test, input_match_device_id(input_dev, &id));
+
+	id.bustype = BUS_I2C;
+	KUNIT_ASSERT_FALSE(test, input_match_device_id(input_dev, &id));
+
+	id.flags = INPUT_DEVICE_ID_MATCH_VENDOR;
+	id.vendor = 1;
+	KUNIT_ASSERT_TRUE(test, input_match_device_id(input_dev, &id));
+
+	id.vendor = 2;
+	KUNIT_ASSERT_FALSE(test, input_match_device_id(input_dev, &id));
+
+	id.flags = INPUT_DEVICE_ID_MATCH_PRODUCT;
+	id.product = 1;
+	KUNIT_ASSERT_TRUE(test, input_match_device_id(input_dev, &id));
+
+	id.product = 2;
+	KUNIT_ASSERT_FALSE(test, input_match_device_id(input_dev, &id));
+
+	id.flags = INPUT_DEVICE_ID_MATCH_VERSION;
+	id.version = 1;
+	KUNIT_ASSERT_TRUE(test, input_match_device_id(input_dev, &id));
+
+	id.version = 2;
+	KUNIT_ASSERT_FALSE(test, input_match_device_id(input_dev, &id));
+
+	id.flags = INPUT_DEVICE_ID_MATCH_EVBIT;
+	__set_bit(EV_KEY, id.evbit);
+	KUNIT_ASSERT_TRUE(test, input_match_device_id(input_dev, &id));
+
+	__set_bit(EV_ABS, id.evbit);
+	KUNIT_ASSERT_FALSE(test, input_match_device_id(input_dev, &id));
+}
+
+static struct kunit_case input_tests[] = {
+	KUNIT_CASE(input_test_polling),
+	KUNIT_CASE(input_test_timestamp),
+	KUNIT_CASE(input_test_match_device_id),
+	{ /* sentinel */ }
+};
+
+static struct kunit_suite input_test_suite = {
+	.name = "input_core",
+	.init = input_test_init,
+	.exit = input_test_exit,
+	.test_cases = input_tests,
+};
+
+kunit_test_suite(input_test_suite);
+
+MODULE_AUTHOR("Javier Martinez Canillas <javierm@redhat.com>");
+MODULE_LICENSE("GPL");