From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from NAM12-MW2-obe.outbound.protection.outlook.com (NAM12-MW2-obe.outbound.protection.outlook.com [40.107.244.111]) by mx.groups.io with SMTP id smtpd.web11.11328.1575670915164739055 for ; Fri, 06 Dec 2019 14:21:55 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@microsoft.com header.s=selector2 header.b=cas23Bba; spf=pass (domain: microsoft.com, ip: 40.107.244.111, mailfrom: bret.barkelew@microsoft.com) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=LdVzQ124ghhE/N7W50jU9mhDoG3iemJIgmWO/sf9VSYs1gLRtLeZ54zTapSH4MrOIvm1Qkv7McVxfAlaussG7p20F8SAWJfklbC44ZOEkp87KKYwjvL8dZxRAcfqWXUog2QMDvZlLkexqyZ4ZH0RiXyS8S14oFU+f1jOwAmIhpQkgPLXLq/vj6AGhNhl0gx4Jeb+mHrahPuCtLlEo23zxPuKfisESrv3u4+iB0M5ysyHaDhdZQv8hL34A3o7DOZ4yv/lpFgW6fNLDc/StlbK9rllkOQDKFpM/NRZzwkTg3du/Zv+pNt48EI1dyaenFVIfC5CsQWXi3+f837+UYwrVQ== 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=auzj6KPZmx7dLbnEH2BZy1LTIUwr/WwNBttkqFboidk=; b=EAK2BH+67VKx5DmM6krRApPQpKdLHpQB4+t2L1d66Iv2ciM0hoRXUOH0aJ8BZ0q3lCuJ2sRKNUHzdbs62c3Rv01WxEMDKf6vz/6wca53GAT7I4E06vlhE85Vz9Gs23fWSzIxCqlqCLQYkoJAY2XIYeR1ZF9mjFWq2N1LcfSNXGGJUmUzI5Ce1YXFxw1J+OIaubcqreBcgaUYdm3QxroBrw2SRPnWbV8vskXc31xD90vWtiEqj/JjbAfa3LKUnHbTrSiGsakZW1PpA7cWmrNYjDVyJTBkCWt4Wx4u8Ro57SeQSm4s4Gu5wuU6MbcEMaJMgkpmFwuowiiUA/DZt3nE6Q== 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=auzj6KPZmx7dLbnEH2BZy1LTIUwr/WwNBttkqFboidk=; b=cas23BbaGwqXRqdR16/yPfe1rdAlyBRILrAgIW+1YLlBSj+/TrJkw7Ih7ptygyYUIcfdVg3dyH3nXYKzN7Uyam9o0nMdNPbPcymKMIFUvma9t+aNRUGL8Po0bm26Rs93cHopLMtauxRi4d+iSzbXp3T9QRL1C34LJRAj5X1p/zY= Received: from CH2PR21MB1400.namprd21.prod.outlook.com (20.180.10.144) by CH2PR21MB1430.namprd21.prod.outlook.com (20.180.8.145) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.2538.4; Fri, 6 Dec 2019 22:21:53 +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.2538.000; Fri, 6 Dec 2019 22:21:53 +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: AQHVoQS12ZW6E6MMrkasWCUh3oP5e6enceDwgALivYCAAAlHOoADZyKb Date: Fri, 6 Dec 2019 22:21:53 +0000 Message-ID: References: ,<0F460E19-2A3B-4FEE-8B6F-BE65B9C5F1D4@apple.com>,<15DD3E3A746110DF.27746@groups.io> In-Reply-To: <15DD3E3A746110DF.27746@groups.io> Accept-Language: en-US X-MS-Has-Attach: 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:2:2997:7e8a:17f3:7262] x-ms-publictraffictype: Email x-ms-office365-filtering-ht: Tenant x-ms-office365-filtering-correlation-id: a28be27c-6446-4a57-46f5-08d77a9ab28c x-ms-traffictypediagnostic: CH2PR21MB1430:|CH2PR21MB1430: x-ld-processed: 72f988bf-86f1-41af-91ab-2d7cd011db47,ExtAddr x-microsoft-antispam-prvs: x-ms-oob-tlc-oobclassifiers: OLM:350; x-forefront-prvs: 0243E5FD68 x-forefront-antispam-report: SFV:NSPM;SFS:(10019020)(136003)(376002)(346002)(366004)(39860400002)(396003)(174864002)(189003)(199004)(30864003)(478600001)(966005)(186003)(55016002)(33656002)(110136005)(19627235002)(81166006)(229853002)(66946007)(76236002)(2906002)(10290500003)(76116006)(91956017)(71190400001)(71200400001)(66556008)(64756008)(66446008)(8936002)(99286004)(8990500004)(66476007)(6506007)(52536014)(54896002)(74316002)(10090500001)(81156014)(4326008)(7696005)(9686003)(86362001)(8676002)(5660300002)(76176011)(102836004)(316002)(53546011)(559001)(569006);DIR:OUT;SFP:1102;SCL:1;SRVR:CH2PR21MB1430;H:CH2PR21MB1400.namprd21.prod.outlook.com;FPR:;SPF:None;LANG:en;PTR:InfoNoRecords;MX:1;A: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: l146Xx1KhnRFShR+buqq1tDtenESMgRbMgjpXmeWxw8oLJBAQFEKIDlUSfxFztrwuBKqkSJv6DdQgRE+eDIXCHEiEiAkOHymSDJD+0j2DZWonmUBbV42wAPk/D+XNUG9HR5TpEdPaseAJw+yZmTEFFLAHqf1ihA8+pu8EpqC/01u9Z+r3zr8X8/hMXZTMRXJK91KaFjoYP+oz8hsjpC0xy9ZLZ+QBWSf9McIXw4j4/YY4//HVx6haTqVHqQT+8w1KHdoY7AqStktOsMkguG82xkda7CDKVl/22JLF5V99JDQa2MWoVZL0GAF0gZz9YdUUrQFrIzvQbz/vWVM1liF7Pf96fXRKywQqbL4XNeNopKyCRznVy1COTOtfcowuw0BENJm1tMw1q/wptpkUXyryVMO2zGr8RGTdBaFB7/LZGybJsnYhXX7fd26tXLWLz5mFpvNglbwKJsxHvk6doWde/j3ZNI0rZesr6iPCt7x+qK/sAMOmjCuDRxpd9fCeg4E7pxwwK/cEJ0yLjdNklXNe5o1p4C40IT/X0faDxkNgHA= x-ms-exchange-transport-forked: True MIME-Version: 1.0 X-OriginatorOrg: microsoft.com X-MS-Exchange-CrossTenant-Network-Message-Id: a28be27c-6446-4a57-46f5-08d77a9ab28c X-MS-Exchange-CrossTenant-originalarrivaltime: 06 Dec 2019 22:21:53.3920 (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: hutsOlS7ue4osmLW4wF2+xfxry7kTsAdDEaa6H+48tkWm04N7qTVMuzpkseF6GBJB2wGSg3xUDQNP0qC4U+Ejw== X-MS-Exchange-Transport-CrossTenantHeadersStamped: CH2PR21MB1430 Content-Language: en-US Content-Type: multipart/alternative; boundary="_000_CH2PR21MB14009637FC55D3A917DE3930EF5F0CH2PR21MB1400namp_" --_000_CH2PR21MB14009637FC55D3A917DE3930EF5F0CH2PR21MB1400namp_ Content-Type: text/plain; charset="Windows-1252" Content-Transfer-Encoding: quoted-printable 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. T= his makes sense, but good to highlight that all packages that use host base= d 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 kno= w. This is an item on our list, but lower priority and not a blocker. 1. MdePkg/HostLibrary/BaseLibHost * Why are there 2 INFs. One with ASM and one without ASM? Can we = just use the one with ASM and assume NASM is installed? * I see the purpose of this lib instance is to call the * SetJump()/LongJump(). So these implementations always work? It = looks like it assumes BASE_LIBRARY_JUMP_BUFFER is identical to the host OS = user mode application setjmp()/longjmp() state. * Why are DRx and CRx registers emulated? I would think and code t= hat depends on read/writing these registers would not be compatible with ho= st based testing. Can we change to ASSERT (FALSE)? * PatchInstructionX86() =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 hav= e 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. Right 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 naming on ot= hers, 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. Given that = this framework is designed to be nimble enough to work in PEI and SMM and o= ther 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. Fingerprint= s are unique IDs to make sure that serialize/unserialized state matches the= tests upon re-entry. I=92m not married to MD5, but it needs to be predicta= bly unique, and carried with the framework. I would not want to make any re= quirements on CryptoLib, since these aren=92t crypto-strong. * Update all the strings to be ASCII? See Unicode vs ASCII above. i. Ideally 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 style 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. That=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 impr= ove the documentation around these functions. I will acknowledge, however, = that this is an interface that I expect to evolve as we figure out what kin= ds of tests the community wants support for =93out of the box=94. If we wan= t to easily enable tests around =96 for example =96 ExitBootServices, we wi= ll likely want to tweak this going forward to its simplest form. The versio= n we have here was sufficient to enable the test cases that we=92ve current= ly 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 di= scuss another implementation, but this is not crypto-strong. It=92s just fo= r uniqueness. A sufficiently long CRC would probably work, too. 1. UnitTestPkg/Library/UnitTestTerminationLibTbd * Do we really need this lib instance now? i. This is her= e so that we can figure out what this should look like for a host. Host obv= iously wouldn=92t reset, but could conceivably quit. Or maybe that doesn=92= t 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_CH2PR21MB14009637FC55D3A917DE3930EF5F0CH2PR21MB1400namp_ Content-Type: text/html; charset="Windows-1252" Content-Transfer-Encoding: quoted-printable
  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

     

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: <= a href=3D"mailto:afish@apple.com">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/ (&nb= sp;*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 int= o modules directory. Maybe we should follow the same pattern as *Pkg/Test a= nd have a Test directory? This will help minimize the number of "reser= ved" directories we need for managing the testing. Have a standardized directory 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/GUID [1] seems a little more complex. It is easy eno= ugh 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 th= ink 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 g= et 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 <= michael.d.kinney@intel.com> wrote:

 

Hi Bret,

 

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

 

I have the following comments:

 

  1. I see that MdePkg <= /span>adds a dependency on UnitTestPkg.  This makes UnitTestPkg <= /span>the root package when building and running host based unit tests.&nbs= p; 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 Depen= dencyCheck plugi= n 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 intended to be used directly from a unit test case?  = ;If some of these are only intended to be used from the framework in= side 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 they be moved into  the =91Test=92 directory?
  3. MdePkg/MdePkgTest= .dsc. 
    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/HostLibrar= y/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
      3. SetJump()/LongJum= p().  So th= ese implementations always work?  It looks like it assumes BASE_LIBRARY_JUMP_BUFFER is identical to the host OS user mode applicationsetjmp()/longjmp() state.
      4. Why are DRx a= nd CRx registe= rs emulated?  I wou= ld 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 = self modifying code. =  Should it be ASSERT (FALSE)?
    1. MdePkg/HostLibrar= y/DebugLibHost
      1. What is =91#ifndef TEST_WITH_KLEE=92
      2. What is the =91PatchFormat()=92 function?&= nbsp; It is always disa= bled with if (0).
      3. Are the PCDs to set the debug message levels disabled on purpose? (DebugPrintEnabled(), DebugPrintLevelEnabled(), DebugCodeEnabled())
    1. MdePkg/HostLibrar= y/BaseMemoryLibHost
      1. Why can=92t we use MdePkg/Library/BaseMe= moryLib/BaseMemoryLib/inf instead an= d reduce the number of host specific lib instances?
    1. MdePkg/HostLibrar= y/MemoryAllocationLibHost
      1. Why is are memcpy(), assert(), memset() used in this li= b?  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 library instance naming conventions.
      1. 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.  Would it make more sense to use =91Host=92 as a prefix instea= d of a postfix in the lib instance names?
    1. Unicode vs ASCII strings
      1. I see InitUnitTestFramework(), CreateUnitTestSuite(), an= d AddTestCase() all take Unicode strings for the labels.  I also see extra code to convert gEfiCallerBase= Name from ASCII to Unicode to use it as a short name of a test framework.  I think it would be simple= r if the parameters to these APIs were ASCII and the framework can convert = to Unicode if needed.
    1. Will InitUnitTestFramework() and CreateUnitTestSuite() a= lways be called in pairs? &nbs= p;If so, can we combine these to a single API? 
      1. I see the SafeIntLib&nb= sp;example create a single framework and multiple test suites. =  Perhaps we can have a single CreateUnitTestSuite() API where Framework is a= n IN/OUT and if it is passed in as NULL, the Framework handle is created.
      2. I see a pattern where the CreateUnitTestSuite() =91Package=92 par= ameter is used as a prefix to every call to AddTestCase() in the =91ClassName=92 parameter.  Can we simplify AddTes= tCase() 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 = 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 fram= ework can manage alloc() and free()?
      2. How are Fingerprints used? &n= bsp;The idea of using as hash as a unique identifier is a good idea.=   How is the hash = calculated?  What unit test code artifacts are used so developers know what parameters must= be unique?  Can w= e just decide to use a specific hash algorithm/size and the structure can b= e 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_CONTEXT - =96 The last field is a variable sized array, so = it can be a formal field.
      5. UNIT_TEST_SAVE_HEADER =96 Can the last 3 fields be changed to pointers and= be formal fields?
      6. Do the structures really need to be packed?  Specially with the changes suggested above?&nbs= p; Is the intent to pot= entially share data stored on disk between different host execution environments that may have different width architectures?
        7. <= /ol>
      1. UnitTestPkg =96 UnitTestLib.h
        1. Can you provide more context for the APIs SaveFrameworkState(), SaveFrameworkStateAndQuit(), SaveFrameworkStateAndReboot()= , SetFrameworkBootNextDevice().  I do not see any Load() functions, so how would a set of tests be resumed?&= nbsp; If these do not a= pply to host based tests, should these be split out to a different lib clas= s?
      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 O= penSSL.
      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 Ba= rkelew via Groups.Io
      Sent: Thursday,= November 21, 2019 11:39 PM
      To: devel@edk2.group= s.io
      Subject: [edk2-= devel] EDK2 Host-Based Unit Test RFC (Now with docs!)

       

      Now that CI has la= nded in edk2/master, we'd like to get the basic framework for unittesting s= taged 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/edk2-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 d= oc here: https://github.com/corthon/edk2-staging/blob/edk2-host-test_v2/Re= adme-RFC.md

      And a usage guide = here: https://github.com/corthon/edk2-staging/blob/edk2-ho= st-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_CH2PR21MB14009637FC55D3A917DE3930EF5F0CH2PR21MB1400namp_--