Re: [edk2-devel] [PATCH 1/3] OvmfPkg/PlatformCI: Expand Ovmf PlatformCI for Shell UnitTest

["duntan" <dun.tan@intel.com>]

Hi all,
Is there anything else I can do to speed up the review process for this patch set? Looking forward to your reply.

Thanks,
Dun

-----Original Message-----
From: Tan, Dun 
Sent: Monday, November 28, 2022 5:34 PM
To: devel@edk2.groups.io; Tan, Dun <dun.tan@intel.com>
Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>; Yao, Jiewen <jiewen.yao@intel.com>; Justen, Jordan L <jordan.l.justen@intel.com>; Gerd Hoffmann <kraxel@redhat.com>; Ni, Ray <ray.ni@intel.com>
Subject: RE: [edk2-devel] [PATCH 1/3] OvmfPkg/PlatformCI: Expand Ovmf PlatformCI for Shell UnitTest

Hi all,
Could you please help to review this patch? Thanks a lot!

Thanks,
Dun

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of duntan
Sent: Tuesday, November 22, 2022 7:48 PM
To: devel@edk2.groups.io
Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>; Yao, Jiewen <jiewen.yao@intel.com>; Justen, Jordan L <jordan.l.justen@intel.com>; Gerd Hoffmann <kraxel@redhat.com>; Ni, Ray <ray.ni@intel.com>
Subject: [edk2-devel] [PATCH 1/3] OvmfPkg/PlatformCI: Expand Ovmf PlatformCI for Shell UnitTest

Expand Ovmf PlatformBuild.py and PlatformBuildLib.py to support building and running specific Shell target UnitTest modules.
In the new CommonPlatform class:
It provides new class attributes and new methods to support build and run specific Shell Unit Test modules.

In the new SettingsManager class for stuart_pr_eval:
It calls new API in CommonPlatform to updates PackagesSupported based on -u ShellUnitTestList input from cmd. The package which contains the module in ShellUnitTestList will be added into PackagesSupported for further evaluation.

In the new PlatformBuilder class for stuart_build:
1.In PlatformPostBuild(), it conditionally calls new API in CommonPlatform to build -u ShellUnitTestList in -p PkgsToBuild and copy them to VirtualDrive folder. If no -p option, all the modules in -u ShellUnitTestList will be built.
2.In FlashRomImage(), it conditionally calls the new API in CommonPlatform to write all efi files name in VirtualDrive into startup.nsh and output UnitTest log into ShellUnitTestLog.
3. After the boot process, it conditionally calls new API in CommonPlatform to check the UnitTest boot log.

Signed-off-by: Dun Tan <dun.tan@intel.com>
Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Ray Ni <ray.ni@intel.com>
---
 OvmfPkg/PlatformCI/PlatformBuild.py    | 112 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 OvmfPkg/PlatformCI/PlatformBuildLib.py |  51 +++++++++++++++++++++++++++++++++++++++++++++------
 2 files changed, 157 insertions(+), 6 deletions(-)

diff --git a/OvmfPkg/PlatformCI/PlatformBuild.py b/OvmfPkg/PlatformCI/PlatformBuild.py
index 6c541cdea4..72cb7e0e9e 100644
--- a/OvmfPkg/PlatformCI/PlatformBuild.py
+++ b/OvmfPkg/PlatformCI/PlatformBuild.py
@@ -6,6 +6,11 @@
 ##
 import os
 import sys
+import shutil
+import logging
+import re
+from edk2toolext.environment import shell_environment from 
+edk2toolext.environment.multiple_workspace import MultipleWorkspace
 
 sys.path.append(os.path.dirname(os.path.abspath(__file__)))
 from PlatformBuildLib import SettingsManager @@ -24,6 +29,10 @@ class CommonPlatform():
     Scopes = ('ovmf', 'edk2-build')
     WorkspaceRoot = os.path.realpath(os.path.join(
         os.path.dirname(os.path.abspath(__file__)), "..", ".."))
+    # Support build and run Shell Unit Test modules
+    UnitTestModuleList = {}
+    RunShellUnitTest   = False
+    ShellUnitTestLog   = ''
 
     @classmethod
     def GetDscName(cls, ArchCsv: str) -> str:
@@ -39,5 +48,108 @@ class CommonPlatform():
         dsc += ".dsc"
         return dsc
 
