Re: [edk2-devel] [PATCH v2 13/13] NetworkPkg: Update the PxeBcDhcp6GoogleTest due to underlying changes
Reviewed-by: Saloni Kasbekar -Original Message- From: Doug Flick Sent: Wednesday, May 8, 2024 10:57 PM To: devel@edk2.groups.io Cc: Kasbekar, Saloni ; Clark-williams, Zachary Subject: [PATCH v2 13/13] NetworkPkg: Update the PxeBcDhcp6GoogleTest due to underlying changes From: Doug Flick This patch updates the PxeBcDhcp6GoogleTest due to the changes in the underlying code. The changes are as follows: - Random now comes from the RngLib Protocol - The TCP ISN is now generated by the hash function Cc: Saloni Kasbekar Cc: Zachary Clark-williams Signed-off-by: Doug Flick [MSFT] --- NetworkPkg/Test/NetworkPkgHostTest.dsc| 1 + NetworkPkg/UefiPxeBcDxe/GoogleTest/UefiPxeBcDxeGoogleTest.inf | 3 +- NetworkPkg/UefiPxeBcDxe/GoogleTest/PxeBcDhcp6GoogleTest.cpp | 102 +++- 3 files changed, 100 insertions(+), 6 deletions(-) diff --git a/NetworkPkg/Test/NetworkPkgHostTest.dsc b/NetworkPkg/Test/NetworkPkgHostTest.dsc index fa301a7a52ab..1772afb05815 100644 --- a/NetworkPkg/Test/NetworkPkgHostTest.dsc +++ b/NetworkPkg/Test/NetworkPkgHostTest.dsc @@ -30,6 +30,7 @@ [Components] NetworkPkg/UefiPxeBcDxe/GoogleTest/UefiPxeBcDxeGoogleTest.inf { UefiRuntimeServicesTableLib|MdePkg/Test/Mock/Library/GoogleTest/MockUefiRuntimeServicesTableLib/MockUefiRuntimeServicesTableLib.inf+ UefiBootServicesTableLib|MdePkg/Test/Mock/Library/GoogleTest/MockUefiBootServicesTableLib/MockUefiBootServicesTableLib.inf } # Despite these library classes being listed in [LibraryClasses] below, they are not needed for the host-based unit tests.diff --git a/NetworkPkg/UefiPxeBcDxe/GoogleTest/UefiPxeBcDxeGoogleTest.inf b/NetworkPkg/UefiPxeBcDxe/GoogleTest/UefiPxeBcDxeGoogleTest.inf index 301dcdf61109..8b092d9291d4 100644 --- a/NetworkPkg/UefiPxeBcDxe/GoogleTest/UefiPxeBcDxeGoogleTest.inf +++ b/NetworkPkg/UefiPxeBcDxe/GoogleTest/UefiPxeBcDxeGoogleTest.inf @@ -14,7 +14,7 @@ [Defines] # # The following information is for reference only and not required by the build tools. #-# VALID_ARCHITECTURES = IA32 X64+# VALID_ARCHITECTURES = IA32 X64 AARCH64 # [Sources]@@ -23,6 +23,7 @@ [Sources] PxeBcDhcp6GoogleTest.h ../PxeBcDhcp6.c ../PxeBcSupport.c+ ../../../MdePkg/Test/Mock/Library/GoogleTest/Protocol/MockRng.cpp [Packages] MdePkg/MdePkg.decdiff --git a/NetworkPkg/UefiPxeBcDxe/GoogleTest/PxeBcDhcp6GoogleTest.cpp b/NetworkPkg/UefiPxeBcDxe/GoogleTest/PxeBcDhcp6GoogleTest.cpp index bd423ebadfce..61736ff79e83 100644 --- a/NetworkPkg/UefiPxeBcDxe/GoogleTest/PxeBcDhcp6GoogleTest.cpp +++ b/NetworkPkg/UefiPxeBcDxe/GoogleTest/PxeBcDhcp6GoogleTest.cpp @@ -7,6 +7,8 @@ #include #include #include +#include +#include extern "C" { #include @@ -165,7 +167,7 @@ protected: // Note: // Testing PxeBcHandleDhcp6Offer() is difficult because it depends on a // properly setup Private structure. Attempting to properly test this function-// without a signficant refactor is a fools errand. Instead, we will test+// without a significant refactor is a fools errand. Instead, we will test // that we can prevent an overflow in the function. TEST_F (PxeBcHandleDhcp6OfferTest, BasicUsageTest) { PXEBC_DHCP6_PACKET_CACHE *Cache6 = NULL;@@ -238,6 +240,7 @@ TEST_F (PxeBcCacheDnsServerAddressesTest, BasicUsageTest) { FreePool (Option); } }+ // Test Description // Test that we can prevent an overflow in the function TEST_F (PxeBcCacheDnsServerAddressesTest, AttemptOverflowTest) {@@ -470,10 +473,15 @@ TEST_F (PxeBcRequestBootServiceTest, AttemptRequestOverFlowExpectFailure) { class PxeBcDhcp6DiscoverTest : public ::testing::Test { public: PXEBC_PRIVATE_DATA Private = { 0 };+ // create a mock md5 hash+ UINT8 Md5Hash[16] = { 0 };+ EFI_UDP6_PROTOCOL Udp6Read; protected: MockUefiRuntimeServicesTableLib RtServicesMock;+ MockUefiBootServicesTableLib BsMock;+ MockRng RngMock;// Add any setup code if needed virtual void@@ -527,8 +535,21 @@ TEST_F (PxeBcDhcp6DiscoverTest, BasicOverflowTest) { Private.Dhcp6Request->Length = (UINT16)(Cursor - (UINT8 *)Private.Dhcp6Request); - EXPECT_CALL (RtServicesMock, gRT_GetTime)- .WillOnce (::testing::Return (0));+ EXPECT_CALL (BsMock, gBS_LocateProtocol)+ .WillOnce (+ ::testing::DoAll (+ ::testing::SetArgPointee<2> (::testing::ByRef (gRngProtocol)),+ ::testing::Return (EFI_SUCCESS)+)+ );++ EXPECT_CALL (RngMock, GetRng)+.WillOnce (+ ::testing::DoAll (+ ::testing::SetArgPointee<3> (::testing::ByRef (Md5Hash[0])),+ ::testing::Return (EFI_SUCCESS)+)+ ); ASSERT_EQ ( PxeBcDhcp6Discover (@@ -558,8 +579,21 @@ TEST_F (PxeBcDhcp6DiscoverTest, BasicUsageTest) { Private.Dhcp6Request->Length = (UINT16)(Cursor - (UINT8 *)Private.Dhcp6Request); - EXPECT_CALL (RtServicesMock, gRT_GetTime)- .WillOnce (::testing::Retu
[edk2-devel] [PATCH v2 13/13] NetworkPkg: Update the PxeBcDhcp6GoogleTest due to underlying changes
From: Doug Flick This patch updates the PxeBcDhcp6GoogleTest due to the changes in the underlying code. The changes are as follows: - Random now comes from the RngLib Protocol - The TCP ISN is now generated by the hash function Cc: Saloni Kasbekar Cc: Zachary Clark-williams Signed-off-by: Doug Flick [MSFT] --- NetworkPkg/Test/NetworkPkgHostTest.dsc| 1 + NetworkPkg/UefiPxeBcDxe/GoogleTest/UefiPxeBcDxeGoogleTest.inf | 3 +- NetworkPkg/UefiPxeBcDxe/GoogleTest/PxeBcDhcp6GoogleTest.cpp | 102 +++- 3 files changed, 100 insertions(+), 6 deletions(-) diff --git a/NetworkPkg/Test/NetworkPkgHostTest.dsc b/NetworkPkg/Test/NetworkPkgHostTest.dsc index fa301a7a52ab..1772afb05815 100644 --- a/NetworkPkg/Test/NetworkPkgHostTest.dsc +++ b/NetworkPkg/Test/NetworkPkgHostTest.dsc @@ -30,6 +30,7 @@ [Components] NetworkPkg/UefiPxeBcDxe/GoogleTest/UefiPxeBcDxeGoogleTest.inf { UefiRuntimeServicesTableLib|MdePkg/Test/Mock/Library/GoogleTest/MockUefiRuntimeServicesTableLib/MockUefiRuntimeServicesTableLib.inf + UefiBootServicesTableLib|MdePkg/Test/Mock/Library/GoogleTest/MockUefiBootServicesTableLib/MockUefiBootServicesTableLib.inf } # Despite these library classes being listed in [LibraryClasses] below, they are not needed for the host-based unit tests. diff --git a/NetworkPkg/UefiPxeBcDxe/GoogleTest/UefiPxeBcDxeGoogleTest.inf b/NetworkPkg/UefiPxeBcDxe/GoogleTest/UefiPxeBcDxeGoogleTest.inf index 301dcdf61109..8b092d9291d4 100644 --- a/NetworkPkg/UefiPxeBcDxe/GoogleTest/UefiPxeBcDxeGoogleTest.inf +++ b/NetworkPkg/UefiPxeBcDxe/GoogleTest/UefiPxeBcDxeGoogleTest.inf @@ -14,7 +14,7 @@ [Defines] # # The following information is for reference only and not required by the build tools. # -# VALID_ARCHITECTURES = IA32 X64 +# VALID_ARCHITECTURES = IA32 X64 AARCH64 # [Sources] @@ -23,6 +23,7 @@ [Sources] PxeBcDhcp6GoogleTest.h ../PxeBcDhcp6.c ../PxeBcSupport.c + ../../../MdePkg/Test/Mock/Library/GoogleTest/Protocol/MockRng.cpp [Packages] MdePkg/MdePkg.dec diff --git a/NetworkPkg/UefiPxeBcDxe/GoogleTest/PxeBcDhcp6GoogleTest.cpp b/NetworkPkg/UefiPxeBcDxe/GoogleTest/PxeBcDhcp6GoogleTest.cpp index bd423ebadfce..61736ff79e83 100644 --- a/NetworkPkg/UefiPxeBcDxe/GoogleTest/PxeBcDhcp6GoogleTest.cpp +++ b/NetworkPkg/UefiPxeBcDxe/GoogleTest/PxeBcDhcp6GoogleTest.cpp @@ -7,6 +7,8 @@ #include #include #include +#include +#include extern "C" { #include @@ -165,7 +167,7 @@ protected: // Note: // Testing PxeBcHandleDhcp6Offer() is difficult because it depends on a // properly setup Private structure. Attempting to properly test this function -// without a signficant refactor is a fools errand. Instead, we will test +// without a significant refactor is a fools errand. Instead, we will test // that we can prevent an overflow in the function. TEST_F (PxeBcHandleDhcp6OfferTest, BasicUsageTest) { PXEBC_DHCP6_PACKET_CACHE *Cache6 = NULL; @@ -238,6 +240,7 @@ TEST_F (PxeBcCacheDnsServerAddressesTest, BasicUsageTest) { FreePool (Option); } } + // Test Description // Test that we can prevent an overflow in the function TEST_F (PxeBcCacheDnsServerAddressesTest, AttemptOverflowTest) { @@ -470,10 +473,15 @@ TEST_F (PxeBcRequestBootServiceTest, AttemptRequestOverFlowExpectFailure) { class PxeBcDhcp6DiscoverTest : public ::testing::Test { public: PXEBC_PRIVATE_DATA Private = { 0 }; + // create a mock md5 hash + UINT8 Md5Hash[16] = { 0 }; + EFI_UDP6_PROTOCOL Udp6Read; protected: MockUefiRuntimeServicesTableLib RtServicesMock; + MockUefiBootServicesTableLib BsMock; + MockRng RngMock; // Add any setup code if needed virtual void @@ -527,8 +535,21 @@ TEST_F (PxeBcDhcp6DiscoverTest, BasicOverflowTest) { Private.Dhcp6Request->Length = (UINT16)(Cursor - (UINT8 *)Private.Dhcp6Request); - EXPECT_CALL (RtServicesMock, gRT_GetTime) -.WillOnce (::testing::Return (0)); + EXPECT_CALL (BsMock, gBS_LocateProtocol) +.WillOnce ( + ::testing::DoAll ( +::testing::SetArgPointee<2> (::testing::ByRef (gRngProtocol)), +::testing::Return (EFI_SUCCESS) +) + ); + + EXPECT_CALL (RngMock, GetRng) +.WillOnce ( + ::testing::DoAll ( +::testing::SetArgPointee<3> (::testing::ByRef (Md5Hash[0])), +::testing::Return (EFI_SUCCESS) +) + ); ASSERT_EQ ( PxeBcDhcp6Discover ( @@ -558,8 +579,21 @@ TEST_F (PxeBcDhcp6DiscoverTest, BasicUsageTest) { Private.Dhcp6Request->Length = (UINT16)(Cursor - (UINT8 *)Private.Dhcp6Request); - EXPECT_CALL (RtServicesMock, gRT_GetTime) -.WillOnce (::testing::Return (0)); + EXPECT_CALL (BsMock, gBS_LocateProtocol) +.WillOnce ( + ::testing::DoAll ( +::testing::SetArgPointee<2> (::testing::ByRef (gRngProtocol)), +::testing::Return (EFI_SUCCESS) +