Re: [edk2-devel] [PATCH v2 13/13] NetworkPkg: Update the PxeBcDhcp6GoogleTest due to underlying changes

2024-05-23 Thread Saloni Kasbekar
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

2024-05-08 Thread Doug Flick via groups.io
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)
+