[REBASE & FF] Add GoogleTest Mocks and STATIC Testing Ability

[os-d]

Description

This PR adds GoogleTest mocks needed for some AdvancedLogger GoogleTests. It also adds the capability to test STATIC functions by undefining the STATIC keyword if a HOST_APPLICATION is being built. The statement is that a HOST_APPLICATION is running with the minimal set of dependencies and should not run into symbol collision.

These will be sent to edk2, but feedback is wanted first, as well as getting the dependent Advanced Logger PR in. If this patch is approved, UnitTestFrameworkPkg's README will also be updated in edk2. This is not done here as that would create a lot of conflicts.

For each item, place an "x" in between [ and ] if true. Example: [x].
(you can also check items in the GitHub UI)

• Impacts functionality?
  • Functionality - Does the change ultimately impact how firmware functions?
  • Examples: Add a new library, publish a new PPI, update an algorithm, ...
• Impacts security?
  • Security - Does the change have a direct security impact on an application,
    flow, or firmware?
  • Examples: Crypto algorithm change, buffer overflow fix, parameter
    validation improvement, ...
• Breaking change?
  • Breaking change - Will anyone consuming this change experience a break
    in build or boot behavior?
  • Examples: Add a new library class, move a module to a different repo, call
    a function in a new library class in a pre-existing module, ...
• Includes tests?
  • Tests - Does the change include any explicit test code?
  • Examples: Unit tests, integration tests, robot tests, ...
• Includes documentation?
  • Documentation - Does the change contain explicit documentation additions
    outside direct code modifications (and comments)?
  • Examples: Update readme file, add feature readme file, link to documentation
    on an a separate Web page, ...

How This Was Tested

Tested with the Advanced Logger GoogleTests

Integration Instructions

Follow the UnitTestFrameworkPkg README.md for instructions on using GoogleTest mocks. To test STATIC functions, compile the relevant C files with your GoogleTest C++ files and you will be able to access the formerly STATIC functions in your test.

Interface tests (that load a library instead of compiling the C files) will still not be able to access STATIC functions. This is intentional as interface tests should be testing a library interface (the class itself) not an instance.

[codecov-commenter]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 1.22%. Comparing base (8fb7503) to head (162f3ca).

Additional details and impacted files

@@               Coverage Diff               @@
##           release/202311     #830   +/-   ##
===============================================
  Coverage            1.22%    1.22%           
===============================================
  Files                1303     1303           
  Lines              335685   335685           
  Branches             3183     3183           
===============================================
  Hits                 4118     4118           
  Misses             331491   331491           
  Partials               76       76           

Flag	Coverage Δ
MdeModulePkg	0.68% <ø> (ø)
MdePkg	5.37% <ø> (ø)
NetworkPkg	0.00% <ø> (ø)
PolicyServicePkg	30.41% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[VivianNK]

Even with the minimum set of dependencies of GOOGLETEST_HOST_UNIT_TEST_BUILD, are we sure there will be no symbol collisions with platform code?

[os-d]

@mdkinney do you have any thoughts on this approach for solving the GoogleTest STATIC testing issue? I will push this to edk2 also, but testing this in Mu first and getting feedback.

[mdkinney]

We are trying to remove use of STATIC in existing code and prevent adding STATIC to new code. This would reverse that idea and if we added GoogleTest coverage across all the components would significant increase the use of STATIC.

There are also several places that 'static' had to be added to modules/libs to prevent symbol collisions between components. This approach would reopen those issues.

• Instead of odd random failures only when building unit tests, one option is to remove STATIC/static all together and address symbol collisions by renaming functions. This reduces symbol hiding the C language supports, so is not a great option.

• #include C file under test from GoogleTest C++ file. There are some know compat issues, but for files that are compatible, this is a viable solution.

[os-d]

We are trying to remove use of STATIC in existing code and prevent adding STATIC to new code. This would reverse that idea and if we added GoogleTest coverage across all the components would significant increase the use of STATIC.

There are also several places that 'static' had to be added to modules/libs to prevent symbol collisions between components. This approach would reopen those issues.

• Instead of odd random failures only when building unit tests, one option is to remove STATIC/static all together and address symbol collisions by renaming functions. This reduces symbol hiding the C language supports, so is not a great option.
• #include C file under test from GoogleTest C++ file. There are some know compat issues, but for files that are compatible, this is a viable solution.

Thanks for the review @mdkinney. We looked at the two options you listed, I think that the first option is not viable, static does provide a useful hiding of internal functions. For the second one, I would be curious if the compat issues can be solved, this is how we used to do it in Cmocka, but we have seen a number of issues, the largest being C++ having stricter rules about implicit casting, which occurs all over the codebase.

Another idea we reviewed was changing STATIC to PRIVATE and then using static for static lifetime variables, since in effect C is using the same keyword for two different ideas. This would change almost every file though and doesn't seem worthwhile to do.

The idea here is that when building a HOST_APPLICATION, you are at the minimal set of dependencies and so we expect that symbol collisions should be rare. It really would just be if there were symbol collisions in the module under test, which I think would be a design flaw for a single module to have symbol collisions. This is not a perfect solution to be sure, but it seemed the lowest lift.

If STATIC is removed, I assume you mean that static would be used instead? If that were the case, I would think we would have to solve the GoogleTest including C files compat issues or else GoogleTest would be unable to unit test many static functions (which I think there is great value to as the author of a library or module to ensure that at a function level your helper functions are working as expected, particularly over time as those functions get changed).

[spbrogan]

@mdkinney - It would be great to explore other ideas your team may have but after trying to find the best compromise we think this makes sense for testing module C files and library class implementation C files.

[spbrogan]

why are you adding all sorts of mock libraries in this PR? I thought we had a plan to only add them as a test needed them?

[os-d]

why are you adding all sorts of mock libraries in this PR? I thought we had a plan to only add them as a test needed them?

@spbrogan they are getting consumed in this PR: microsoft/mu_plus#471, so they are needed.