Before:
Expected str == "world", but str == hello "world" == world
After:
Expected str == "world", but str == "hello"
<we don't need to tell the user that "world" == "world">
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
<output for next failure>
Signed-off-by: Daniel Latypov dlatypov@google.com --- 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
On Fri, Feb 5, 2021 at 2:18 PM Daniel Latypov dlatypov@google.com wrote:
Before:
Expected str == "world", but str == hello "world" == world
After:
Expected str == "world", but str == "hello"
<we don't need to tell the user that "world" == "world">
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
<output for next failure>
Signed-off-by: Daniel Latypov dlatypov@google.com
Reviewed-by: Brendan Higgins brendanhiggins@google.com
On 4/2/21 3:35 AM, Brendan Higgins wrote:
On Fri, Feb 5, 2021 at 2:18 PM Daniel Latypov dlatypov@google.com wrote:
Before:
Expected str == "world", but str == hello "world" == world
After:
Expected str == "world", but str == "hello"
<we don't need to tell the user that "world" == "world">
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
<output for next failure>
Signed-off-by: Daniel Latypov dlatypov@google.com
Reviewed-by: Brendan Higgins brendanhiggins@google.com
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
On Fri, Apr 2, 2021 at 10:47 AM Shuah Khan skhan@linuxfoundation.org wrote:
On 4/2/21 3:35 AM, Brendan Higgins wrote:
On Fri, Feb 5, 2021 at 2:18 PM Daniel Latypov dlatypov@google.com wrote:
Before:
Expected str == "world", but str == hello "world" == world
After:
Expected str == "world", but str == "hello"
<we don't need to tell the user that "world" == "world">
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
<output for next failure>
Signed-off-by: Daniel Latypov dlatypov@google.com
Reviewed-by: Brendan Higgins brendanhiggins@google.com
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
On 4/2/21 1:09 PM, Daniel Latypov wrote:
On Fri, Apr 2, 2021 at 10:47 AM Shuah Khan skhan@linuxfoundation.org wrote:
On 4/2/21 3:35 AM, Brendan Higgins wrote:
On Fri, Feb 5, 2021 at 2:18 PM Daniel Latypov dlatypov@google.com wrote:
Before:
Expected str == "world", but str == hello "world" == world
After:
Expected str == "world", but str == "hello"
<we don't need to tell the user that "world" == "world">
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
<output for next failure>
Signed-off-by: Daniel Latypov dlatypov@google.com
Reviewed-by: Brendan Higgins brendanhiggins@google.com
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
On Fri, Apr 2, 2021 at 12:19 PM Shuah Khan skhan@linuxfoundation.org wrote:
On 4/2/21 1:09 PM, Daniel Latypov wrote:
On Fri, Apr 2, 2021 at 10:47 AM Shuah Khan skhan@linuxfoundation.org wrote:
On 4/2/21 3:35 AM, Brendan Higgins wrote:
On Fri, Feb 5, 2021 at 2:18 PM Daniel Latypov dlatypov@google.com wrote:
Before:
Expected str == "world", but str == hello "world" == world
After:
Expected str == "world", but str == "hello"
<we don't need to tell the user that "world" == "world">
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
<output for next failure>
Signed-off-by: Daniel Latypov dlatypov@google.com
Reviewed-by: Brendan Higgins brendanhiggins@google.com
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-dlatypov@goo...
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
linux-kselftest-mirror@lists.linaro.org
-
Brendan Higgins -
Daniel Latypov -
Shuah Khan