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
On Tue, Nov 3, 2020 at 5:37 AM Daniel Latypov dlatypov@google.com wrote:
usage.rst goes into a detailed about faking out classes, but currently
Nit: a detailed what?
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
I suspect we'll eventually want to document the _MSG variants here as well, though it will bloat the page somewhat. In any case, it can be left to a separate patch.
Signed-off-by: Daniel Latypov dlatypov@google.com
Thanks for writing this -- it's definitely a common test pattern which it'd be nice to encourage and explain a bit better.
Cheers, -- David
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 +----------------------
Nit: "various" isn't hugely descriptive here. Maybe something like "Testing against multiple inputs" would be better?
+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``.
This is a bit of a nitpick, but I don't think this is quite conveying the usefulness of table-based testing. Maybe it's that a hypothetical "undo_md5sum" is too unrealistic an example? Maybe, instead of having both the macro-based and table-driven examples based around md5sum(), the table-based one could use something more obviously invertible / reusable, and include both in the example code. E.g, something akin to toupper() and tolower() or some other conversion function. I think having a better example here is probably more useful than having both the table- and macro- driven examples test the same thing.
.. _kunit-on-non-uml:
KUnit on non-UML architectures
base-commit: 77c8473edf7f7664137f555cfcdc8c460bbd947d
2.29.1.341.ge80a0c044ae-goog
On Fri, Nov 6, 2020 at 8:21 PM David Gow davidgow@google.com wrote:
On Tue, Nov 3, 2020 at 5:37 AM Daniel Latypov dlatypov@google.com wrote:
usage.rst goes into a detailed about faking out classes, but currently
Nit: a detailed what?
Thanks for the catch, added "detailed section" locally.
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
I suspect we'll eventually want to document the _MSG variants here as well, though it will bloat the page somewhat. In any case, it can be left to a separate patch.
Agreed.
Signed-off-by: Daniel Latypov dlatypov@google.com
Thanks for writing this -- it's definitely a common test pattern which it'd be nice to encourage and explain a bit better.
Apologies for the delayed response. Noting here that having talked offline with David, this section will have to change for parameterized testing (which is basically just formalized support for table-driven tests). But it seems it'll take a while to resolve the debate on TAP output, so this docs change shouldn't be blocked on that going in.
Cheers, -- David
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 +----------------------
Nit: "various" isn't hugely descriptive here. Maybe something like "Testing against multiple inputs" would be better?
Changed. As you can tell from the name of this patch ("many inputs"), I had been unsure what to put here. "multiple inputs" works fine, I think. I had initially changed from that, since I had wanted to convey that these patterns are more useful when you have a larger number of inputs to go through. But in hindsight "multiple inputs" is just more clear.
+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``.
This is a bit of a nitpick, but I don't think this is quite conveying the usefulness of table-based testing. Maybe it's that a hypothetical "undo_md5sum" is too unrealistic an example? Maybe, instead of having both the macro-based and table-driven examples based around md5sum(), the table-based one could use something more obviously invertible / reusable, and include both in the example code. E.g, something akin to toupper() and tolower() or some other conversion function. I think having a better example here is probably more useful than having both the table- and macro- driven examples test the same thing.
Heh, I was worried about this a bit as well. Perhaps an inverse md5 breaks "suspension of disbelief" too much, even in this hypothetical context.
I had considered toupper()/tolower() but they aren't truly inverses, e.g. tolower(toupper("Hello")), and felt a bit too trivial perhaps. I'm also unsure I would test these functions on strs as opposed to chars (unless we're dealing with non-ascii), whereas I probably would test a checksum like this. I wouldn't be too opposed to switching over to tolower/toupper().
But perhaps something like
struct sha_test_case { const char *str; const char *sha1, *sha256; };
and reusing the same table would be good enough to demonstrate a different kind of "reuse" that is somewhat common for table-driven tests?
.. _kunit-on-non-uml:
KUnit on non-UML architectures
base-commit: 77c8473edf7f7664137f555cfcdc8c460bbd947d
2.29.1.341.ge80a0c044ae-goog
On Mon, Nov 2, 2020 at 1:37 PM Daniel Latypov dlatypov@google.com wrote:
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
Aside from the minor comment I made below, I like the patch; it is a definite improvement, but I think the test you wrote that ultimately led to this documentation fix had more information in it than this documentation. I think it only contains the pattern that you outlined here, but I think it does include some other best practices. Maybe we should add some more documentation patches with more code examples in the future?
Anyway, like I said, I think this patch in and of itself looks pretty good.
Reviewed-by: Brendan Higgins brendanhiggins@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 +----------------------
Since this, by my count, the second test pattern that we are introducing here, could we maybe call that out with a subheading or a new section or something? It would be nice if we could sort of build up a cookbook of testing patterns.
+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
On Mon, Nov 23, 2020 at 10:32 AM Brendan Higgins brendanhiggins@google.com wrote:
On Mon, Nov 2, 2020 at 1:37 PM Daniel Latypov dlatypov@google.com wrote:
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
Aside from the minor comment I made below, I like the patch; it is a definite improvement, but I think the test you wrote that ultimately led to this documentation fix had more information in it than this documentation. I think it only contains the pattern that you outlined here, but I think it does include some other best practices. Maybe we should add some more documentation patches with more code examples in the future?
Anyway, like I said, I think this patch in and of itself looks pretty good.
Reviewed-by: Brendan Higgins brendanhiggins@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 +----------------------
Since this, by my count, the second test pattern that we are introducing here, could we maybe call that out with a subheading or a new section or something? It would be nice if we could sort of build up a cookbook of testing patterns.
Good point, I noticed now the "Organization of this document" section would need to be updated. Perhaps something like
-This document is organized into two main sections: Testing and Isolating -Behavior. The first covers what unit tests are and how to use KUnit to write -them. The second covers how to use KUnit to isolate code and make it possible -to unit test code that was otherwise un-unit-testable. +This document is organized into two main sections: Testing and Common Patterns. +The first covers what unit tests are and how to use KUnit to write them. The +second covers common testing patterns, e.g. how to isolate code and make it +possible to unit test code that was otherwise un-unit-testable.
I'll send out a V2 shortly, changing the example per David's suggestion and with the above.
+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
linux-kselftest-mirror@lists.linaro.org
-
Brendan Higgins -
Daniel Latypov -
David Gow