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

["Yao, Jiewen" <jiewen.yao@intel.com>]

Thanks. I think this is good addition.

Acked-by: Jiewen Yao <Jiewen.yao@intel.com>

Need CI expert to give reviewed-by.

> -----Original Message-----
> From: Tan, Dun <dun.tan@intel.com>
> Sent: Monday, December 5, 2022 11:57 AM
> 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>; Sean
> Brogan <sean.brogan@microsoft.com>; Kinney, Michael D
> <michael.d.kinney@intel.com>
> Subject: RE: [edk2-devel] [PATCH 1/3] OvmfPkg/PlatformCI: Expand Ovmf
> PlatformCI for Shell UnitTest
> 
> 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
> 
> 
> 
> 
>