<html xmlns:o="urn:schemas-microsoft-com:office:office" xmlns:w="urn:schemas-microsoft-com:office:word" xmlns:m="http://schemas.microsoft.com/office/2004/12/omml" xmlns="http://www.w3.org/TR/REC-html40">
<head>
<meta http-equiv="Content-Type" content="text/html; charset=us-ascii">
<meta name="Generator" content="Microsoft Word 15 (filtered medium)">
<style><!--
/* Font Definitions */
@font-face
        {font-family:"Cambria Math";
        panose-1:2 4 5 3 5 4 6 3 2 4;}
@font-face
        {font-family:DengXian;
        panose-1:2 1 6 0 3 1 1 1 1 1;}
@font-face
        {font-family:Calibri;
        panose-1:2 15 5 2 2 2 4 3 2 4;}
@font-face
        {font-family:"\@DengXian";
        panose-1:2 1 6 0 3 1 1 1 1 1;}
/* Style Definitions */
p.MsoNormal, li.MsoNormal, div.MsoNormal
        {margin:0in;
        margin-bottom:.0001pt;
        font-size:11.0pt;
        font-family:"Calibri",sans-serif;}
.MsoChpDefault
        {mso-style-type:export-only;}
@page WordSection1
        {size:8.5in 11.0in;
        margin:1.0in 1.0in 1.0in 1.0in;}
div.WordSection1
        {page:WordSection1;}
--></style>
</head>
<body lang="EN-US" link="blue" vlink="#954F72">
<div class="WordSection1">
<p class="MsoNormal">Did you make sure to stuart_update? It will pull a dependency repo that is only needed for CI.</p>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal">- Bret</p>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<hr style="display:inline-block;width:98%" tabindex="-1">
<div id="divRplyFwdMsg" dir="ltr"><font face="Calibri, sans-serif" style="font-size:11pt" color="#000000"><b>From:</b> Kinney, Michael D <michael.d.kinney@intel.com><br>
<b>Sent:</b> Monday, December 16, 2019 2:31:13 PM<br>
<b>To:</b> rfc@edk2.groups.io <rfc@edk2.groups.io>; Bret Barkelew <Bret.Barkelew@microsoft.com>; devel@edk2.groups.io <devel@edk2.groups.io>; Andrew Fish <afish@apple.com>; Kinney, Michael D <michael.d.kinney@intel.com><br>
<b>Subject:</b> RE: [edk2-rfc] [EXTERNAL] Re: [edk2-devel] EDK2 Host-Based Unit Test RFC (Now with docs!)</font>
<div> </div>
</div>
<div class="BodyFragment"><font size="2"><span style="font-size:11pt;">
<div class="PlainText">Hi Bret,<br>
<br>
I am looking at the latest version of the content on your branch.<br>
<br>
I am confused by MdePkg/Test/MdePkgTest.dsc.  It makes references<br>
to lib classes and packages that do not exist.<br>
<br>
  CmockaLib|CmockaHostUnitTestPkg/Library/CmockaLib/CmockaLib.inf<br>
  OsServiceLib|HostBasedUnitTestPkg/Library/OsServiceLibHost/OsServiceLibHost.inf<br>
  UnitTestAssertLib|CmockaHostUnitTestPkg/Library/UnitTestAssertLibcmocka/UnitTestAssertLibcmocka.inf<br>
  UnitTestLib|CmockaHostUnitTestPkg/Library/UnitTestLibcmocka/UnitTestLibcmocka.inf<br>
