LinuxLists.cc - [PATCH v6 0/3] Introduce KUNIT_EXPECT_MEMEQ and KUNIT_EXPECT

2022-10-18 19:10:36

Subject: [PATCH v6 0/3] Introduce KUNIT_EXPECT_MEMEQ and KUNIT_EXPECT_MEMNEQ macros

2022-10-18 19:13:27

Subject: [PATCH v6 3/3] kunit: Use KUNIT_EXPECT_MEMEQ macro

Use KUNIT_EXPECT_MEMEQ to compare memory blocks in replacement of the
KUNIT_EXPECT_EQ macro. Therefor, the statement

KUNIT_EXPECT_EQ(test, memcmp(foo, bar, size), 0);

is replaced by:

KUNIT_EXPECT_MEMEQ(test, foo, bar, size);

Signed-off-by: Maíra Canal <[email protected]>
Acked-by: Daniel Latypov <[email protected]>
Reviewed-by: David Gow <[email protected]>
---
drivers/gpu/drm/tests/drm_format_helper_test.c | 12 ++++++------
net/core/dev_addr_lists_test.c | 4 ++--
2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/tests/drm_format_helper_test.c b/drivers/gpu/drm/tests/drm_format_helper_test.c
index 8d86c250c2ec..dbbf53a8dd2f 100644
--- a/drivers/gpu/drm/tests/drm_format_helper_test.c
+++ b/drivers/gpu/drm/tests/drm_format_helper_test.c
@@ -315,7 +315,7 @@ static void drm_test_fb_xrgb8888_to_gray8(struct kunit *test)
iosys_map_set_vaddr(&src, xrgb8888);

drm_fb_xrgb8888_to_gray8(&dst, &result->dst_pitch, &src, &fb, &params->clip);
- KUNIT_EXPECT_EQ(test, memcmp(buf, result->expected, dst_size), 0);
+ KUNIT_EXPECT_MEMEQ(test, buf, result->expected, dst_size);
}

static void drm_test_fb_xrgb8888_to_rgb332(struct kunit *test)
@@ -345,7 +345,7 @@ static void drm_test_fb_xrgb8888_to_rgb332(struct kunit *test)
iosys_map_set_vaddr(&src, xrgb8888);

drm_fb_xrgb8888_to_rgb332(&dst, &result->dst_pitch, &src, &fb, &params->clip);
- KUNIT_EXPECT_EQ(test, memcmp(buf, result->expected, dst_size), 0);
+ KUNIT_EXPECT_MEMEQ(test, buf, result->expected, dst_size);
}

static void drm_test_fb_xrgb8888_to_rgb565(struct kunit *test)
@@ -375,10 +375,10 @@ static void drm_test_fb_xrgb8888_to_rgb565(struct kunit *test)
iosys_map_set_vaddr(&src, xrgb8888);

drm_fb_xrgb8888_to_rgb565(&dst, &result->dst_pitch, &src, &fb, &params->clip, false);
- KUNIT_EXPECT_EQ(test, memcmp(buf, result->expected, dst_size), 0);
+ KUNIT_EXPECT_MEMEQ(test, buf, result->expected, dst_size);

drm_fb_xrgb8888_to_rgb565(&dst, &result->dst_pitch, &src, &fb, &params->clip, true);
- KUNIT_EXPECT_EQ(test, memcmp(buf, result->expected_swab, dst_size), 0);
+ KUNIT_EXPECT_MEMEQ(test, buf, result->expected_swab, dst_size);
}

static void drm_test_fb_xrgb8888_to_rgb888(struct kunit *test)
@@ -408,7 +408,7 @@ static void drm_test_fb_xrgb8888_to_rgb888(struct kunit *test)
iosys_map_set_vaddr(&src, xrgb8888);

drm_fb_xrgb8888_to_rgb888(&dst, &result->dst_pitch, &src, &fb, &params->clip);
- KUNIT_EXPECT_EQ(test, memcmp(buf, result->expected, dst_size), 0);
+ KUNIT_EXPECT_MEMEQ(test, buf, result->expected, dst_size);
}

static void drm_test_fb_xrgb8888_to_xrgb2101010(struct kunit *test)
@@ -439,7 +439,7 @@ static void drm_test_fb_xrgb8888_to_xrgb2101010(struct kunit *test)