+    @classmethod
+    def UpdatePackagesSupported(cls, ShellUnitTestListArg):
+        ''' Update PackagesSupported by -u ShellUnitTestList from cmd line. '''
+        UnitTestModuleListStr = ','.join(ShellUnitTestListArg)
+        if not re.search(r'.+.inf:.+.dsc', UnitTestModuleListStr):
+            raise Exception('No valid ModulePath:DscPath in the -u {}'.format(UnitTestModuleListStr))
+        UnitTestModuleList = UnitTestModuleListStr.split(',')
+        PackagesSupported = []
+        for KeyValue in UnitTestModuleList:
+            PkgName = KeyValue.split("Pkg")[0] + 'Pkg'
+            if PkgName not in PackagesSupported:
+                PackagesSupported.append(PkgName)
+        cls.PackagesSupported = tuple(PackagesSupported)
+        print('PackagesSupported for UnitTest is
+ {}'.format(cls.PackagesSupported))
+
+    @classmethod
+    def UpdateUnitTestConfig(cls, args):
+        ''' Update UnitTest config by -u ShellUnitTestList and -p PkgsToBuildForUT from cmd line.
+            ShellUnitTestList is in this format: {module1:dsc1, module2:dsc2, module3:dsc2...}.
+            Only the modules which are in the PkgsToBuildForUT list are added into self.UnitTestModuleList.
+        '''
+        UnitTestModuleListStr = ','.join(args.ShellUnitTestList)
+        if not re.search(r'.+.inf:.+.dsc', UnitTestModuleListStr):
+            raise Exception('No valid ModulePath:DscPath in the -u {}'.format(args.ShellUnitTestList))
+        UnitTestModuleList = UnitTestModuleListStr.split(',')
+        if args.PkgsToBuildForUT is None or all(['Pkg' not in Pkg for Pkg in args.PkgsToBuildForUT]):
+            # No invalid Pkgs from input. Build all modules in -u UnitTestModuleList.
+            for KeyValue in UnitTestModuleList:
+                UnitTestPath = os.path.normpath(KeyValue.split(":")[0])
+                DscPath      = os.path.normpath(KeyValue.split(":")[1])
+                cls.UnitTestModuleList[UnitTestPath] = DscPath
+        else:
+            PkgsToBuildForUT = ','.join(args.PkgsToBuildForUT).split(',')
+            for KeyValue in UnitTestModuleList:
+                UnitTestPath = os.path.normpath(KeyValue.split(":")[0])
+                DscPath      = os.path.normpath(KeyValue.split(":")[1])
+                PkgName      = UnitTestPath.split("Pkg")[0] + 'Pkg'
+                if PkgName in PkgsToBuildForUT:
+                    cls.UnitTestModuleList[UnitTestPath] = DscPath
+        if len(cls.UnitTestModuleList) > 0:
+            cls.RunShellUnitTest = True
+            cls.ShellUnitTestLog = os.path.join(cls.WorkspaceRoot, 'Build', "BUILDLOG_UnitTest.txt")
+            print('UnitTestModuleList is
+ {}'.format(cls.UnitTestModuleList))
+
+    def BuildUnitTest(self):
+        ''' Build specific DSC for modules in UnitTestModuleList '''
+        self.env = shell_environment.GetBuildVars()
+        self.ws  = PlatformBuilder.GetWorkspaceRoot(self)
+        self.mws = MultipleWorkspace()
+        self.pp  = ''
+        VirtualDrive = os.path.join(self.env.GetValue("BUILD_OUTPUT_BASE"), "VirtualDrive")
+        os.makedirs(VirtualDrive, exist_ok=True)
+
+        # DSC by self.GetDscName() should have been built in BUILD process.
+        BuiltDsc = [CommonPlatform.GetDscName(",".join(self.env.GetValue("TARGET_ARCH").split(' ')))]
+        for UnitTestPath, DscPath in CommonPlatform.UnitTestModuleList.items():
+            if DscPath not in BuiltDsc:
+                ModuleName = os.path.split(UnitTestPath)[1].split('.inf')[0]
+                logging.info('Build {0} for {1}'.format(DscPath, ModuleName))
+                BuiltDsc.append(DscPath)
+                Arch = self.env.GetValue("TARGET_ARCH").split(" ")
+                if 'X64' in Arch:
+                    UTArch = 'X64'
+                else:
+                    UTArch = 'IA32'
+                self.env.AllowOverride("ACTIVE_PLATFORM")
+                self.env.SetValue("ACTIVE_PLATFORM", DscPath, "For UnitTest")
+                self.env.AllowOverride("TARGET_ARCH")
+                self.env.SetValue("TARGET_ARCH", UTArch, "For UnitTest") # Set UnitTest arch the same as Ovmf Shell module.
+                ret = PlatformBuilder.Build(self)                        # Build specific dsc for UnitTest modules
+                if (ret != 0):
+                    return ret
+            ret = PlatformBuilder.ParseDscFile(self) # Parse OUTPUT_DIRECTORY from dsc files
+            if(ret != 0):
+                return ret
+            OutputPath = os.path.normpath(os.path.join(self.ws, self.env.GetValue("OUTPUT_DIRECTORY")))
+            EfiPath    = os.path.join(OutputPath, self.env.GetValue("TARGET") + "_" + self.env.GetValue("TOOL_CHAIN_TAG"),
+                         UTArch, UnitTestPath.split('.inf')[0], "OUTPUT", ModuleName + '.efi')
+            logging.info('Copy {0}.efi from:{1}'.format(ModuleName, EfiPath))
+            shutil.copy(EfiPath, VirtualDrive)
+        return 0
+
+    @staticmethod
+    def WriteEfiToStartup(EfiFolder, FileObj):
+        ''' Write all the .efi files' name in VirtualDrive into Startup.nsh '''
+        for Root,Dirs,Files in os.walk(EfiFolder):
+            for File in Files:
+                if os.path.splitext(File)[1] == '.efi':
+                    FileObj.write("{0} \n".format(File))
+
+    @classmethod
+    def CheckUnitTestLog(cls):
+        ''' Check the boot log for UnitTest '''
+        File        = open(cls.ShellUnitTestLog, "r")
+        FileContent = File.readlines()
+        logging.info('Check the UnitTest boot log:{0}'.format(cls.ShellUnitTestLog))
+        for Index in range(len(FileContent)):
+            if 'FAILURE MESSAGE:' in FileContent[Index]:
+                if FileContent[Index + 1].strip() != '':
+                    FailureMessage = FileContent[Index + 1] + FileContent[Index + 2]
+                    return FailureMessage
+        return 0
+
 import PlatformBuildLib
 PlatformBuildLib.CommonPlatform = CommonPlatform diff --git a/OvmfPkg/PlatformCI/PlatformBuildLib.py b/OvmfPkg/PlatformCI/PlatformBuildLib.py
index bfef9849c7..b42235b2ac 100644
--- a/OvmfPkg/PlatformCI/PlatformBuildLib.py
+++ b/OvmfPkg/PlatformCI/PlatformBuildLib.py
@@ -103,15 +103,28 @@ class SettingsManager(UpdateSettingsManager, SetupSettingsManager, PrEvalSetting
 
         return build_these_packages
 
+    def AddCommandLineOptions(self, parserObj):
+        parserObj.add_argument('-u', '--UnitTest', dest='ShellUnitTestList', type=str,
+            help='Optional - Key:Value that contains Shell UnitTest list and corresponding DscPath you want to test.(workspace relative)'
+            'Can list multiple by doing -u <UTPath1:DscPath1>,<UTPath2:DscPath2> or -u <UTPath3:DscPath3> -u <UTPath4:DscPath4>',
+            action="append", default=None)
+
+    def RetrieveCommandLineOptions(self, args):
+        if args.ShellUnitTestList:
+            
+ CommonPlatform.UpdatePackagesSupported(args.ShellUnitTestList)
+
     def GetPlatformDscAndConfig(self) -> tuple:
         ''' If a platform desires to provide its DSC then Policy 4 will evaluate if
         any of the changes will be built in the dsc.
 
         The tuple should be (<workspace relative path to dsc file>, <input dictionary of dsc key value pairs>)
+        This Policy 4 can only be applied when PackagesSupported only contains OvmfPkg Since it doesn't support
+        mutiple packages evaluation.
         '''
-        dsc = CommonPlatform.GetDscName(",".join(self.ActualArchitectures))
-        return (f"OvmfPkg/{dsc}", {})
-
+        if (len(CommonPlatform.PackagesSupported) == 1) and (CommonPlatform.PackagesSupported[0] == 'OvmfPkg'):
+            dsc = CommonPlatform.GetDscName(",".join(self.ActualArchitectures))
+            return (f"OvmfPkg/{dsc}", {})
+        return None
 
     # ####################################################################################### #
     #                         Actual Configuration for Platform Build                         #
@@ -126,6 +139,14 @@ class PlatformBuilder( UefiBuilder, BuildSettingsManager):
             help="Optional - CSV of architecture to build.  IA32 will use IA32 for Pei & Dxe. "
             "X64 will use X64 for both PEI and DXE.  IA32,X64 will use IA32 for PEI and "
             "X64 for DXE. default is IA32,X64")
+        parserObj.add_argument('-p', '--pkg', '--pkg-dir', dest='PkgsToBuildForUT', type=str,
+            help='Optional - Package list you want to build for UnitTest.efi. (workspace relative).'
+            'Can list multiple by doing -p <pkg1>,<pkg2> or -p <pkg3> -p <pkg4>.If no valid input -p, build and run all -u UnitTest',
+            action="append", default=None)
+        parserObj.add_argument('-u', '--UnitTest', dest='ShellUnitTestList', type=str,
+            help='Optional - Key:Value that contains Shell UnitTest list and corresponding DscPath you want to test.(workspace relative)'
+            'Can list multiple by doing -u <UTPath1:DscPath1>,<UTPath2:DscPath2> or -u <UTPath3:DscPath3> -u <UTPath4:DscPath4>',
+            action="append", default=None)
 
     def RetrieveCommandLineOptions(self, args):
         '''  Retrieve command line options from the argparser '''
@@ -133,6 +154,10 @@ class PlatformBuilder( UefiBuilder, BuildSettingsManager):
         shell_environment.GetBuildVars().SetValue("TARGET_ARCH"," ".join(args.build_arch.upper().split(",")), "From CmdLine")
         dsc = CommonPlatform.GetDscName(args.build_arch)
         shell_environment.GetBuildVars().SetValue("ACTIVE_PLATFORM", f"OvmfPkg/{dsc}", "From CmdLine")
+        self.RunShellUnitTest = False
+        if args.ShellUnitTestList:
+            CommonPlatform.UpdateUnitTestConfig(args)
+            self.RunShellUnitTest = CommonPlatform.RunShellUnitTest
 
     def GetWorkspaceRoot(self):
         ''' get WorkspacePath '''
@@ -176,6 +201,11 @@ class PlatformBuilder( UefiBuilder, BuildSettingsManager):
         return 0
 
     def PlatformPostBuild(self):
+        if self.RunShellUnitTest:
+            ret = CommonPlatform.BuildUnitTest(self)
+            if ret !=0:
+                logging.critical("Build UnitTest failed")
+                return ret
         return 0
 
     def FlashRomImage(self):
@@ -210,9 +240,13 @@ class PlatformBuilder( UefiBuilder, BuildSettingsManager):
         else:
             args += " -pflash " + os.path.join(OutputPath_FV, "OVMF.fd")    # path to firmware
 
-
         if (self.env.GetValue("MAKE_STARTUP_NSH").upper() == "TRUE"):
             f = open(os.path.join(VirtualDrive, "startup.nsh"), "w")
+            if self.RunShellUnitTest:
+                # When RunShellUnitTest is True, write all efi files name into startup.nsh.
+                CommonPlatform.WriteEfiToStartup(VirtualDrive, f)
+                # Output UnitTest log into ShellUnitTestLog.
+                args += " -serial
+ file:{}".format(CommonPlatform.ShellUnitTestLog)
             f.write("BOOT SUCCESS !!! \n")
             ## add commands here
             f.write("reset -s\n")
@@ -222,6 +256,11 @@ class PlatformBuilder( UefiBuilder, BuildSettingsManager):
 
         if ret == 0xc0000005:
             #for some reason getting a c0000005 on successful return
-            return 0
-
+            ret = 0
+        if self.RunShellUnitTest and ret == 0:
+            # Check the UnitTest boot log.
+            UnitTestResult = CommonPlatform.CheckUnitTestLog()
+            if (UnitTestResult):
+                logging.info("UnitTest failed with this FAILURE MESSAGE:\n{}".format(UnitTestResult))
+                return UnitTestResult
         return ret
--
2.31.1.windows.1