Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Sender: "dlatypov via sendgmr" <dlatypov@dlatypov.svl.corp.google.com>
Date:   Mon,  2 Nov 2020 13:36:56 -0800
Message-Id: <20201102213656.2700500-1-dlatypov@google.com>
Mime-Version: 1.0
Subject: [PATCH] Documentation: kunit: provide guidance for testing many inputs
From:   Daniel Latypov <dlatypov@google.com>
To:     brendanhiggins@google.com, andriy.shevchenko@linux.intel.com
Cc:     davidgow@google.com, linux-kernel@vger.kernel.org,
        linux-kselftest@vger.kernel.org, skhan@linuxfoundation.org,
        Daniel Latypov <dlatypov@google.com>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

usage.rst goes into a detailed about faking out classes, but currently
lacks wording about how one might idiomatically test a range of inputs.

Give an example of how one might test a hash function via macros/helper
funcs and a table-driven test and very briefly discuss pros and cons.

Also highlight the KUNIT_EXPECT_*_MSG() variants (that aren't mentioned
elsewhere [1]) which are particularly useful in these situations.

It is also criminally underused at the moment, only appearing in 2
tests (both written by people involved in KUnit).

[1] not even on
https://www.kernel.org/doc/html/latest/dev-tools/kunit/api/test.html

Signed-off-by: Daniel Latypov <dlatypov@google.com>
---
 Documentation/dev-tools/kunit/usage.rst | 66 +++++++++++++++++++++++++
 1 file changed, 66 insertions(+)

diff --git a/Documentation/dev-tools/kunit/usage.rst b/Documentation/dev-tools/kunit/usage.rst
index 62142a47488c..317390df2b96 100644
--- a/Documentation/dev-tools/kunit/usage.rst
+++ b/Documentation/dev-tools/kunit/usage.rst
@@ -451,6 +451,72 @@ We can now use it to test ``struct eeprom_buffer``:
 		destroy_eeprom_buffer(ctx->eeprom_buffer);
 	}
 
+Testing various inputs
+----------------------
+
+Testing just a few inputs might not be enough to have confidence that the code
+works correctly, e.g. for a hash function.
+
+In such cases, it can be helpful to have a helper macro or function, e.g. this
+fictitious example for ``md5sum(1)``
+
+.. code-block:: c
+
+	/* Note: the cast is to satisfy overly strict type-checking. */
+	#define TEST_MD5(in, want) \
+		md5sum(in, out); \
+		KUNIT_EXPECT_STREQ_MSG(test, (char *)out, want, "md5sum(%s)", in);
+
+	char out[16];
+	TEST_MD5("hello world",   "5eb63bbbe01eeed093cb22bb8f5acdc3");
+	TEST_MD5("hello world!",  "fc3ff98e8c6a0d3087d515c0473f8677");
+
+Note the use of ``KUNIT_EXPECT_STREQ_MSG`` to give more context when it fails
+and make it easier to track down. (Yes, in this example, ``want`` is likely
+going to be unique enough on its own).
+
+The ``_MSG`` variants are even more useful when the same expectation is called
+multiple times (in a loop or helper function) and thus the line number isn't
+enough to identify what failed, like below.
+
+In some cases, it can be helpful to write a *table-driven test* instead, e.g.
+
+.. code-block:: c
+
+	int i;
+	char out[16];
+
+	struct md5_test_case {
+		const char *str;
+		const char *md5;
+	};
+
+	struct md5_test_case cases[] = {
+		{
+			.str = "hello world",
+			.md5 = "5eb63bbbe01eeed093cb22bb8f5acdc3",
+		},
+		{
+			.str = "hello world!",
+			.md5 = "fc3ff98e8c6a0d3087d515c0473f8677",
+		},
+	};
+	for (i = 0; i < ARRAY_SIZE(cases); ++i) {
+		md5sum(cases[i].str, out);
+		KUNIT_EXPECT_STREQ_MSG(test, (char *)out, cases[i].md5,
+		                      "md5sum(%s)", cases[i].str);
+	}
+
+
+There's more boilerplate involved, but it can:
+
+* be more readable when there are multiple inputs/outputs thanks to field names,
+
+  * E.g. see ``fs/ext4/inode-test.c`` for an example of both.
+* reduce duplication if test cases can be shared across multiple tests.
+
+  * E.g. if we had a magical ``undo_md5sum`` function, we could reuse ``cases``.
+
 .. _kunit-on-non-uml:
 
 KUnit on non-UML architectures

base-commit: 77c8473edf7f7664137f555cfcdc8c460bbd947d
-- 
2.29.1.341.ge80a0c044ae-goog