From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mga03.intel.com (mga03.intel.com [134.134.136.65]) by mx.groups.io with SMTP id smtpd.web12.291.1576370303722132968 for ; Sat, 14 Dec 2019 16:38:24 -0800 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: intel.com, ip: 134.134.136.65, mailfrom: jiewen.yao@intel.com) X-Amp-Result: UNKNOWN X-Amp-Original-Verdict: FILE UNKNOWN X-Amp-File-Uploaded: False Received: from fmsmga006.fm.intel.com ([10.253.24.20]) by orsmga103.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 14 Dec 2019 16:38:18 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.69,315,1571727600"; d="png'150?scan'150,208,217,150";a="416051618" Received: from fmsmsx104.amr.corp.intel.com ([10.18.124.202]) by fmsmga006.fm.intel.com with ESMTP; 14 Dec 2019 16:38:18 -0800 Received: from FMSMSX110.amr.corp.intel.com (10.18.116.10) by fmsmsx104.amr.corp.intel.com (10.18.124.202) with Microsoft SMTP Server (TLS) id 14.3.439.0; Sat, 14 Dec 2019 16:38:17 -0800 Received: from shsmsx104.ccr.corp.intel.com (10.239.4.70) by fmsmsx110.amr.corp.intel.com (10.18.116.10) with Microsoft SMTP Server (TLS) id 14.3.439.0; Sat, 14 Dec 2019 16:38:17 -0800 Received: from shsmsx102.ccr.corp.intel.com ([169.254.2.109]) by SHSMSX104.ccr.corp.intel.com ([169.254.5.90]) with mapi id 14.03.0439.000; Sun, 15 Dec 2019 08:38:15 +0800 From: "Yao, Jiewen" To: "devel@edk2.groups.io" , "bret.barkelew@microsoft.com" , Andrew Fish , "Kinney, Michael D" CC: "rfc@edk2.groups.io" Subject: Re: [EXTERNAL] Re: [edk2-devel] EDK2 Host-Based Unit Test RFC (Now with docs!) Thread-Topic: [EXTERNAL] Re: [edk2-devel] EDK2 Host-Based Unit Test RFC (Now with docs!) Thread-Index: AQHVoQS12ZW6E6MMrkasWCUh3oP5e6enceDwgALivYCAAAlHOoADZyKbgAsoujaAAYecgA== Date: Sun, 15 Dec 2019 00:38:14 +0000 Message-ID: <74D8A39837DF1E4DA445A8C0B3885C503F8A1CEB@shsmsx102.ccr.corp.intel.com> References: ,<0F460E19-2A3B-4FEE-8B6F-BE65B9C5F1D4@apple.com>,<15DD3E3A746110DF.27746@groups.io>, In-Reply-To: Accept-Language: zh-CN, en-US X-MS-Has-Attach: yes X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiOGRmYmU2MGItMjdiYy00OWM4LWFjYzYtOWIzYTUwZDA0ZGVjIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX05UIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE3LjEwLjE4MDQuNDkiLCJUcnVzdGVkTGFiZWxIYXNoIjoiT0h3T1BmZUJrY1M1RExEQjNDUUJ6cGRROG5qWElmN1laZTNLckRnMEpab3ExN0RSQUhqMW5OYkZGeFlOdGRkMyJ9 x-ctpclassification: CTP_NT dlp-product: dlpe-windows dlp-version: 11.2.0.6 dlp-reaction: no-action msip_labels: MSIP_Label_f42aa342-8706-4288-bd11-ebb85995028c_Enabled=True;MSIP_Label_f42aa342-8706-4288-bd11-ebb85995028c_SiteId=72f988bf-86f1-41af-91ab-2d7cd011db47;MSIP_Label_f42aa342-8706-4288-bd11-ebb85995028c_SetDate=2019-12-04T18:23:45.2427147Z;MSIP_Label_f42aa342-8706-4288-bd11-ebb85995028c_ContentBits=0;MSIP_Label_f42aa342-8706-4288-bd11-ebb85995028c_Method=Privileged x-originating-ip: [10.239.127.40] MIME-Version: 1.0 Return-Path: jiewen.yao@intel.com X-Groupsio-MsgNum: 52235 Content-Language: en-US Content-Type: multipart/related; boundary="_004_74D8A39837DF1E4DA445A8C0B3885C503F8A1CEBshsmsx102ccrcor_"; type="multipart/alternative" --_004_74D8A39837DF1E4DA445A8C0B3885C503F8A1CEBshsmsx102ccrcor_ Content-Type: multipart/alternative; boundary="_000_74D8A39837DF1E4DA445A8C0B3885C503F8A1CEBshsmsx102ccrcor_" --_000_74D8A39837DF1E4DA445A8C0B3885C503F8A1CEBshsmsx102ccrcor_ Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Answer the question 5,6,7,8, which are related to HBFA. For other MSFT unit test, I don't have strong opinion. I am OK with that. Thank you Yao Jiewen From: devel@edk2.groups.io On Behalf Of Bret Barkel= ew via Groups.Io Sent: Saturday, December 14, 2019 8:47 AM To: devel@edk2.groups.io; Andrew Fish ; Kinney, Michael D= Cc: rfc@edk2.groups.io Subject: Re: [EXTERNAL] Re: [edk2-devel] EDK2 Host-Based Unit Test RFC (No= w with docs!) Mike, I think I've gotten all the feedback here and all the action items from ou= r call. The current branch should be good to go, minus the couple of things= that Intel was going to look into. Thanks! - Bret From: Bret Barkelew Sent: Friday, December 6, 2019 2:21 PM To: devel@edk2.groups.io; Andrew Fish; Kinney, Michael D Cc: rfc@edk2.groups.io Subject: RE: [EXTERNAL] Re: [edk2-devel] EDK2 Host-Based Unit Test RFC (No= w with docs!) 1. I see that MdePkg adds a dependency on UnitTestPkg. This makes Unit= TestPkg the root package when building and running host based unit tests. = This makes sense, but good to highlight that all packages that use host bas= ed tests will introduce a new package dependency on UnitTestPkg. * Since the dependency only applies to unit tests, can we update th= e DependencyCheck plugin to support listing dependencies for FW components = separate from dependencies for unit tests? * Can move. Capability is there, but mistakenly added to the wrong = section. 2. I see UnitTestPkg declares 6 new lib classes. Are all 6 classes int= ended to be used directly from a unit test case? If some of these are only= intended to be used from the framework inside the UnitTestPkg can we make = them private? * Because there are different instances in multiple places (and con= ceivably more in the future), we don't see how we could make them private. 3. In the MdePkg, I see a new top level directory called 'HostLibrary'.= Since these lib instances are only used from a host based test, can they b= e moved into the 'Test' directory? * Can move. 4. MdePkg/MdePkgTest.dsc. * Can this DSC file be moved into the 'Test' directory? i. Yes * I see this DSC file only supports VS today. How much work is it = to add GCC support? i. Don't know. Thi= s is an item on our list, but lower priority and not a blocker. 1. MdePkg/HostLibrary/BaseLibHost * Why are there 2 INFs. One with ASM and one without ASM? Can we = just use the one with ASM and assume NASM is installed? * I see the purpose of this lib instance is to call the * SetJump()/LongJump(). So these implementations always work? It = looks like it assumes BASE_LIBRARY_JUMP_BUFFER is identical to the host OS = user mode application setjmp()/longjmp() state. * Why are DRx and CRx registers emulated? I would think and code t= hat depends on read/writing these registers would not be compatible with ho= st based testing. Can we change to ASSERT (FALSE)? * PatchInstructionX86() - I suspect this will not work in a host ba= sed environment because it is self modifying code. Should it be ASSERT (FA= LSE)? * Libraries were taken directly from the Intel work in HBFA. They w= orked so we kept them. We're just as interested in the answers to these que= stions as you are. [Jiewen] Here is the answer: Some tools requires source build, such AFL-CC or KLEE to get the result or= optimization. They do analysis besides compiling. These tools only work on= C-language. That why we need non-asm build. Some test case need access CRx/DRx just as the pre-condition and post-cond= ition, such as MTRR test. We cannot use ASSERT. Generally, a big problem of current based lib is that it mixes the hardwar= e register access (such as CR/DR, CPUIR, MSR, mode switch) into a common so= ftware lib (such as linked list, math, string, checksum). I hope we can ref= actory the based lib to separate the hardware access part, such as a BaseRe= gAccessLib, similar to IoLib, PciLib. Agree on PatchInstructionX86(). We should use ASSERT(). If we encounter a = case that requires this function, we consider how to make it work later. 1. MdePkg/HostLibrary/DebugLibHost * What is '#ifndef TEST_WITH_KLEE' * What is the 'PatchFormat()' function? It is always disabled with= if (0). * Are the PCDs to set the debug message levels disabled on purpose?= (DebugPrintEnabled(), DebugPrintLevelEnabled(), DebugCodeEnabled()) * Libraries were taken directly from the Intel work in HBFA. They w= orked so we kept them. We're just as interested in the answers to these que= stions as you are. [Jiewen] Here is the answer: This is special optimization for KLEE - a symbolic executio= n tool. We hope to reduce the unnecessary branch. Or the KELL might not res= olve the function. PatchFormat() is an old function. Deadcode can be removed n= ow. In the beginning, PCD is not supported. And we don't find a= use case to control PCD in such detail level. To simplify thing, we just d= on't use it. An on/off control (via HOST_DEBUG_MESSAGE) is good enough. If = there is real use case coming up, we can see how to enable it. 1. MdePkg/HostLibrary/BaseMemoryLibHost * Why can't we use MdePkg/Library/BaseMemoryLib/BaseMemoryLib/inf i= nstead and reduce the number of host specific lib instances? * Libraries were taken directly from the Intel work in HBFA. They w= orked so we kept them. We're just as interested in the answers to these que= stions as you are. [Jiewen] Here is the answer: This is special optimization for fuzzing tool. If we use st= andard C lib, the compiler knows it and don't insert branch analysis. If we= use our own implementation, the compiler has no idea, and try to do analys= is. That is unnecessary. 1. MdePkg/HostLibrary/MemoryAllocationLibHost * Why is are memcpy(), assert(), memset() used in this lib? I woul= d expect this lib instance to match the UefiMemoryAllocationLib with the on= ly the use of malloc() and free() to replace the UEFI Boot Services calls. * Libraries were taken directly from the Intel work in HBFA. They w= orked so we kept them. We're just as interested in the answers to these que= stions as you are. [Jiewen] Here is the answer: Similar to 7, using standard-C function can reduce the unne= cessary analysis for analysis tool. 1. Host library instance naming conventions. * The current naming convention uses the environment as a prefix(e.= g. Pei, Smm, Uefi) and the services the lib instance uses as a post fix. W= ould it make more sense to use 'Host' as a prefix instead of a postfix in t= he lib instance names? * I don't know if there's a 1:1 relationship with these. For some l= ibrary classes (that you might have host versions of), the class itself is = the PeiSomethingLib, and the host version would be the PeiSomethingLibHost.= No? 2. Unicode vs ASCII strings * I see InitUnitTestFramework(), CreateUnitTestSuite(), and AddTest= Case() all take Unicode strings for the labels. I also see extra code to c= onvert gEfiCallerBaseName from ASCII to Unicode to use it as a short name o= f a test framework. I think it would be simpler if the parameters to these= APIs were ASCII and the framework can convert to Unicode if needed. * No strong feelings, but we already have a bunch of tests written = this way. Since the UnitTestLib is an abstraction that works as well in She= ll as in the Host, going with Unicode strings seemed to match the art for S= hell apps (since the framework was written for Shell first). 3. Will InitUnitTestFramework() and CreateUnitTestSuite() always be cal= led in pairs? If so, can we combine these to a single API? * I see the SafeIntLib example create a single framework and multip= le test suites. Perhaps we can have a single CreateUnitTestSuite() API whe= re Framework is an IN/OUT and if it is passed in as NULL, the Framework han= dle is created. i. It's not always= in pairs. You would only have a single framework init, but could have mult= iple suites. * I see a pattern where the CreateUnitTestSuite() 'Package' paramet= er is used as a prefix to every call to AddTestCase() in the 'ClassName' pa= rameter. Can we simplify AddTestCase() so it only need to pass in the name= of the unit test case, and the framework can append Package and the unit = test case name? i. Right now, our = tests just coincidentally share similar names with the packages, but we don= 't feel this is axiomatic and don't want to force this naming on others, wh= o may be trying to bolt into other reporting structures. 1. I see the use of the 'Fw' variable as a shorthand for 'Framework'. = Can we use something other than 'Fw'. It may be confused with 'Firmware'. * No real arguments. Wouldn't fight a find-replace, but it'll just = be a bunch of touches as we commit. 2. UnitTestPkg/Include/UnitTestTypes.h * I see several hard coded string lengths. Since this runs in a ho= st environment and strings can always be allocated, can the hard coded leng= ths be removed? Update the structs to use pointers to strings instead of s= tring arrays, and the framework can manage alloc() and free()? i. Given that this= framework is designed to be nimble enough to work in PEI and SMM and other= constrictive environments, we wanted to keep fixed sizing. * How are Fingerprints used? The idea of using as hash as a unique= identifier is a good idea. How is the hash calculated? What unit test co= de artifacts are used so developers know what parameters must be unique? C= an we just decide to use a specific hash algorithm/size and the structure c= an be a pointer to an allocated buffer instead of a fixed size array to mak= e it easy to change the algorithm/size in the future? i. Fingerprints ar= e unique IDs to make sure that serialize/unserialized state matches the tes= ts upon re-entry. I'm not married to MD5, but it needs to be predictably un= ique, and carried with the framework. I would not want to make any requirem= ents on CryptoLib, since these aren't crypto-strong. * Update all the strings to be ASCII? See Unicode vs ASCII above. i. Ideally not, un= less there's a strong use case. * UNIT_TEST_SAVE_TEST - The last field is a variable sized array, s= o it can be a formal field. i. Not opposed, bu= t don't really want to make the change myself. Is there a style guide that = this is breaking? * UNIT_TEST_SAVE_CONTEXT - - The last field is a variable sized arr= ay, so it can be a formal field. * UNIT_TEST_SAVE_HEADER - Can the last 3 fields be changed to point= ers and be formal fields? * Do the structures really need to be packed? Specially with the c= hanges suggested above? Is the intent to potentially share data stored on = disk between different host execution environments that may have different = width architectures? i. That's an inter= esting point. Could you draw up your suggestion and submit a PR? 1. UnitTestPkg - UnitTestLib.h * Can you provide more context for the APIs SaveFrameworkState(), S= aveFrameworkStateAndQuit(), SaveFrameworkStateAndReboot(), SetFrameworkBoot= NextDevice(). I do not see any Load() functions, so how would a set of tes= ts be resumed? If these do not apply to host based tests, should these be = split out to a different lib class? i. I'll improve th= e documentation around these functions. I will acknowledge, however, that t= his is an interface that I expect to evolve as we figure out what kinds of = tests the community wants support for "out of the box". If we want to easil= y enable tests around - for example - ExitBootServices, we will likely want= to tweak this going forward to its simplest form. The version we have here= was sufficient to enable the test cases that we've currently written. 1. UnitTestPkg/Library/UnitTestLib * I see an implementation of MD5. We should not do our own. We sh= ould use an approved implementation such as OpenSSL. i. Happy to discus= s another implementation, but this is not crypto-strong. It's just for uniq= ueness. A sufficiently long CRC would probably work, too. 1. UnitTestPkg/Library/UnitTestTerminationLibTbd * Do we really need this lib instance now? i. This is here so= that we can figure out what this should look like for a host. Host obvious= ly wouldn't reset, but could conceivably quit. Or maybe that doesn't make a= ny sense for a host. - Bret From: devel@edk2.groups.io > on behalf of Bret Barkelew via Groups.Io = > Sent: Wednesday, December 4, 2019 10:24:21 AM To: Andrew Fish >; devel@edk2.grou= ps.io >; Kinney, Michael D > Cc: rfc@edk2.groups.io > Subject: Re: [EXTERNAL] Re: [edk2-devel] EDK2 Host-Based Unit Test RFC (No= w with docs!) Andrew, I agree with your points. Mike, You've got a lot more there. Let me take a look and update the draft. I'll= ping back ASAP. - Bret From: Andrew Fish Sent: Wednesday, December 4, 2019 9:50 AM To: devel@edk2.groups.io; Kinney, Michael D Cc: Bret Barkelew; rfc@edk2.groups.io<= mailto:rfc@edk2.groups.io> Subject: [EXTERNAL] Re: [edk2-devel] EDK2 Host-Based Unit Test RFC (Now wi= th docs!) Mike, I like and agree with your comments. On the UnitTestPkg(s) dependency issue I think it would make sense to move= as much as possible into the *Pkg/Test/ ( *Pkg/HostLibrary, MdePkg/MdePkg= Test.dsc, etc.) directory structure. It looks like for implementation speci= fic tests we skip the Test directory and drop the UnitTest or FuzzTest dire= ctly into modules directory. Maybe we should follow the same pattern as *Pk= g/Test and have a Test directory? This will help minimize the number of "re= served" directories we need for managing the testing. Have a standardized d= irectory structure will make it easier to differentiate the code from tests= while doing `git grep` or other forms of code spelunking. A Host-Based Unit Test for a Library makes sense as you can link directly = against the library and test it. A Host-Based Unit Test for Protocol/PPI/GU= ID [1] seems a little more complex. It is easy enough to write tests for th= e interfaces but what APIs do you call to get access to the protocol? Per our conversation at the Stewards meeting I think make sense to try to = roll out the testing of libraries as the 1st phase. The mocking required fo= r drivers could get quite complex and let us not get bogged down in all tha= t to get something working. [1] *Pkg/Test/UnitTest/[Library|Protocol|Ppi|Guid] Thanks, Andrew Fish On Dec 2, 2019, at 3:12 PM, Michael D Kinney > wrote: Hi Bret, Thanks for posting this content. Host based unit testing is a very valuab= le addition to the CI checks. I have the following comments: 1. I see that MdePkg adds a dependency on UnitTestPkg. This makes Unit= TestPkg the root package when building and running host based unit tests. = This makes sense, but good to highlight that all packages that use host bas= ed tests will introduce a new package dependency on UnitTestPkg. * Since the dependency only applies to unit tests, can we update th= e DependencyCheck plugin to support listing dependencies for FW components = separate from dependencies for unit tests? 1. I see UnitTestPkg declares 6 new lib classes. Are all 6 classes int= ended to be used directly from a unit test case? If some of these are only= intended to be used from the framework inside the UnitTestPkg can we make = them private? 2. In the MdePkg, I see a new top level directory called 'HostLibrary'.= Since these lib instances are only used from a host based test, can they b= e moved into the 'Test' directory? 3. MdePkg/MdePkgTest.dsc. * Can this DSC file be moved into the 'Test' directory? * I see this DSC file only supports VS today. How much work is it = to add GCC support? 1. MdePkg/HostLibrary/BaseLibHost * Why are there 2 INFs. One with ASM and one without ASM? Can we = just use the one with ASM and assume NASM is installed? * I see the purpose of this lib instance is to call the * SetJump()/LongJump(). So these implementations always work? It = looks like it assumes BASE_LIBRARY_JUMP_BUFFER is identical to the host OS = user mode applicationsetjmp()/longjmp() state. * Why are DRx and CRx registers emulated? I would think and code t= hat depends on read/writing these registers would not be compatible with ho= st based testing. Can we change to ASSERT (FALSE)? * PatchInstructionX86() - I suspect this will not work in a host ba= sed environment because it is self modifying code. Should it be ASSERT (FA= LSE)? 1. MdePkg/HostLibrary/DebugLibHost * What is '#ifndef TEST_WITH_KLEE' * What is the 'PatchFormat()' function? It is always disabled with= if (0). * Are the PCDs to set the debug message levels disabled on purpose?= (DebugPrintEnabled(), DebugPrintLevelEnabled(), DebugCodeEnabled()) 1. MdePkg/HostLibrary/BaseMemoryLibHost * Why can't we use MdePkg/Library/BaseMemoryLib/BaseMemoryLib/inf i= nstead and reduce the number of host specific lib instances? 1. MdePkg/HostLibrary/MemoryAllocationLibHost * Why is are memcpy(), assert(), memset() used in this lib? I woul= d expect this lib instance to match the UefiMemoryAllocationLib with the on= ly the use of malloc() and free() to replace the UEFI Boot Services calls. 1. Host library instance naming conventions. * The current naming convention uses the environment as a prefix(e.= g. Pei, Smm, Uefi) and the services the lib instance uses as a post fix. W= ould it make more sense to use 'Host' as a prefix instead of a postfix in t= he lib instance names? 1. Unicode vs ASCII strings * I see InitUnitTestFramework(), CreateUnitTestSuite(), and AddTest= Case() all take Unicode strings for the labels. I also see extra code to c= onvert gEfiCallerBaseName from ASCII to Unicode to use it as a short name o= f a test framework. I think it would be simpler if the parameters to these= APIs were ASCII and the framework can convert to Unicode if needed. 1. Will InitUnitTestFramework() and CreateUnitTestSuite() always be cal= led in pairs? If so, can we combine these to a single API? * I see the SafeIntLib example create a single framework and multip= le test suites. Perhaps we can have a single CreateUnitTestSuite() API whe= re Framework is an IN/OUT and if it is passed in as NULL, the Framework han= dle is created. * I see a pattern where the CreateUnitTestSuite() 'Package' paramet= er is used as a prefix to every call to AddTestCase() in the 'ClassName' pa= rameter. Can we simplify AddTestCase() so it only need to pass in the name= of the unit test case, and the framework can append Package and the unit = test case name? 1. I see the use of the 'Fw' variable as a shorthand for 'Framework'. = Can we use something other than 'Fw'. It may be confused with 'Firmware'. 2. UnitTestPkg/Include/UnitTestTypes.h * I see several hard coded string lengths. Since this runs in a ho= st environment and strings can always be allocated, can the hard coded leng= ths be removed? Update the structs to use pointers to strings instead of s= tring arrays, and the framework can manage alloc() and free()? * How are Fingerprints used? The idea of using as hash as a unique= identifier is a good idea. How is the hash calculated? What unit test co= de artifacts are used so developers know what parameters must be unique? C= an we just decide to use a specific hash algorithm/size and the structure c= an be a pointer to an allocated buffer instead of a fixed size array to mak= e it easy to change the algorithm/size in the future? * Update all the strings to be ASCII? See Unicode vs ASCII above. * UNIT_TEST_SAVE_TEST - The last field is a variable sized array, s= o it can be a formal field. * UNIT_TEST_SAVE_CONTEXT - - The last field is a variable sized arr= ay, so it can be a formal field. * UNIT_TEST_SAVE_HEADER - Can the last 3 fields be changed to point= ers and be formal fields? * Do the structures really need to be packed? Specially with the c= hanges suggested above? Is the intent to potentially share data stored on = disk between different host execution environments that may have different = width architectures? 1. UnitTestPkg - UnitTestLib.h * Can you provide more context for the APIs SaveFrameworkState(), S= aveFrameworkStateAndQuit(), SaveFrameworkStateAndReboot(), SetFrameworkBoot= NextDevice(). I do not see any Load() functions, so how would a set of tes= ts be resumed? If these do not apply to host based tests, should these be = split out to a different lib class? 1. UnitTestPkg/Library/UnitTestLib * I see an implementation of MD5. We should not do our own. We sh= ould use an approved implementation such as OpenSSL. 1. UnitTestPkg/Library/UnitTestTerminationLibTbd * Do we really need this lib instance now? Thanks, Mike From: devel@edk2.groups.io > On Behalf Of Bret Barkelew via Groups.Io Sent: Thursday, November 21, 2019 11:39 PM To: devel@edk2.groups.io Subject: [edk2-devel] EDK2 Host-Based Unit Test RFC (Now with docs!) Now that CI has landed in edk2/master, we'd like to get the basic framewor= k for unittesting staged as well. Target intercept date would be immediatel= y after the 2019/11 stabilization, so we wanted to go ahead and get comment= s now. The host unittest framework consists of five primary pieces: - The test library (Cmocka) - Support libraries (Found in UnitTestPkg) - The test support plugins (HostUnitTestComilerPlugin, HostUnitTestDxeComp= leteCheck, HostBasedUnitTestRunner) - The configuration in the package-based *.ci.yaml file and package-based = Test.dsc - The tests themselves We have a demo branch set up at: https://github.com/corthon/edk2-staging/tree/edk2-host-test_v2 We also have a demo build pipeline at: https://dev.azure.com/tianocore/edk2-ci-play/_build?definitionId=3D36&_a= =3Dsummary The current demo branch contains a single test in MdePkg for SafeIntLib (m= odule file MdePkg\Test\UnitTest\Library\BaseSafeIntLib\TestBaseSafeIntLib.i= nf). This test is automatically detected by the HostUnitTestComilerPlugin a= nd run by the HostBasedUnitTestRunner as part of the CI process. A few notes about the current demo: 1) The demo currently only works on Windows build chains, but there's no r= eason to believe that it can't work equally well on Linux build chains, and= are happy to work with anyone to get it there. 2) The demo currently has four failing conditions that can be seen towards= the end of MdePkg "Build and Test" log file for this build: https://dev.azure.com/tianocore/edk2-ci-play/_build/results?buildId=3D2813= "WARNING - Test SafeInt16ToChar8 - Status d:\a\1\s\MdePkg\Test\UnitTest\Library\BaseSafeIntLib\TestBaseSafeIntLib.c:= 302: error: Failure! WARNING - TestBaseSafeIntLib.exe Test Failed WARNING - Test SafeInt32ToChar8 - Status d:\a\1\s\MdePkg\Test\UnitTest\Library\BaseSafeIntLib\TestBaseSafeIntLib.c:= 638: error: Failure! WARNING - TestBaseSafeIntLib.exe Test Failed WARNING - Test SafeIntnToChar8 - Status d:\a\1\s\MdePkg\Test\UnitTest\Library\BaseSafeIntLib\TestBaseSafeIntLib.c:= 1051: error: Failure! WARNING - TestBaseSafeIntLib.exe Test Failed WARNING - Test SafeInt64ToChar8 - Status d:\a\1\s\MdePkg\Test\UnitTest\Library\BaseSafeIntLib\TestBaseSafeIntLib.c:= 1456: error: Failure!" These failures seem to be legitimate and further work should be done by th= e community to decide whether the test case is correct or the library is co= rrect, but one of them needs to change. 3) Current demo pulls in test collateral from a fork of the edk2-test repo= . This repo is currently *very* heavy due to the history of the UEFI SCT pr= oject and the number of binaries that it pulls down. It's possible that we = (the community) need to select a better place for this code to live. Maybe = in edk2 primary (though it's not explicitly firmware code, so it seems unne= cessary). Maybe in a new edk2-test2 repo or something like that. There's an RFC doc here: https://github.com/corthon/edk2-staging/blob/edk2= -host-test_v2/Readme-RFC.md And a usage guide here: https://github.com/corthon/edk2-staging/blob/edk2-= host-test_v2/UnitTestPkg/ReadMe.md Once again, would love to get this into EDK2 master after stabilization, a= nd most of this has previously been shopped around in other discussion thre= ads. This is just where the rubber meets the road and is the minimal subset= of code that needs to land for folks to start contributing unittests again= st the core libraries that can be run as part of any CI pass. Thanks! - Bret --_000_74D8A39837DF1E4DA445A8C0B3885C503F8A1CEBshsmsx102ccrcor_ Content-Type: text/html; charset="us-ascii" Content-Transfer-Encoding: quoted-printable

