From: "Maíra Canal" <mairacanal@riseup.net>
To: David Gow <davidgow@google.com>
Cc: "Brendan Higgins" <brendanhiggins@google.com>,
"Daniel Latypov" <dlatypov@google.com>,
"David Airlie" <airlied@linux.ie>,
"Daniel Vetter" <daniel@ffwll.ch>,
"David S. Miller" <davem@davemloft.net>,
"Jakub Kicinski" <kuba@kernel.org>,
"José Expósito" <jose.exposito89@gmail.com>,
"Javier Martinez Canillas" <javierm@redhat.com>,
andrealmeid@riseup.net, melissa.srw@gmail.com,
siqueirajordao@riseup.net,
"Isabella Basso" <isabbasso@riseup.net>,
magalilemes00@gmail.com, tales.aparecida@gmail.com,
"open list:KERNEL SELFTEST FRAMEWORK"
<linux-kselftest@vger.kernel.org>,
"KUnit Development" <kunit-dev@googlegroups.com>,
"Linux Kernel Mailing List" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v3 0/3] Introduce KUNIT_EXPECT_MEMEQ and KUNIT_EXPECT_MEMNEQ macros
Date: Fri, 5 Aug 2022 09:18:12 -0300 [thread overview]
Message-ID: <ad483719-24b2-3207-cdcc-f5055d7a8895@riseup.net> (raw)
In-Reply-To: <CABVgOS=Yq2+uDw_iVK11eqBjEB-BErEr6naYpTswtTvyehAzYw@mail.gmail.com>
On 8/5/22 01:44, David Gow wrote:
> On Thu, Aug 4, 2022 at 5:59 AM 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, 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.
>>
>> Other than the style changes, this v3 brings alignment to the bytes, making
>> it easier to identify the faulty bytes. So, on the previous version, the
>> output from a failure would be:
>> [14:27:42] # xrgb8888_to_rgb565_test: EXPECTATION FAILED at drivers/gpu/drm/tests/drm_format_helper_test.c:248
>> [14:27:42] Expected dst == result->expected, but
>> [14:27:42] dst ==
>> [14:27:42] 33 0a <60> 12 00 a8 00 00 <00> 00 8e 6b <33> 0a 60 12
>> [14:27:42] 00 00 <00> 00 00 a8 <8e> 6b 33 0a 00 00 <00> 00
>> [14:27:42] result->expected ==
>> [14:27:42] 33 0a <61> 12 00 a8 00 00 <01> 00 8e 6b <31> 0a 60 12
>> [14:27:42] 00 00 <01> 00 00 a8 <81> 6b 33 0a 00 00 <01> 00
>>
>> Now, with the alignment, the output is:
>> [14:27:42] # xrgb8888_to_rgb565_test: EXPECTATION FAILED at drivers/gpu/drm/tests/drm_format_helper_test.c:248
>> [14:27:42] Expected dst == result->expected, but
>> [14:27:42] dst ==
>> [14:27:42] 33 0a <60> 12 00 a8 00 00 <00> 00 8e 6b <33> 0a 60 12
>> [14:27:42] 00 00 <00> 00 00 a8 <8e> 6b 33 0a 00 00 <00> 00
>> [14:27:42] result->expected ==
>> [14:27:42] 33 0a <61> 12 00 a8 00 00 <01> 00 8e 6b <31> 0a 60 12
>> [14:27:42] 00 00 <01> 00 00 a8 <81> 6b 33 0a 00 00 <01> 00
>>
>> Moreover, on the raw output, there were some indentation problems. Those
>> problems were solved with the use of KUNIT_SUBSUBTEST_INDENT.
>>
>> 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/2a0dcd75-5461-5266-2749-808f638f4c50@riseup.net/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/20220802212621.420840-1-mairacanal@riseup.net/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).
>>
>> Maíra Canal (3):
>> kunit: Introduce KUNIT_EXPECT_MEMEQ and KUNIT_EXPECT_MEMNEQ macros
>> kunit: Add KUnit memory block assertions to the example_all_expect_macros_test
>> kunit: Use KUNIT_EXPECT_MEMEQ macro
>>
>> .../gpu/drm/tests/drm_format_helper_test.c | 6 +-
>> include/kunit/assert.h | 34 +++++++++
>> include/kunit/test.h | 76 +++++++++++++++++++
>> lib/kunit/assert.c | 56 ++++++++++++++
>> lib/kunit/kunit-example-test.c | 7 ++
>> net/core/dev_addr_lists_test.c | 4 +-
>> 6 files changed, 178 insertions(+), 5 deletions(-)
>>
>> --
>> 2.37.1
>>
>> --
>> You received this message because you are subscribed to the Google Groups "KUnit Development" group.
>> To unsubscribe from this group and stop receiving emails from it, send an email to kunit-dev+unsubscribe@googlegroups.com.
>> To view this discussion on the web visit https://groups.google.com/d/msgid/kunit-dev/20220803215855.258704-1-mairacanal%40riseup.net.
>
> These patches look pretty good to me overall, but I was unable to
> apply v3 to test -- it looks like the mail client has wrapped some
> lines or something...
>
> davidgow@slicestar:~/linux-kselftest$ git am
> ./v3_20220803_mairacanal_introduce_kunit_expect_memeq_and_kunit_expect_memneq_macros.mbx
> Applying: kunit: Introduce KUNIT_EXPECT_MEMEQ and KUNIT_EXPECT_MEMNEQ macros
> error: corrupt patch at line 24
> Patch failed at 0001 kunit: Introduce KUNIT_EXPECT_MEMEQ and
> KUNIT_EXPECT_MEMNEQ macros
>
> Checkpatch also picks up an issue:
> ERROR: patch seems to be corrupt (line wrapped?)
> #62: FILE: include/kunit/assert.h:255:
> const struct va_format *message,
>
> v2 applied clearnly, so it seems to be specific to v3.
I'll check this issue and submit a v4. Thank you!
>
> In general, I like the patches, though. While I think there are a few
> places it'd be slightly suboptimale if it's being used to compare more
> structured data, such as the prospect of comparing padding between
> elements, as well as the output formatting not being ideal. It's
> perfect for the cases where memcmp() otherwise would be used, though.
Do you any take on how to make the output formatting more ideal?
Best Regards,
- Maíra Canal
>
> Cheers,
> -- David
next prev parent reply other threads:[~2022-08-05 12:18 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-08-03 21:58 [PATCH v3 0/3] Introduce KUNIT_EXPECT_MEMEQ and KUNIT_EXPECT_MEMNEQ macros Maíra Canal
2022-08-03 21:58 ` [PATCH v3 1/3] kunit: " Maíra Canal
2022-08-06 14:20 ` Muhammad Usama Anjum
2022-08-03 21:58 ` [PATCH v3 2/3] kunit: Add KUnit memory block assertions to the example_all_expect_macros_test Maíra Canal
2022-08-03 21:58 ` [PATCH v3 3/3] kunit: Use KUNIT_EXPECT_MEMEQ macro Maíra Canal
2022-08-05 4:44 ` [PATCH v3 0/3] Introduce KUNIT_EXPECT_MEMEQ and KUNIT_EXPECT_MEMNEQ macros David Gow
2022-08-05 12:18 ` Maíra Canal [this message]
2022-08-05 22:46 ` David Gow
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=ad483719-24b2-3207-cdcc-f5055d7a8895@riseup.net \
--to=mairacanal@riseup.net \
--cc=airlied@linux.ie \
--cc=andrealmeid@riseup.net \
--cc=brendanhiggins@google.com \
--cc=daniel@ffwll.ch \
--cc=davem@davemloft.net \
--cc=davidgow@google.com \
--cc=dlatypov@google.com \
--cc=isabbasso@riseup.net \
--cc=javierm@redhat.com \
--cc=jose.exposito89@gmail.com \
--cc=kuba@kernel.org \
--cc=kunit-dev@googlegroups.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-kselftest@vger.kernel.org \
--cc=magalilemes00@gmail.com \
--cc=melissa.srw@gmail.com \
--cc=siqueirajordao@riseup.net \
--cc=tales.aparecida@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).