From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mga18.intel.com (mga18.intel.com [134.134.136.126]) by mx.groups.io with SMTP id smtpd.web11.12797.1576531711915660559 for ; Mon, 16 Dec 2019 13:28:32 -0800 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: intel.com, ip: 134.134.136.126, mailfrom: michael.d.kinney@intel.com) X-Amp-Result: UNKNOWN X-Amp-Original-Verdict: FILE UNKNOWN X-Amp-File-Uploaded: False Received: from fmsmga004.fm.intel.com ([10.253.24.48]) by orsmga106.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 16 Dec 2019 13:28:30 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.69,323,1571727600"; d="png'150?scan'150,208,217,150";a="240177180" Received: from orsmsx110.amr.corp.intel.com ([10.22.240.8]) by fmsmga004.fm.intel.com with ESMTP; 16 Dec 2019 13:28:29 -0800 Received: from orsmsx113.amr.corp.intel.com ([169.254.9.100]) by ORSMSX110.amr.corp.intel.com ([169.254.10.84]) with mapi id 14.03.0439.000; Mon, 16 Dec 2019 13:28:29 -0800 From: "Michael D Kinney" To: "Yao, Jiewen" , "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: AQHVst/yGaEy50YdAkSogz1zzl4ccqe9SQ7w Date: Mon, 16 Dec 2019 21:28:28 +0000 Message-ID: References: ,<0F460E19-2A3B-4FEE-8B6F-BE65B9C5F1D4@apple.com>,<15DD3E3A746110DF.27746@groups.io>, <74D8A39837DF1E4DA445A8C0B3885C503F8A1CEB@shsmsx102.ccr.corp.intel.com> In-Reply-To: <74D8A39837DF1E4DA445A8C0B3885C503F8A1CEB@shsmsx102.ccr.corp.intel.com> Accept-Language: en-US X-MS-Has-Attach: yes X-MS-TNEF-Correlator: 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.22.254.139] MIME-Version: 1.0 X-Groupsio-MsgNum: 52261 Content-Language: en-US Content-Type: multipart/related; boundary="_004_E92EE9817A31E24EB0585FDF735412F5B9E4D005ORSMSX113amrcor_"; type="multipart/alternative" --_004_E92EE9817A31E24EB0585FDF735412F5B9E4D005ORSMSX113amrcor_ Content-Type: multipart/alternative; boundary="_000_E92EE9817A31E24EB0585FDF735412F5B9E4D005ORSMSX113amrcor_" --_000_E92EE9817A31E24EB0585FDF735412F5B9E4D005ORSMSX113amrcor_ Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Hi Jiewen, Many of your responses are related to fuzz testing. Can we focus current infrastructure on host based unit testing and minimiz= ing the content to support that testing type. Then, add a new subdirectory below Test in the future to support fuzz test= ing. This way, lib instances can be tuned to the test infrastructure/subsy= stem in use. Trying to make one version of the host libs for multiple test= methodologies it making is more complex and hander to maintain. Thanks, Mike From: Yao, Jiewen Sent: Saturday, December 14, 2019 4:38 PM 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 (No= w with docs!) 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 Barkelew 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. 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? * 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. 1. 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. 1. 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. This is an item on our list, but lower priority and not a block= er. 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? 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. * 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). 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. = It's not always in pairs. You would only have a single framework init, but = could have multiple 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 packa= ges, but we don't feel this is axiomatic and don't want to force this namin= g 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 '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. 1. 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 an= d 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 are unique IDs to make sure that serialize/unserialized state = matches the tests upon re-entry. I'm not married to MD5, but it needs to be= predictably unique, and carried with the framework. I would not want to ma= ke any requirements on CryptoLib, since these aren't crypto-strong. * Update all the strings to be ASCII? See Unicode vs ASCII above. i. = Ideally not, unless 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, but don't really want to make the change myself. Is there a st= yle 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 interesting 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 the documentation around these functions. I will acknowledge, = however, that this 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 w= e want to easily enable tests around - for example - ExitBootServices, we w= ill likely want to tweak this going forward to its simplest form. The versi= on we have here was sufficient to enable the test cases that we've currentl= y 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 discuss another implementation, but this is not crypto-strong. It'= s just for uniqueness. 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 hos= t. Host obviously wouldn't reset, but could conceivably quit. Or maybe that= doesn't make any 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_E92EE9817A31E24EB0585FDF735412F5B9E4D005ORSMSX113amrcor_ Content-Type: text/html; charset="us-ascii" Content-Transfer-Encoding: quoted-printable