<br>
Am I looking at an old file?<br>
<br>
I am just trying to do a local build of the unit tests for the SafeIntLib.<br>
<br>
Thanks,<br>
<br>
Mike<br>
<br>
<br>
> -----Original Message-----<br>
> From: rfc@edk2.groups.io <rfc@edk2.groups.io> On Behalf<br>
> Of Bret Barkelew via Groups.Io<br>
> Sent: Saturday, December 14, 2019 12:07 PM<br>
> To: devel@edk2.groups.io; Andrew Fish<br>
> <afish@apple.com>; Kinney, Michael D<br>
> <michael.d.kinney@intel.com><br>
> Cc: rfc@edk2.groups.io<br>
> Subject: Re: [edk2-rfc] [EXTERNAL] Re: [edk2-devel]<br>
> EDK2 Host-Based Unit Test RFC (Now with docs!)<br>
> <br>
> The host-based tests now build on Linux/GCC, but the<br>
> final executables don't seem to get created. Don't know<br>
> where the disconnect is there. I can see the test get<br>
> compiled (along with all the libraries), but it doesn't<br>
> seem to link.<br>
> <br>
> Can you take a look? Thanks!<br>
> <br>
> - Bret<br>
> <br>
> From: Bret Barkelew<<a href="mailto:Bret.Barkelew@microsoft.com">mailto:Bret.Barkelew@microsoft.com</a>><br>
> Sent: Friday, December 13, 2019 4:46 PM<br>
> To: devel@edk2.groups.io<mailto:devel@edk2.groups.io>;<br>
> Andrew Fish<<a href="mailto:afish@apple.com">mailto:afish@apple.com</a>>; Kinney, Michael<br>
> D<<a href="mailto:michael.d.kinney@intel.com">mailto:michael.d.kinney@intel.com</a>><br>
> Cc: rfc@edk2.groups.io<<a href="mailto:rfc@edk2.groups.io">mailto:rfc@edk2.groups.io</a>><br>
> Subject: RE: [EXTERNAL] Re: [edk2-devel] EDK2 Host-<br>
> Based Unit Test RFC (Now with docs!)<br>
> <br>
> Mike,<br>
> <br>
> I think I've gotten all the feedback here and all the<br>
> action items from our call. The current branch should<br>
> be good to go, minus the couple of things that Intel<br>
> was going to look into.<br>
> <br>
> Thanks!<br>
> <br>
> - Bret<br>
> <br>
> From: Bret Barkelew<<a href="mailto:Bret.Barkelew@microsoft.com">mailto:Bret.Barkelew@microsoft.com</a>><br>
> Sent: Friday, December 6, 2019 2:21 PM<br>
> To: devel@edk2.groups.io<mailto:devel@edk2.groups.io>;<br>
> Andrew Fish<<a href="mailto:afish@apple.com">mailto:afish@apple.com</a>>; Kinney, Michael<br>
> D<<a href="mailto:michael.d.kinney@intel.com">mailto:michael.d.kinney@intel.com</a>><br>
> Cc: rfc@edk2.groups.io<<a href="mailto:rfc@edk2.groups.io">mailto:rfc@edk2.groups.io</a>><br>
> Subject: RE: [EXTERNAL] Re: [edk2-devel] EDK2 Host-<br>
> Based Unit Test RFC (Now with docs!)<br>
> <br>
> <br>
>   1.  I see that MdePkg adds a dependency on<br>
> UnitTestPkg.  This makes UnitTestPkg the root package<br>
> when building and running host based unit tests.  This<br>
> makes sense, but good to highlight that all packages<br>
> that use host based tests will introduce a new package<br>
> dependency on UnitTestPkg.<br>
>      *   Since the dependency only applies to unit<br>
> tests, can we update the DependencyCheck plugin to<br>
> support listing dependencies for FW components separate<br>
> from dependencies for unit tests?<br>
>      *   Can move. Capability is there, but mistakenly<br>
> added to the wrong section.<br>
>   2.  I see UnitTestPkg declares 6 new lib classes.<br>
> Are all 6 classes intended to be used directly from a<br>
> unit test case?  If some of these are only intended to<br>
> be used from the framework inside the UnitTestPkg can<br>
> we make them private?<br>
>      *   Because there are different instances in<br>
> multiple places (and conceivably more in the future),<br>
> we don't see how we could make them private.<br>
>   3.  In the MdePkg, I see a new top level directory<br>
> called 'HostLibrary'. Since these lib instances are<br>
> only used from a host based test, can they be moved<br>
> into  the 'Test' directory?<br>
>      *   Can move.<br>
>   4.  MdePkg/MdePkgTest.dsc.<br>
>      *   Can this DSC file be moved into the 'Test'<br>
> directory?<br>
> <br>
> <br>
> i.      Yes<br>
> <br>
>      *   I see this DSC file only supports VS today.<br>
> How much work is it to add GCC support?<br>
> <br>
> <br>
> i.      Don't know. This is an item on our list, but<br>
> lower priority and not a blocker.<br>
> <br>
>   1.  MdePkg/HostLibrary/BaseLibHost<br>
>      *   Why are there 2 INFs.  One with ASM and one<br>
> without ASM?  Can we just use the one with ASM and<br>
> assume NASM is installed?<br>
>      *   I see the purpose of this lib instance is to<br>
> call the<br>
>      *   SetJump()/LongJump().  So these<br>
> implementations always work?  It looks like it assumes<br>
> BASE_LIBRARY_JUMP_BUFFER is identical to the host OS<br>
> user mode application setjmp()/longjmp() state.<br>
>      *   Why are DRx and CRx registers emulated?  I<br>
> would think and code that depends on read/writing these<br>
> registers would not be compatible with host based<br>
> testing.  Can we change to ASSERT (FALSE)?<br>
>      *   PatchInstructionX86() - I suspect this will<br>
> not work in a host based environment because it is self<br>
> modifying code.  Should it be ASSERT (FALSE)?<br>
>      *   Libraries were taken directly from the Intel<br>
> work in HBFA. They worked so we kept them. We're just<br>
> as interested in the answers to these questions as you<br>
> are.<br>
>   2.  MdePkg/HostLibrary/DebugLibHost<br>
>      *   What is '#ifndef TEST_WITH_KLEE'<br>
>      *   What is the 'PatchFormat()' function?  It is<br>
> always disabled with if (0).<br>
>      *   Are the PCDs to set the debug message levels<br>
> disabled on purpose? (DebugPrintEnabled(),<br>
> DebugPrintLevelEnabled(), DebugCodeEnabled())<br>
>      *   Libraries were taken directly from the Intel<br>
> work in HBFA. They worked so we kept them. We're just<br>
> as interested in the answers to these questions as you<br>
> are.<br>
>   3.  MdePkg/HostLibrary/BaseMemoryLibHost<br>
>      *   Why can't we use<br>
> MdePkg/Library/BaseMemoryLib/BaseMemoryLib/inf instead<br>
> and reduce the number of host specific lib instances?<br>
>      *   Libraries were taken directly from the Intel<br>
> work in HBFA. They worked so we kept them. We're just<br>
> as interested in the answers to these questions as you<br>
> are.<br>
>   4.  MdePkg/HostLibrary/MemoryAllocationLibHost<br>
>      *   Why is are memcpy(), assert(), memset() used<br>
> in this lib?  I would expect this lib instance to match<br>
> the UefiMemoryAllocationLib with the only the use of<br>
> malloc() and free() to replace the UEFI Boot Services<br>
> calls.<br>
>      *   Libraries were taken directly from the Intel<br>
> work in HBFA. They worked so we kept them. We're just<br>
> as interested in the answers to these questions as you<br>
> are.<br>
>   5.  Host library instance naming conventions.<br>
>      *   The current naming convention uses the<br>
> environment as a prefix(e.g. Pei, Smm, Uefi) and the<br>
> services the lib instance uses as a post fix.  Would it<br>
> make more sense to use 'Host' as a prefix instead of a<br>
> postfix in the lib instance names?<br>
>      *   I don't know if there's a 1:1 relationship<br>
> with these. For some library classes (that you might<br>
> have host versions of), the class itself is the<br>
> PeiSomethingLib, and the host version would be the<br>
> PeiSomethingLibHost. No?<br>
>   6.  Unicode vs ASCII strings<br>
>      *   I see InitUnitTestFramework(),<br>
> CreateUnitTestSuite(), and AddTestCase() all take<br>
> Unicode strings for the labels.  I also see extra code<br>
> to convert gEfiCallerBaseName from ASCII to Unicode to<br>
> use it as a short name of a test framework.  I think it<br>
> would be simpler if the parameters to these APIs were<br>
> ASCII and the framework can convert to Unicode if<br>
> needed.<br>
>      *   No strong feelings, but we already have a<br>
> bunch of tests written this way. Since the UnitTestLib<br>
> is an abstraction that works as well in Shell as in the<br>
> Host, going with Unicode strings seemed to match the<br>
> art for Shell apps (since the framework was written for<br>
> Shell first).<br>
>   7.  Will InitUnitTestFramework() and<br>
> CreateUnitTestSuite() always be called in pairs?  If<br>
> so, can we combine these to a single API?<br>
>      *   I see the SafeIntLib example create a single<br>
> framework and multiple test suites.  Perhaps we can<br>
> have a single CreateUnitTestSuite() API where Framework<br>
> is an IN/OUT and if it is passed in as NULL, the<br>
> Framework handle is created.<br>
> <br>
> <br>
> i.      It's not always in pairs. You would only have a<br>
> single framework init, but could have multiple suites.<br>
> <br>
>      *   I see a pattern where the<br>
> CreateUnitTestSuite() 'Package' parameter is used as a<br>
> prefix to every call to AddTestCase() in the<br>
> 'ClassName' parameter.  Can we simplify AddTestCase()<br>
> so it only need to pass in the name of the unit test<br>
> case, and the framework can append Package  and the<br>
> unit test case name?<br>
> <br>
> <br>
> i.      Right now, our tests just coincidentally share<br>
> similar names with the packages, but we don't feel this<br>
> is axiomatic and don't want to force this naming on<br>
> others, who may be trying to bolt into other reporting<br>
> structures.<br>
> <br>
>   1.  I see the use of the 'Fw' variable as a shorthand<br>
> for 'Framework'.  Can we use something other than 'Fw'.<br>
> It may be confused with 'Firmware'.<br>
>      *   No real arguments. Wouldn't fight a find-<br>
> replace, but it'll just be a bunch of touches as we<br>
> commit.<br>
>   2.  UnitTestPkg/Include/UnitTestTypes.h<br>
>      *   I see several hard coded string lengths.<br>
> Since this runs in a host environment and strings can<br>
> always be allocated, can the hard coded lengths be<br>
> removed?  Update the structs to use pointers to strings<br>
> instead of string arrays, and the framework can manage<br>
> alloc() and free()?<br>
> <br>
> <br>
> i.      Given that this framework is designed to be<br>
> nimble enough to work in PEI and SMM and other<br>
> constrictive environments, we wanted to keep fixed<br>
> sizing.<br>
> <br>
>      *   How are Fingerprints used?  The idea of using<br>
> as hash as a unique identifier is a good idea.  How is<br>
> the hash calculated?  What unit test code artifacts are<br>
> used so developers know what parameters must be unique?<br>
> Can we just decide to use a specific hash<br>
> algorithm/size and the structure can be a pointer to an<br>
> allocated buffer instead of a fixed size array to make<br>
> it easy to change the algorithm/size in the future?<br>
> <br>
> <br>
> i.      Fingerprints are unique IDs to make sure that<br>
> serialize/unserialized state matches the tests upon re-<br>
> entry. I'm not married to MD5, but it needs to be<br>
> predictably unique, and carried with the framework. I<br>
> would not want to make any requirements on CryptoLib,<br>
> since these aren't crypto-strong.<br>
> <br>
>      *   Update all the strings to be ASCII?  See<br>
> Unicode vs ASCII above.<br>
> <br>
> <br>
> i.      Ideally not, unless there's a strong use case.<br>
> <br>
>      *   UNIT_TEST_SAVE_TEST - The last field is a<br>
> variable sized array, so it can be a formal field.<br>
> <br>
> <br>
> i.      Not opposed, but don't really want to make the<br>
> change myself. Is there a style guide that this is<br>
> breaking?<br>
> <br>
>      *   UNIT_TEST_SAVE_CONTEXT - - The last field is a<br>
> variable sized array, so it can be a formal field.<br>
>      *   UNIT_TEST_SAVE_HEADER - Can the last 3 fields<br>
> be changed to pointers and be formal fields?<br>
>      *   Do the structures really need to be packed?<br>
> Specially with the changes suggested above?  Is the<br>
> intent to potentially share data stored on disk between<br>
> different host execution environments that may have<br>
> different width architectures?<br>
> <br>
> <br>
> i.      That's an interesting point. Could you draw up<br>
> your suggestion and submit a PR?<br>
> <br>
>   1.  UnitTestPkg - UnitTestLib.h<br>
>      *   Can you provide more context for the APIs<br>
> SaveFrameworkState(), SaveFrameworkStateAndQuit(),<br>
> SaveFrameworkStateAndReboot(),<br>
> SetFrameworkBootNextDevice().  I do not see any Load()<br>
> functions, so how would a set of tests be resumed?  If<br>
> these do not apply to host based tests, should these be<br>
> split out to a different lib class?<br>
> <br>
> <br>
> i.      I'll improve the documentation around these<br>
> functions. I will acknowledge, however, that this is an<br>
> interface that I expect to evolve as we figure out what<br>
> kinds of tests the community wants support for "out of<br>
> the box". If we want to easily enable tests around -<br>
> for example - ExitBootServices, we will likely want to<br>
> tweak this going forward to its simplest form. The<br>
> version we have here was sufficient to enable the test<br>
> cases that we've currently written.<br>
> <br>
>   1.  UnitTestPkg/Library/UnitTestLib<br>
>      *   I see an implementation of MD5.  We should not<br>
> do our own.  We should use an approved implementation<br>
> such as OpenSSL.<br>
> <br>
> <br>
> i.      Happy to discuss another implementation, but<br>
> this is not crypto-strong. It's just for uniqueness. A<br>
> sufficiently long CRC would probably work, too.<br>
> <br>
>   1.  UnitTestPkg/Library/UnitTestTerminationLibTbd<br>
>      *   Do we really need this lib instance now?<br>
> <br>
> <br>
> i.      This is here so that we can figure out what<br>
> this should look like for a host. Host obviously<br>
> wouldn't reset, but could conceivably quit. Or maybe<br>
> that doesn't make any sense for a host.<br>
> <br>
> <br>
> <br>
> - Bret<br>
> <br>
> <br>
> From: devel@edk2.groups.io <devel@edk2.groups.io> on<br>
> behalf of Bret Barkelew via Groups.Io<br>
> <bret.barkelew=microsoft.com@groups.io><br>
> Sent: Wednesday, December 4, 2019 10:24:21 AM<br>
> To: Andrew Fish <afish@apple.com>; devel@edk2.groups.io<br>
> <devel@edk2.groups.io>; Kinney, Michael D<br>
> <michael.d.kinney@intel.com><br>
> Cc: rfc@edk2.groups.io <rfc@edk2.groups.io><br>
> Subject: Re: [EXTERNAL] Re: [edk2-devel] EDK2 Host-<br>
> Based Unit Test RFC (Now with docs!)<br>
> <br>
> <br>
> Andrew,<br>
> <br>
> I agree with your points.<br>
> <br>
> <br>
> <br>
> Mike,<br>
> <br>
> You've got a lot more there. Let me take a look and<br>
> update the draft. I'll ping back ASAP.<br>
> <br>
> <br>
> <br>
> - Bret<br>
> <br>
> <br>
> <br>
> From: Andrew Fish<<a href="mailto:afish@apple.com">mailto:afish@apple.com</a>><br>
> Sent: Wednesday, December 4, 2019 9:50 AM<br>
> To: devel@edk2.groups.io<mailto:devel@edk2.groups.io>;<br>
> Kinney, Michael D<<a href="mailto:michael.d.kinney@intel.com">mailto:michael.d.kinney@intel.com</a>><br>
> Cc: Bret Barkelew<<a href="mailto:Bret.Barkelew@microsoft.com">mailto:Bret.Barkelew@microsoft.com</a>>;<br>
> rfc@edk2.groups.io<<a href="mailto:rfc@edk2.groups.io">mailto:rfc@edk2.groups.io</a>><br>
> Subject: [EXTERNAL] Re: [edk2-devel] EDK2 Host-Based<br>
> Unit Test RFC (Now with docs!)<br>
> <br>
> <br>
> <br>
> Mike,<br>
> <br>
> <br>
> <br>
> I like and agree with your comments.<br>
> <br>
> <br>
> <br>
> On the UnitTestPkg(s) dependency issue I think it would<br>
> make sense to move as much as possible into the<br>
> *Pkg/Test/ ( *Pkg/HostLibrary,  MdePkg/MdePkgTest.dsc,<br>
> etc.) directory structure. It looks like for<br>
> implementation specific tests we skip the Test<br>
> directory and drop the UnitTest or FuzzTest directly<br>
> into modules directory. Maybe we should follow the same<br>
> pattern as *Pkg/Test and have a Test directory? This<br>
> will help minimize the number of "reserved" directories<br>
> we need for managing the testing. Have a standardized<br>
> directory structure will make it easier to<br>
> differentiate the code from tests while doing `git<br>
> grep` or other forms of code spelunking.<br>
> <br>
> <br>
> <br>
> A Host-Based Unit Test for a Library makes sense as you<br>
> can link directly against the library and test it. A<br>
> Host-Based Unit Test for Protocol/PPI/GUID [1] seems a<br>
> little more complex. It is easy enough to write tests<br>
> for the interfaces but what APIs do you call to get<br>
> access to the protocol?<br>
> <br>
> <br>
> <br>
> Per our conversation at the Stewards meeting I think<br>
> make sense to try to roll out the testing of libraries<br>
> as the 1st phase. The mocking required for drivers<br>
> could get quite complex and let us not get bogged down<br>
> in all that to get something working.<br>
> <br>
> <br>
> <br>
> [1] *Pkg/Test/UnitTest/[Library|Protocol|Ppi|Guid]<br>
> <br>
> <br>
> <br>
> Thanks,<br>
> <br>
> <br>
> <br>
> Andrew Fish<br>
> <br>
> <br>
> <br>
> <br>
> <br>
> On Dec 2, 2019, at 3:12 PM, Michael D Kinney<br>
> <michael.d.kinney@intel.com<mailto:michael.d.kinney@int<br>
> el.com>> wrote:<br>
> <br>
> <br>
> <br>
> Hi Bret,<br>
> <br>
> <br>
> <br>
> Thanks for posting this content.  Host based unit<br>
> testing is a very valuable addition to the CI checks.<br>
> <br>
> <br>
> <br>
> I have the following comments:<br>
> <br>
> <br>
> <br>
>   1.  I see that MdePkg adds a dependency on<br>
> UnitTestPkg.  This makes UnitTestPkg the root package<br>
> when building and running host based unit tests.  This<br>
> makes sense, but good to highlight that all packages<br>
> that use host based tests will introduce a new package<br>
> dependency on UnitTestPkg.<br>
> <br>
>      *   Since the dependency only applies to unit<br>
> tests, can we update the DependencyCheck plugin to<br>
> support listing dependencies for FW components separate<br>
> from dependencies for unit tests?<br>
> <br>
>   1.  I see UnitTestPkg declares 6 new lib classes.<br>
> Are all 6 classes intended to be used directly from a<br>
> unit test case?  If some of these are only intended to<br>
> be used from the framework inside the UnitTestPkg can<br>
> we make them private?<br>
>   2.  In the MdePkg, I see a new top level directory<br>
> called 'HostLibrary'. Since these lib instances are<br>
> only used from a host based test, can they be moved<br>
> into  the 'Test' directory?<br>
>   3.  MdePkg/MdePkgTest.dsc.<br>
> <br>
>      *   Can this DSC file be moved into the 'Test'<br>
> directory?<br>
>      *   I see this DSC file only supports VS today.<br>
> How much work is it to add GCC support?<br>
> <br>
>   1.  MdePkg/HostLibrary/BaseLibHost<br>
> <br>
>      *   Why are there 2 INFs.  One with ASM and one<br>
> without ASM?  Can we just use the one with ASM and<br>
> assume NASM is installed?<br>
>      *   I see the purpose of this lib instance is to<br>
> call the<br>
>      *   SetJump()/LongJump().  So these<br>
> implementations always work?  It looks like it assumes<br>
> BASE_LIBRARY_JUMP_BUFFER is identical to the host OS<br>
> user mode applicationsetjmp()/longjmp() state.<br>
>      *   Why are DRx and CRx registers emulated?  I<br>
> would think and code that depends on read/writing these<br>
> registers would not be compatible with host based<br>
> testing.  Can we change to ASSERT (FALSE)?<br>
>      *   PatchInstructionX86() - I suspect this will<br>
> not work in a host based environment because it is self<br>
> modifying code.  Should it be ASSERT (FALSE)?<br>
> <br>
>   1.  MdePkg/HostLibrary/DebugLibHost<br>
> <br>
>      *   What is '#ifndef TEST_WITH_KLEE'<br>
>      *   What is the 'PatchFormat()' function?  It is<br>
> always disabled with if (0).<br>
>      *   Are the PCDs to set the debug message levels<br>
> disabled on purpose? (DebugPrintEnabled(),<br>
> DebugPrintLevelEnabled(), DebugCodeEnabled())<br>
> <br>
>   1.  MdePkg/HostLibrary/BaseMemoryLibHost<br>
> <br>
>      *   Why can't we use<br>
> MdePkg/Library/BaseMemoryLib/BaseMemoryLib/inf instead<br>
> and reduce the number of host specific lib instances?<br>
> <br>
>   1.  MdePkg/HostLibrary/MemoryAllocationLibHost<br>
> <br>
>      *   Why is are memcpy(), assert(), memset() used<br>
> in this lib?  I would expect this lib instance to match<br>
> the UefiMemoryAllocationLib with the only the use of<br>
> malloc() and free() to replace the UEFI Boot Services<br>
> calls.<br>
> <br>
>   1.  Host library instance naming conventions.<br>
> <br>
>      *   The current naming convention uses the<br>
> environment as a prefix(e.g. Pei, Smm, Uefi) and the<br>
> services the lib instance uses as a post fix.  Would it<br>
> make more sense to use 'Host' as a prefix instead of a<br>
> postfix in the lib instance names?<br>
> <br>
>   1.  Unicode vs ASCII strings<br>
> <br>
>      *   I see InitUnitTestFramework(),<br>
> CreateUnitTestSuite(), and AddTestCase() all take<br>
> Unicode strings for the labels.  I also see extra code<br>
> to convert gEfiCallerBaseName from ASCII to Unicode to<br>
> use it as a short name of a test framework.  I think it<br>
> would be simpler if the parameters to these APIs were<br>
> ASCII and the framework can convert to Unicode if<br>
> needed.<br>
> <br>
>   1.  Will InitUnitTestFramework() and<br>
> CreateUnitTestSuite() always be called in pairs?  If<br>
> so, can we combine these to a single API?<br>
> <br>
>      *   I see the SafeIntLib example create a single<br>
> framework and multiple test suites.  Perhaps we can<br>
> have a single CreateUnitTestSuite() API where Framework<br>
> is an IN/OUT and if it is passed in as NULL, the<br>
> Framework handle is created.<br>
>      *   I see a pattern where the<br>
> CreateUnitTestSuite() 'Package' parameter is used as a<br>
> prefix to every call to AddTestCase() in the<br>
> 'ClassName' parameter.  Can we simplify AddTestCase()<br>
> so it only need to pass in the name of the unit test<br>
> case, and the framework can append Package  and the<br>
> unit test case name?<br>
> <br>
>   1.  I see the use of the 'Fw' variable as a shorthand<br>
> for 'Framework'.  Can we use something other than 'Fw'.<br>
> It may be confused with 'Firmware'.<br>
>   2.  UnitTestPkg/Include/UnitTestTypes.h<br>
> <br>
>      *   I see several hard coded string lengths.<br>
> Since this runs in a host environment and strings can<br>
> always be allocated, can the hard coded lengths be<br>
> removed?  Update the structs to use pointers to strings<br>
> instead of string arrays, and the framework can manage<br>
> alloc() and free()?<br>
>      *   How are Fingerprints used?  The idea of using<br>
> as hash as a unique identifier is a good idea.  How is<br>
> the hash calculated?  What unit test code artifacts are<br>
> used so developers know what parameters must be unique?<br>
> Can we just decide to use a specific hash<br>
> algorithm/size and the structure can be a pointer to an<br>
> allocated buffer instead of a fixed size array to make<br>
> it easy to change the algorithm/size in the future?<br>
>      *   Update all the strings to be ASCII?  See<br>
> Unicode vs ASCII above.<br>
>      *   UNIT_TEST_SAVE_TEST - The last field is a<br>
> variable sized array, so it can be a formal field.<br>
>      *   UNIT_TEST_SAVE_CONTEXT - - The last field is a<br>
> variable sized array, so it can be a formal field.<br>
>      *   UNIT_TEST_SAVE_HEADER - Can the last 3 fields<br>
> be changed to pointers and be formal fields?<br>
>      *   Do the structures really need to be packed?<br>
> Specially with the changes suggested above?  Is the<br>
> intent to potentially share data stored on disk between<br>
> different host execution environments that may have<br>
> different width architectures?<br>
> <br>
>   1.  UnitTestPkg - UnitTestLib.h<br>
> <br>
>      *   Can you provide more context for the APIs<br>
> SaveFrameworkState(), SaveFrameworkStateAndQuit(),<br>
> SaveFrameworkStateAndReboot(),<br>
> SetFrameworkBootNextDevice().  I do not see any Load()<br>
> functions, so how would a set of tests be resumed?  If<br>
> these do not apply to host based tests, should these be<br>
> split out to a different lib class?<br>
> <br>
>   1.  UnitTestPkg/Library/UnitTestLib<br>
> <br>
>      *   I see an implementation of MD5.  We should not<br>
> do our own.  We should use an approved implementation<br>
> such as OpenSSL.<br>
> <br>
>   1.  UnitTestPkg/Library/UnitTestTerminationLibTbd<br>
> <br>
>      *   Do we really need this lib instance now?<br>
> <br>
> <br>
> <br>
> Thanks,<br>
> <br>
> <br>
> <br>
> Mike<br>
> <br>
> <br>
> <br>
> From: devel@edk2.groups.io<mailto:devel@edk2.groups.io><br>
> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>> On<br>
> Behalf Of Bret Barkelew via Groups.Io<br>
> Sent: Thursday, November 21, 2019 11:39 PM<br>
> To: devel@edk2.groups.io<mailto:devel@edk2.groups.io><br>
> Subject: [edk2-devel] EDK2 Host-Based Unit Test RFC<br>
> (Now with docs!)<br>
> <br>
> <br>
> <br>
> Now that CI has landed in edk2/master, we'd like to get<br>
> the basic framework for unittesting staged as well.<br>
> Target intercept date would be immediately after the<br>
> 2019/11 stabilization, so we wanted to go ahead and get<br>
> comments now.<br>
> <br>
> The host unittest framework consists of five primary<br>
> pieces:<br>
> - The test library (Cmocka)<br>
> - Support libraries (Found in UnitTestPkg)<br>
> - The test support plugins (HostUnitTestComilerPlugin,<br>
> HostUnitTestDxeCompleteCheck, HostBasedUnitTestRunner)<br>
> - The configuration in the package-based *.ci.yaml file<br>
> and package-based Test.dsc<br>
> - The tests themselves<br>
> <br>
> We have a demo branch set up at:<br>
> <a href="https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fcorthon%2Fedk2-staging%2Ftree%2Fedk2-host-&amp;data=02%7C01%7Cbret.barkelew%40microsoft.com%7Caf69d76bb76f4a4e152e08d78277a97e%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637121322769998529&amp;sdata=0%2FEUzZG6J3F%2FLnCt0sICGwsxpwjXf7cbUMnQQWeSbtw%3D&amp;reserved=0">
https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fcorthon%2Fedk2-staging%2Ftree%2Fedk2-host-&amp;data=02%7C01%7Cbret.barkelew%40microsoft.com%7Caf69d76bb76f4a4e152e08d78277a97e%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637121322769998529&amp;sdata=0%2FEUzZG6J3F%2FLnCt0sICGwsxpwjXf7cbUMnQQWeSbtw%3D&amp;reserved=0</a><br>
> test_v2<<a href=""></a>https://nam06.safelinks.protection.outlook.com/<br>
> ?url=https%3A%2F%2Fgithub.com%2Fcorthon%2Fedk2-<br>
> staging%2Ftree%2Fedk2-host-<br>
> test_v2&data=02%7C01%7Cbret.barkelew%40microsoft.com%7C<br>
> 6eac9932f3f640dd65ac08d778e731e3%7C72f988bf86f141af91ab<br>
> 2d7cd011db47%7C1%7C0%7C637110806683189828&sdata=JQ%2BoW<br>
> xlEBOK2sH55cRAPVa3OpAxTsm6eHcdbWSo9CVo%3D&reserved=0><br>
> We also have a demo build pipeline at:<br>
> <a href="https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fdev.azure.com%2Ftianocore%2Fedk2-ci-&amp;data=02%7C01%7Cbret.barkelew%40microsoft.com%7Caf69d76bb76f4a4e152e08d78277a97e%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637121322770003516&amp;sdata=12H%2FC8YVt%2Fn5ud%2BYOp9vAzs6vk2by46YFTzplhcz1xs%3D&amp;reserved=0">
https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fdev.azure.com%2Ftianocore%2Fedk2-ci-&amp;data=02%7C01%7Cbret.barkelew%40microsoft.com%7Caf69d76bb76f4a4e152e08d78277a97e%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637121322770003516&amp;sdata=12H%2FC8YVt%2Fn5ud%2BYOp9vAzs6vk2by46YFTzplhcz1xs%3D&amp;reserved=0</a><br>
> play/_build?definitionId=36&_a=summary<https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fnam06.sa&amp;data=02%7C01%7Cbret.barkelew%40microsoft.com%7Caf69d76bb76f4a4e152e08d78277a97e%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637121322770003516&amp;sdata=TrBwMkL6jCSS0XiZhegHkA4VQheFvj3auBWkOWD5ysg%3D&amp;reserved=0<br>
> felinks.protection.outlook.com/?url=https%3A%2F%2Fdev.a<br>
> zure.com%2Ftianocore%2Fedk2-ci-<br>
> play%2F_build%3FdefinitionId%3D36%26_a%3Dsummary&data=0<br>
> 2%7C01%7Cbret.barkelew%40microsoft.com%7C6eac9932f3f640<br>
> dd65ac08d778e731e3%7C72f988bf86f141af91ab2d7cd011db47%7<br>
> C1%7C0%7C637110806683189828&sdata=wBwn1ehjyTmYNKVSvlEZS<br>
> XK5qyeu4EPAL7FzdYntnt4%3D&reserved=0><br>
> <br>
> The current demo branch contains a single test in<br>
> MdePkg for SafeIntLib (module file<br>
> MdePkg\Test\UnitTest\Library\BaseSafeIntLib\TestBaseSaf<br>
> eIntLib.inf). This test is automatically detected by<br>
> the HostUnitTestComilerPlugin and run by the<br>
> HostBasedUnitTestRunner as part of the CI process.<br>
> <br>
> A few notes about the current demo:<br>
> 1) The demo currently only works on Windows build<br>
> chains, but there's no reason to believe that it can't<br>
> work equally well on Linux build chains, and are happy<br>
> to work with anyone to get it there.<br>
> <br>
> 2) The demo currently has four failing conditions that<br>
> can be seen towards the end of MdePkg "Build and Test"<br>
> log file for this build:<br>
> <a href="https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fdev.azure.com%2Ftianocore%2Fedk2-ci-&amp;data=02%7C01%7Cbret.barkelew%40microsoft.com%7Caf69d76bb76f4a4e152e08d78277a97e%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637121322770003516&amp;sdata=12H%2FC8YVt%2Fn5ud%2BYOp9vAzs6vk2by46YFTzplhcz1xs%3D&amp;reserved=0">
https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fdev.azure.com%2Ftianocore%2Fedk2-ci-&amp;data=02%7C01%7Cbret.barkelew%40microsoft.com%7Caf69d76bb76f4a4e152e08d78277a97e%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637121322770003516&amp;sdata=12H%2FC8YVt%2Fn5ud%2BYOp9vAzs6vk2by46YFTzplhcz1xs%3D&amp;reserved=0</a><br>
> play/_build/results?buildId=2813<https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fnam06.safelink&amp;data=02%7C01%7Cbret.barkelew%40microsoft.com%7Caf69d76bb76f4a4e152e08d78277a97e%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637121322770003516&amp;sdata=OrhL0m%2BJC5obLQoU%2BgmFS9StJgvv2eEfSj8jsmrt9hI%3D&amp;reserved=0<br>
> s.protection.outlook.com/?url=https%3A%2F%2Fdev.azure.c<br>
> om%2Ftianocore%2Fedk2-ci-<br>
> play%2F_build%2Fresults%3FbuildId%3D2590&data=02%7C01%7<br>
> Cbret.barkelew%40microsoft.com%7C6eac9932f3f640dd65ac08<br>
> d778e731e3%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7<br>
> C637110806683199782&sdata=1zd2oq8zRZUn3%2FUiGDuJZEZ5M0s<br>
> rtc2bqoa0%2BLbZB3s%3D&reserved=0><br>
> "WARNING -   Test SafeInt16ToChar8 - Status<br>
> d:\a\1\s\MdePkg\Test\UnitTest\Library\BaseSafeIntLib\Te<br>
> stBaseSafeIntLib.c:302: error: Failure!<br>
> WARNING - TestBaseSafeIntLib.exe Test Failed<br>
> WARNING -   Test SafeInt32ToChar8 - Status<br>
> d:\a\1\s\MdePkg\Test\UnitTest\Library\BaseSafeIntLib\Te<br>
> stBaseSafeIntLib.c:638: error: Failure!<br>
> WARNING - TestBaseSafeIntLib.exe Test Failed<br>
> WARNING -   Test SafeIntnToChar8 - Status<br>
> d:\a\1\s\MdePkg\Test\UnitTest\Library\BaseSafeIntLib\Te<br>
> stBaseSafeIntLib.c:1051: error: Failure!<br>
> WARNING - TestBaseSafeIntLib.exe Test Failed<br>
> WARNING -   Test SafeInt64ToChar8 - Status<br>
> d:\a\1\s\MdePkg\Test\UnitTest\Library\BaseSafeIntLib\Te<br>
> stBaseSafeIntLib.c:1456: error: Failure!"<br>
> These failures seem to be legitimate and further work<br>
> should be done by the community to decide whether the<br>
> test case is correct or the library is correct, but one<br>
> of them needs to change.<br>
> <br>
> 3) Current demo pulls in test collateral from a fork of<br>
> the edk2-test repo. This repo is currently *very* heavy<br>
> due to the history of the UEFI SCT project and the<br>
> number of binaries that it pulls down. It's possible<br>
> that we (the community) need to select a better place<br>
> for this code to live. Maybe in edk2 primary (though<br>
> it's not explicitly firmware code, so it seems<br>
> unnecessary). Maybe in a new edk2-test2 repo or<br>
> something like that.<br>
> <br>
> There's an RFC doc here:<br>
> <a href="https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fcorthon%2Fedk2-staging%2Fblob%2Fedk2-host-&amp;data=02%7C01%7Cbret.barkelew%40microsoft.com%7Caf69d76bb76f4a4e152e08d78277a97e%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637121322770003516&amp;sdata=mroz5b5qSrDo%2FBtl%2BxWqrTDPpaFclIdhgco6yHJFIFo%3D&amp;reserved=0">
https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fcorthon%2Fedk2-staging%2Fblob%2Fedk2-host-&amp;data=02%7C01%7Cbret.barkelew%40microsoft.com%7Caf69d76bb76f4a4e152e08d78277a97e%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637121322770003516&amp;sdata=mroz5b5qSrDo%2FBtl%2BxWqrTDPpaFclIdhgco6yHJFIFo%3D&amp;reserved=0</a><br>
> test_v2/Readme-<br>
> RFC.md<<a href=""></a>https://nam06.safelinks.protection.outlook.com/?<br>
> url=https%3A%2F%2Fgithub.com%2Fcorthon%2Fedk2-<br>
> staging%2Fblob%2Fedk2-host-test_v2%2FReadme-<br>
> RFC.md&data=02%7C01%7Cbret.barkelew%40microsoft.com%7C6<br>
> eac9932f3f640dd65ac08d778e731e3%7C72f988bf86f141af91ab2<br>
> d7cd011db47%7C1%7C0%7C637110806683199782&sdata=1nq1mHjs<br>
> bSZj4IQM5RBwSrvaQxO5cmDlTvf7VYNDV%2FA%3D&reserved=0><br>
> <br>
> And a usage guide here:<br>
> <a href="https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fcorthon%2Fedk2-staging%2Fblob%2Fedk2-host-&amp;data=02%7C01%7Cbret.barkelew%40microsoft.com%7Caf69d76bb76f4a4e152e08d78277a97e%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637121322770003516&amp;sdata=mroz5b5qSrDo%2FBtl%2BxWqrTDPpaFclIdhgco6yHJFIFo%3D&amp;reserved=0">
https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fcorthon%2Fedk2-staging%2Fblob%2Fedk2-host-&amp;data=02%7C01%7Cbret.barkelew%40microsoft.com%7Caf69d76bb76f4a4e152e08d78277a97e%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637121322770003516&amp;sdata=mroz5b5qSrDo%2FBtl%2BxWqrTDPpaFclIdhgco6yHJFIFo%3D&amp;reserved=0</a><br>
> test_v2/UnitTestPkg/ReadMe.md<https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fnam06.safelinks.p&amp;data=02%7C01%7Cbret.barkelew%40microsoft.com%7Caf69d76bb76f4a4e152e08d78277a97e%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637121322770003516&amp;sdata=hPFhk5UWEA4JzSl9KyhX8%2Fj44ls5hx6T4i2bz8H9SB0%3D&amp;reserved=0<br>
> rotection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fc<br>
> orthon%2Fedk2-staging%2Fblob%2Fedk2-host-<br>
> test_v2%2FUnitTestPkg%2FReadMe.md&data=02%7C01%7Cbret.b<br>
> arkelew%40microsoft.com%7C6eac9932f3f640dd65ac08d778e73<br>
> 1e3%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637110<br>
> 806683209750&sdata=0jRQ2Rzr9PWSYJ1YDs7l2aFS3PfAnTbTousY<br>
> Ye8IWTw%3D&reserved=0><br>
> <br>
> Once again, would love to get this into EDK2 master<br>
> after stabilization, and most of this has previously<br>
> been shopped around in other discussion threads. This<br>
> is just where the rubber meets the road and is the<br>
> minimal subset of code that needs to land for folks to<br>
> start contributing unittests against the core libraries<br>
> that can be run as part of any CI pass.<br>
> <br>
> Thanks!<br>
> - Bret<br>
> <br>
> <br>
> <br>
> <br>
> <br>
> <br>
> <br>
> <br>
> <br>
<br>
</div>
</span></font></div>
</body>
</html>

<div width="1" style="color:white;clear:both">_._,_._,_</div>
<hr>
Groups.io Links:<p>

You receive all messages sent to this group.


<p>

<a target="_blank" href="https://edk2.groups.io/g/devel/message/52266">View/Reply Online (#52266)</a> |


  


|


  
    <a target="_blank" href="https://groups.io/mt/68745360/1813853">Mute This Topic</a>
  

| <a href="https://edk2.groups.io/g/devel/post">New Topic</a><br>



<br>

<a href="https://edk2.groups.io/g/devel/editsub/1813853">Your Subscription</a> |
<a href="mailto:devel+owner@edk2.groups.io">Contact Group Owner</a> |

<a href="https://edk2.groups.io/g/devel/unsub">Unsubscribe</a>

 [edk2-devel-archive@redhat.com]<br>
<div width="1" style="color:white;clear:both">_._,_._,_</div>