From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from NAM04-BN3-obe.outbound.protection.outlook.com (NAM04-BN3-obe.outbound.protection.outlook.com [40.107.68.93]) by mx.groups.io with SMTP id smtpd.web10.10734.1576354050511774142 for ; Sat, 14 Dec 2019 12:07:31 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@microsoft.com header.s=selector2 header.b=FsFnad/K; spf=pass (domain: microsoft.com, ip: 40.107.68.93, mailfrom: bret.barkelew@microsoft.com) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=VZ2U7tXMOQ3b7nA2eCBE1fLZ3Z+O0DJjYxuCH4mojdVi/K4adEo+MjxxL5bjfiIZmqapOgNnPNz5j1R1DM4drboI0uh2Qs3s2TWhQPy8KP4TuwWH3XVKGvxLDmw5NF2W6tiaYf2tc5U2CBQKl3GbZviwOedOtsOlzrCwRUnEFARNkEByxWcsOHyVxoAGoUfGZzejZ7ECNMPdYa9zkhx2cakvDgwvqzCxH1J1NAv3bprMqMuk0S+ia8xX4XDC/cJWqlvqmbqy72SCOYHstAFscg0gXMfNlxho/9nLKOnp2SFak0MAink1cKmuBsQldNTZTt16Bdv3tetd8IWZkWFUdw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=k7DqP0ZboPhTDJ1eNj1cxtdv5oAea59LZhwm72u1RX4=; b=BX6rG1FCHAlV+DX/7ISSZxl59X/gDpxoywpMWMV3Q188fDImvCHIOKh7KNfNnv3Qcri6FpqBBKwBvM0dVpuUQpP0LuepD8gGQ3t9CegLZYThO4skdeBkEyjPKY6S0a/ibyi+4e4U0H5X2cBgfnJkk7MC6zp/xyv5vFbw8Lmlqs6TC85IVD9jDFCdaP9V0KWl2F9pcXu5Xq1SqDU71CWadl1Db5FXCpZr9TER4CK3kRa0RhfmAnV3N3ADqBrXBdX4L3FcgRxj1VdkuJhCBjTjF7cZSszbep28ZB0qo8LYybh7ZIAssjcJPaJ77LPbnVVQJp7QwwT/7SNsgwwVO6D/fA== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=microsoft.com; dmarc=pass action=none header.from=microsoft.com; dkim=pass header.d=microsoft.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=selector2; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=k7DqP0ZboPhTDJ1eNj1cxtdv5oAea59LZhwm72u1RX4=; b=FsFnad/K5yaEtMz0x+q/5zW/jFP9XaNQd4h5MnoGbq69go8QdyWCAitSgT2GPUFHsoAlhZuyRxlKbGZLQ8YphSWBLIdTas4UkxZWJYLJldHL4/4YUuleXvrcN3rrNSGY4vE4JzJRzNCN+bXFiawje76+Y5o1uqtxR089ugM6Wzs= Received: from CH2PR21MB1400.namprd21.prod.outlook.com (20.180.10.144) by CH2PR21MB1382.namprd21.prod.outlook.com (20.180.10.203) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.2559.8; Sat, 14 Dec 2019 20:07:28 +0000 Received: from CH2PR21MB1400.namprd21.prod.outlook.com ([fe80::a147:c42f:38de:e8bd]) by CH2PR21MB1400.namprd21.prod.outlook.com ([fe80::a147:c42f:38de:e8bd%4]) with mapi id 15.20.2559.012; Sat, 14 Dec 2019 20:07:28 +0000 From: "Bret Barkelew" 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 (Now with docs!) Thread-Topic: [EXTERNAL] Re: [edk2-devel] EDK2 Host-Based Unit Test RFC (Now with docs!) Thread-Index: AQHVoQS12ZW6E6MMrkasWCUh3oP5e6enceDwgALivYCAAAlHOoADZyKbgAsoujaAAUMtSw== Date: Sat, 14 Dec 2019 20:07:28 +0000 Message-ID: References: ,<0F460E19-2A3B-4FEE-8B6F-BE65B9C5F1D4@apple.com>,<15DD3E3A746110DF.27746@groups.io>,, In-Reply-To: Accept-Language: en-US X-MS-Has-Attach: yes X-MS-TNEF-Correlator: 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 authentication-results: spf=none (sender IP is ) smtp.mailfrom=Bret.Barkelew@microsoft.com; x-originating-ip: [2001:4898:80e8:f:6997:3a06:7b62:b798] x-ms-publictraffictype: Email x-ms-office365-filtering-ht: Tenant x-ms-office365-filtering-correlation-id: 13083011-beae-4d54-944e-08d780d13ec2 x-ms-traffictypediagnostic: CH2PR21MB1382:|CH2PR21MB1382: x-ld-processed: 72f988bf-86f1-41af-91ab-2d7cd011db47,ExtAddr x-microsoft-antispam-prvs: x-ms-oob-tlc-oobclassifiers: OLM:350; x-forefront-prvs: 025100C802 x-forefront-antispam-report: SFV:NSPM;SFS:(10019020)(376002)(346002)(396003)(136003)(39860400002)(366004)(174864002)(199004)(189003)(2940100002)(2906002)(186003)(316002)(33656002)(76236002)(10290500003)(6506007)(66446008)(64756008)(66556008)(66476007)(86362001)(8936002)(53546011)(4326008)(7696005)(55016002)(8990500004)(66576008)(66946007)(8676002)(71200400001)(76116006)(966005)(81166006)(110136005)(5660300002)(81156014)(91956017)(9686003)(52536014)(30864003)(478600001)(579004)(569006);DIR:OUT;SFP:1102;SCL:1;SRVR:CH2PR21MB1382;H:CH2PR21MB1400.namprd21.prod.outlook.com;FPR:;SPF:None;LANG:en;PTR:InfoNoRecords;A:1;MX:1; received-spf: None (protection.outlook.com: microsoft.com does not designate permitted sender hosts) x-ms-exchange-senderadcheck: 1 x-microsoft-antispam: BCL:0; x-microsoft-antispam-message-info: EeT2UTjT+zSNcqM90yInFv1XlXCckaFM+RwcR0VnjXRi3ZyCHIG8ISYEkW0/061oB609Px76t6VigelqdiFp6/iVQEHjJm3wYcw1W+pebWFvWkxMVck3CmQUMMz1Y15gyhePaQiSPOnO4JnyV9i+V+uq7S8Y5nV/+YySwDCWUpRfwnVJuqZG7bcXz0ru3VvbrMbFUjQvOW3D88ONyoVTpmFU/T0ucBCBOSbfv1UjTC2bR19mQAr3qpnx3mTv6QUBccX8+jwJ6qXS9O0jp8bNCRSD07JJ1qnt8jTI7Nl59ZtpugpvlX87ncpCmGje4cxuT/Cda4zab1uqIEGCvhDFOJDCjTvWybmaFQrVjpyhRiYKd2m2BVFqdchVKAFEcOOvnHezrXmW+brjpFKJfAME6Y27ovdZQEvvPPtSMMO/zggqt4f8XfP5rt51QwMrKw975818kCmihn0nsC/vZjNrOp7ylKFcPO9YwvUTNlY6Y3gMBNTzkfccqSo36z99+rfQgzpq0CI3rK0wymUhU296dLUQkxg1i0sC/Ib3U9WY+1c= x-ms-exchange-transport-forked: True MIME-Version: 1.0 X-OriginatorOrg: microsoft.com X-MS-Exchange-CrossTenant-Network-Message-Id: 13083011-beae-4d54-944e-08d780d13ec2 X-MS-Exchange-CrossTenant-originalarrivaltime: 14 Dec 2019 20:07:28.4339 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: 72f988bf-86f1-41af-91ab-2d7cd011db47 X-MS-Exchange-CrossTenant-mailboxtype: HOSTED X-MS-Exchange-CrossTenant-userprincipalname: dJOqLCIVIq0hGMZ6XLuzVu227fcV5dmelp6xUx3a6mcTQx+MEyOqr1eNEVwYhSRJVWjcem2/L60gpo86t6+ZWg== X-MS-Exchange-Transport-CrossTenantHeadersStamped: CH2PR21MB1382 X-Groupsio-MsgNum: 52234 Content-Language: en-US Content-Type: multipart/related; boundary="_005_CH2PR21MB1400635686DC7CEE81BA276BEF570CH2PR21MB1400namp_"; type="multipart/alternative" --_005_CH2PR21MB1400635686DC7CEE81BA276BEF570CH2PR21MB1400namp_ Content-Type: multipart/alternative; boundary="_000_CH2PR21MB1400635686DC7CEE81BA276BEF570CH2PR21MB1400namp_" --_000_CH2PR21MB1400635686DC7CEE81BA276BEF570CH2PR21MB1400namp_ Content-Type: text/plain; charset="Windows-1252" Content-Transfer-Encoding: quoted-printable The host-based tests now build on Linux/GCC, but the final executables don= =92t seem to get created. Don=92t know where the disconnect is there. I ca= n see the test get compiled (along with all the libraries), but it doesn=92= t seem to link. Can you take a look? Thanks! - Bret From: Bret Barkelew Sent: Friday, December 13, 2019 4:46 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!) Mike, I think I=92ve gotten all the feedback here and all the action items from = our call. The current branch should be good to go, minus the couple of thin= gs 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=92t see how we could make them private= . 3. In the MdePkg, I see a new top level directory called =91HostLibrary= = =92. Since these lib instances are only used from a host based test, can t= hey be moved into the =91Test=92 directory? * Can move. 4. MdePkg/MdePkgTest.dsc. * Can this DSC file be moved into the =91Test=92 directory? i. Yes * I see this DSC file only supports VS today. How much work is it = to add GCC support? i. Don= = =92t 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() =96 I suspect this will not work in a host = based environment because it is self modifying code. Should it be ASSERT (= FALSE)? * Libraries were taken directly from the Intel work in HBFA. They w= orked so we kept them. We=92re just as interested in the answers to these q= uestions as you are. 2. MdePkg/HostLibrary/DebugLibHost * What is =91#ifndef TEST_WITH_KLEE=92 * What is the =91PatchFormat()=92 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=92re just as interested in the answers to these q= uestions as you are. 3. MdePkg/HostLibrary/BaseMemoryLibHost * Why can=92t we use MdePkg/Library/BaseMemoryLib/BaseMemoryLib/inf= instead 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=92re just as interested in the answers to these q= uestions as you are. 4. 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=92re just as interested in the answers to these q= uestions as you are. 5. 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 =91Host=92 as a prefix instead of a postfix = in the lib instance names? * I don=92t know if there=92s a 1:1 relationship with these. For so= me library classes (that you might have host versions of), the class itself= is the PeiSomethingLib, and the host version would be the PeiSomethingLibH= ost. No? 6. 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). 7. 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= =92s not always in pairs. You would only have a single framework init, but= could have multiple suites. * I see a pattern where the CreateUnitTestSuite() =91Package=92 par= ameter is used as a prefix to every call to AddTestCase() in the =91ClassNa= me=92 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 t= he unit test case name? i. Rig= ht now, our tests just coincidentally share similar names with the packages= , but we don=92t feel this is axiomatic and don=92t want to force this nami= ng on others, who may be trying to bolt into other reporting structures. 1. I see the use of the =91Fw=92 variable as a shorthand for =91Framewo= rk=92. Can we use something other than =91Fw=92. It may be confused with = = =91Firmware=92. * No real arguments. Wouldn=92t fight a find-replace, but it=92ll j= ust 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. Giv= en that this framework is designed to be nimble enough to work in PEI and S= MM 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. Fin= gerprints are unique IDs to make sure that serialize/unserialized state mat= ches the tests upon re-entry. I=92m not married to MD5, but it needs to be = predictably unique, and carried with the framework. I would not want to mak= e any requirements on CryptoLib, since these aren=92t crypto-strong. * Update all the strings to be ASCII? See Unicode vs ASCII above. i. Ide= ally not, unless there=92s a strong use case. * UNIT_TEST_SAVE_TEST =96 The last field is a variable sized array,= so it can be a formal field. i. Not= opposed, but don=92t really want to make the change myself. Is there a sty= le guide that this is breaking? * UNIT_TEST_SAVE_CONTEXT - =96 The last field is a variable sized a= rray, so it can be a formal field. * UNIT_TEST_SAVE_HEADER =96 Can the last 3 fields be changed to poi= nters 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. Tha= t=92s an interesting point. Could you draw up your suggestion and submit a = PR? 1. UnitTestPkg =96 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= =92ll improve the documentation around these functions. I will acknowledge= , however, that this is an interface that I expect to evolve as we figure o= ut what kinds of tests the community wants support for =93out of the box=94= . If we want to easily enable tests around =96 for example =96 ExitBootServ= ices, 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= =92ve 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. Hap= py to discuss another implementation, but this is not crypto-strong. It=92s= just for uniqueness. A sufficiently long CRC would probably work, too. 1. UnitTestPkg/Library/UnitTestTerminationLibTbd * Do we really need this lib instance now? i. Thi= s is here so that we can figure out what this should look like for a host. = Host obviously wouldn=92t reset, but could conceivably quit. Or maybe that = doesn=92t make any sense for a host. - Bret From: devel@edk2.groups.io on behalf of Bret Barkel= ew via Groups.Io Sent: Wednesday, December 4, 2019 10:24:21 AM To: Andrew Fish ; devel@edk2.groups.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=92ve got a lot more there. Let me take a look and update the draft. I= =92ll 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 =91HostLibrary= = =92. Since these lib instances are only used from a host based test, can t= hey be moved into the =91Test=92 directory? 3. MdePkg/MdePkgTest.dsc. * Can this DSC file be moved into the =91Test=92 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() =96 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/DebugLibHost * What is =91#ifndef TEST_WITH_KLEE=92 * What is the =91PatchFormat()=92 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=92t we use MdePkg/Library/BaseMemoryLib/BaseMemoryLib/inf= instead 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 =91Host=92 as a prefix instead of a postfix = in the 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() =91Package=92 par= ameter is used as a prefix to every call to AddTestCase() in the =91ClassNa= me=92 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 t= he unit test case name? 1. I see the use of the =91Fw=92 variable as a shorthand for =91Framewo= rk=92. Can we use something other than =91Fw=92. It may be confused with = = =91Firmware=92. 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 =96 The last field is a variable sized array,= so it can be a formal field. * UNIT_TEST_SAVE_CONTEXT - =96 The last field is a variable sized a= rray, so it can be a formal field. * UNIT_TEST_SAVE_HEADER =96 Can the last 3 fields be changed to poi= nters 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 =96 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=92s an RFC doc here: https://github.com/corthon/edk2-staging/blob/ed= k2-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_CH2PR21MB1400635686DC7CEE81BA276BEF570CH2PR21MB1400namp_ Content-Type: text/html; charset="Windows-1252" Content-Transfer-Encoding: quoted-printable