H= i Jiewen,

<= o:p> 

M= any of your responses are related to fuzz testing.

<= o:p> 

C= an we focus current infrastructure on host based unit testing and minimizin= g the content to support that testing type.

<= o:p> 

T= hen, add a new subdirectory below Test in the future to support fuzz testin= g.  This way, lib instances can be tuned to the test infrastructure/sub= system in use.  Trying to make one version of the host libs for multiple test metho= dologies it making is more complex and hander to maintain.

<= o:p> 

T= hanks,

<= o:p> 

M= ike

<= o:p> 

From: Yao, Jiewen <jiewen.yao@intel.com>
Sent: Saturday, December 14, 2019 4:38 PM
To: devel@edk2.groups.io; bret.barkelew@microsoft.com; Andrew Fish = <afish@apple.com>; Kinney, Michael D <michael.d.kinney@intel.com&g= t;
Cc: rfc@edk2.groups.io
Subject: RE: [EXTERNAL] Re: [edk2-devel] EDK2 Host-Based Unit Test = RFC (Now with docs!)

 

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: devel@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&g= t;; Kinney, Michael D <mic= hael.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 UnitTestPkg = the root package when building and running host based unit tests.  Thi= s makes sense, but good to highlight that all packages that use host based tests will introduce a new package dependency on Unit= TestPkg.
    1. Since = the dependency only applies to unit tests, can we update the DependencyChec= k plugin to support listing dependencies for FW components separate from de= pendencies for unit tests?
    2. Can move. Capability is there, but mistakenly added to the wrong sectio= n.
    3. I see = UnitTestPkg declares 6 new lib classes.  Are all 6 classes intended to= be used directly from a unit test case?  If some of these are only in= tended to be used from the framework inside the UnitTestPkg can we make them private?
      1. Because there are different instances in multiple places (and conceivab= ly more in the future), we don’t see how we could make them private.<= /span><= o:p>
    1. 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?
      2. <= /ol>
        1. Can move.
      1. MdePkg= /MdePkgTest.dsc. 
        1. Can th= is DSC file be moved into the ‘Test’ directory?

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

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

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

      1. MdePkg= /HostLibrary/BaseLibHost
        1. Why ar= e there 2 INFs.  One with ASM and one without ASM?  Can we just u= se the one with ASM and assume NASM is installed?
          2. I see = the purpose of this lib instance is to call the
        2. SetJum= p()/LongJump().  So these implementations always work?  It looks = like it assumes BASE_LIBRARY_JUMP_BUFFER is identical to the host OS user m= ode application setjmp()/longjmp() state.
        3. Why ar= e DRx and CRx registers emulated?  I would think and code that depends= on read/writing these registers would not be compatible with host based te= sting.  Can we change to ASSERT (FALSE)?
        4. PatchI= nstructionX86() – I suspect this will not work in a host based enviro= nment because it is self modifying code.  Should it be ASSERT (FALSE)?=
        5. Libraries were taken directly from the Intel work in HBFA. They worked = 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 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 i= s ‘#ifndef TEST_WITH_KLEE’
        2. What i= s the ‘PatchFormat()’ function?  It is always disabled wit= h if (0).
        3. Are th= e PCDs to set the debug message levels disabled on purpose? (DebugPrintEnab= led(), DebugPrintLevelEnabled(), DebugCodeEnabled())
          4. =
        4. Libraries were taken directly from the Intel work in HBFA. They worked = so we kept them. We’re just as interested in the answers to these que= stions 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 ca= n’t we use MdePkg/Library/BaseMemoryLib/BaseMemoryLib/inf instead and= reduce the number of host specific lib instances?
        2. Libraries were taken directly from the Intel work in HBFA. They worked = so we kept them. We’re just as interested in the answers to these que= stions 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 expect th= is lib instance to match the UefiMemoryAllocationLib with the only the use = of malloc() and free() to replace the UEFI Boot Services calls.
        2. Libraries were taken directly from the Intel work in HBFA. They worked = so we kept them. We’re just as interested in the answers to these que= stions 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 l= ibrary instance naming conventions.
        1. The cu= rrent naming convention uses the environment as a prefix(e.g. Pei, Smm, Uef= i) and the services the lib instance uses as a post fix.  Would it mak= e more sense to use ‘Host’ as a prefix instead of a postfix in the lib instance names?
        2. I don’t know if there’s a 1:1 relationship with these. For = some library classes (that you might have host versions of), the class itse= lf is the PeiSomethingLib, and the host version would be the PeiSomethingLibHost. No?
      1. Unicod= e vs ASCII strings
        1. I see = InitUnitTestFramework(), CreateUnitTestSuite(), and AddTestCase() all take = Unicode strings for the labels.  I also see extra code to convert gEfi= CallerBaseName from ASCII to Unicode to use it as a short name of a test framework.  I think it would be simpler if th= e parameters to these APIs were ASCII and the framework can convert to Unic= ode if needed.
        2. No strong feelings, but we already have a bunch of tests written this w= ay. Since the UnitTestLib is an abstraction that works as well in Shell as = in the Host, going with Unicode strings seemed to match the art for Shell apps (since the framework was written for Shel= l first).
      1. Will I= nitUnitTestFramework() 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 suites.&= nbsp; Perhaps we can have a single CreateUnitTestSuite() API where Framewor= k is an IN/OUT and if it is passed in as NULL, the Framework handle is created.

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

        1. I see = a pattern where the CreateUnitTestSuite() ‘Package’ parameter i= s used as a prefix to every call to AddTestCase() in the ‘ClassName&#= 8217; parameter.  Can we simplify AddTestCase() so it only need to pas= s in the name of the unit test case, and the framework can append Package&n= bsp; and the unit test case name?

                 &n= bsp;            = ;            &n= bsp;            = ;            &n= bsp;    i. &nbs= p;    Right now, our tests just coincidentally share similar names with the pa= ckages, but we don’t feel this is axiomatic and don’t want to f= orce this naming on others, who may be trying to bolt into other reporting structur= es.

      1. I see = the use of the ‘Fw’ variable as a shorthand for ‘Framewor= k’.  Can we use something other than ‘Fw’.  It = may be confused with ‘Firmware’. 
        1. No real arguments. Wouldn’t fight a find-replace, but it’ll= just be a bunch of touches as we commit.
      1. UnitTe= stPkg/Include/UnitTestTypes.h
        1. I see = several hard coded string lengths.  Since this runs in a host environm= ent and strings can always be allocated, can the hard coded lengths be remo= ved?  Update the structs to use pointers to strings instead of string arrays, and the framework can manage alloc() and free()= ?

                 &n= bsp;            = ;            &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 sizin= g.

        1. How ar= e Fingerprints used?  The idea of using as hash as a unique identifier= is a good idea.  How is the hash calculated?  What unit test cod= e 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 fix= ed size array to make it easy to change the algorithm/size in the future?

                 &n= bsp;            = ;            &n= bsp;            = ;            &n= bsp;    i. &nbs= p;    Fingerprints are unique IDs to make sure that serialize/unserialized sta= te matches the tests upon re-entry. I’m not married to MD5, but it ne= eds to be predictably unique, and carried with the framework. I would not wan= t to make any requirements on CryptoLib, since these aren’t crypto-st= rong.

        1. Update= all the strings to be ASCII?  See Unicode vs ASCII above.<= /span>

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

        1. UNIT_T= EST_SAVE_TEST – The last field is a variable sized array, so it can b= e a formal field.

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

        1. UNIT_T= EST_SAVE_CONTEXT - – The last field is a variable sized array, so it = can be a formal field.
        2. UNIT_T= EST_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 changes sugg= ested above?  Is the intent to potentially share data stored on disk b= etween different host execution environments that may have different width architectures?

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

      1. UnitTe= stPkg – UnitTestLib.h
        1. Can yo= u provide more context for the APIs SaveFrameworkState(), SaveFrameworkStat= eAndQuit(), SaveFrameworkStateAndReboot(), SetFrameworkBootNextDevice().&nb= sp; I do not see any Load() functions, so how would a set of tests be resumed?  If these do not apply to host based test= s, should these be split out to a different lib class?

                 &n= bsp;            = ;            &n= bsp;            = ;            &n= bsp;    i. &nbs= p;    I’ll improve the documentation around these functions. I will ackn= owledge, however, that this is an interface that I expect to evolve as we f= igure out what kinds of tests the community wants support for “out of the= box”. If we want to easily enable tests around – for example &= #8211; 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.<= /o:p>

      1. UnitTe= stPkg/Library/UnitTestLib
        1. I see = an implementation of MD5.  We should not do our own.  We should u= se an approved implementation such as OpenSSL.

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

      1. UnitTe= stPkg/Library/UnitTestTerminationLibTbd
        1. Do we = really need this lib instance now?

                 &n= bsp;            = ;            &n= bsp;            = ;            &n= bsp;    i. &nbs= p;    This is here so that we can figure out what this should look like for a = host. Host obviously wouldn’t reset, but could conceivably quit. Or m= aybe that doesn’t make any sense for a host.

       

       

       

      - Bret

       

      <= o:p>

      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: 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
      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 DependencyChec= k plugin to support listing dependencies for FW components separate from dependenc= ies for unit tests?
      1. I see<= span class=3D"xapple-converted-space"> = UnitTestPkg decla= res 6 new lib classes.  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 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 be moved into  the ‘Test’ directory?<= o:p>
      3. MdePkg/MdePkgTest.dsc. 
        1. Can th= is 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/= BaseLibHost
        1. Why ar= e 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
        3. SetJump()/LongJump(= ).  So these implem= entations always work?  It = looks like it assumes BASE_LIBRARY_JUMP_BUFFER is identical to the host OS = user mode applicationsetjmp()/longjmp() state.
        4. Why ar= e DRx and 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. PatchI= nstructionX86() – I suspect this will not work in a host based enviro= nment because it is self modifying code.  <= /span>Should it be ASSERT (FALSE)?
      1. MdePkg/HostLibrary/= DebugLibHost
        1. What i= s ‘#ifndef TEST_WITH_KLEE’
        2. What i= s the ‘PatchFormat()’ function?&= nbsp; It is always disab= led with if (0).
        3. Are th= e PCDs to set the debug message levels disabled on purpose? (DebugPrintEnabled(), DebugPrintLevelEnabled(), Debu= gCodeEnabled())
      1. MdePkg/HostLibrary/= BaseMemoryLibHost
        1. Why ca= n’t we use MdePkg/Library/BaseMemoryLi= b/BaseMemoryLib/inf instead and reduce the number of host specific lib instances?
      1. MdePkg/HostLibrary/= MemoryAllocationLibHost
        1. Why is= are memcpy(), assert(), = ;memset() used in this lib?  I would expect this lib instance to match the UefiMemoryAllocationLib with the only the use = of malloc() and free() to replace the UEFI Boot Services calls.
      1. Host l= ibrary instance naming conventions.
        1. The cu= rrent naming convention uses the environment as a prefix(e.g. Pei, Smm, Uefi) and the services the lib instance uses as a post fix.  Would it make more sense to use ̵= 6;Host’ as a prefix instead of a postfix in the lib instance names?
      1. Unicod= e vs ASCII strings
        1. I see<= span class=3D"xapple-converted-space"> = InitUnitTestFramework(), = ;CreateUnitTestSuite(), and AddTestCa= se() 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 I= nitUnitTestFramework() and&nb= sp;CreateUnitTestSuite() always be called in pairs? &n= bsp;If so, can we combine these to a single API? 
        1. I see = the SafeIntLib ex= ample create a single framework and multiple test suites.  Perhaps we can have a single <= span class=3D"xspelle">CreateUnitTestSuite() API where Framework is = an IN/OUT and if it is passed in as NULL, the Framework handle is created.<= o:p>
        2. I see = a pattern where the CreateUnitTestSuite() ‘Package’ para= meter 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’ variable as = a shorthand for ‘Framework’.  Can we use something other than ‘Fw’.  It may be confused with ‘Firmware’.  
      2. UnitTestPkg/Include/UnitTe= stTypes.h
        1. I see = several hard coded string lengths.  Since this runs in a host environment and strings can alwa= ys 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()?
        2. How ar= e Fingerprints used?  The idea of using as hash as a unique identifier is a good idea.  How is the hash calculate= d?  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? &= nbsp;See Unicode vs ASCII above.
        4. UNIT_T= EST_SAVE_TEST – The last field is a variable sized array, so it can b= e a formal field.
        5. UNIT_T= EST_SAVE_CONTEXT - – The last field is a variable sized array, so it = can be a formal field.
        6. UNIT_T= EST_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 potentially share data stored on disk between different host execution environments t= hat may have different width architectures?
      1. UnitTestPkg=  – UnitTestLib.h
        1. Can yo= u provide more context for the APIs&= nbsp;SaveFrameworkState(), SaveFrame= workStateAndQuit(), SaveFrameworkStateAndReboot(), SetFramew= orkBootNextDevice(). &nb= sp;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/UnitTe= stLib
        1. I see = an implementation of MD5.  = ;We should not do our own.  We should use an approved implementation such as OpenSSL.
      1. UnitTestPkg/Library/UnitTe= stTerminationLibTbd
        1. Do we = really need this lib instance now?

       

      Thanks,

       

      Mike

       

       

      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_E92EE9817A31E24EB0585FDF735412F5B9E4D005ORSMSX113amrcor_-- --_004_E92EE9817A31E24EB0585FDF735412F5B9E4D005ORSMSX113amrcor_ Content-Type: image/png; name="image002.png" Content-Description: image002.png Content-Disposition: inline; filename="image002.png"; size=204; creation-date="Mon, 16 Dec 2019 21:28:28 GMT"; modification-date="Mon, 16 Dec 2019 21:28:28 GMT" Content-ID: Content-Transfer-Encoding: base64 iVBORw0KGgoAAAANSUhEUgAAA0EAAAADCAYAAABVhQQjAAAAAXNSR0ICQMB9xQAAAAlwSFlzAAAW JQAAFiUBSVIk8AAAABl0RVh0U29mdHdhcmUATWljcm9zb2Z0IE9mZmljZX/tNXEAAABMSURBVGje 7dnBCYAwEADB9GJb1uqB7wT8akArEEEL0GAZYR5TxLIpSn0j13vK2wkAANC7NC97i6BjjLwOAAAA vUv/CSr1aS4AAIDefe2+4bmJmYghAAAAAElFTkSuQmCC --_004_E92EE9817A31E24EB0585FDF735412F5B9E4D005ORSMSX113amrcor_--