drm_fb_xrgb8888_to_xrgb2101010(&dst, &result->dst_pitch, &src, &fb, &params->clip);
buf = le32buf_to_cpu(test, buf, TEST_BUF_SIZE);
- KUNIT_EXPECT_EQ(test, memcmp(buf, result->expected, dst_size), 0);
+ KUNIT_EXPECT_MEMEQ(test, buf, result->expected, dst_size);
}

static struct kunit_case drm_format_helper_test_cases[] = {
diff --git a/net/core/dev_addr_lists_test.c b/net/core/dev_addr_lists_test.c
index 049cfbc58aa9..90e7e3811ae7 100644
--- a/net/core/dev_addr_lists_test.c
+++ b/net/core/dev_addr_lists_test.c
@@ -71,11 +71,11 @@ static void dev_addr_test_basic(struct kunit *test)

memset(addr, 2, sizeof(addr));
eth_hw_addr_set(netdev, addr);
- KUNIT_EXPECT_EQ(test, 0, memcmp(netdev->dev_addr, addr, sizeof(addr)));
+ KUNIT_EXPECT_MEMEQ(test, netdev->dev_addr, addr, sizeof(addr));

memset(addr, 3, sizeof(addr));
dev_addr_set(netdev, addr);
- KUNIT_EXPECT_EQ(test, 0, memcmp(netdev->dev_addr, addr, sizeof(addr)));
+ KUNIT_EXPECT_MEMEQ(test, netdev->dev_addr, addr, sizeof(addr));
}

static void dev_addr_test_sync_one(struct kunit *test)
--
2.37.3

2022-10-18 19:29:18

by Maira Canal

[permalink] [raw]

Subject: [PATCH v6 1/3] kunit: Introduce KUNIT_EXPECT_MEMEQ and KUNIT_EXPECT_MEMNEQ macros

Currently, in order to compare memory blocks in KUnit, the KUNIT_EXPECT_EQ
or KUNIT_EXPECT_FALSE macros are used in conjunction with the memcmp
function, such as:
KUNIT_EXPECT_EQ(test, memcmp(foo, bar, size), 0);

Although this usage produces correct results for the test cases, when
the expectation fails, the error message is not very helpful,
indicating only the return of the memcmp function.

Therefore, create a new set of macros KUNIT_EXPECT_MEMEQ and
KUNIT_EXPECT_MEMNEQ that compare memory blocks until a specified size.
In case of expectation failure, those macros print the hex dump of the
memory blocks, making it easier to debug test failures for memory blocks.

That said, the expectation

KUNIT_EXPECT_EQ(test, memcmp(foo, bar, size), 0);

would translate to the expectation

KUNIT_EXPECT_MEMEQ(test, foo, bar, size);

Signed-off-by: Maíra Canal <[email protected]>
Reviewed-by: Daniel Latypov <[email protected]>
Reviewed-by: Muhammad Usama Anjum <[email protected]>
Reviewed-by: David Gow <[email protected]>
---
include/kunit/assert.h | 33 ++++++++++++++++
include/kunit/test.h | 87 ++++++++++++++++++++++++++++++++++++++++++
lib/kunit/assert.c | 56 +++++++++++++++++++++++++++
3 files changed, 176 insertions(+)

diff --git a/include/kunit/assert.h b/include/kunit/assert.h
index ace3de8d1ee7..fd59ac4a63b9 100644
--- a/include/kunit/assert.h
+++ b/include/kunit/assert.h
@@ -240,4 +240,37 @@ void kunit_binary_str_assert_format(const struct kunit_assert *assert,
const struct va_format *message,
struct string_stream *stream);

+#define KUNIT_INIT_MEM_ASSERT_STRUCT(text_, left_val, right_val, size_) \
+ { \
+ .text = text_, \
+ .left_value = left_val, \
+ .right_value = right_val, .size = size_, \
+ }
+
+/**
+ * struct kunit_mem_assert - An expectation/assertion that compares two
+ * memory blocks.
+ * @assert: The parent of this type.
+ * @text: Holds the textual representations of the operands and comparator.
+ * @left_value: The actual evaluated value of the expression in the left slot.
+ * @right_value: The actual evaluated value of the expression in the right slot.
+ * @size: Size of the memory block analysed in bytes.
+ *
+ * Represents an expectation/assertion that compares two memory blocks. For
+ * example, to expect that the first three bytes of foo is equal to the
+ * first three bytes of bar, you can use the expectation
+ * KUNIT_EXPECT_MEMEQ(test, foo, bar, 3);
+ */
+struct kunit_mem_assert {
+ struct kunit_assert assert;
+ const struct kunit_binary_assert_text *text;
+ const void *left_value;
+ const void *right_value;
+ const size_t size;
+};
+
+void kunit_mem_assert_format(const struct kunit_assert *assert,
+ const struct va_format *message,
+ struct string_stream *stream);
+
#endif /* _KUNIT_ASSERT_H */
diff --git a/include/kunit/test.h b/include/kunit/test.h
index b1ab6b32216d..cde97dc4eed5 100644
--- a/include/kunit/test.h
+++ b/include/kunit/test.h
@@ -658,6 +658,39 @@ do { \
##__VA_ARGS__); \
} while (0)