Answer the question 5,6,7,8, which are related to H= BFA.

 

For other MSFT unit test, I don’t have strong= opinion. I am OK with that.

 

Thank you

Yao Jiewen

 

From: de= vel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Bret Barkelew via Groups.Io
Sent: Saturday, December 14, 2019 8:47 AM
To: devel@edk2.groups.io; Andrew Fish <afish@apple.com>; Kinn= ey, Michael D <michael.d.kinney@intel.com>
Cc: rfc@edk2.groups.io
Subject: Re: [EXTERNAL] Re: [edk2-devel] EDK2 Host-Based Unit Test = RFC (Now with docs!)

 

Mike,

 

I think I’ve gotten all the feedback here and= all the action items from our call. The current branch should be good to g= o, minus the couple of things that Intel was going to look into.=

 

Thanks!

 

- Bret

 

From: Bret Barkelew
Sent: Friday, December 6, 2019 2:21 PM
To: devel@edk2.groups.io; Andrew Fish; Kinney, Mic= hael D
Cc: rfc@edk2.groups.io Subject: RE: [EXTERNAL] Re: [edk2-devel] EDK2 Host-Based Unit Test = RFC (Now with docs!)

 

  1. I see that MdePkg adds a dependency on UnitTestPkg.  This makes UnitT= estPkg the root package when building and running host based unit tests.&nb= sp; This makes sense, but good to highlight that all packages that use host= based tests will introduce a new package dependency on UnitTestPkg.
    1. Since the dependency only applies to unit tests, can we update the Depende= ncyCheck plugin to support listing dependencies for FW components separate = from dependencies for unit tests?
    2. Can move. Capability is there, but mistakenly ad= ded to the wrong section.
  2. I see UnitTestPkg declares 6 new lib classes.  Are all 6 classes inte= nded to be used directly from a unit test case?  If some of these are = only intended to be used from the framework inside the UnitTestPkg can we m= ake them private?
    1. Because there are different instances in multipl= e places (and conceivably more in the future), we don’t see how we co= uld make them private.
  3. In the MdePkg, I see a new top level directory called ‘HostLibrary&#= 8217;. Since these lib instances are only used from a host based test, can = they be moved into  the ‘Test’ directory?
    1. Can move.
  4. MdePkg/MdePkgTest.dsc. 
    1. Can this DSC file be moved into the ‘Test’ directory?

       = ;            &n= bsp;            = ;            &n= bsp;       i. &nbs= p;   Yes=

    1. I see this DSC file only supports VS today.  How much work is it to a= dd GCC support?

       = ;            &n= bsp;            = ;            &n= bsp;       i. &nbs= p;   Don’= t know. This is an item on our list, but lower priority and not a blocker.<= /span>

  1. MdePkg/HostLibrary/BaseLibHost
    1. Why are there 2 INFs.  One with ASM and one without ASM?  Can we= just use the one with ASM and assume NASM is installed?
      2. I see the purpose of this lib instance is to call the
    2. SetJump()/LongJump().  So these implementations always work?  It= looks like it assumes BASE_LIBRARY_JUMP_BUFFER is identical to the host OS= user mode application setjmp()/longjmp() state.
    3. Why are DRx and CRx registers emulated?  I would think and code that = depends on read/writing these registers would not be compatible with host b= ased testing.  Can we change to ASSERT (FALSE)?
    4. PatchInstructionX86() – I suspect this will not work in a host based= environment because it is self modifying code.  Should it be ASSERT (= FALSE)?
    5. Libraries were taken directly from the Intel wor= k in HBFA. They worked so we kept them. We’re just as interested in t= he answers to these questions as you are.

