Re: [edk2-devel] [PATCH 2/2] MdePkg/Test: Add google tests for BaseLib

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

With one comment below addressed

Reviewed-by: Michael D Kinney <michael.d.kinney@intel.com>

> -----Original Message-----
> From: Pedro Falcato <pedro.falcato@gmail.com>
> Sent: Wednesday, November 29, 2023 6:46 PM
> To: devel@edk2.groups.io
> Cc: Savva Mitrofanov <savvamtr@gmail.com>; Pedro Falcato
> <pedro.falcato@gmail.com>; Gao, Liming <gaoliming@byosoft.com.cn>; Kinney,
> Michael D <michael.d.kinney@intel.com>; Liu, Zhiguang
> <zhiguang.liu@intel.com>
> Subject: [PATCH 2/2] MdePkg/Test: Add google tests for BaseLib
> 
> Add GoogleTestBaseLib, which contains gtest unit tests for BaseLib.
> For now, only add checksum tests for CRC32C and CRC16; these tests check
> for correctness on various inputs using precomputed hashes.
> 
> Signed-off-by: Pedro Falcato <pedro.falcato@gmail.com>
> Cc: Liming Gao <gaoliming@byosoft.com.cn>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Cc: Zhiguang Liu <zhiguang.liu@intel.com>
> ---
>  .../Library/BaseLib/GoogleTestBaseLib.inf     | 31 +++++++++
>  .../Library/BaseLib/TestBaseLibMain.cpp       | 23 +++++++
>  .../Library/BaseLib/TestCheckSum.cpp          | 64 +++++++++++++++++++
>  .../SafeIntLibUintnIntnUnitTests64.cpp        |  4 +-
>  MdePkg/Test/MdePkgHostTest.dsc                |  5 ++
>  5 files changed, 125 insertions(+), 2 deletions(-)
>  create mode 100644
> MdePkg/Test/GoogleTest/Library/BaseLib/GoogleTestBaseLib.inf
>  create mode 100644
> MdePkg/Test/GoogleTest/Library/BaseLib/TestBaseLibMain.cpp
>  create mode 100644
> MdePkg/Test/GoogleTest/Library/BaseLib/TestCheckSum.cpp
> 
> diff --git a/MdePkg/Test/GoogleTest/Library/BaseLib/GoogleTestBaseLib.inf
> b/MdePkg/Test/GoogleTest/Library/BaseLib/GoogleTestBaseLib.inf
> new file mode 100644
> index 000000000000..c859e5f86b9e
> --- /dev/null
> +++ b/MdePkg/Test/GoogleTest/Library/BaseLib/GoogleTestBaseLib.inf
> @@ -0,0 +1,31 @@
> +## @file
> +# Host OS based Application that unit tests BaseLib using Google Test
> +#
> +# Copyright (c) 2023, Pedro Falcato. All rights reserved.
> +# SPDX-License-Identifier: BSD-2-Clause-Patent
> +##
> +
> +[Defines]
> +  INF_VERSION     = 0x00010005
> +  BASE_NAME       = GoogleTestBaseLib
> +  FILE_GUID       = 34D8CBBA-2442-455F-8454-5B06B12A8B62
> +  MODULE_TYPE     = HOST_APPLICATION
> +  VERSION_STRING  = 1.0
> +
> +#
> +# The following information is for reference only and not required by the
> build tools.
> +#
> +#  VALID_ARCHITECTURES           = IA32 X64
> +#
> +
> +[Sources]
> +  TestCheckSum.cpp
> +  TestBaseLibMain.cpp
> +
> +[Packages]
> +  MdePkg/MdePkg.dec
> +  UnitTestFrameworkPkg/UnitTestFrameworkPkg.dec
> +
> +[LibraryClasses]
> +  GoogleTestLib
> +  BaseLib
> diff --git a/MdePkg/Test/GoogleTest/Library/BaseLib/TestBaseLibMain.cpp
> b/MdePkg/Test/GoogleTest/Library/BaseLib/TestBaseLibMain.cpp
> new file mode 100644
> index 000000000000..1a9941492be6
> --- /dev/null
> +++ b/MdePkg/Test/GoogleTest/Library/BaseLib/TestBaseLibMain.cpp
> @@ -0,0 +1,23 @@
> +/** @file
> +  Main routine for BaseLib google tests.
> +
> +  Copyright (c) 2023 Pedro Falcato. All rights reserved<BR>
> +  SPDX-License-Identifier: BSD-2-Clause-Patent
> +**/
> +
> +#include <gtest/gtest.h>
> +
> +// Note: Until we can --whole-archive libs, we're forced to include
> secondary files from the main one.
> +// Yuck.

