Re: [PATCH] kunit: make KUNIT_EXPECT_STREQ() quote values, don't print literals
On Fri, Apr 2, 2021 at 12:19 PM Shuah Khan wrote: > > On 4/2/21 1:09 PM, Daniel Latypov wrote: > > On Fri, Apr 2, 2021 at 10:47 AM Shuah Khan > > wrote: > >> > >> On 4/2/21 3:35 AM, Brendan Higgins wrote: > >>> On Fri, Feb 5, 2021 at 2:18 PM Daniel Latypov wrote: > > Before: > >Expected str == "world", but > >str == hello > >"world" == world > > After: > >Expected str == "world", but > >str == "hello" > > > Note: like the literal ellision for integers, this doesn't handle the > case of > KUNIT_EXPECT_STREQ(test, "hello", "world") > since we don't expect it to realistically happen in checked in tests. > (If you really wanted a test to fail, KUNIT_FAIL("msg") exists) > > In that case, you'd get: > >Expected "hello" == "world", but > > > Signed-off-by: Daniel Latypov > >>> > >>> Reviewed-by: Brendan Higgins > >>> > >> > >> Hi Daniel, > >> > >> Please run checkpatch on your patches in the future. I am seeing > >> a few checkpatch readability type improvements that can be made. > >> > >> Please make changes and send v2 with Brendan's Reviewed-by. > > > > Are there some flags you'd like me to pass to checkpatch? > > > > $ ./scripts/checkpatch.pl --git HEAD > > total: 0 errors, 0 warnings, 42 lines checked > > > > My commit script uses --strict which shows readability errors. Oh neat, TIL. I'll make sure to use that in the future, thanks! v2: https://lore.kernel.org/linux-kselftest/20210402193357.819176-1-dlaty...@google.com/ > > > Commit f66884e8b831 ("kunit: make KUNIT_EXPECT_STREQ() quote values, > > don't print literals") has no obvious style problems and is ready for > > submission. > > > > I just rebased onto linus/master again since I know checkpatch.pl's > > default behavior had changed recently, but I didn't see any errors > > there. > > > > I know this commit made some lines go just over 80 characters, so > > $ ./scripts/checkpatch.pl --max-line-length=80 --git HEAD > > ... > > total: 0 errors, 4 warnings, 42 lines checked > > > > Don't worry about line wrap warns. I just ignore them. :) > > thanks, > -- Shuah > > >
Re: [PATCH] kunit: make KUNIT_EXPECT_STREQ() quote values, don't print literals
On 4/2/21 1:09 PM, Daniel Latypov wrote: On Fri, Apr 2, 2021 at 10:47 AM Shuah Khan wrote: On 4/2/21 3:35 AM, Brendan Higgins wrote: On Fri, Feb 5, 2021 at 2:18 PM Daniel Latypov wrote: Before: Expected str == "world", but str == hello "world" == world After: Expected str == "world", but str == "hello" Note: like the literal ellision for integers, this doesn't handle the case of KUNIT_EXPECT_STREQ(test, "hello", "world") since we don't expect it to realistically happen in checked in tests. (If you really wanted a test to fail, KUNIT_FAIL("msg") exists) In that case, you'd get: Expected "hello" == "world", but Signed-off-by: Daniel Latypov Reviewed-by: Brendan Higgins Hi Daniel, Please run checkpatch on your patches in the future. I am seeing a few checkpatch readability type improvements that can be made. Please make changes and send v2 with Brendan's Reviewed-by. Are there some flags you'd like me to pass to checkpatch? $ ./scripts/checkpatch.pl --git HEAD total: 0 errors, 0 warnings, 42 lines checked My commit script uses --strict which shows readability errors. Commit f66884e8b831 ("kunit: make KUNIT_EXPECT_STREQ() quote values, don't print literals") has no obvious style problems and is ready for submission. I just rebased onto linus/master again since I know checkpatch.pl's default behavior had changed recently, but I didn't see any errors there. I know this commit made some lines go just over 80 characters, so $ ./scripts/checkpatch.pl --max-line-length=80 --git HEAD ... total: 0 errors, 4 warnings, 42 lines checked Don't worry about line wrap warns. I just ignore them. :) thanks, -- Shuah
Re: [PATCH] kunit: make KUNIT_EXPECT_STREQ() quote values, don't print literals
On Fri, Apr 2, 2021 at 10:47 AM Shuah Khan wrote: > > On 4/2/21 3:35 AM, Brendan Higgins wrote: > > On Fri, Feb 5, 2021 at 2:18 PM Daniel Latypov wrote: > >> > >> Before: > >>> Expected str == "world", but > >>> str == hello > >>> "world" == world > >> > >> After: > >>> Expected str == "world", but > >>> str == "hello" > >> > >> > >> Note: like the literal ellision for integers, this doesn't handle the > >> case of > >>KUNIT_EXPECT_STREQ(test, "hello", "world") > >> since we don't expect it to realistically happen in checked in tests. > >> (If you really wanted a test to fail, KUNIT_FAIL("msg") exists) > >> > >> In that case, you'd get: > >>> Expected "hello" == "world", but > >> > >> > >> Signed-off-by: Daniel Latypov > > > > Reviewed-by: Brendan Higgins > > > > Hi Daniel, > > Please run checkpatch on your patches in the future. I am seeing > a few checkpatch readability type improvements that can be made. > > Please make changes and send v2 with Brendan's Reviewed-by. Are there some flags you'd like me to pass to checkpatch? $ ./scripts/checkpatch.pl --git HEAD total: 0 errors, 0 warnings, 42 lines checked Commit f66884e8b831 ("kunit: make KUNIT_EXPECT_STREQ() quote values, don't print literals") has no obvious style problems and is ready for submission. I just rebased onto linus/master again since I know checkpatch.pl's default behavior had changed recently, but I didn't see any errors there. I know this commit made some lines go just over 80 characters, so $ ./scripts/checkpatch.pl --max-line-length=80 --git HEAD ... total: 0 errors, 4 warnings, 42 lines checked I can go and line wrap these but had figured they were more readable this way if checkpatch.pl no longer complained by default. Thanks, Daniel > > thanks, > -- Shuah
Re: [PATCH] kunit: make KUNIT_EXPECT_STREQ() quote values, don't print literals
On 4/2/21 3:35 AM, Brendan Higgins wrote: On Fri, Feb 5, 2021 at 2:18 PM Daniel Latypov wrote: Before: Expected str == "world", but str == hello "world" == world After: Expected str == "world", but str == "hello" Note: like the literal ellision for integers, this doesn't handle the case of KUNIT_EXPECT_STREQ(test, "hello", "world") since we don't expect it to realistically happen in checked in tests. (If you really wanted a test to fail, KUNIT_FAIL("msg") exists) In that case, you'd get: Expected "hello" == "world", but Signed-off-by: Daniel Latypov Reviewed-by: Brendan Higgins Hi Daniel, Please run checkpatch on your patches in the future. I am seeing a few checkpatch readability type improvements that can be made. Please make changes and send v2 with Brendan's Reviewed-by. thanks, -- Shuah
Re: [PATCH] kunit: make KUNIT_EXPECT_STREQ() quote values, don't print literals
On Fri, Feb 5, 2021 at 2:18 PM Daniel Latypov wrote: > > Before: > > Expected str == "world", but > > str == hello > > "world" == world > > After: > > Expected str == "world", but > > str == "hello" > > > Note: like the literal ellision for integers, this doesn't handle the > case of > KUNIT_EXPECT_STREQ(test, "hello", "world") > since we don't expect it to realistically happen in checked in tests. > (If you really wanted a test to fail, KUNIT_FAIL("msg") exists) > > In that case, you'd get: > > Expected "hello" == "world", but > > > Signed-off-by: Daniel Latypov Reviewed-by: Brendan Higgins
[PATCH] kunit: make KUNIT_EXPECT_STREQ() quote values, don't print literals
Before: > Expected str == "world", but > str == hello > "world" == world After: > Expected str == "world", but > str == "hello" Note: like the literal ellision for integers, this doesn't handle the case of KUNIT_EXPECT_STREQ(test, "hello", "world") since we don't expect it to realistically happen in checked in tests. (If you really wanted a test to fail, KUNIT_FAIL("msg") exists) In that case, you'd get: > Expected "hello" == "world", but Signed-off-by: Daniel Latypov --- lib/kunit/assert.c | 30 -- 1 file changed, 24 insertions(+), 6 deletions(-) diff --git a/lib/kunit/assert.c b/lib/kunit/assert.c index e0ec7d6fed6f..176ef547fa94 100644 --- a/lib/kunit/assert.c +++ b/lib/kunit/assert.c @@ -156,6 +156,22 @@ void kunit_binary_ptr_assert_format(const struct kunit_assert *assert, } EXPORT_SYMBOL_GPL(kunit_binary_ptr_assert_format); +/* Checks if KUNIT_EXPECT_STREQ() args were string literals. + * Note: `text` will have ""s where as `value` will not. + */ +static bool is_str_literal(const char *text, const char *value) +{ + int len; + + len = strlen(text); + if (len < 2) + return false; + if (text[0] != '\"' || text[len-1] != '\"') + return false; + + return strncmp(text+1, value, len-2) == 0; +} + void kunit_binary_str_assert_format(const struct kunit_assert *assert, struct string_stream *stream) { @@ -168,12 +184,14 @@ void kunit_binary_str_assert_format(const struct kunit_assert *assert, binary_assert->left_text, binary_assert->operation, binary_assert->right_text); - string_stream_add(stream, KUNIT_SUBSUBTEST_INDENT "%s == %s\n", - binary_assert->left_text, - binary_assert->left_value); - string_stream_add(stream, KUNIT_SUBSUBTEST_INDENT "%s == %s", - binary_assert->right_text, - binary_assert->right_value); + if (!is_str_literal(binary_assert->left_text, binary_assert->left_value)) + string_stream_add(stream, KUNIT_SUBSUBTEST_INDENT "%s == \"%s\"\n", + binary_assert->left_text, + binary_assert->left_value); + if (!is_str_literal(binary_assert->right_text, binary_assert->right_value)) + string_stream_add(stream, KUNIT_SUBSUBTEST_INDENT "%s == \"%s\"", + binary_assert->right_text, + binary_assert->right_value); kunit_assert_print_msg(assert, stream); } EXPORT_SYMBOL_GPL(kunit_binary_str_assert_format); base-commit: 1e0d27fce010b0a4a9e595506b6ede75934c31be prerequisite-patch-id: 290f8022f30763cbfb6aec969b038a6f60a57482 -- 2.30.0.478.g8a0d178c01-goog