[Jiewen] Here is the answer:

Some tools requi= res source build, such AFL-CC or KLEE to get the result or optimization. Th= ey do analysis besides compiling. These tools only work on C-language. That= why we need non-asm build.

Some test case n= eed access CRx/DRx just as the pre-condition and post-condition, such as MT= RR test. We cannot use ASSERT.

Generally, a big= problem of current based lib is that it mixes the hardware register access= (such as CR/DR, CPUIR, MSR, mode switch) into a common software lib (such = as linked list, math, string, checksum). I hope we can refactory the based lib to separate the hardware access par= t, such as a BaseRegAccessLib, similar to IoLib, PciLib.

Agree on PatchIn= structionX86(). We should use ASSERT(). If we encounter a case that require= s this function, we consider how to make it work later.

 

  1. MdePkg/HostLibrary/DebugLibHost
    1. What is ‘#ifndef TEST_WITH_KLEE’
    2. What is the ‘PatchFormat()’ function?  It is always disab= led with if (0).
    3. Are the PCDs to set the debug message levels disabled on purpose? (DebugPr= intEnabled(), DebugPrintLevelEnabled(), DebugCodeEnabled())
      4. =
    4. Libraries were taken directly from the Intel wor= k in HBFA. They worked so we kept them. We’re just as interested in t= he answers to these questions as you are.

 

