v1: https://patchwork.kernel.org/project/linux-kselftest/list/?series=881867 v2: keep numerical values (David)
Cc: David Gow davidgow@google.com Cc: Rae Moar rmoar@google.com
Michal Wajdeczko (3): kunit: Improve format of the NOT_ERR_OR_NULL assertion kunit: Improve format of the PTR_EQ|NE|NULL assertion kunit: Improve format of the KUNIT_EXPECT_EQ assertion
lib/kunit/assert.c | 71 +++++++++++++++++++++++++++++++++++++--------- 1 file changed, 57 insertions(+), 14 deletions(-)
Diagnostic message for failed KUNIT_ASSERT|EXPECT_NOT_ERR_OR_NULL shows only raw error code, like for this example:
void *myptr = ERR_PTR(-ENOMEM); KUNIT_EXPECT_NOT_ERR_OR_NULL(test, myptr);
we will get:
[ ] Expected myptr is not error, but is: -12
but we can improve it by using more friendly error pointer format:
[ ] Expected myptr is not error, but is -ENOMEM (-12)
Signed-off-by: Michal Wajdeczko michal.wajdeczko@intel.com Reviewed-by: David Gow davidgow@google.com --- v2: keep numerical error value (David,kunit-assert) --- lib/kunit/assert.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/lib/kunit/assert.c b/lib/kunit/assert.c index 867aa5c4bccf..aa3ae225f49f 100644 --- a/lib/kunit/assert.c +++ b/lib/kunit/assert.c @@ -77,13 +77,14 @@ void kunit_ptr_not_err_assert_format(const struct kunit_assert *assert, assert);
if (!ptr_assert->value) { - string_stream_add(stream, - KUNIT_SUBTEST_INDENT "Expected %s is not null, but is\n", + string_stream_add(stream, KUNIT_SUBTEST_INDENT + "Expected %s is not null, but is\n", ptr_assert->text); } else if (IS_ERR(ptr_assert->value)) { - string_stream_add(stream, - KUNIT_SUBTEST_INDENT "Expected %s is not error, but is: %ld\n", + string_stream_add(stream, KUNIT_SUBTEST_INDENT + "Expected %s is not error, but is %pe (%ld)\n", ptr_assert->text, + ptr_assert->value, PTR_ERR(ptr_assert->value)); } kunit_assert_print_msg(message, stream);
Diagnostic message for failed KUNIT_ASSERT|EXPECT_PTR_EQ|NE|NULL shows only raw pointer value, like for this example:
void *ptr1 = ERR_PTR(-ENOMEM); void *ptr2 = NULL; KUNIT_EXPECT_PTR_EQ(test, ptr1, ptr2); KUNIT_EXPECT_NULL(test, ptr1);
we will get:
[ ] Expected ptr1 == ptr2, but [ ] ptr1 == fffffffffffffff4 [ ] ptr2 == 0000000000000000 [ ] Expected ptr1 == ((void *)0), but [ ] ptr1 == ffffffffffffffe4 [ ] ((void *)0) == 0000000000000000
but we can improve this by detecting whether pointer was NULL or error, and use friendly error pointer format if possible:
[ ] Expected ptr1 == ptr2, but [ ] ptr1 == fffffffffffffff4 (-ENOMEM) [ ] ptr2 == 0000000000000000 (NULL) [ ] Expected ptr1 == ((void *)0), but [ ] ptr1 == fffffffffffffff4 (-ENOMEM) [ ] ((void *)0) == 0000000000000000 (NULL)
Signed-off-by: Michal Wajdeczko michal.wajdeczko@intel.com --- Cc: David Gow davidgow@google.com Cc: Rae Moar rmoar@google.com --- v2: keep '==' and raw value (David) --- lib/kunit/assert.c | 32 ++++++++++++++++++++++++++------ 1 file changed, 26 insertions(+), 6 deletions(-)
diff --git a/lib/kunit/assert.c b/lib/kunit/assert.c index aa3ae225f49f..414474841b61 100644 --- a/lib/kunit/assert.c +++ b/lib/kunit/assert.c @@ -156,12 +156,32 @@ void kunit_binary_ptr_assert_format(const struct kunit_assert *assert, binary_assert->text->left_text, binary_assert->text->operation, binary_assert->text->right_text); - string_stream_add(stream, KUNIT_SUBSUBTEST_INDENT "%s == %px\n", - binary_assert->text->left_text, - binary_assert->left_value); - string_stream_add(stream, KUNIT_SUBSUBTEST_INDENT "%s == %px", - binary_assert->text->right_text, - binary_assert->right_value); + if (!binary_assert->left_value) + string_stream_add(stream, KUNIT_SUBSUBTEST_INDENT "%s == %px (NULL)\n", + binary_assert->text->left_text, + binary_assert->left_value); + else if (IS_ERR(binary_assert->left_value)) + string_stream_add(stream, KUNIT_SUBSUBTEST_INDENT "%s == %px (%pe)\n", + binary_assert->text->left_text, + binary_assert->left_value, + binary_assert->left_value); + else + string_stream_add(stream, KUNIT_SUBSUBTEST_INDENT "%s == %px\n", + binary_assert->text->left_text, + binary_assert->left_value); + if (!binary_assert->right_value) + string_stream_add(stream, KUNIT_SUBSUBTEST_INDENT "%s == %px (NULL)\n", + binary_assert->text->right_text, + binary_assert->right_value); + else if (IS_ERR(binary_assert->right_value)) + string_stream_add(stream, KUNIT_SUBSUBTEST_INDENT "%s == %px (%pe)\n", + binary_assert->text->right_text, + binary_assert->right_value, + binary_assert->right_value); + else + string_stream_add(stream, KUNIT_SUBSUBTEST_INDENT "%s == %px\n", + binary_assert->text->right_text, + binary_assert->right_value); kunit_assert_print_msg(message, stream); } EXPORT_SYMBOL_GPL(kunit_binary_ptr_assert_format);
Diagnostic message for failed KUNIT_ASSERT|EXPECT_EQ shows in case of integers only raw values, like for this example:
bool flag; KUNIT_EXPECT_EQ(test, -ENODATA, kstrtobool("dunno", &flag));
we will get:
[ ] Expected -61 == kstrtobool("dunno", &flag), but [ ] kstrtobool("dunno", &flag) == -22 (0xffffffffffffffea)
but we can improve it if the value is within MAX_ERRNO range by using more friendly error format:
[ ] Expected -61 == kstrtobool("dunno", &flag), but [ ] -61 == 0xffffffffffffffc3 (-ENODATA) [ ] kstrtobool("dunno", &flag) == -22 (-EINVAL)
Signed-off-by: Michal Wajdeczko michal.wajdeczko@intel.com Reviewed-by: David Gow davidgow@google.com #v1 --- v2: keep '==' and raw values (David) use better example (Michal) --- lib/kunit/assert.c | 30 ++++++++++++++++++++++++++---- 1 file changed, 26 insertions(+), 4 deletions(-)
diff --git a/lib/kunit/assert.c b/lib/kunit/assert.c index 414474841b61..5f0a2e2f5231 100644 --- a/lib/kunit/assert.c +++ b/lib/kunit/assert.c @@ -129,15 +129,37 @@ void kunit_binary_assert_format(const struct kunit_assert *assert, binary_assert->text->operation, binary_assert->text->right_text); if (!is_literal(binary_assert->text->left_text, binary_assert->left_value)) - string_stream_add(stream, KUNIT_SUBSUBTEST_INDENT "%s == %lld (0x%llx)\n", + if (IS_ERR_VALUE(binary_assert->left_value)) + string_stream_add(stream, KUNIT_SUBSUBTEST_INDENT "%s == %lld (%pe)\n", + binary_assert->text->left_text, + binary_assert->left_value, + ERR_PTR(binary_assert->left_value)); + else + string_stream_add(stream, KUNIT_SUBSUBTEST_INDENT "%s == %lld (%#llx)\n", + binary_assert->text->left_text, + binary_assert->left_value, + binary_assert->left_value); + else if (IS_ERR_VALUE(binary_assert->left_value)) + string_stream_add(stream, KUNIT_SUBSUBTEST_INDENT "%s == %#llx (%pe)\n", binary_assert->text->left_text, binary_assert->left_value, - binary_assert->left_value); + ERR_PTR(binary_assert->left_value)); if (!is_literal(binary_assert->text->right_text, binary_assert->right_value)) - string_stream_add(stream, KUNIT_SUBSUBTEST_INDENT "%s == %lld (0x%llx)", + if (IS_ERR_VALUE(binary_assert->right_value)) + string_stream_add(stream, KUNIT_SUBSUBTEST_INDENT "%s == %lld (%pe)", + binary_assert->text->right_text, + binary_assert->right_value, + ERR_PTR(binary_assert->right_value)); + else + string_stream_add(stream, KUNIT_SUBSUBTEST_INDENT "%s == %lld (%#llx)", + binary_assert->text->right_text, + binary_assert->right_value, + binary_assert->right_value); + else if (IS_ERR_VALUE(binary_assert->right_value)) + string_stream_add(stream, KUNIT_SUBSUBTEST_INDENT "%s == %#llx (%pe)", binary_assert->text->right_text, binary_assert->right_value, - binary_assert->right_value); + ERR_PTR(binary_assert->right_value)); kunit_assert_print_msg(message, stream); } EXPORT_SYMBOL_GPL(kunit_binary_assert_format);
linux-kselftest-mirror@lists.linaro.org