| Message ID | 20220803215855.258704-1-mairacanal@riseup.net (mailing list archive) |
|---|---|
| Headers | show |
| Series | Introduce KUNIT_EXPECT_MEMEQ and KUNIT_EXPECT_MEMNEQ macros | expand |
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. 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. Cheers, -- David
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
On Fri, Aug 5, 2022 at 8:18 PM Maíra Canal <mairacanal@riseup.net> wrote: > > 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! > Thanks! > > > > 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? > I don't actually think we need to change any of the formatting in this patch, I'm just noting that usinng MEMEQ()/MEMNEQ() might not be the best choice for comparing, e.g., structs (and that comparing their members individually might be better there). _Maybe_ that's something that could be mentioned in the documentation, but I wouldn't change the code at all. Cheers, -- David