[Jiewen] Here is the answer:

        &nb= sp;      This is special optimization for KLEE = 211; a symbolic execution tool. We hope to reduce the unnecessary branch. O= r the KELL might not resolve the function.

        &nb= sp;      PatchFormat() is an old function. Deadcod= e can be removed now.

        &nb= sp;      In the beginning, PCD is not supported. A= nd we don’t find a use case to control PCD in such detail level. To s= implify thing, we just don’t use it. An on/off control (via HOST_DEBU= G_MESSAGE) is good enough. If there is real use case coming up, we c= an see how to enable it.

 

  1. MdePkg/HostLibrary/BaseMemoryLibHost
    1. Why can’t we use MdePkg/Library/BaseMemoryLib/BaseMemoryLib/inf inst= ead and reduce the number of host specific lib instances?
    2. Libraries were taken directly from the Intel wor= k in HBFA. They worked so we kept them. We’re just as interested in t= he answers to these questions as you are.

[Jiewen] Here is the answer:

        &nb= sp;      This is special optimization for fuzzing = tool. If we use standard C lib, the compiler knows it and don’t inser= t branch analysis. If we use our own implementation, the compiler has no id= ea, and try to do analysis. That is unnecessary.

 

 

  1. MdePkg/HostLibrary/MemoryAllocationLibHost
    1. Why is are memcpy(), assert(), memset() used in this lib?  I would ex= pect this lib instance to match the UefiMemoryAllocationLib with the only t= he use of malloc() and free() to replace the UEFI Boot Services calls.
    2. Libraries were taken directly from the Intel wor= k in HBFA. They worked so we kept them. We’re just as interested in t= he answers to these questions as you are.

