diff mbox series

kunit: log numbers in decimal and hex

Message ID 20221005175149.611068-1-mark.rutland@arm.com
State Accepted
Commit 7b1dd2cf06e1da9a0982937e82736daa6cd400ee
Headers show
Series kunit: log numbers in decimal and hex | expand

Commit Message

Mark Rutland Oct. 5, 2022, 5:51 p.m. UTC
When KUNIT_EXPECT_EQ() or KUNIT_ASSERT_EQ() log a failure, they log the
two values being compared, with numerical values logged in decimal.

In some cases, decimal output is painful to consume, and hexadecimal
output would be more helpful. For example, this is the case for tests
I'm currently developing for the arm64 insn encoding/decoding code,
where comparing two 32-bit instruction opcodes results in output such
as:

|  # test_insn_add_shifted_reg: EXPECTATION FAILED at arch/arm64/lib/test_insn.c:2791
|  Expected obj_insn == gen_insn, but
|      obj_insn == 2332164128
|      gen_insn == 1258422304

To make this easier to consume, this patch logs the values in both
decimal and hexadecimal:

|  # test_insn_add_shifted_reg: EXPECTATION FAILED at arch/arm64/lib/test_insn.c:2791
|  Expected obj_insn == gen_insn, but
|      obj_insn == 2332164128 (0x8b020020)
|      gen_insn == 1258422304 (0x4b020020)

As can be seen from the example, having hexadecimal makes it
significantly easier for a human to spot which specific bits are
incorrect.

Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Cc: Brendan Higgins <brendan.higgins@linux.dev>
Cc: David Gow <davidgow@google.com>
Cc: linux-kselftest@vger.kernel.org
Cc: kunit-dev@googlegroups.com
---
 lib/kunit/assert.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

Comments

David Gow Oct. 15, 2022, 4:04 a.m. UTC | #1
On Thu, Oct 6, 2022 at 1:52 AM Mark Rutland <mark.rutland@arm.com> wrote:
>
> When KUNIT_EXPECT_EQ() or KUNIT_ASSERT_EQ() log a failure, they log the
> two values being compared, with numerical values logged in decimal.
>
> In some cases, decimal output is painful to consume, and hexadecimal
> output would be more helpful. For example, this is the case for tests
> I'm currently developing for the arm64 insn encoding/decoding code,
> where comparing two 32-bit instruction opcodes results in output such
> as:
>
> |  # test_insn_add_shifted_reg: EXPECTATION FAILED at arch/arm64/lib/test_insn.c:2791
> |  Expected obj_insn == gen_insn, but
> |      obj_insn == 2332164128
> |      gen_insn == 1258422304
>
> To make this easier to consume, this patch logs the values in both
> decimal and hexadecimal:
>
> |  # test_insn_add_shifted_reg: EXPECTATION FAILED at arch/arm64/lib/test_insn.c:2791
> |  Expected obj_insn == gen_insn, but
> |      obj_insn == 2332164128 (0x8b020020)
> |      gen_insn == 1258422304 (0x4b020020)
>
> As can be seen from the example, having hexadecimal makes it
> significantly easier for a human to spot which specific bits are
> incorrect.
>
> Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> Cc: Brendan Higgins <brendan.higgins@linux.dev>
> Cc: David Gow <davidgow@google.com>
> Cc: linux-kselftest@vger.kernel.org
> Cc: kunit-dev@googlegroups.com
> ---

Thanks very much: this will definitely be useful. I tend to agree with
Daniel that this could clutter things up a bit, but I think the other
options (a separate ASSERT_EQ_HEX() macro, or a heuristic to remove it
for, e.g., values <= 9) add more complexity than benefit there.

So let's go with this as-is.

Reviewed-by: David Gow <davidgow@google.com>

Cheers,
-- David

>  lib/kunit/assert.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/lib/kunit/assert.c b/lib/kunit/assert.c
> index d00d6d181ee8..24dec5b48722 100644
> --- a/lib/kunit/assert.c
> +++ b/lib/kunit/assert.c
> @@ -127,13 +127,15 @@ void kunit_binary_assert_format(const struct kunit_assert *assert,
>                           binary_assert->text->right_text);
>         if (!is_literal(stream->test, binary_assert->text->left_text,
>                         binary_assert->left_value, stream->gfp))
> -               string_stream_add(stream, KUNIT_SUBSUBTEST_INDENT "%s == %lld\n",
> +               string_stream_add(stream, KUNIT_SUBSUBTEST_INDENT "%s == %lld (0x%llx)\n",
>                                   binary_assert->text->left_text,
> +                                 binary_assert->left_value,
>                                   binary_assert->left_value);
>         if (!is_literal(stream->test, binary_assert->text->right_text,
>                         binary_assert->right_value, stream->gfp))
> -               string_stream_add(stream, KUNIT_SUBSUBTEST_INDENT "%s == %lld",
> +               string_stream_add(stream, KUNIT_SUBSUBTEST_INDENT "%s == %lld (0x%llx)",
>                                   binary_assert->text->right_text,
> +                                 binary_assert->right_value,
>                                   binary_assert->right_value);
>         kunit_assert_print_msg(message, stream);
>  }
> --
> 2.30.2
>
diff mbox series

Patch

diff --git a/lib/kunit/assert.c b/lib/kunit/assert.c
index d00d6d181ee8..24dec5b48722 100644
--- a/lib/kunit/assert.c
+++ b/lib/kunit/assert.c
@@ -127,13 +127,15 @@  void kunit_binary_assert_format(const struct kunit_assert *assert,
 			  binary_assert->text->right_text);
 	if (!is_literal(stream->test, binary_assert->text->left_text,
 			binary_assert->left_value, stream->gfp))
-		string_stream_add(stream, KUNIT_SUBSUBTEST_INDENT "%s == %lld\n",
+		string_stream_add(stream, KUNIT_SUBSUBTEST_INDENT "%s == %lld (0x%llx)\n",
 				  binary_assert->text->left_text,
+				  binary_assert->left_value,
 				  binary_assert->left_value);
 	if (!is_literal(stream->test, binary_assert->text->right_text,
 			binary_assert->right_value, stream->gfp))
-		string_stream_add(stream, KUNIT_SUBSUBTEST_INDENT "%s == %lld",
+		string_stream_add(stream, KUNIT_SUBSUBTEST_INDENT "%s == %lld (0x%llx)",
 				  binary_assert->text->right_text,
+				  binary_assert->right_value,
 				  binary_assert->right_value);
 	kunit_assert_print_msg(message, stream);
 }