__EXPECT() currently prints the stringified arguments on failure but not their value, which is not very helpful.
Let's use the same message as the original macro in kselftest_harness.h, printing the values too. For simplicity's sake they are always printed as signed, which is generally appropriate (only 64-bit unsigned values with the top bit set are an issue, and these are rarely encountered).
Signed-off-by: Kevin Brodsky kevin.brodsky@arm.com --- tools/testing/selftests/arm64/morello/freestanding.h | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/tools/testing/selftests/arm64/morello/freestanding.h b/tools/testing/selftests/arm64/morello/freestanding.h index 2beb52eafae1..8dab905d2b54 100644 --- a/tools/testing/selftests/arm64/morello/freestanding.h +++ b/tools/testing/selftests/arm64/morello/freestanding.h @@ -91,8 +91,12 @@ int __attribute__ ((format(printf, 1, 2))) printf(const char *fmt, ...); /* this macro emulates its harness counterpart but is not API compatible */ #define __EXPECT(exp, seen, op, exit_on_fail) \ do { \ - if (!((exp) op (seen))) { \ - __TH_LOG_ERROR("'(%s) %s (%s)' was false", #exp, #op, #seen); \ + __typeof__(exp) __exp = (exp); \ + __typeof__(seen) __seen = (seen); \ + if (!((__exp) op (__seen))) { \ + __TH_LOG_ERROR("Expected %s (%lld) %s %s (%lld)", \ + #exp, (long long)__exp, #op, \ + #seen, (long long)__seen); \ __cur_test->message = 1; \ } \ } while (0); \
On 13/10/2023 10:02, Kevin Brodsky wrote:
__EXPECT() currently prints the stringified arguments on failure but not their value, which is not very helpful.
Let's use the same message as the original macro in kselftest_harness.h, printing the values too. For simplicity's sake they are always printed as signed, which is generally appropriate (only 64-bit unsigned values with the top bit set are an issue, and these are rarely encountered).
I guess that will be pretty rare. Although since the code to output the correct signedness is already there[1] it's not *that* much more complicated. Maybe it will save some confusion in future and always correct seems better than mostly correct :)
1: i.e. the switch case in kselftest_harness.h __EXPECT def: switch (is_signed_type(__exp) * 2 + is_signed_type(__seen)) { [...]
Best, Zach
Signed-off-by: Kevin Brodsky kevin.brodsky@arm.com
tools/testing/selftests/arm64/morello/freestanding.h | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/tools/testing/selftests/arm64/morello/freestanding.h b/tools/testing/selftests/arm64/morello/freestanding.h index 2beb52eafae1..8dab905d2b54 100644 --- a/tools/testing/selftests/arm64/morello/freestanding.h +++ b/tools/testing/selftests/arm64/morello/freestanding.h @@ -91,8 +91,12 @@ int __attribute__ ((format(printf, 1, 2))) printf(const char *fmt, ...); /* this macro emulates its harness counterpart but is not API compatible */ #define __EXPECT(exp, seen, op, exit_on_fail) \ do { \
if (!((exp) op (seen))) { \__TH_LOG_ERROR("'(%s) %s (%s)' was false", #exp, #op, #seen); \
__typeof__(exp) __exp = (exp); \__typeof__(seen) __seen = (seen); \if (!((__exp) op (__seen))) { \__TH_LOG_ERROR("Expected %s (%lld) %s %s (%lld)", \#exp, (long long)__exp, #op, \#seen, (long long)__seen); \ __cur_test->message = 1; \
On 19/10/2023 18:28, Zachary Leaf wrote:
On 13/10/2023 10:02, Kevin Brodsky wrote:
__EXPECT() currently prints the stringified arguments on failure but not their value, which is not very helpful.
Let's use the same message as the original macro in kselftest_harness.h, printing the values too. For simplicity's sake they are always printed as signed, which is generally appropriate (only 64-bit unsigned values with the top bit set are an issue, and these are rarely encountered).
I guess that will be pretty rare. Although since the code to output the correct signedness is already there[1] it's not *that* much more complicated. Maybe it will save some confusion in future and always correct seems better than mostly correct :)
1: i.e. the switch case in kselftest_harness.h __EXPECT def: switch (is_signed_type(__exp) * 2 + is_signed_type(__seen)) { [...]
Sure, I'm aware, that was my starting point. Still I don't think the complication is really justified, worst case you get a large negative value that can easily be converted back. (When is the MSB ever set in practice?) I hope we can just switch to kselftest_harness.h at some point.
Kevin
Best, Zach
Signed-off-by: Kevin Brodsky kevin.brodsky@arm.com
tools/testing/selftests/arm64/morello/freestanding.h | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/tools/testing/selftests/arm64/morello/freestanding.h b/tools/testing/selftests/arm64/morello/freestanding.h index 2beb52eafae1..8dab905d2b54 100644 --- a/tools/testing/selftests/arm64/morello/freestanding.h +++ b/tools/testing/selftests/arm64/morello/freestanding.h @@ -91,8 +91,12 @@ int __attribute__ ((format(printf, 1, 2))) printf(const char *fmt, ...); /* this macro emulates its harness counterpart but is not API compatible */ #define __EXPECT(exp, seen, op, exit_on_fail) \ do { \
if (!((exp) op (seen))) { \__TH_LOG_ERROR("'(%s) %s (%s)' was false", #exp, #op, #seen); \
__typeof__(exp) __exp = (exp); \__typeof__(seen) __seen = (seen); \if (!((__exp) op (__seen))) { \__TH_LOG_ERROR("Expected %s (%lld) %s %s (%lld)", \#exp, (long long)__exp, #op, \#seen, (long long)__seen); \ __cur_test->message = 1; \
On 20/10/2023 08:03, Kevin Brodsky wrote:
On 19/10/2023 18:28, Zachary Leaf wrote:
On 13/10/2023 10:02, Kevin Brodsky wrote:
__EXPECT() currently prints the stringified arguments on failure but not their value, which is not very helpful.
Let's use the same message as the original macro in kselftest_harness.h, printing the values too. For simplicity's sake they are always printed as signed, which is generally appropriate (only 64-bit unsigned values with the top bit set are an issue, and these are rarely encountered).
I guess that will be pretty rare. Although since the code to output the correct signedness is already there[1] it's not *that* much more complicated. Maybe it will save some confusion in future and always correct seems better than mostly correct :)
1: i.e. the switch case in kselftest_harness.h __EXPECT def: switch (is_signed_type(__exp) * 2 + is_signed_type(__seen)) { [...]
Sure, I'm aware, that was my starting point. Still I don't think the complication is really justified, worst case you get a large negative value that can easily be converted back. (When is the MSB ever set in practice?)
Maybe in a bitset? Then you probably would be converting what ever is printed anyway. Okay, EXPECT'ing large numbers is rare. I guess you would recognise the value looks "off". I just would choose 100% correct over 99.9% if we have the choice :D
I get that printing the value isn't exactly critical though, so I'm okay with it. LGTM.
I hope we can just switch to kselftest_harness.h at some point.
What would be required for that, #ifdef'ing the Morello stuff in there?
Thanks, Zach
Kevin
Best, Zach
Signed-off-by: Kevin Brodsky kevin.brodsky@arm.com
tools/testing/selftests/arm64/morello/freestanding.h | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/tools/testing/selftests/arm64/morello/freestanding.h b/tools/testing/selftests/arm64/morello/freestanding.h index 2beb52eafae1..8dab905d2b54 100644 --- a/tools/testing/selftests/arm64/morello/freestanding.h +++ b/tools/testing/selftests/arm64/morello/freestanding.h @@ -91,8 +91,12 @@ int __attribute__ ((format(printf, 1, 2))) printf(const char *fmt, ...); /* this macro emulates its harness counterpart but is not API compatible */ #define __EXPECT(exp, seen, op, exit_on_fail) \ do { \
if (!((exp) op (seen))) { \__TH_LOG_ERROR("'(%s) %s (%s)' was false", #exp, #op, #seen); \
__typeof__(exp) __exp = (exp); \__typeof__(seen) __seen = (seen); \if (!((__exp) op (__seen))) { \__TH_LOG_ERROR("Expected %s (%lld) %s %s (%lld)", \#exp, (long long)__exp, #op, \#seen, (long long)__seen); \ __cur_test->message = 1; \
On 20/10/2023 09:45, Zachary Leaf wrote:
On 20/10/2023 08:03, Kevin Brodsky wrote:
On 19/10/2023 18:28, Zachary Leaf wrote:
On 13/10/2023 10:02, Kevin Brodsky wrote:
__EXPECT() currently prints the stringified arguments on failure but not their value, which is not very helpful.
Let's use the same message as the original macro in kselftest_harness.h, printing the values too. For simplicity's sake they are always printed as signed, which is generally appropriate (only 64-bit unsigned values with the top bit set are an issue, and these are rarely encountered).
I guess that will be pretty rare. Although since the code to output the correct signedness is already there[1] it's not *that* much more complicated. Maybe it will save some confusion in future and always correct seems better than mostly correct :)
1: i.e. the switch case in kselftest_harness.h __EXPECT def: switch (is_signed_type(__exp) * 2 + is_signed_type(__seen)) { [...]
Sure, I'm aware, that was my starting point. Still I don't think the complication is really justified, worst case you get a large negative value that can easily be converted back. (When is the MSB ever set in practice?)
Maybe in a bitset? Then you probably would be converting what ever is printed anyway. Okay, EXPECT'ing large numbers is rare. I guess you would recognise the value looks "off". I just would choose 100% correct over 99.9% if we have the choice :D
I get that printing the value isn't exactly critical though, so I'm okay with it. LGTM.
Cool, thanks for chiming in either way. Now applied on next.
I hope we can just switch to kselftest_harness.h at some point.
What would be required for that, #ifdef'ing the Morello stuff in there?
It's the same issue as the rest, i.e. kselftest_harness.h uses standard (libc) headers. Once we get to the point of building the kselftests normally in purecap, it should be easy to build the Morello ones against libc too.
Kevin
Thanks, Zach
Kevin
Best, Zach
Signed-off-by: Kevin Brodsky kevin.brodsky@arm.com
tools/testing/selftests/arm64/morello/freestanding.h | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/tools/testing/selftests/arm64/morello/freestanding.h b/tools/testing/selftests/arm64/morello/freestanding.h index 2beb52eafae1..8dab905d2b54 100644 --- a/tools/testing/selftests/arm64/morello/freestanding.h +++ b/tools/testing/selftests/arm64/morello/freestanding.h @@ -91,8 +91,12 @@ int __attribute__ ((format(printf, 1, 2))) printf(const char *fmt, ...); /* this macro emulates its harness counterpart but is not API compatible */ #define __EXPECT(exp, seen, op, exit_on_fail) \ do { \
if (!((exp) op (seen))) { \__TH_LOG_ERROR("'(%s) %s (%s)' was false", #exp, #op, #seen); \
__typeof__(exp) __exp = (exp); \__typeof__(seen) __seen = (seen); \if (!((__exp) op (__seen))) { \__TH_LOG_ERROR("Expected %s (%lld) %s %s (%lld)", \#exp, (long long)__exp, #op, \#seen, (long long)__seen); \ __cur_test->message = 1; \
linux-morello@op-lists.linaro.org
-
Kevin Brodsky -
Zachary Leaf