+#define KUNIT_MEM_ASSERTION(test, \
+ assert_type, \
+ left, \
+ op, \
+ right, \
+ size, \
+ fmt, \
+ ...) \
+do { \
+ const void *__left = (left); \
+ const void *__right = (right); \
+ const size_t __size = (size); \
+ static const struct kunit_binary_assert_text __text = { \
+ .operation = #op, \
+ .left_text = #left, \
+ .right_text = #right, \
+ }; \
+ \
+ if (likely(memcmp(__left, __right, __size) op 0)) \
+ break; \
+ \
+ _KUNIT_FAILED(test, \
+ assert_type, \
+ kunit_mem_assert, \
+ kunit_mem_assert_format, \
+ KUNIT_INIT_MEM_ASSERT_STRUCT(&__text, \
+ __left, \
+ __right, \
+ __size), \
+ fmt, \
+ ##__VA_ARGS__); \
+} while (0)
+
#define KUNIT_PTR_NOT_ERR_OR_NULL_MSG_ASSERTION(test, \
assert_type, \
ptr, \
@@ -928,6 +961,60 @@ do { \
fmt, \
##__VA_ARGS__)

+/**
+ * KUNIT_EXPECT_MEMEQ() - Expects that the first @size bytes of @left and @right are equal.
+ * @test: The test context object.
+ * @left: An arbitrary expression that evaluates to the specified size.
+ * @right: An arbitrary expression that evaluates to the specified size.
+ * @size: Number of bytes compared.
+ *
+ * Sets an expectation that the values that @left and @right evaluate to are
+ * equal. This is semantically equivalent to
+ * KUNIT_EXPECT_TRUE(@test, !memcmp((@left), (@right), (@size))). See
+ * KUNIT_EXPECT_TRUE() for more information.
+ *
+ * Although this expectation works for any memory block, it is not recommended
+ * for comparing more structured data, such as structs. This expectation is
+ * recommended for comparing, for example, data arrays.
+ */
+#define KUNIT_EXPECT_MEMEQ(test, left, right, size) \
+ KUNIT_EXPECT_MEMEQ_MSG(test, left, right, size, NULL)
+
+#define KUNIT_EXPECT_MEMEQ_MSG(test, left, right, size, fmt, ...) \
+ KUNIT_MEM_ASSERTION(test, \
+ KUNIT_EXPECTATION, \
+ left, ==, right, \
+ size, \
+ fmt, \
+ ##__VA_ARGS__)
+
+/**
+ * KUNIT_EXPECT_MEMNEQ() - Expects that the first @size bytes of @left and @right are not equal.
+ * @test: The test context object.
+ * @left: An arbitrary expression that evaluates to the specified size.
+ * @right: An arbitrary expression that evaluates to the specified size.
+ * @size: Number of bytes compared.
+ *
+ * Sets an expectation that the values that @left and @right evaluate to are
+ * not equal. This is semantically equivalent to
+ * KUNIT_EXPECT_TRUE(@test, memcmp((@left), (@right), (@size))). See
+ * KUNIT_EXPECT_TRUE() for more information.
+ *
+ * Although this expectation works for any memory block, it is not recommended
+ * for comparing more structured data, such as structs. This expectation is
+ * recommended for comparing, for example, data arrays.
+ */
+#define KUNIT_EXPECT_MEMNEQ(test, left, right, size) \
+ KUNIT_EXPECT_MEMNEQ_MSG(test, left, right, size, NULL)
+
+#define KUNIT_EXPECT_MEMNEQ_MSG(test, left, right, size, fmt, ...) \
+ KUNIT_MEM_ASSERTION(test, \
+ KUNIT_EXPECTATION, \
+ left, !=, right, \
+ size, \
+ fmt, \
+ ##__VA_ARGS__)
+
/**
* KUNIT_EXPECT_NULL() - Expects that @ptr is null.
* @test: The test context object.
diff --git a/lib/kunit/assert.c b/lib/kunit/assert.c
index d00d6d181ee8..c346a8d7fa6e 100644
--- a/lib/kunit/assert.c
+++ b/lib/kunit/assert.c
@@ -204,3 +204,59 @@ void kunit_binary_str_assert_format(const struct kunit_assert *assert,
kunit_assert_print_msg(message, stream);
}
EXPORT_SYMBOL_GPL(kunit_binary_str_assert_format);
+
+/* Adds a hexdump of a buffer to a string_stream comparing it with
+ * a second buffer. The different bytes are marked with <>.
+ */
+static void kunit_assert_hexdump(struct string_stream *stream,
+ const void *buf,
+ const void *compared_buf,
+ const size_t len)
+{
+ size_t i;
+ const u8 *buf1 = buf;
+ const u8 *buf2 = compared_buf;
+
+ string_stream_add(stream, KUNIT_SUBSUBTEST_INDENT);
+
+ for (i = 0; i < len; ++i) {
+ if (!(i % 16) && i)
+ string_stream_add(stream, "\n" KUNIT_SUBSUBTEST_INDENT);
+
+ if (buf1[i] != buf2[i])
+ string_stream_add(stream, "<%02x>", buf1[i]);
+ else
+ string_stream_add(stream, " %02x ", buf1[i]);
+ }
+}
+
+void kunit_mem_assert_format(const struct kunit_assert *assert,
+ const struct va_format *message,
+ struct string_stream *stream)
+{
+ struct kunit_mem_assert *mem_assert;
+
+ mem_assert = container_of(assert, struct kunit_mem_assert,
+ assert);
+
+ string_stream_add(stream,
+ KUNIT_SUBTEST_INDENT "Expected %s %s %s, but\n",
+ mem_assert->text->left_text,
+ mem_assert->text->operation,
+ mem_assert->text->right_text);
+
+ string_stream_add(stream, KUNIT_SUBSUBTEST_INDENT "%s ==\n",
+ mem_assert->text->left_text);
+ kunit_assert_hexdump(stream, mem_assert->left_value,
+ mem_assert->right_value, mem_assert->size);
+
+ string_stream_add(stream, "\n");
+
+ string_stream_add(stream, KUNIT_SUBSUBTEST_INDENT "%s ==\n",
+ mem_assert->text->right_text);
+ kunit_assert_hexdump(stream, mem_assert->right_value,
+ mem_assert->left_value, mem_assert->size);
+
+ kunit_assert_print_msg(message, stream);
+}
+EXPORT_SYMBOL_GPL(kunit_mem_assert_format);
--
2.37.3

2022-10-18 19:38:58

by Maira Canal

[permalink] [raw]

Subject: [PATCH v6 2/3] kunit: Add KUnit memory block assertions to the example_all_expect_macros_test

Augment the example_all_expect_macros_test with the KUNIT_EXPECT_MEMEQ
and KUNIT_EXPECT_MEMNEQ macros by creating a test with memory block
assertions.

Signed-off-by: Maíra Canal <[email protected]>
Reviewed-by: Daniel Latypov <[email protected]>
Reviewed-by: David Gow <[email protected]>
---
lib/kunit/kunit-example-test.c | 7 +++++++
1 file changed, 7 insertions(+)

diff --git a/lib/kunit/kunit-example-test.c b/lib/kunit/kunit-example-test.c
index f8fe582c9e36..66cc4e2365ec 100644
--- a/lib/kunit/kunit-example-test.c
+++ b/lib/kunit/kunit-example-test.c
@@ -86,6 +86,9 @@ static void example_mark_skipped_test(struct kunit *test)
*/
static void example_all_expect_macros_test(struct kunit *test)
{
+ const u32 array1[] = { 0x0F, 0xFF };
+ const u32 array2[] = { 0x1F, 0xFF };
+
/* Boolean assertions */
KUNIT_EXPECT_TRUE(test, true);
KUNIT_EXPECT_FALSE(test, false);
@@ -109,6 +112,10 @@ static void example_all_expect_macros_test(struct kunit *test)
KUNIT_EXPECT_STREQ(test, "hi", "hi");
KUNIT_EXPECT_STRNEQ(test, "hi", "bye");

+ /* Memory block assertions */
+ KUNIT_EXPECT_MEMEQ(test, array1, array1, sizeof(array1));
+ KUNIT_EXPECT_MEMNEQ(test, array1, array2, sizeof(array1));
+
/*
* There are also ASSERT variants of all of the above that abort test
* execution if they fail. Useful for memory allocations, etc.
--
2.37.3

2022-10-18 21:24:41

by Daniel Latypov

[permalink] [raw]

Subject: Re: [PATCH v6 1/3] kunit: Introduce KUNIT_EXPECT_MEMEQ and KUNIT_EXPECT_MEMNEQ macros

On Tue, Oct 18, 2022 at 12:06 PM > diff --git a/include/kunit/test.h > index b1ab6b32216d..cde97dc4eed5 > --- a/include/kunit/test.h
> +++ b/include/kunit/test.h
> @@ -658,6 +658,39 @@ do { > ##__VA_ARGS__); > } while (0)
>
> +#define KUNIT_MEM_ASSERTION(test, > + assert_type, > + left, > + op, > + right, > + size, > + fmt, > + ...) > +do { > + const void *__left = (left); > + const void *__right = (right); > + const size_t __size = (size); > + static const struct > + .operation = #op, > + .left_text = #left, > + .right_text = #right, > + }; > + > + if (likely(memcmp(__left, > + break; > + > + _KUNIT_FAILED(test, > + assert_type, > + kunit_mem_assert, > + kunit_mem_assert_format, > + KUNIT_INIT_MEM > + > + > + > + fmt, > + ##__VA_ARGS__); > +} while (0)
> +

I think this should have been the (to resolve the merge conflict I Looks good to me. I think this series
Sorry for changing the assertion
Daniel
Maíra Canal <[email protected]> wrote:
b/include/kunit/test.h
100644
\
\
\
\
\
\
\
\
\
\
\
\
\
\
kunit_binary_assert_text __text = { \
\
\
\
\
\
__right, __size) op 0)) \
\
\
\
\
\
\
_ASSERT_STRUCT(&__text, \
__left, \
__right, \
__size), \
\
\
only real change we needed to make
created).
is still good to go.
internals out from under you!

2022-10-19 07:25:02

by David Gow

[permalink] [raw]

Subject: Re: [PATCH v6 1/3] kunit: Introduce KUNIT_EXPECT_MEMEQ and KUNIT_EXPECT_MEMNEQ macros

On Wed, Oct 19, 2022 at 3:06 AM Maíra Canal <[email protected]> wrote:
>
> Currently, in order to compare memory blocks in KUnit, the KUNIT_EXPECT_EQ
> or KUNIT_EXPECT_FALSE macros are used in conjunction with the memcmp
> function, such as:
> KUNIT_EXPECT_EQ(test, memcmp(foo, bar, size), 0);
>
> Although this usage produces correct results for the test cases, when
> the expectation fails, the error message is not very helpful,
> indicating only the return of the memcmp function.
>
> Therefore, create a new set of macros KUNIT_EXPECT_MEMEQ and
> KUNIT_EXPECT_MEMNEQ that compare memory blocks until a specified size.
> In case of expectation failure, those macros print the hex dump of the
> memory blocks, making it easier to debug test failures for memory blocks.
>
> That said, the expectation
>
> KUNIT_EXPECT_EQ(test, memcmp(foo, bar, size), 0);
>
> would translate to the expectation
>
> KUNIT_EXPECT_MEMEQ(test, foo, bar, size);
>
> Signed-off-by: Maíra Canal <[email protected]>
> Reviewed-by: Daniel Latypov <[email protected]>
> Reviewed-by: Muhammad Usama Anjum <[email protected]>
> Reviewed-by: David Gow <[email protected]>
> ---

Thanks very much for rebasing this! I'm using it a bit in some tests
I'm working on, and it's been very useful!

One minor formatting nitpick below, otherwise we'll pull this into the
kunit branch as soon as possible.

And this is still
Reviewed-by: David Gow <[email protected]>

Cheers,
-- David

> include/kunit/assert.h | 33 ++++++++++++++++
> include/kunit/test.h | 87 ++++++++++++++++++++++++++++++++++++++++++
> lib/kunit/assert.c | 56 +++++++++++++++++++++++++++
> 3 files changed, 176 insertions(+)
>
> diff --git a/include/kunit/assert.h b/include/kunit/assert.h
> index ace3de8d1ee7..fd59ac4a63b9 100644
> --- a/include/kunit/assert.h
> +++ b/include/kunit/assert.h
> @@ -240,4 +240,37 @@ void kunit_binary_str_assert_format(const struct kunit_assert *assert,
> const struct va_format *message,
> struct string_stream *stream);
>
> +#define KUNIT_INIT_MEM_ASSERT_STRUCT(text_, left_val, right_val, size_) \
> + { \
> + .text = text_, \
> + .left_value = left_val, \
> + .right_value = right_val, .size = size_, \

Nit: Could we have the style of this macro match the other
INIT_*_ASSERT_STRUCT() ones: in particular, having the open brace on
the first line, and the close brace non-indented. It'd also be nice to
have .size set on its own line, like the other struct members.

> + }
> +
> +/**
> + * struct kunit_mem_assert - An expectation/assertion that compares two
> + * memory blocks.
> + * @assert: The parent of this type.
> + * @text: Holds the textual representations of the operands and comparator.
> + * @left_value: The actual evaluated value of the expression in the left slot.
> + * @right_value: The actual evaluated value of the expression in the right slot.
> + * @size: Size of the memory block analysed in bytes.
> + *
> + * Represents an expectation/assertion that compares two memory blocks. For
> + * example, to expect that the first three bytes of foo is equal to the
> + * first three bytes of bar, you can use the expectation
> + * KUNIT_EXPECT_MEMEQ(test, foo, bar, 3);
> + */
> +struct kunit_mem_assert {
> + struct kunit_assert assert;
> + const struct kunit_binary_assert_text *text;
> + const void *left_value;
> + const void *right_value;
> + const size_t size;
> +};
> +
> +void kunit_mem_assert_format(const struct kunit_assert *assert,
> + const struct va_format *message,
> + struct string_stream *stream);
> +
> #endif /* _KUNIT_ASSERT_H */
> diff --git a/include/kunit/test.h b/include/kunit/test.h
> index b1ab6b32216d..cde97dc4eed5 100644
> --- a/include/kunit/test.h
> +++ b/include/kunit/test.h
> @@ -658,6 +658,39 @@ do { \
> ##__VA_ARGS__); \
> } while (0)
>
> +#define KUNIT_MEM_ASSERTION(test, \
> + assert_type, \
> + left, \
> + op, \
> + right, \
> + size, \
> + fmt, \
> + ...) \
> +do { \
> + const void *__left = (left); \
> + const void *__right = (right); \
> + const size_t __size = (size); \
> + static const struct kunit_binary_assert_text __text = { \
> + .operation = #op, \
> + .left_text = #left, \
> + .right_text = #right, \
> + }; \
> + \
> + if (likely(memcmp(__left, __right, __size) op 0)) \
> + break; \
> + \
> + _KUNIT_FAILED(test, \
> + assert_type, \
> + kunit_mem_assert, \
> + kunit_mem_assert_format, \
> + KUNIT_INIT_MEM_ASSERT_STRUCT(&__text, \
> + __left, \
> + __right, \
> + __size), \
> + fmt, \
> + ##__VA_ARGS__); \
> +} while (0)
> +
> #define KUNIT_PTR_NOT_ERR_OR_NULL_MSG_ASSERTION(test, \
> assert_type, \
> ptr, \
> @@ -928,6 +961,60 @@ do { \
> fmt, \
> ##__VA_ARGS__)
>
> +/**
> + * KUNIT_EXPECT_MEMEQ() - Expects that the first @size bytes of @left and @right are equal.
> + * @test: The test context object.
> + * @left: An arbitrary expression that evaluates to the specified size.
> + * @right: An arbitrary expression that evaluates to the specified size.
> + * @size: Number of bytes compared.
> + *
> + * Sets an expectation that the values that @left and @right evaluate to are
> + * equal. This is semantically equivalent to
> + * KUNIT_EXPECT_TRUE(@test, !memcmp((@left), (@right), (@size))). See
> + * KUNIT_EXPECT_TRUE() for more information.
> + *
> + * Although this expectation works for any memory block, it is not recommended
> + * for comparing more structured data, such as structs. This expectation is
> + * recommended for comparing, for example, data arrays.
> + */
> +#define KUNIT_EXPECT_MEMEQ(test, left, right, size) \
> + KUNIT_EXPECT_MEMEQ_MSG(test, left, right, size, NULL)
> +
> +#define KUNIT_EXPECT_MEMEQ_MSG(test, left, right, size, fmt, ...) \
> + KUNIT_MEM_ASSERTION(test, \
> + KUNIT_EXPECTATION, \
> + left, ==, right, \
> + size, \
> + fmt, \
> + ##__VA_ARGS__)
> +
> +/**
> + * KUNIT_EXPECT_MEMNEQ() - Expects that the first @size bytes of @left and @right are not equal.
> + * @test: The test context object.
> + * @left: An arbitrary expression that evaluates to the specified size.
> + * @right: An arbitrary expression that evaluates to the specified size.
> + * @size: Number of bytes compared.
> + *
> + * Sets an expectation that the values that @left and @right evaluate to are
> + * not equal. This is semantically equivalent to
> + * KUNIT_EXPECT_TRUE(@test, memcmp((@left), (@right), (@size))). See
> + * KUNIT_EXPECT_TRUE() for more information.
> + *
> + * Although this expectation works for any memory block, it is not recommended
> + * for comparing more structured data, such as structs. This expectation is
> + * recommended for comparing, for example, data arrays.
> + */
> +#define KUNIT_EXPECT_MEMNEQ(test, left, right, size) \
> + KUNIT_EXPECT_MEMNEQ_MSG(test, left, right, size, NULL)
> +
> +#define KUNIT_EXPECT_MEMNEQ_MSG(test, left, right, size, fmt, ...) \
> + KUNIT_MEM_ASSERTION(test, \
> + KUNIT_EXPECTATION, \
> + left, !=, right, \
> + size, \
> + fmt, \
> + ##__VA_ARGS__)
> +
> /**
> * KUNIT_EXPECT_NULL() - Expects that @ptr is null.
> * @test: The test context object.
> diff --git a/lib/kunit/assert.c b/lib/kunit/assert.c
> index d00d6d181ee8..c346a8d7fa6e 100644
> --- a/lib/kunit/assert.c
> +++ b/lib/kunit/assert.c
> @@ -204,3 +204,59 @@ void kunit_binary_str_assert_format(const struct kunit_assert *assert,
> kunit_assert_print_msg(message, stream);
> }
> EXPORT_SYMBOL_GPL(kunit_binary_str_assert_format);
> +
> +/* Adds a hexdump of a buffer to a string_stream comparing it with
> + * a second buffer. The different bytes are marked with <>.
> + */
> +static void kunit_assert_hexdump(struct string_stream *stream,
> + const void *buf,
> + const void *compared_buf,
> + const size_t len)
> +{
> + size_t i;
> + const u8 *buf1 = buf;
> + const u8 *buf2 = compared_buf;
> +
> + string_stream_add(stream, KUNIT_SUBSUBTEST_INDENT);
> +
> + for (i = 0; i < len; ++i) {
> + if (!(i % 16) && i)
> + string_stream_add(stream, "\n" KUNIT_SUBSUBTEST_INDENT);
> +
> + if (buf1[i] != buf2[i])
> + string_stream_add(stream, "<%02x>", buf1[i]);
> + else
> + string_stream_add(stream, " %02x ", buf1[i]);
> + }
> +}
> +
> +void kunit_mem_assert_format(const struct kunit_assert *assert,
> + const struct va_format *message,
> + struct string_stream *stream)
> +{
> + struct kunit_mem_assert *mem_assert;
> +
> + mem_assert = container_of(assert, struct kunit_mem_assert,
> + assert);
> +
> + string_stream_add(stream,
> + KUNIT_SUBTEST_INDENT "Expected %s %s %s, but\n",
> + mem_assert->text->left_text,
> + mem_assert->text->operation,
> + mem_assert->text->right_text);
> +
> + string_stream_add(stream, KUNIT_SUBSUBTEST_INDENT "%s ==\n",
> + mem_assert->text->left_text);
> + kunit_assert_hexdump(stream, mem_assert->left_value,
> + mem_assert->right_value, mem_assert->size);
> +
> + string_stream_add(stream, "\n");
> +
> + string_stream_add(stream, KUNIT_SUBSUBTEST_INDENT "%s ==\n",
> + mem_assert->text->right_text);
> + kunit_assert_hexdump(stream, mem_assert->right_value,
> + mem_assert->left_value, mem_assert->size);
> +
> + kunit_assert_print_msg(message, stream);
> +}
> +EXPORT_SYMBOL_GPL(kunit_mem_assert_format);
> --
> 2.37.3
>

2022-10-19 07:37:31

by David Gow

[permalink] [raw]

Subject: Re: [PATCH v6 0/3] Introduce KUNIT_EXPECT_MEMEQ and KUNIT_EXPECT_MEMNEQ macros

On Wed, Oct 19, 2022 at 3:06 AM Maíra Canal <[email protected]> wrote:
>
> Currently, in order to compare memory blocks in KUnit, the KUNIT_EXPECT_EQ or
> KUNIT_EXPECT_FALSE macros are used in conjunction with the memcmp function,
> such as:
> KUNIT_EXPECT_EQ(test, memcmp(foo, bar, size), 0);
>
> Although this usage produces correct results for the test cases, if the
> expectation fails the error message is not very helpful, indicating only the
> return of the memcmp function.
>
> Therefore, create a new set of macros KUNIT_EXPECT_MEMEQ and
> KUNIT_EXPECT_MEMNEQ that compare memory blocks until a determined size. In
> case of expectation failure, those macros print the hex dump of the memory
> blocks, making it easier to debug test failures for memory blocks.
>
> The v6 has some changes on the first patch, due to rebase on top of Linux 6.1,
> specially the renaming of KUNIT_ASSERTION macro to _KUNIT_FAILED
> (97d453bc4007d4ac148c2ba89904026612b91ec9). Moreover, the DRM KUnit tests were
> mainlined in 6.1.
>
> The first patch of the series introduces the KUNIT_EXPECT_MEMEQ and
> KUNIT_EXPECT_MEMNEQ. The second patch adds an example of memory block
> expectations on the kunit-example-test.c. And the last patch replaces the
> KUNIT_EXPECT_EQ for KUNIT_EXPECT_MEMEQ on the existing occurrences.
>
> Best Regards,
> - Maíra Canal
>
> v1 -> v2: https://lore.kernel.org/linux-kselftest/[email protected]/T/#m402cc72eb01fb3b88d6706cf7d1705fdd51e5da2
>
> - Change "determinated" to "specified" (Daniel Latypov).
> - Change the macro KUNIT_EXPECT_ARREQ to KUNIT_EXPECT_MEMEQ, in order to make
> it easier for users to infer the right size unit (Daniel Latypov).
> - Mark the different bytes on the failure message with a <> (Daniel Latypov).
> - Replace a constant number of array elements for ARRAY_SIZE() (André Almeida).
> - Rename "array" and "expected" variables to "array1" and "array2" (Daniel Latypov).
>
> v2 -> v3: https://lore.kernel.org/linux-kselftest/[email protected]/T/#t
>
> - Make the bytes aligned at output.
> - Add KUNIT_SUBSUBTEST_INDENT to the output for the indentation (Daniel Latypov).
> - Line up the trailing \ at macros using tabs (Daniel Latypov).
> - Line up the params to the functions (Daniel Latypov).
> - Change "Increament" to "Augment" (Daniel Latypov).
> - Use sizeof() for array sizes (Daniel Latypov).
> - Add Daniel Latypov's tags.
>
> v3 -> v4: https://lore.kernel.org/linux-kselftest/CABVgOSm_59Yr82deQm2C=18jjSv_akmn66zs4jxx3hfziXPeHg@mail.gmail.com/T/#t
>
> - Fix wrapped lines by the mail client (David Gow).
> - Mention on documentation that KUNIT_EXPECT_MEMEQ is not recommended for
> structured data (David Gow).
> - Add Muhammad Usama Anjum's tag.
>
> v4 -> v5: https://lore.kernel.org/linux-kselftest/[email protected]/
>
> - Rebase on top of drm-misc-next.
> - Add David Gow's tags.
>
> v5 -> v6: https://lore.kernel.org/linux-kselftest/[email protected]/
>
> - Rebase on top of Linux 6.1.
> - Change KUNIT_ASSERTION macro to _KUNIT_FAILED.
>

Thanks a bunch for rebasing this. It works well here, and I'm planning
to use it in some tests I'm writing!

One minor formatting comment on patch 1/3, otherwise this whole series
is good to go.

Cheers,
-- David

Attachments:

smime.p7s (3.91 kB)
S/MIME Cryptographic Signature