On Wed, Mar 29, 2023 at 2:23 AM Javier Martinez Canillas javierm@redhat.com wrote:
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
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
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 +++++++++++++++++++++++++++++++
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)
4 files changed, 160 insertions(+) create mode 100644 drivers/input/tests/Makefile create mode 100644 drivers/input/tests/input_test.c
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_TESTSdepends on INPUT && KUNIT=ydefault KUNIT_ALL_TESTShelpSay Y here if you want to build the KUnit tests for the inputsubsystem. For more information about KUnit and unit tests ingeneral, please refer to the KUnit documentation inDocumentation/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.
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);
(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); }
KUNIT_ASSERT_FAILURE() is like KUNIT_ASSERT_*(), but doesn't do any comparison. It just unconditionally aborts the test after printing the given message.
We could alternatively do kunit_err(test, "failed to register device: %d", ret); return ret; but this version won't show a file and line number.
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);
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)
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);+}
Thanks, Daniel