| Message ID | 20220802212621.420840-2-mairacanal@riseup.net (mailing list archive) |
|---|---|
| State | New |
| Delegated to: | Brendan Higgins |
| Headers | show |
| Series | [v2,1/3] kunit: Introduce KUNIT_EXPECT_MEMEQ and KUNIT_EXPECT_MEMNEQ macros | expand |
On Tue, Aug 2, 2022 at 2:26 PM Maíra Canal <mairacanal@riseup.net> 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 <mairacanal@riseup.net> Reviewed-by: Daniel Latypov <dlatypov@google.com> Thanks, this is nice to have and I think the clarity issues have been resolved. Some various optional nits about whitespace below. I'd see if anyone has any complaints about the output format before wasting any time on those nits. Looking at example output now, I see Expected array2 == array1, but array2 == <1f> 00 00 00 ff 00 00 00 array1 == <0f> 00 00 00 ff 00 00 00 not ok 4 - example_all_expect_macros_test Looks good to me. > --- > v1 -> v2: > - 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). > --- > include/kunit/assert.h | 35 +++++++++++++++++++ > include/kunit/test.h | 76 ++++++++++++++++++++++++++++++++++++++++++ > lib/kunit/assert.c | 54 ++++++++++++++++++++++++++++++ > 3 files changed, 165 insertions(+) > > diff --git a/include/kunit/assert.h b/include/kunit/assert.h > index 4b52e12c2ae8..a54f5253b997 100644 > --- a/include/kunit/assert.h > +++ b/include/kunit/assert.h > @@ -256,4 +256,39 @@ 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_) \ > + { \ > + .assert = { .format = kunit_mem_assert_format }, \ > + .text = text_, \ > + .left_value = left_val, \ > + .right_value = right_val, .size = size_, \ > + } very nit: the trailing \s aren't quite lined up. In this particular case, I'm planning on deleting this block in the future, so it doesn't matter too much. > diff --git a/include/kunit/test.h b/include/kunit/test.h > index 8ffcd7de9607..1925d648eec8 100644 > --- a/include/kunit/test.h > +++ b/include/kunit/test.h > @@ -684,6 +684,36 @@ do { \ > ##__VA_ARGS__); \ > } while (0) > > +#define KUNIT_MEM_ASSERTION(test, \ very nit: the trailing \s are also a bit out of line here. We can fix this particular line by just adding another \t after "test," In general, lining these up is just a matter of adding a \t after the text and then maybe add or delete some of the " "s before the \s. E.g. with vim's `:set list`, after lining up the \s, I get #define KUNIT_MEM_ASSERTION(test,^I^I^I^I^I \$ ^I^I^I^I assert_type,^I^I^I^I \$ ^I^I^I^I left,^I^I^I^I \$ ^I^I^I^I op,^I^I^I^I^I \$ ^I^I^I^I right,^I^I^I^I \$ ^I^I^I^I size,^I^I \$ ^I^I^I^I fmt,^I^I^I^I^I \$ ^I^I^I^I ...)^I^I^I^I^I \$ do {^I^I^I^I^I^I^I^I^I \$ ^Iconst void *__left = (left);^I^I^I^I^I \$ ... > diff --git a/lib/kunit/assert.c b/lib/kunit/assert.c > index d00d6d181ee8..abd434bc7ec6 100644 > --- a/lib/kunit/assert.c > +++ b/lib/kunit/assert.c > @@ -204,3 +204,57 @@ 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; > + > + for (i = 0; i < len; ++i) { > + if (i % 16) > + string_stream_add(stream, " "); > + else if (i) > + string_stream_add(stream, "\n "); > + > + 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) very nit: the func above doesn't line up the params, but I don't think it matters too much. It uses \t\t whereas this one is \t\t\t + some spaces. I think it'd be fine if they were consistent with each other, or if you're willing to mess around with spaces, we could get the parameters to line up. (e.g. see how kunit_binary_ptr_assert_format and others do their line breaks)
diff --git a/include/kunit/assert.h b/include/kunit/assert.h index 4b52e12c2ae8..a54f5253b997 100644 --- a/include/kunit/assert.h +++ b/include/kunit/assert.h @@ -256,4 +256,39 @@ 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_) \ + { \ + .assert = { .format = kunit_mem_assert_format }, \ + .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 8ffcd7de9607..1925d648eec8 100644 --- a/include/kunit/test.h +++ b/include/kunit/test.h @@ -684,6 +684,36 @@ 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, \ + }; \ + \ + KUNIT_ASSERTION(test, \ + assert_type, \ + memcmp(__left, __right, __size) op 0, \ + kunit_mem_assert, \ + 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, \ @@ -952,6 +982,52 @@ 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. + */ +#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. + */ +#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..abd434bc7ec6 100644 --- a/lib/kunit/assert.c +++ b/lib/kunit/assert.c @@ -204,3 +204,57 @@ 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; + + for (i = 0; i < len; ++i) { + if (i % 16) + string_stream_add(stream, " "); + else if (i) + string_stream_add(stream, "\n "); + + 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);
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 <mairacanal@riseup.net> --- v1 -> v2: - 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). --- include/kunit/assert.h | 35 +++++++++++++++++++ include/kunit/test.h | 76 ++++++++++++++++++++++++++++++++++++++++++ lib/kunit/assert.c | 54 ++++++++++++++++++++++++++++++ 3 files changed, 165 insertions(+)