Re: [edk2-devel] [PATCH] UnitTestFrameworkPkg/UnitTestLib: Print expected Status on ASSERT fail

["Michael D Kinney" <michael.d.kinney@intel.com>]

Hi Jeshuas,

This is a good idea to show the expected value.

%p was used on purpose because unit tests can be built for 32-bit or 64-bit and
the EFI_STATUS is same as RETURN_STATUS which is same as UINTN.  UINTN is 32-bits
for 32-bit unit test apps and 64-bit for 64-bit unit test apps.  %p prints a 
pointer sized value, which happens to match the UINTN for support CPU archs.

A couple options to consider:
1) Keep using %p instead of %llx.
2) Use UINT64 local variables to hold Status and Expected values and use %llx.
3) Use the MdePkg PrintLib to convert Status and Expected to string names and
   update message to show the name of the status value instead of the hex value.
4) Don't add a dependency on PrintLib and instead convert to string names in
   this same C file.

Best regards,

Mike


> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Jeshua Smith via groups.io
> Sent: Wednesday, November 30, 2022 12:39 PM
> To: devel@edk2.groups.io
> Cc: Kinney, Michael D <michael.d.kinney@intel.com>; mikuback@linux.microsoft.com; sean.brogan@microsoft.com; Jeshua Smith
> <jeshuas@nvidia.com>
> Subject: [edk2-devel] [PATCH] UnitTestFrameworkPkg/UnitTestLib: Print expected Status on ASSERT fail
> 
> Update the UnitTestAssertStatusEqual error message to print out the
> expected value in addition to the seen value.
> 
> Signed-off-by: Jeshua Smith <jeshuas@nvidia.com>
> ---
>  UnitTestFrameworkPkg/Library/UnitTestLib/AssertCmocka.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/UnitTestFrameworkPkg/Library/UnitTestLib/AssertCmocka.c
> b/UnitTestFrameworkPkg/Library/UnitTestLib/AssertCmocka.c
> index dc05bbd438..322daf318a 100644
> --- a/UnitTestFrameworkPkg/Library/UnitTestLib/AssertCmocka.c
> +++ b/UnitTestFrameworkPkg/Library/UnitTestLib/AssertCmocka.c
> @@ -290,7 +290,7 @@ UnitTestAssertStatusEqual (
>  {
>    CHAR8  TempStr[MAX_STRING_SIZE];
> 
> -  snprintf (TempStr, sizeof (TempStr), "UT_ASSERT_STATUS_EQUAL(%s:%p)", Description, (VOID *)Status);
> +  snprintf (TempStr, sizeof (TempStr), "UT_ASSERT_STATUS_EQUAL(%s:0x%llx expected:0x%llx)", Description, Status,
> Expected);
>    _assert_true ((Status == Expected), TempStr, FileName, (INT32)LineNumber);
> 
>    return (Status == Expected);
> --
> 2.25.1
> 
> 
> 
> 
>