Though I agree with this comment, it does not need to be in the source
code.

Not sure I understand how --whole-archive can help, so please start 
a new discussion or enter a BZ with the details.

> +
> +#include "TestCheckSum.cpp"
> +
> +int
> +main (
> +  int   argc,
> +  char  *argv[]
> +  )
> +{
> +  testing::InitGoogleTest (&argc, argv);
> +  return RUN_ALL_TESTS ();
> +}
> diff --git a/MdePkg/Test/GoogleTest/Library/BaseLib/TestCheckSum.cpp
> b/MdePkg/Test/GoogleTest/Library/BaseLib/TestCheckSum.cpp
> new file mode 100644
> index 000000000000..fa97f11dfa9c
> --- /dev/null
> +++ b/MdePkg/Test/GoogleTest/Library/BaseLib/TestCheckSum.cpp
> @@ -0,0 +1,64 @@
> +/** @file
> +  Unit tests for BaseLib's checksum capabilities.
> +
> +  Copyright (c) 2023 Pedro Falcato. All rights reserved<BR>
> +  SPDX-License-Identifier: BSD-2-Clause-Patent
> +**/
> +
> +#include <gtest/gtest.h>
> +extern "C" {
> +#include <Base.h>
> +#include <Library/BaseLib.h>
> +}
> +
> +// Precomputed crc32c and crc16-ansi for "hello" (without the null byte)
> +constexpr STATIC UINT32  mHelloCrc32c = 0x9A71BB4C;
> +constexpr STATIC UINT16  mHelloCrc16  = 0x34F6;
> +
> +TEST (Crc32c, BasicCheck) {
> +  // Note: The magic numbers below are precomputed checksums
> +  // Check for basic operation on even and odd numbers of bytes
> +  EXPECT_EQ (CalculateCrc32c ("hello", 5, 0), mHelloCrc32c);
> +  EXPECT_EQ (
> +    CalculateCrc32c ("longlonglonglonglong", sizeof
> ("longlonglonglonglong") - 1, 0),
> +    0xC50F869D
> +    );
> +  EXPECT_EQ (CalculateCrc32c ("h", 1, 0), 0xB96298FC);
> +
> +  // Check if a checksum with no bytes correctly yields 0
> +  EXPECT_EQ (CalculateCrc32c ("", 0, 0), 0U);
> +}
> +
> +TEST (Crc32c, MultipartCheck) {
> +  // Test multi-part crc32c calculation. So that given a string of bytes
> +  // s[N], crc32c(s, N, 0) == crc32c(s[N - 1], 1, crc32c(s, N - 1, 0))
> +  // and all other sorts of combinations one might imagine.
> +  UINT32  val;
> +
> +  val = CalculateCrc32c ("hel", 3, 0);
> +  EXPECT_EQ (CalculateCrc32c (&"hello"[3], 2, val), mHelloCrc32c);
> +}
> +
> +TEST (Crc16, BasicCheck) {
> +  // Note: The magic numbers below are precomputed checksums
> +  // Check for basic operation on even and odd numbers of bytes
> +  EXPECT_EQ (CalculateCrc16Ansi ("hello", 5, CRC16ANSI_INIT),
> mHelloCrc16);
> +  EXPECT_EQ (
> +    CalculateCrc16Ansi ("longlonglonglonglong", sizeof
> ("longlonglonglonglong") - 1, CRC16ANSI_INIT),
> +    0xF723
> +    );
> +  EXPECT_EQ (CalculateCrc16Ansi ("h", 1, CRC16ANSI_INIT), 0xAEBE);
> +
> +  // Check if a checksum with no bytes correctly yields CRC16ANSI_INIT
> +  EXPECT_EQ (CalculateCrc16Ansi ("", 0, CRC16ANSI_INIT), CRC16ANSI_INIT);
> +}
> +
> +TEST (Crc16, MultipartCheck) {
> +  // Test multi-part crc16 calculation. So that given a string of bytes
> +  // s[N], crc16(s, N, 0) == crc16(s[N - 1], 1, crc16(s, N - 1, 0))
> +  // and all other sorts of combinations one might imagine.
> +  UINT16  val;
> +
> +  val = CalculateCrc16Ansi ("hel", 3, CRC16ANSI_INIT);
> +  EXPECT_EQ (CalculateCrc16Ansi (&"hello"[3], 2, val), mHelloCrc16);
> +}
> diff --git
> a/MdePkg/Test/GoogleTest/Library/BaseSafeIntLib/SafeIntLibUintnIntnUnitTes
> ts64.cpp
> b/MdePkg/Test/GoogleTest/Library/BaseSafeIntLib/SafeIntLibUintnIntnUnitTes
> ts64.cpp
> index 6efdd3be7c5e..9fc60c5c71f3 100644
> ---
> a/MdePkg/Test/GoogleTest/Library/BaseSafeIntLib/SafeIntLibUintnIntnUnitTes
> ts64.cpp
> +++
> b/MdePkg/Test/GoogleTest/Library/BaseSafeIntLib/SafeIntLibUintnIntnUnitTes
> ts64.cpp
> @@ -10,8 +10,8 @@
> 
>  #include <gtest/gtest.h>
>  extern "C" {
> -  #include <Base.h>
> -  #include <Library/SafeIntLib.h>
> +#include <Base.h>
> +#include <Library/SafeIntLib.h>
>  }
> 
>  TEST (ConversionTestSuite, TestSafeInt32ToUintn) {
> diff --git a/MdePkg/Test/MdePkgHostTest.dsc
> b/MdePkg/Test/MdePkgHostTest.dsc
> index b92b564d434a..583f8fc0ddd8 100644
> --- a/MdePkg/Test/MdePkgHostTest.dsc
> +++ b/MdePkg/Test/MdePkgHostTest.dsc
> @@ -20,6 +20,7 @@
>  !include UnitTestFrameworkPkg/UnitTestFrameworkPkgHost.dsc.inc
> 
>  [LibraryClasses]
> +  BaseLib|MdePkg/Library/BaseLib/BaseLib.inf
>    SafeIntLib|MdePkg/Library/BaseSafeIntLib/BaseSafeIntLib.inf
> 
> DevicePathLib|MdePkg/Library/UefiDevicePathLib/UefiDevicePathLibBase.inf
> 
> @@ -31,6 +32,10 @@
>    MdePkg/Test/UnitTest/Library/BaseLib/BaseLibUnitTestsHost.inf
> 
> MdePkg/Test/GoogleTest/Library/BaseSafeIntLib/GoogleTestBaseSafeIntLib.inf
>    MdePkg/Test/UnitTest/Library/DevicePathLib/TestDevicePathLibHost.inf
> +  #
> +  # BaseLib tests
> +  #
> +  MdePkg/Test/GoogleTest/Library/BaseLib/GoogleTestBaseLib.inf
> 
>    #
>    # Build HOST_APPLICATION Libraries
> --
> 2.43.0



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111911): https://edk2.groups.io/g/devel/message/111911
Mute This Topic: https://groups.io/mt/102886794/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/1913456212/xyzzy [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-