On Sun, Jun 13, 2021 at 11:42 PM Christoph Hellwig hch@lst.de wrote:
+config UUID_KUNIT_TEST
tristate "Unit test for UUID" if !KUNIT_ALL_TESTSdepends on KUNITdefault KUNIT_ALL_TESTShelpThis builds the UUID unit test.Does this first help line really add any value if we have this second line:
Tests parsing functions for UUID/GUID strings.?
If unsure, say N.Not specific to this case, but IMHO we can drop this line for all kunit tests as it is completely obvious.
@@ -354,5 +353,6 @@ obj-$(CONFIG_LIST_KUNIT_TEST) += list-test.o obj-$(CONFIG_LINEAR_RANGES_TEST) += test_linear_ranges.o obj-$(CONFIG_BITS_TEST) += test_bits.o obj-$(CONFIG_CMDLINE_KUNIT_TEST) += cmdline_kunit.o +obj-$(CONFIG_UUID_KUNIT_TEST) += test_uuid.o
Another meta-comment on the kunit tests: Wouldn't it make more sense to name them all as CONFIG_KUNIT_TEST_FOO to allow for easier grepping?
But putting them in a "kunit namespace" by prefixing them as such would be misleading, IMO. The tests live adjacent to the code they test and are owned by the same maintainers, or at least that's the intent.
And if the goal is just to find configs, then I don't see much difference between "config.*KUNIT_TEST" and "config KUNIT_TEST.*"
-struct test_uuid_data { +struct test_data { const char *uuid; guid_t le; uuid_t be; };
-static const struct test_uuid_data test_uuid_test_data[] = { +static const struct test_data correct_data[] = {
What is the reason for these renames? Is this a pattern used for other kunit tests?
No, this is not a pattern. The structs can be renamed back.
+static void uuid_correct_le(struct kunit *test) {
guid_t le;const struct test_data *data = (const struct test_data *)(test->param_value);Overly long line. But as far as I can tell there is no need for the case that causes this mess anyway given that param_value is a "const void *".
There is no need for the cast or the brace, yes. This is my fault.
The documentation has both since I had thought that would make how it works more clear: https://www.kernel.org/doc/html/latest/dev-tools/kunit/usage.html#parameteri... I don't really understand my past thought process...
Same for all the other instances of this.
+static void uuid_wrong_le(struct kunit *test) { guid_t le;
const char **data = (const char **)(test->param_value);No need for the second pair of braces. Same for various other instances.