The host-based tests now build on Linux/GCC, but th= e final executables don=92t seem to get created. Don=92t know where the dis= connect is there. I can see the test get compiled (along with all the libra= ries), but it doesn=92t seem to link.

 

Can you take a look? Thanks!

 

- Bret

 

From: Bret Barkelew
Sent: Friday, December 13, 2019 4:46 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!)

 

Mike,

 

I think I=92ve gotten all the feedback here and all= the action items from our call. The current branch should be good to go, m= inus 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=92t see how we could = make them private.
  3. In the MdePkg, I see a new top level directory called =91HostLibrary=92. S= ince these lib instances are only used from a host based test, can they be = moved into  the =91Test=92 directory?
    1. Can move.
  4. MdePkg/MdePkgTest.dsc. 
    1. Can this DSC file be moved into the =91Test=92 directory?
      2. =

       = ;            &n= bsp;            = ;            &n= bsp;            = ;      i. &nbs= p;    Yes<= /span>

    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= =92t know. This is an item on our list, but lower priority and not a block= er.

  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() =96 I suspect this will not work in a host based env= ironment because it is self modifying code.  Should it be ASSERT (FALS= E)?
    5. Libraries were taken directly from the Intel wor= k in HBFA. They worked so we kept them. We=92re just as interested in the a= nswers to these questions as you are.
  2. MdePkg/HostLibrary/DebugLibHost
    1. What is =91#ifndef TEST_WITH_KLEE=92
    2. What is the =91PatchFormat()=92 function?  It is always disabled 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=92re just as interested in the a= nswers to these questions as you are.
  3. MdePkg/HostLibrary/BaseMemoryLibHost
    1. Why can=92t 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 wor= k in HBFA. They worked so we kept them. We=92re just as interested in the a= nswers to these questions as you are.
  4. 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=92re just as interested in the a= nswers to these questions as you are.
  5. 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 =91Host=92 as a prefix instead of a postfix in t= he lib instance names?
    2. I don=92t know if there=92s a 1:1 relationship w= ith these. For some library classes (that you might have host versions of),= the class itself is the PeiSomethingLib, and the host version would be the= PeiSomethingLibHost. No?
  6. 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).
  7. 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= =92s not always in pairs. You would only have a single framework init, but= could have multiple suites.

    1. I see a pattern where the CreateUnitTestSuite() =91Package=92 parameter is= used as a prefix to every call to AddTestCase() in the =91ClassName=92 par= ameter.  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;    Righ= t now, our tests just coincidentally share similar names with the packages,= but we don=92t feel this is axiomatic and don=92t 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 =91Fw=92 variable as a shorthand for =91Framework=92.=   Can we use something other than =91Fw=92.  It may be confused w= ith =91Firmware=92. 
      1. No real arguments. Wouldn=92t fight a find-repla= ce, but it=92ll just be a bunch of touches as we commit.<= /li>
    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;    Give= n that this framework is designed to be nimble enough to work in PEI and SM= M 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;    Fing= erprints are unique IDs to make sure that serialize/unserialized state matc= hes the tests upon re-entry. I=92m not married to MD5, but it needs to be predictably unique, and carried with the framework. I would not want to m= ake any requirements on CryptoLib, since these aren=92t crypto-strong.

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

           = ;            &n= bsp;            = ;            &n= bsp;            = ;      i. &nbs= p;    Idea= lly not, unless there=92s a strong use case.

      1. UNIT_TEST_SAVE_TEST =96 The last field is a variable sized array, so it ca= n be a formal field.

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

      1. UNIT_TEST_SAVE_CONTEXT - =96 The last field is a variable sized array, so = it can be a formal field.
      2. UNIT_TEST_SAVE_HEADER =96 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= = =92s an interesting point. Could you draw up your suggestion and submit a = PR?

    1. UnitTestPkg =96 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=92= ll improve the documentation around these functions. I will acknowledge, ho= wever, that this is an interface that I expect to evolve as we figure out what kinds of tests the community wants support for =93out of the box=94.= If we want to easily enable tests around =96 for example =96 ExitBootServi= ces, 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=92ve 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;    Happ= y to discuss another implementation, but this is not crypto-strong. It=92s = just 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 here so that we can figure out what this should look like for a host. H= ost obviously wouldn=92t reset, but could conceivably quit. Or maybe that doesn=92t make any sense for a host.

     

     

     

    - Bret

     

    3D"cid:image001.png@01D5B1D4.E=

    From: devel@edk2.groups.io <devel@edk2.groups.io> = on behalf of 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 <d= evel@edk2.groups.io>; Kinney, Michael D <michael.d.kinney@intel.com&g= t;
    Cc: rfc@edk2.groups.io <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=92ve got a lot more there. Let me take a look = and update the draft. I=92ll 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 =91HostLibrary=92. Since these lib instances are only use= d from a host based test, can they be moved into  the =91Test=92 directory?
    3. MdePkg/MdePkgTest.d= sc. 
      1. Can this DSC file be moved into the =91Test=92 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() =96 I suspect this will not work in a host based env= ironment because it is <= span class=3D"xspelle">self modifying code. &nbs= p;Should it be ASSERT (FALSE)?
    1. MdePkg/HostLibrary<= /span>/DebugLibHost
      1. What is =91#ifndef TEST_WITH_KLEE=92
      2. What is the =91PatchFormat()=92 function?&n= bsp; It is always disabl= ed with if (0).
      3. DebugPrintEnabled(), DebugPrintLevelEnabled(= ), DebugCodeEnabled())
    1. MdePkg/HostLibrary<= /span>/BaseMemoryLibHost
      1. Why can=92t we use MdePkg/Library/BaseMemor= yLib/BaseMemoryLib/inf instead and re= duce 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 =91Host=92 as a prefix instead of a pos= tfix 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() =91Package=92 param= eter is used as a prefix to every call to AddTestCase() in the =91ClassName=92 parameter.  Can we simplify AddTestCa= se() 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 =91Fw=92 variable as a= shorthand for =91Framework=92.  Can we use something other than =91Fw= =92.  It may be confused with =91Firmware=92.  
      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 =96 The last field is a variable sized array, so it ca= n be a formal field.
        5. UNIT_TEST_SAVE_CONTEXT - =96 The last field is a variable sized array, so = it can be a formal field.
        6. UNIT_TEST_SAVE_HEADER =96 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 =96 = 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=92s an RFC do= c here: https://github.com/corthon/edk2-staging/blob/edk= 2-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_CH2PR21MB1400635686DC7CEE81BA276BEF570CH2PR21MB1400namp_-- --_005_CH2PR21MB1400635686DC7CEE81BA276BEF570CH2PR21MB1400namp_ Content-Type: image/png; name="0F02058703214FAEB779864D35F743F9.png" Content-Description: 0F02058703214FAEB779864D35F743F9.png Content-Disposition: inline; filename="0F02058703214FAEB779864D35F743F9.png"; size=169; creation-date="Sat, 14 Dec 2019 20:07:27 GMT"; modification-date="Sat, 14 Dec 2019 20:07:27 GMT" Content-ID: Content-Transfer-Encoding: base64 iVBORw0KGgoAAAANSUhEUgAAAz8AAAADCAYAAAC9BbEIAAAAAXNSR0IArs4c6QAAAARnQU1BAACx jwv8YQUAAAAJcEhZcwAADsMAAA7DAcdvqGQAAAA+SURBVGhD7dcxDcAwDABBcwr/dMuWLZuzFYhL oAise+lAfDw7CwAAoLtY56257whJkiRJ6tzfEQEAAPSS9QFv5xeVQL05XgAAAABJRU5ErkJggg== --_005_CH2PR21MB1400635686DC7CEE81BA276BEF570CH2PR21MB1400namp_ Content-Type: image/png; name="8836278201304A9BAB5D003376CEB97F.png" Content-Description: 8836278201304A9BAB5D003376CEB97F.png Content-Disposition: inline; filename="8836278201304A9BAB5D003376CEB97F.png"; size=148; creation-date="Sat, 14 Dec 2019 20:07:27 GMT"; modification-date="Sat, 14 Dec 2019 20:07:27 GMT" Content-ID: Content-Transfer-Encoding: base64 iVBORw0KGgoAAAANSUhEUgAAA1EAAAACCAYAAAC5d1ZuAAAAAXNSR0IArs4c6QAAAARnQU1BAACx jwv8YQUAAAAJcEhZcwAADsMAAA7DAcdvqGQAAAApSURBVFhH7dfBAAAACASw/KUiiOBgSiGAPQax 6skCAADwI1EAAABv2QNqnYvXLKWONgAAAABJRU5ErkJggg== --_005_CH2PR21MB1400635686DC7CEE81BA276BEF570CH2PR21MB1400namp_--