[Jiewen] Here is the answer:

        &nb= sp;      Similar to 7, using standard-C function c= an reduce the unnecessary analysis for analysis tool.

 

  1. Host library instance naming conventions.
    1. The current naming convention uses the environment as a prefix(e.g. Pei, S= mm, Uefi) and the services the lib instance uses as a post fix.  Would= it make more sense to use ‘Host’ as a prefix instead of a post= fix in the lib instance names?
    2. I don’t know if there’s a 1:1 relati= onship with these. For some library classes (that you might have host versi= ons of), the class itself is the PeiSomethingLib, and the host version woul= d be the PeiSomethingLibHost. No?
  2. Unicode vs ASCII strings
    1. I see InitUnitTestFramework(), CreateUnitTestSuite(), and AddTestCase() al= l take Unicode strings for the labels.  I also see extra code to conve= rt gEfiCallerBaseName from ASCII to Unicode to use it as a short name of a = test framework.  I think it would be simpler if the parameters to these APIs were ASCII and the framework can convert = to Unicode if needed.
    2. No strong feelings, but we already have a bunch = of tests written this way. Since the UnitTestLib is an abstraction that wor= ks as well in Shell as in the Host, going with Unicode strings seemed to ma= tch the art for Shell apps (since the framework was written for Shell first).
  3. Will InitUnitTestFramework() and CreateUnitTestSuite() always be called in= pairs?  If so, can we combine these to a single API? 
    1. I see the SafeIntLib example create a single framework and multiple test s= uites.  Perhaps we can have a single CreateUnitTestSuite() API where F= ramework is an IN/OUT and if it is passed in as NULL, the Framework handle = is created.

       = ;            &n= bsp;            = ;            &n= bsp;       i. &nbs= p;   It’s= not always in pairs. You would only have a single framework init, but coul= d have multiple suites.

    1. I see a pattern where the CreateUnitTestSuite() ‘Package’ para= meter is used as a prefix to every call to AddTestCase() in the ‘Clas= sName’ parameter.  Can we simplify AddTestCase() so it only need= to pass in the name of the unit test case, and the framework can append Package  and the unit test case name?

           = ;            &n= bsp;            = ;            &n= bsp;       i. &nbs= p;   Right now,= our tests just coincidentally share similar names with the packages, but w= e don’t feel this is axiomatic and don’t want to force this nam= ing on others, who may be trying to bolt into other reporting structures.

    1. I see the use of the ‘Fw’ variable as a shorthand for ‘F= ramework’.  Can we use something other than ‘Fw’.&nb= sp; It may be confused with ‘Firmware’. 
      1. No real arguments. Wouldn’t fight a find-r= eplace, but it’ll just be a bunch of touches as we commit.
    2. UnitTestPkg/Include/UnitTestTypes.h
      1. I see several hard coded string lengths.  Since this runs in a host e= nvironment and strings can always be allocated, can the hard coded lengths = be removed?  Update the structs to use pointers to strings instead of = string arrays, and the framework can manage alloc() and free()?

           = ;            &n= bsp;            = ;            &n= bsp;       i. &nbs= p;   Given that= this framework is designed to be nimble enough to work in PEI and SMM and = other constrictive environments, we wanted to keep fixed sizing.

      1. How are Fingerprints used?  The idea of using as hash as a unique ide= ntifier is a good idea.  How is the hash calculated?  What unit t= est code artifacts are used so developers know what parameters must be uniq= ue?  Can we just decide to use a specific hash algorithm/size and the structure can be a pointer to an allocated buffer instead of a fi= xed size array to make it easy to change the algorithm/size in the future?<= o:p>

           = ;            &n= bsp;            = ;            &n= bsp;       i. &nbs= p;   Fingerprin= ts are unique IDs to make sure that serialize/unserialized state matches th= e tests upon re-entry. I’m not married to MD5, but it needs to be pre= dictably unique, and carried with the framework. I would not want to make any requ= irements on CryptoLib, since these aren’t crypto-strong.<= /o:p>

      1. Update all the strings to be ASCII?  See Unicode vs ASCII above.=

           = ;            &n= bsp;            = ;            &n= bsp;       i. &nbs= p;   Ideally no= t, unless there’s a strong use case.

      1. UNIT_TEST_SAVE_TEST – The last field is a variable sized array, so i= t can be a formal field.

           = ;            &n= bsp;            = ;            &n= bsp;       i. &nbs= p;   Not oppose= d, but don’t really want to make the change myself. Is there a style = guide that this is breaking?

      1. UNIT_TEST_SAVE_CONTEXT - – The last field is a variable sized array,= so it can be a formal field.
      2. UNIT_TEST_SAVE_HEADER – Can the last 3 fields be changed to pointers= and be formal fields?
      3. Do the structures really need to be packed?  Specially with the chang= es suggested above?  Is the intent to potentially share data stored on= disk between different host execution environments that may have different= width architectures?

           = ;            &n= bsp;            = ;            &n= bsp;       i. &nbs= p;   That’= ;s an interesting point. Could you draw up your suggestion and submit a PR?=

    1. UnitTestPkg – UnitTestLib.h
      1. Can you provide more context for the APIs SaveFrameworkState(), SaveFramew= orkStateAndQuit(), SaveFrameworkStateAndReboot(), SetFrameworkBootNextDevic= e().  I do not see any Load() functions, so how would a set of tests b= e resumed?  If these do not apply to host based tests, should these be split out to a different lib class?

           = ;            &n= bsp;            = ;            &n= bsp;       i. &nbs= p;   I’ll= improve the documentation around these functions. I will acknowledge, howe= ver, that this is an interface that I expect to evolve as we figure out wha= t kinds of tests the community wants support for “out of the box̶= 1;. If we want to easily enable tests around – for example – Ex= itBootServices, we will likely want to tweak this going forward to its simp= lest form. The version we have here was sufficient to enable the test cases that we’ve currently written.

    1. UnitTestPkg/Library/UnitTestLib
      1. I see an implementation of MD5.  We should not do our own.  We s= hould use an approved implementation such as OpenSSL.

           = ;            &n= bsp;            = ;            &n= bsp;       i. &nbs= p;   Happy to d= iscuss another implementation, but this is not crypto-strong. It’s ju= st for uniqueness. A sufficiently long CRC would probably work, too.=

    1. UnitTestPkg/Library/UnitTestTerminationLibTbd
      1. Do we really need this lib instance now?

           = ;            &n= bsp;            = ;            &n= bsp;       i. &nbs= p;   This is he= re so that we can figure out what this should look like for a host. Host ob= viously wouldn’t reset, but could conceivably quit. Or maybe that doe= sn’t make any sense for a host.

     

     

     

    - Bret

     

    From: devel@edk2.groups.io <devel@edk2.groups.io> on behalf o= f Bret Barkelew via Groups.Io <bret.barkelew=3Dmicrosoft.com@groups.io>
    Sent: Wednesday, December 4, 2019 10:24:21 AM
    To: Andrew Fish <afish@apple.= com>; devel@edk2.groups.io <devel= @edk2.groups.io>; Kinney, Michael D <michael.d.kinney@intel.com>
    Cc: rfc@edk2.groups.io &l= t;rfc@edk2.groups.io>
    Subject: Re: [EXTERNAL] Re: [edk2-devel] EDK2 Host-Based Unit Test = RFC (Now with docs!)

     

    Andrew,

    I agree with your points.

     

    Mike,

    You’ve got a lot more there. Let me take a l= ook and update the draft. I’ll ping back ASAP.

     

    - Bret

     

    From: An= drew Fish
    Sent: Wednesday, December 4, 2019 9:50 AM
    To: devel@edk2.groups.io; Kinney, Michael D
    Cc: Bret Barkelew; rfc@edk2.groups.io
    Subject: [EXTERNAL] Re: [edk2-devel] EDK2 Host-Based Unit Test RFC = (Now with docs!)

     

    Mike,

     

    I like and agree with your comments.

     

    On the UnitTestPkg(s) dependency issue I think it = would make sense to move as much as possible into the *Pkg/Test/ (&nbs= p;*Pkg/HostLibrary,  MdePkg/MdePkgTest.dsc, etc.) directory structure.= It looks like for implementation specific tests we skip the Test directory and drop the UnitTest or FuzzTest directly into m= odules directory. Maybe we should follow the same pattern as *Pkg/Test and = have a Test directory? This will help minimize the number of "reserved= " directories we need for managing the testing. Have a standardized directory structure will make it easier to d= ifferentiate the code from tests while doing `git grep` or other forms of c= ode spelunking. 

     

    A Host-Based Unit Test for a Library makes sense a= s you can link directly against the library and test it. A Host-Based Unit = Test for Protocol/PPI/GUID [1] seems a little more complex. It is easy enou= gh to write tests for the interfaces but what APIs do you call to get access to the protocol? =

     

    Per our conversation at the Stewards meeting I thi= nk make sense to try to roll out the testing of libraries as the 1st phase.= The mocking required for drivers could get quite complex and let us not ge= t bogged down in all that to get something working. 

     

    [1] *Pkg/Test/UnitTest/[Library|Protocol|Ppi|= Guid] 

     

    Thanks,

     

    Andrew Fish

     

     

    On Dec 2, 2019, at 3:12 PM, Michael D Kinney <<= a href=3D"mailto:michael.d.kinney@intel.com">michael.d.kinney@intel.com= > wrote:

     

    Hi Bret,

     

    Thanks for posting this content.  Host based unit testing is a very= valuable addition to the CI checks.

     

    I have the following comments:

     

    1. I see that MdePkg adds a dependency on UnitTestPkg.  This makes UnitTestPkg the root package when building and running host based unit tests. <= span class=3D"xapple-converted-space"> This makes sense, but good to highlight that all packages that use host based tests = will introduce a new package dependency on UnitTestPkg.
      1. Since the dependency only applies to unit tests, can we update the Depende= ncyCheck plugin t= o support listing dependencies for FW components separate from dependencies for unit tests?
    1. I see UnitTestPkg declares 6 new lib classes. &= nbsp;Are all 6 classes intended to be used directly from a unit test case?  If some of these are only intended to be used from the framework inside t= he UnitTestPkg ca= n we make them private?
    2. In the MdePkg, I see a new top level directory called ‘HostLibrary’. Since these lib instances are = only used from a host based test, can they be moved into  the ‘Test’ directory?
    3. MdePkg/MdePkgTest.d= sc. 
      1. Can this DSC file be moved into the ‘Test’ directory? 
      2. I see this DSC file only supports VS today.  How much work is it to add GCC support?
    1. MdePkg/HostLibrary<= /span>/BaseLibHost
      1. Why are there 2 INFs. &= nbsp;One with ASM and one without ASM?  Can we just use the one with ASM and assume NASM is installed?
      2. I see the purpose of this lib instance is to call the
      3. SetJump()/LongJump<= /span>().  So these= implementations always work? &= nbsp;It looks like it assumes BASE_LIBRARY_JUMP_BUFFER is identical to the host OS user mode application= setjmp()/longjmp() state.<= /li>
      4. Why are DRx and<= span class=3D"xapple-converted-space"> = CRx registers emulated?  I woul= d think and code that depends on read/writing these registers would not be = compatible with host based testing.  Can we change to ASSERT (FALSE)?
      5. PatchInstructionX86() – I suspect this will not work in a host based= environment because it is self modifying code. =  Should it be ASSERT (FALSE)?
    1. MdePkg/HostLibrary<= /span>/DebugLibHost
      1. What is ‘#ifndef TEST_WITH_KLEE’
      2. What is the ‘PatchFormat()’ fun= ction?  It is alway= s disabled with if (0).
      3. Are the PCDs to set the debug message levels disabled on purpose? (DebugPrintEnabled(), DebugPrintLevelEnabled(= ), DebugCodeEnabled())
    1. MdePkg/HostLibrary<= /span>/BaseMemoryLibHost
      1. Why can’t we use = MdePkg/Library/BaseM= emoryLib/BaseMemoryLib/inf instead an= d reduce the number of host specific lib instances?
    1. MdePkg/HostLibrary<= /span>/MemoryAllocationLibHost
      1. Why is are memcpy(), assert(), memset() used in this lib?&= nbsp; I would expect this lib instance to match the UefiMemoryAllocationLib with the only the use of mal= loc() and free() to replace the UEFI Boot Services calls.
    1. Host library instance naming conventions.
      1. The current naming convention uses the environment as a prefix(e.g. Pei, S= mm, Uefi) and the services the lib instance uses as a post fix.  Would it make more sense to use ‘Host’ as a prefix instead = of a postfix in the lib instance names?
    1. Unicode vs ASCII strings
      1. I see InitUnitTestFramework(), CreateUnitTestSuite(), and Ad= dTestCase() all take Unicode strings for the labels.  I also see extra code to convert gEfiCallerBaseNam= e from ASCII to Unicode to use it as a short name of a test framework.  I think it would be simpler= if the parameters to these APIs were ASCII and the framework can convert t= o Unicode if needed.
    1. Will InitUnitTestFramework() and CreateUnitTestSuite() alway= s be called in pairs?  If so, can we combine these to a single API? 
      1. I see the SafeIntLib = ;example create a single framework and multiple test suites.  Perhaps we can have a single CreateUnitTestSuite() API where Framework is an IN/O= UT and if it is passed in as NULL, the Framework handle is created.
      2. I see a pattern where the CreateUnitTestSuite() ‘Package̵= 7; parameter is used as a prefix to every call to AddTestCase() in the ‘ClassName’ parameter.&= nbsp; Can we simplify Ad= dTestCase() so it only need to pass in the name of the unit test case, and the framework can append Package  and the unit test case name?
      1. I see the use of the ‘Fw’ varia= ble as a shorthand for ‘Framework’.  Can we use something other than ‘Fw’.  It may be confused with ‘Firmware’.  
      2. UnitTestPkg/Include/UnitTestTypes.h
        1. I see several hard coded string lengths.  Since this runs in a host environment and strings c= an always be allocated, can the hard coded lengths be removed?  Update the structs to use pointers to strings instead of string arrays, and the framework ca= n manage alloc() and free()?
        2. How are Fingerprints used? &nb= sp;The idea of using as hash as a unique identifier is a good idea.&= nbsp; How is the hash ca= lculated?  What unit test code artifacts are used so developers know what parameters must= be unique?  Can we= just decide to use a specific hash algorithm/size and the structure can be= a pointer to an allocated buffer instead of a fixed size array to make it easy to change the algorithm/size in the fu= ture?
        3. Update all the strings to be ASCII?  See Unicode vs ASCII above.
        4. UNIT_TEST_SAVE_TEST – The last field is a variable sized array, so i= t can be a formal field.
        5. UNIT_TEST_SAVE_CONTEXT - – The last field is a variable sized array,= so it can be a formal field.
        6. UNIT_TEST_SAVE_HEADER – Can the last 3 fields be changed to pointers= and be formal fields?
        7. Do the structures really need to be packed?  Specially with the changes suggested above? = ; Is the intent to poten= tially share data stored on disk between different host execution environments that may have different width architectures?
          8. =
      1. UnitTestPkg – UnitTestLib.h
        1. Can you provide more context for the APIs SaveFrameworkState(), Sav= eFrameworkStateAndQuit(),&nbs= p;SaveFrameworkStateAndReboot(), SetF= rameworkBootNextDevice().  I do not see any Load() functions, so how would a set of tests be resumed?&= nbsp; If these do not ap= ply to host based tests, should these be split out to a different lib class= ?
      1. UnitTestPkg/Library/UnitTestLib
        1. I see an implementation of MD5.  We should not do our own.  We should use an approved implementation such as Ope= nSSL.
      1. UnitTestPkg/Library/UnitTestTerminationLibTbd
        1. Do we really need this lib instance now?

       

      Thanks,

       

      Mike

       

      From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Bret Bar= kelew via Groups.Io
      Sent: Thursday, = November 21, 2019 11:39 PM
      To: devel@edk2.groups= .io
      Subject: [edk2-d= evel] EDK2 Host-Based Unit Test RFC (Now with docs!)

       

      Now that CI has lan= ded in edk2/master, we'd like to get the basic framework for unittesting st= aged as well. Target intercept date would be immediately after the 2019/11 = stabilization, so we wanted to go ahead and get comments now.

      The host unittest framework consists of five primary pieces:
      - The test library (Cmocka)
      - Support libraries (Found in UnitTestPkg)
      - The test support plugins (HostUnitTestComilerPlugin, HostUnitTestDxeComp= leteCheck, HostBasedUnitTestRunner)
      - The configuration in the package-based *.ci.yaml file and package-based = Test.dsc
      - The tests themselves

      We have a demo branch set up at:
      https://github.com/corthon/edk2-staging/tree/edk= 2-host-test_v2
      We also have a demo build pipeline at:
      https://dev.azure.com/tianoc= ore/edk2-ci-play/_build?definitionId=3D36&_a=3Dsummary

      The current demo branch contains a single test in MdePkg for SafeIntLib (m= odule file MdePkg\Test\UnitTest\Library\BaseSafeIntLib\TestBaseSafeIntLib.i= nf). This test is automatically detected by the HostUnitTestComilerPlugin a= nd run by the HostBasedUnitTestRunner as part of the CI process.

      A few notes about the current demo:
      1) The demo currently only works on Windows build chains, but there's no r= eason to believe that it can't work equally well on Linux build chains, and= are happy to work with anyone to get it there.

      2) The demo currently has four failing conditions that can be seen towards= the end of MdePkg "Build and Test" log file for this build:
      https://dev.azure.com/tianocore/= edk2-ci-play/_build/results?buildId=3D2813
      "WARNING -   Test SafeInt16ToChar8 - Status
      d:\a\1\s\MdePkg\Test\UnitTest\Library\BaseSafeIntLib\TestBaseSafeIntLib.c:= 302: error: Failure!
      WARNING - TestBaseSafeIntLib.exe Test Failed
      WARNING -   Test SafeInt32ToChar8 - Status
      d:\a\1\s\MdePkg\Test\UnitTest\Library\BaseSafeIntLib\TestBaseSafeIntLib.c:= 638: error: Failure!
      WARNING - TestBaseSafeIntLib.exe Test Failed
      WARNING -   Test SafeIntnToChar8 - Status
      d:\a\1\s\MdePkg\Test\UnitTest\Library\BaseSafeIntLib\TestBaseSafeIntLib.c:= 1051: error: Failure!
      WARNING - TestBaseSafeIntLib.exe Test Failed
      WARNING -   Test SafeInt64ToChar8 - Status
      d:\a\1\s\MdePkg\Test\UnitTest\Library\BaseSafeIntLib\TestBaseSafeIntLib.c:= 1456: error: Failure!"
      These failures seem to be legitimate and further work should be done by th= e community to decide whether the test case is correct or the library is co= rrect, but one of them needs to change.

      3) Current demo pulls in test collateral from a fork of the edk2-test repo= . This repo is currently *very* heavy due to the history of the UEFI SCT pr= oject and the number of binaries that it pulls down. It's possible that we = (the community) need to select a better place for this code to live. Maybe in edk2 primary (though it's no= t explicitly firmware code, so it seems unnecessary). Maybe in a new edk2-t= est2 repo or something like that.

      There’s an RF= C doc here: = https://github.com/corthon/edk2-staging/blob/= edk2-host-test_v2/Readme-RFC.md

      And a usage guide h= ere: https://github.com/corthon/edk2-staging/blob= /edk2-host-test_v2/UnitTestPkg/ReadMe.md


      Once again, would love to get this into EDK2 master after stabilization, a= nd most of this has previously been shopped around in other discussion thre= ads. This is just where the rubber meets the road and is the minimal subset= of code that needs to land for folks to start contributing unittests against the core libraries that can be ru= n as part of any CI pass.

      Thanks!
      - Bret

     

     

     

--_000_74D8A39837DF1E4DA445A8C0B3885C503F8A1CEBshsmsx102ccrcor_-- --_004_74D8A39837DF1E4DA445A8C0B3885C503F8A1CEBshsmsx102ccrcor_ Content-Type: image/png; name="image002.png" Content-Description: image002.png Content-Disposition: inline; filename="image002.png"; size=182; creation-date="Sun, 15 Dec 2019 00:38:14 GMT"; modification-date="Sun, 15 Dec 2019 00:38:14 GMT" Content-ID: Content-Transfer-Encoding: base64 iVBORw0KGgoAAAANSUhEUgAAArYAAAADAQMAAABcVd5AAAAAAXNSR0IArs4c6QAAAAZQTFRFAAAA v83bBDCivQAAAAF0Uk5TAEDm2GYAAAAJcEhZcwAAEnQAABJ0Ad5mH3gAAAAZdEVYdFNvZnR3YXJl AE1pY3Jvc29mdCBPZmZpY2V/7TVxAAAAF0lEQVQoU2P4TxPwh6GBgTaAJs79/wcAufGtzXuVZ5IA AAAASUVORK5CYII= --_004_74D8A39837DF1E4DA445A8C0B3885C503F8A1CEBshsmsx102ccrcor_--