Re: [edk2-rfc] [EXTERNAL] Re: [edk2-devel] EDK2 Host-Based Unit Test RFC (Now with docs!)

["Bret Barkelew" <bret.barkelew@microsoft.com>]

[-- Attachment #1: Type: text/plain, Size: 36413 bytes --]

Yeah, if we don't want to carry Cmocka in edk2, there's a necessary trade off of having to keep a dependency for the host-based tests. You wouldn't need this dependency for a simple shell-based test, I'm pretty sure.

- Bret
________________________________
From: Kinney, Michael D <michael.d.kinney@intel.com>
Sent: Monday, December 16, 2019 6:19:14 PM
To: Bret Barkelew <Bret.Barkelew@microsoft.com>; rfc@edk2.groups.io <rfc@edk2.groups.io>; devel@edk2.groups.io <devel@edk2.groups.io>; Andrew Fish <afish@apple.com>; Kinney, Michael D <michael.d.kinney@intel.com>
Subject: RE: [edk2-rfc] [EXTERNAL] Re: [edk2-devel] EDK2 Host-Based Unit Test RFC (Now with docs!)


Hi Bret,



No.  I did not do that step yet.  I will try that next.



I was just trying to build the new DSC in the MdePkg.  The external dependency for a set of simple lib API unit tests in MdePkg was not expected.   A Readme.md in MdePkg/Test might help.



The RFC and Usage guide links in this thread are no longer available.



Thanks,



Mike



From: Bret Barkelew <Bret.Barkelew@microsoft.com>
Sent: Monday, December 16, 2019 3:53 PM
To: Kinney, Michael D <michael.d.kinney@intel.com>; rfc@edk2.groups.io; devel@edk2.groups.io; Andrew Fish <afish@apple.com>
Subject: RE: [edk2-rfc] [EXTERNAL] Re: [edk2-devel] EDK2 Host-Based Unit Test RFC (Now with docs!)



Did you make sure to stuart_update? It will pull a dependency repo that is only needed for CI.



- Bret



________________________________

From: Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>
Sent: Monday, December 16, 2019 2:31:13 PM
To: rfc@edk2.groups.io<mailto:rfc@edk2.groups.io> <rfc@edk2.groups.io<mailto:rfc@edk2.groups.io>>; Bret Barkelew <Bret.Barkelew@microsoft.com<mailto:Bret.Barkelew@microsoft.com>>; devel@edk2.groups.io<mailto:devel@edk2.groups.io> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>>; Andrew Fish <afish@apple.com<mailto:afish@apple.com>>; Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>
Subject: RE: [edk2-rfc] [EXTERNAL] Re: [edk2-devel] EDK2 Host-Based Unit Test RFC (Now with docs!)



Hi Bret,

I am looking at the latest version of the content on your branch.

I am confused by MdePkg/Test/MdePkgTest.dsc.  It makes references
to lib classes and packages that do not exist.

  CmockaLib|CmockaHostUnitTestPkg/Library/CmockaLib/CmockaLib.inf
  OsServiceLib|HostBasedUnitTestPkg/Library/OsServiceLibHost/OsServiceLibHost.inf
  UnitTestAssertLib|CmockaHostUnitTestPkg/Library/UnitTestAssertLibcmocka/UnitTestAssertLibcmocka.inf
  UnitTestLib|CmockaHostUnitTestPkg/Library/UnitTestLibcmocka/UnitTestLibcmocka.inf

Am I looking at an old file?

I am just trying to do a local build of the unit tests for the SafeIntLib.

Thanks,

Mike


> -----Original Message-----
> From: rfc@edk2.groups.io<mailto:rfc@edk2.groups.io> <rfc@edk2.groups.io<mailto:rfc@edk2.groups.io>> On Behalf
> Of Bret Barkelew via Groups.Io
> Sent: Saturday, December 14, 2019 12:07 PM
> To: devel@edk2.groups.io<mailto:devel@edk2.groups.io>; Andrew Fish
> <afish@apple.com<mailto:afish@apple.com>>; Kinney, Michael D
> <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>
> Cc: rfc@edk2.groups.io<mailto:rfc@edk2.groups.io>
> Subject: Re: [edk2-rfc] [EXTERNAL] Re: [edk2-devel]
> EDK2 Host-Based Unit Test RFC (Now with docs!)
>
> The host-based tests now build on Linux/GCC, but the
> final executables don't seem to get created. Don't know
> where the disconnect is there. I can see the test get
> compiled (along with all the libraries), but it doesn't
> seem to link.
>
> Can you take a look? Thanks!
>
> - Bret
>
> From: Bret Barkelew<mailto:Bret.Barkelew@microsoft.com>
> Sent: Friday, December 13, 2019 4:46 PM
> To: devel@edk2.groups.io<mailto:devel@edk2.groups.io<mailto:devel@edk2.groups.io%3cmailto:devel@edk2.groups.io>>;
> Andrew Fish<mailto:afish@apple.com>; Kinney, Michael
> D<mailto:michael.d.kinney@intel.com>
> Cc: rfc@edk2.groups.io<mailto:rfc@edk2.groups.io>
> Subject: RE: [EXTERNAL] Re: [edk2-devel] EDK2 Host-
> Based Unit Test RFC (Now with docs!)
>
> Mike,
>
> I think I've gotten all the feedback here and all the
> action items from our call. The current branch should
> be good to go, minus the couple of things that Intel
> was going to look into.
>
> Thanks!
>
> - Bret
>
> From: Bret Barkelew<mailto:Bret.Barkelew@microsoft.com>
> Sent: Friday, December 6, 2019 2:21 PM
> To: devel@edk2.groups.io<mailto:devel@edk2.groups.io<mailto:devel@edk2.groups.io%3cmailto:devel@edk2.groups.io>>;
> Andrew Fish<mailto:afish@apple.com>; Kinney, Michael
> D<mailto:michael.d.kinney@intel.com>
> Cc: rfc@edk2.groups.io<mailto:rfc@edk2.groups.io>
> Subject: RE: [EXTERNAL] Re: [edk2-devel] EDK2 Host-
> Based Unit Test RFC (Now with docs!)
>
>
>   1.  I see that MdePkg adds a dependency on
> UnitTestPkg.  This makes UnitTestPkg the root package
> when building and running host based unit tests.  This
> makes sense, but good to highlight that all packages
> that use host based tests will introduce a new package
> dependency on UnitTestPkg.
>      *   Since the dependency only applies to unit
> tests, can we update the 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 intended to be used directly from a
> unit test case?  If some of these are only intended to
> be used from the framework inside the UnitTestPkg can
> we make them private?
>      *   Because there are different instances in
> multiple places (and conceivably more in the future),
> we don't see how we could make them private.
>   3.  In the MdePkg, I see a new top level directory
> called 'HostLibrary'. Since these lib instances are
> only used from a host based test, can they be moved
> into  the 'Test' directory?
>      *   Can move.
>   4.  MdePkg/MdePkgTest.dsc.
>      *   Can this DSC file be moved into the 'Test'
> directory?
>
>
> i.      Yes
>
>      *   I see this DSC file only supports VS today.
> How much work is it to add GCC support?
>
>
> i.      Don't know. 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 that depends on read/writing these
> registers would not be compatible with host based
> testing.  Can we change to ASSERT (FALSE)?
>      *   PatchInstructionX86() - 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 worked so we kept them. We're just
> as interested in the answers to these questions as you
> are.
>   2.  MdePkg/HostLibrary/DebugLibHost
>      *   What is '#ifndef TEST_WITH_KLEE'
>      *   What is the 'PatchFormat()' function?  It is
> always disabled with if (0).
>      *   Are the PCDs to set the debug message levels
> disabled on purpose? (DebugPrintEnabled(),
> DebugPrintLevelEnabled(), DebugCodeEnabled())
>      *   Libraries were taken directly from the Intel
> work in HBFA. They worked so we kept them. We're just
> as interested in the answers to these questions as you
> are.
>   3.  MdePkg/HostLibrary/BaseMemoryLibHost
>      *   Why can't 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 worked so we kept them. We're just
> as interested in the answers to these questions as you
> are.
>   4.  MdePkg/HostLibrary/MemoryAllocationLibHost
>      *   Why is are memcpy(), assert(), memset() used
> in this lib?  I would expect this lib instance to match
> the UefiMemoryAllocationLib with the only the use of
> malloc() and free() to replace the UEFI Boot Services
> calls.
>      *   Libraries were taken directly from the Intel
> work in HBFA. They worked so we kept them. We're just
> as interested in the answers to these questions 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.  Would it
> make more sense to use 'Host' as a prefix instead of a
> postfix in the lib instance names?
>      *   I don't know if there's a 1:1 relationship
> with these. For some library classes (that you might
> have host versions of), the class itself is the
> PeiSomethingLib, and the host version would be the
> PeiSomethingLibHost. No?
>   6.  Unicode vs ASCII strings
>      *   I see InitUnitTestFramework(),
> CreateUnitTestSuite(), and AddTestCase() all take
> Unicode strings for the labels.  I also see extra code
> to convert 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.
>      *   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 Shell as in the
> Host, going with Unicode strings seemed to match 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?
>      *   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/OUT and if it is passed in as NULL, the
> Framework handle is created.
>
>
> i.      It's not always in pairs. You would only have a
> single framework init, but could have multiple suites.
>
>      *   I see a pattern where the
> CreateUnitTestSuite() 'Package' parameter is used as a
> prefix to every call to AddTestCase() in the
> 'ClassName' parameter.  Can we simplify AddTestCase()
> so it only need to pass in the name of the unit test
> case, and the framework can append Package  and the
> unit test case name?
>
>
> i.      Right now, our tests just coincidentally share
> similar names with the packages, but we don't feel this
> is axiomatic and don't want to force this naming on
> others, who may be trying to bolt into other reporting
> structures.
>
>   1.  I see the use of the 'Fw' variable as a shorthand
> for 'Framework'.  Can we use something other than 'Fw'.
> It may be confused with 'Firmware'.
>      *   No real arguments. Wouldn't fight a find-
> replace, but it'll just be a bunch of touches as we
> commit.
>   2.  UnitTestPkg/Include/UnitTestTypes.h
>      *   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 framework can manage
> alloc() and free()?
>
>
> i.      Given that this framework is designed to be
> nimble enough to work in PEI and SMM and other
> constrictive environments, we wanted to keep fixed
> sizing.
>
>      *   How are Fingerprints used?  The idea of using
> as hash as a unique identifier is a good idea.  How is
> the hash calculated?  What unit test 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 future?
>
>
> i.      Fingerprints are unique IDs to make sure that
> serialize/unserialized state matches the tests upon re-
> entry. I'm not married to MD5, but it needs to be
> predictably unique, and carried with the framework. I
> would not want to make any requirements on CryptoLib,
> since these aren't crypto-strong.
>
>      *   Update all the strings to be ASCII?  See
> Unicode vs ASCII above.
>
>
> i.      Ideally not, unless there's a strong use case.
>
>      *   UNIT_TEST_SAVE_TEST - The last field is a
> variable sized array, so it can be a formal field.
>
>
> i.      Not opposed, but don't really want to make the
> change myself. Is there a style guide that this is
> breaking?
>
>      *   UNIT_TEST_SAVE_CONTEXT - - The last field is a
> variable sized array, so it can be a formal field.
>      *   UNIT_TEST_SAVE_HEADER - Can the last 3 fields
> be changed to pointers and be formal fields?
>      *   Do the structures really need to be packed?
> Specially with the changes suggested above?  Is the
> intent to potentially share data stored on disk between
> different host execution environments that may have
> different width architectures?
>
>
> i.      That's an interesting point. Could you draw up
> your suggestion and submit a PR?
>
>   1.  UnitTestPkg - UnitTestLib.h
>      *   Can you provide more context for the APIs
> SaveFrameworkState(), SaveFrameworkStateAndQuit(),
> SaveFrameworkStateAndReboot(),
> SetFrameworkBootNextDevice().  I do not see any Load()
> functions, so how would a set of tests be resumed?  If
> these do not apply to host based tests, should these be
> split out to a different lib class?
>
>
> i.      I'll improve the documentation around these
> functions. I will acknowledge, however, that this is an
> interface that I expect to evolve as we figure out what
> kinds of tests the community wants support for "out of
> the box". If we want to easily enable tests around -
> for example - ExitBootServices, we will likely want to
> tweak this going forward to its simplest form. The
> version we have here was sufficient to enable the test
> cases that we've currently written.
>
>   1.  UnitTestPkg/Library/UnitTestLib
>      *   I see an implementation of MD5.  We should not
> do our own.  We should use an approved implementation
> such as OpenSSL.
>
>
> i.      Happy to discuss another implementation, but
> this is not crypto-strong. It's just for uniqueness. A
> sufficiently long CRC would probably work, too.
>
>   1.  UnitTestPkg/Library/UnitTestTerminationLibTbd
>      *   Do we really need this lib instance now?
>
>
> i.      This is here so that we can figure out what
> this should look like for a host. Host obviously
> wouldn't reset, but could conceivably quit. Or maybe
> that doesn't make any sense for a host.
>
>
>
> - Bret
>
>
> From: devel@edk2.groups.io<mailto:devel@edk2.groups.io> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>> on
> behalf of Bret Barkelew via Groups.Io
> <bret.barkelew=microsoft.com@groups.io<mailto:bret.barkelew=microsoft.com@groups.io>>
> Sent: Wednesday, December 4, 2019 10:24:21 AM
> To: Andrew Fish <afish@apple.com<mailto:afish@apple.com>>; devel@edk2.groups.io<mailto:devel@edk2.groups.io>
> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>>; Kinney, Michael D
> <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>
> Cc: rfc@edk2.groups.io<mailto:rfc@edk2.groups.io> <rfc@edk2.groups.io<mailto:rfc@edk2.groups.io>>
> Subject: Re: [EXTERNAL] Re: [edk2-devel] EDK2 Host-
> Based Unit Test RFC (Now with docs!)
>
>
> Andrew,
>
> I agree with your points.
>
>
>
> Mike,
>
> You've got a lot more there. Let me take a look and
> update the draft. I'll ping back ASAP.
>
>
>
> - Bret
>
>
>
> From: Andrew Fish<mailto:afish@apple.com>
> Sent: Wednesday, December 4, 2019 9:50 AM
> To: devel@edk2.groups.io<mailto:devel@edk2.groups.io<mailto:devel@edk2.groups.io%3cmailto:devel@edk2.groups.io>>;
> Kinney, Michael D<mailto:michael.d.kinney@intel.com>
> Cc: Bret Barkelew<mailto:Bret.Barkelew@microsoft.com>;
> rfc@edk2.groups.io<mailto: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/ ( *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 modules 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
> 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 enough 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 think
> 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 get 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<mailto:michael.d.kinney@int
<mailto:michael.d.kinney@intel.com%3cmailto:michael.d.kinney@int%0b>> el.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.  This
> makes sense, but good to highlight that all packages
> that use host based tests will introduce a new package
> dependency on UnitTestPkg.
>
>      *   Since the dependency only applies to unit
> tests, can we update the 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 intended to be used directly from a
> unit test case?  If some of these are only intended to
> be used from the framework inside the UnitTestPkg can
> we make them private?
>   2.  In the MdePkg, I see a new top level directory
> called 'HostLibrary'. Since these lib instances are
> only used from a host based test, can they be moved
> into  the 'Test' directory?
>   3.  MdePkg/MdePkgTest.dsc.
>
>      *   Can this DSC file be moved into the 'Test'
> directory?
>      *   I see this DSC file only supports VS today.
> How much work is it to add GCC support?
>
>   1.  MdePkg/HostLibrary/BaseLibHost
>
>      *   Why are there 2 INFs.  One with ASM and one
> without ASM?  Can we just use the one with ASM and
> assume NASM is installed?
>      *   I see the purpose of this lib instance is to
> call the
>      *   SetJump()/LongJump().  So these
> implementations always work?  It looks like it assumes
> BASE_LIBRARY_JUMP_BUFFER is identical to the host OS
> user mode applicationsetjmp()/longjmp() state.
>      *   Why are DRx and CRx registers emulated?  I
> would think and code that depends on read/writing these
> registers would not be compatible with host based
> testing.  Can we change to ASSERT (FALSE)?
>      *   PatchInstructionX86() - I suspect this will
> not work in a host based environment because it is self
> modifying code.  Should it be ASSERT (FALSE)?
>
>   1.  MdePkg/HostLibrary/DebugLibHost
>
>      *   What is '#ifndef TEST_WITH_KLEE'
>      *   What is the 'PatchFormat()' function?  It is
> always disabled with if (0).
>      *   Are the PCDs to set the debug message levels
> disabled on purpose? (DebugPrintEnabled(),
> DebugPrintLevelEnabled(), DebugCodeEnabled())
>
>   1.  MdePkg/HostLibrary/BaseMemoryLibHost
>
>      *   Why can't we use
> MdePkg/Library/BaseMemoryLib/BaseMemoryLib/inf 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 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.
>
>      *   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 'Host' as a prefix instead of a
> postfix in the lib instance names?
>
>   1.  Unicode vs ASCII strings
>
>      *   I see InitUnitTestFramework(),
> CreateUnitTestSuite(), and AddTestCase() all take
> Unicode strings for the labels.  I also see extra code
> to convert 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.
>
>   1.  Will InitUnitTestFramework() and
> CreateUnitTestSuite() always be called in pairs?  If
> so, can we combine these to a single API?
>
>      *   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/OUT and if it is passed in as NULL, the
> Framework handle is created.
>      *   I see a pattern where the
> CreateUnitTestSuite() 'Package' parameter is used as a
> prefix to every call to AddTestCase() in the
> 'ClassName' parameter.  Can we simplify AddTestCase()
> so it only need to pass in the name of the unit test
> case, and the framework can append Package  and the
> unit test case name?
>
>   1.  I see the use of the 'Fw' variable as a shorthand
> for 'Framework'.  Can we use something other than 'Fw'.
> It may be confused with 'Firmware'.
>   2.  UnitTestPkg/Include/UnitTestTypes.h
>
>      *   I see several hard coded string lengths.
> Since this runs in a 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 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 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 future?
>      *   Update all the strings to be ASCII?  See
> Unicode vs ASCII above.
>      *   UNIT_TEST_SAVE_TEST - The last field is a
> variable sized array, so it can be a formal field.
>      *   UNIT_TEST_SAVE_CONTEXT - - The last field is a
> variable sized array, so it can be a formal field.
>      *   UNIT_TEST_SAVE_HEADER - Can the last 3 fields
> be changed to pointers and be formal fields?
>      *   Do the structures really need to be packed?
> Specially with the changes suggested above?  Is the
> intent to potentially share data stored on disk between
> different host execution environments that may have
> different width architectures?
>
>   1.  UnitTestPkg - UnitTestLib.h
>
>      *   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?  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 should 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<mailto:devel@edk2.groups.io<mailto:devel@edk2.groups.io%3cmailto:devel@edk2.groups.io>>
> <devel@edk2.groups.io<mailto:devel@edk2.groups.io<mailto:devel@edk2.groups.io%3cmailto: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<mailto:devel@edk2.groups.io<mailto:devel@edk2.groups.io%3cmailto: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 framework for unittesting staged 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,
> HostUnitTestDxeCompleteCheck, 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://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-&data=02%7C01%7CBret.Barkelew%40microsoft.com%7Ca68d03ae548e4bade27208d7829784c7%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637121459602891276&sdata=AjP4y1VmSz3hVazhVi10t9s%2B61LRUTU382x%2BC1U7Xgs%3D&reserved=0>
> test_v2<https://nam06.safelinks.protection.outlook.com/
<https://nam06.safelinks.protection.outlook.com/%0b>> ?url=https%3A%2F%2Fgithub.com%2Fcorthon%2Fedk2-
> staging%2Ftree%2Fedk2-host-
> test_v2&data=02%7C01%7Cbret.barkelew%40microsoft.com%7C
> 6eac9932f3f640dd65ac08d778e731e3%7C72f988bf86f141af91ab
> 2d7cd011db47%7C1%7C0%7C637110806683189828&sdata=JQ%2BoW
> xlEBOK2sH55cRAPVa3OpAxTsm6eHcdbWSo9CVo%3D&reserved=0>
> We also have a demo build pipeline at:
> 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-&data=02%7C01%7CBret.Barkelew%40microsoft.com%7Ca68d03ae548e4bade27208d7829784c7%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637121459602901235&sdata=ESuFb4DgAymJXeZCdStXOz1xSV8OfC1efwKUFFk8svI%3D&reserved=0>
> 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
<https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fnam06.sa&data=02%7C01%7CBret.Barkelew%40microsoft.com%7Ca68d03ae548e4bade27208d7829784c7%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637121459602911189&sdata=jfLSkCtEYEZsa0hAMRC4onDC39cfjPXMbTVytC5M8Pc%3D&reserved=0>> felinks.protection.outlook.com/?url=https%3A%2F%2Fdev.a
> zure.com%2Ftianocore%2Fedk2-ci-
> play%2F_build%3FdefinitionId%3D36%26_a%3Dsummary&data=0
> 2%7C01%7Cbret.barkelew%40microsoft.com%7C6eac9932f3f640
> dd65ac08d778e731e3%7C72f988bf86f141af91ab2d7cd011db47%7
> C1%7C0%7C637110806683189828&sdata=wBwn1ehjyTmYNKVSvlEZS
> XK5qyeu4EPAL7FzdYntnt4%3D&reserved=0>
>
> The current demo branch contains a single test in
> MdePkg for SafeIntLib (module file
> MdePkg\Test\UnitTest\Library\BaseSafeIntLib\TestBaseSaf
> eIntLib.inf). This test is automatically detected by
> the HostUnitTestComilerPlugin and 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 reason 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://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-&data=02%7C01%7CBret.Barkelew%40microsoft.com%7Ca68d03ae548e4bade27208d7829784c7%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637121459602911189&sdata=htsFVfl9pz96NZq0tkIlS5cLe1LvvikAsP0DZMkzhR0%3D&reserved=0>
> 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
<https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fnam06.safelink&data=02%7C01%7CBret.Barkelew%40microsoft.com%7Ca68d03ae548e4bade27208d7829784c7%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637121459602921146&sdata=Rbt%2BikKJftdP7sIcAMfNi67N4gm8p7btnwik0gQA0QQ%3D&reserved=0>> s.protection.outlook.com/?url=https%3A%2F%2Fdev.azure.c
> om%2Ftianocore%2Fedk2-ci-
> play%2F_build%2Fresults%3FbuildId%3D2590&data=02%7C01%7
> Cbret.barkelew%40microsoft.com%7C6eac9932f3f640dd65ac08
> d778e731e3%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7
> C637110806683199782&sdata=1zd2oq8zRZUn3%2FUiGDuJZEZ5M0s
> rtc2bqoa0%2BLbZB3s%3D&reserved=0>
> "WARNING -   Test SafeInt16ToChar8 - Status
> d:\a\1\s\MdePkg\Test\UnitTest\Library\BaseSafeIntLib\Te
> stBaseSafeIntLib.c:302: error: Failure!
> WARNING - TestBaseSafeIntLib.exe Test Failed
> WARNING -   Test SafeInt32ToChar8 - Status
> d:\a\1\s\MdePkg\Test\UnitTest\Library\BaseSafeIntLib\Te
> stBaseSafeIntLib.c:638: error: Failure!
> WARNING - TestBaseSafeIntLib.exe Test Failed
> WARNING -   Test SafeIntnToChar8 - Status
> d:\a\1\s\MdePkg\Test\UnitTest\Library\BaseSafeIntLib\Te
> stBaseSafeIntLib.c:1051: error: Failure!
> WARNING - TestBaseSafeIntLib.exe Test Failed
> WARNING -   Test SafeInt64ToChar8 - Status
> d:\a\1\s\MdePkg\Test\UnitTest\Library\BaseSafeIntLib\Te
> stBaseSafeIntLib.c:1456: error: Failure!"
> These failures seem to be legitimate and further work
> should be done by the community to decide whether the
> test case is correct or the library is correct, 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 project 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
> unnecessary). Maybe in a new edk2-test2 repo or
> something like that.
>
> There's an RFC doc here:
> 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-&data=02%7C01%7CBret.Barkelew%40microsoft.com%7Ca68d03ae548e4bade27208d7829784c7%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637121459602921146&sdata=FgVQ%2B%2BnxYGezf9xcUurNnW1vHZnxnCQ%2Bfl352i%2B2NNk%3D&reserved=0>
> test_v2/Readme-
> RFC.md<https://nam06.safelinks.protection.outlook.com/?
<https://nam06.safelinks.protection.outlook.com/?%0b>> url=https%3A%2F%2Fgithub.com%2Fcorthon%2Fedk2-
> staging%2Fblob%2Fedk2-host-test_v2%2FReadme-
> RFC.md&data=02%7C01%7Cbret.barkelew%40microsoft.com%7C6
> eac9932f3f640dd65ac08d778e731e3%7C72f988bf86f141af91ab2
> d7cd011db47%7C1%7C0%7C637110806683199782&sdata=1nq1mHjs
> bSZj4IQM5RBwSrvaQxO5cmDlTvf7VYNDV%2FA%3D&reserved=0>
>
> And a usage guide here:
> 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-&data=02%7C01%7CBret.Barkelew%40microsoft.com%7Ca68d03ae548e4bade27208d7829784c7%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637121459602931100&sdata=kpie8vUTd0p3uuJALz6JEhot3mbaU0%2BDOYcXlPr1IWI%3D&reserved=0>
> 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
<https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fnam06.safelinks.p&data=02%7C01%7CBret.Barkelew%40microsoft.com%7Ca68d03ae548e4bade27208d7829784c7%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637121459602931100&sdata=4g4c3wXMSoSUZ9xpjWCTADAuj3gDL4eKV44Wio0%2FVsg%3D&reserved=0>> rotection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fc
> orthon%2Fedk2-staging%2Fblob%2Fedk2-host-
> test_v2%2FUnitTestPkg%2FReadMe.md&data=02%7C01%7Cbret.b
> arkelew%40microsoft.com%7C6eac9932f3f640dd65ac08d778e73
> 1e3%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637110
> 806683209750&sdata=0jRQ2Rzr9PWSYJ1YDs7l2aFS3PfAnTbTousY
> Ye8IWTw%3D&reserved=0>
>
> Once again, would love to get this into EDK2 master
> after stabilization, and most of this has previously
> been shopped around in other discussion threads. 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 run as part of any CI pass.
>
> Thanks!
> - Bret
>
>
>
>
>
>
>
>
> 

[-- Attachment #2: Type: text/html, Size: 51870 bytes --]