| Message ID | 20220802161206.228707-3-mairacanal@riseup.net (mailing list archive) |
|---|---|
| State | New |
| Delegated to: | Brendan Higgins |
| Headers | show |
| Series | [1/3] kunit: Introduce KUNIT_EXPECT_ARREQ and KUNIT_EXPECT_ARRNEQ macros | expand |
Às 13:12 de 02/08/22, Maíra Canal escreveu: > Increament the example_all_expect_macros_test with the > KUNIT_EXPECT_ARREQ and KUNIT_EXPECT_ARRNEQ macros by creating a test > with array assertions. > > Signed-off-by: Maíra Canal <mairacanal@riseup.net> > --- > 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..fc81a45d9cbc 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 array[] = { 0x0F, 0xFF }; > + const u32 expected[] = { 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"); > > + /* Array assertions */ > + KUNIT_EXPECT_ARREQ(test, expected, expected, 2); > + KUNIT_EXPECT_ARRNEQ(test, array, expected, 2); ARRAY_SIZE() is usually better than constants is this case. > + > /* > * There are also ASSERT variants of all of the above that abort test > * execution if they fail. Useful for memory allocations, etc.
On Tue, Aug 2, 2022 at 9:19 AM André Almeida <andrealmeid@riseup.net> wrote: > Às 13:12 de 02/08/22, Maíra Canal escreveu: > > Increament the example_all_expect_macros_test with the > > KUNIT_EXPECT_ARREQ and KUNIT_EXPECT_ARRNEQ macros by creating a test > > with array assertions. > > > > Signed-off-by: Maíra Canal <mairacanal@riseup.net> > > --- > > 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..fc81a45d9cbc 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 array[] = { 0x0F, 0xFF }; > > + const u32 expected[] = { 0x1F, 0xFF }; Given the distance between the definition and their use, perhaps we can give them clearer names. E.g. array + diff_array, or array1 + array2, etc. I think something to indicate they're arrays and that they're different. The current name `expected` is a bit unclear. > > + > > /* 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"); > > > > + /* Array assertions */ > > + KUNIT_EXPECT_ARREQ(test, expected, expected, 2); > > + KUNIT_EXPECT_ARRNEQ(test, array, expected, 2); > > ARRAY_SIZE() is usually better than constants is this case. Note: that's actually incorrect! Ah right, this was the other blocker I had in mind. I wasn't sure how we'd handle the size parameter. Users might think ARRAY_SIZE() is fine and copy-paste it. But the size parameter is in units of bytes, not array elements! If the element types are not 1 byte, it'll silently not compare the full array. We'd want people to use KUNIT_EXPECT_ARREQ(test, expected, expected, sizeof(expected)); But this doesn't work for `u32 *array`, since it'll silently just compare 1 byte if people get them mixed up. I don't know how we make a maximally fool-proof version of this macro :\
On 8/2/22 15:15, 'Daniel Latypov' via KUnit Development wrote: > On Tue, Aug 2, 2022 at 9:19 AM André Almeida <andrealmeid@riseup.net> wrote: >> Às 13:12 de 02/08/22, Maíra Canal escreveu: >>> Increament the example_all_expect_macros_test with the >>> KUNIT_EXPECT_ARREQ and KUNIT_EXPECT_ARRNEQ macros by creating a test >>> with array assertions. >>> >>> Signed-off-by: Maíra Canal <mairacanal@riseup.net> >>> --- >>> 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..fc81a45d9cbc 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 array[] = { 0x0F, 0xFF }; >>> + const u32 expected[] = { 0x1F, 0xFF }; > > Given the distance between the definition and their use, perhaps we > can give them clearer names. > E.g. array + diff_array, or array1 + array2, etc. > > I think something to indicate they're arrays and that they're different. > The current name `expected` is a bit unclear. Thank you for the note, I'll address it at v2. > >>> + >>> /* 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"); >>> >>> + /* Array assertions */ >>> + KUNIT_EXPECT_ARREQ(test, expected, expected, 2); >>> + KUNIT_EXPECT_ARRNEQ(test, array, expected, 2); >> >> ARRAY_SIZE() is usually better than constants is this case. > > Note: that's actually incorrect! > Yep, that's my bad! > Ah right, this was the other blocker I had in mind. > I wasn't sure how we'd handle the size parameter. > > Users might think ARRAY_SIZE() is fine and copy-paste it. > But the size parameter is in units of bytes, not array elements! > If the element types are not 1 byte, it'll silently not compare the full array. > > We'd want people to use > KUNIT_EXPECT_ARREQ(test, expected, expected, sizeof(expected)); > > But this doesn't work for `u32 *array`, since it'll silently just > compare 1 byte if people get them mixed up. > > I don't know how we make a maximally fool-proof version of this macro :\ This is a hard one also. I believe that use KUNIT_EXPECT_ARREQ(test, expected, expected, sizeof(expected)); is more compliant to the memcpy/memset/memcmp signature. Moreover, this problem also occur for the KUNIT_EXPECT_EQ(test, memcmp(expected, expected, sizeof(expected)), 0); I believe that the number of array elements will make it easier for users to avoid mistakes. I'll change it internally for size_bytes = (size) * sizeof((left)[0]) on v2. Best Regards, - Maíra Canal >
On Tue, Aug 2, 2022 at 12:00 PM Maíra Canal <mairacanal@riseup.net> wrote: > > I don't know how we make a maximally fool-proof version of this macro :\ > > This is a hard one also. I believe that use KUNIT_EXPECT_ARREQ(test, > expected, expected, sizeof(expected)); is more compliant to the > memcpy/memset/memcmp signature. Moreover, this problem also occur for > the KUNIT_EXPECT_EQ(test, memcmp(expected, expected, sizeof(expected)), 0); > > I believe that the number of array elements will make it easier for > users to avoid mistakes. Actually, another idea: perhaps KUNIT_EXPECT_MEMEQ? I think that might be clearer in terms of the semantics and people could more easily infer the right unit (bytes). Daniel
diff --git a/lib/kunit/kunit-example-test.c b/lib/kunit/kunit-example-test.c index f8fe582c9e36..fc81a45d9cbc 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 array[] = { 0x0F, 0xFF }; + const u32 expected[] = { 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"); + /* Array assertions */ + KUNIT_EXPECT_ARREQ(test, expected, expected, 2); + KUNIT_EXPECT_ARRNEQ(test, array, expected, 2); + /* * There are also ASSERT variants of all of the above that abort test * execution if they fail. Useful for memory allocations, etc.
Increament the example_all_expect_macros_test with the KUNIT_EXPECT_ARREQ and KUNIT_EXPECT_ARRNEQ macros by creating a test with array assertions. Signed-off-by: Maíra Canal <mairacanal@riseup.net> --- lib/kunit/kunit-example-test.c | 7 +++++++ 1 file changed, 7 insertions(+)