Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MdePkg: Add Google Mock Library for BaseMemoryLib #1289

Conversation

SeolforHsieh
Copy link
Contributor

Description

Added MockBaseMemoryLib to be used by Googletests

  • Impacts functionality?
  • Impacts security?
  • Breaking change?
  • Includes tests?
  • Includes documentation?
  • Backport to release branch?

How This Was Tested

Added MockBaseMemoryLib to GoogleTests and able to successfully include its functions

Integration Instructions

N/A

@github-actions github-actions bot added impact:non-functional Does not have a functional impact type:backport Backport changes in a dev branch PR to its release branch. labels Mar 3, 2025
@SeolforHsieh
Copy link
Contributor Author

@apop5 and @VivianNK,
please review it, thanks.

PR of edk2: tianocore/edk2#10810

@codecov-commenter
Copy link

codecov-commenter commented Mar 3, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 1.42%. Comparing base (b077d2f) to head (d0ca698).

Additional details and impacted files
@@              Coverage Diff               @@
##           dev/202405    #1289      +/-   ##
==============================================
- Coverage        1.52%    1.42%   -0.11%     
==============================================
  Files            1383     1383              
  Lines          359247   359091     -156     
  Branches         5524     5524              
==============================================
- Hits             5493     5122     -371     
- Misses         353466   353498      +32     
- Partials          288      471     +183     
Flag Coverage Δ
MdeModulePkg 0.67% <ø> (ø)
MdePkg 5.59% <ø> (ø)
NetworkPkg 0.35% <ø> (ø)
PolicyServicePkg 30.41% <ø> (+12.01%) ⬆️
UefiCpuPkg 2.83% <ø> (-1.86%) ⬇️

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.

@SeolforHsieh SeolforHsieh marked this pull request as draft March 3, 2025 08:49
@SeolforHsieh SeolforHsieh marked this pull request as ready for review March 3, 2025 08:49
@SeolforHsieh SeolforHsieh reopened this Mar 3, 2025
@VivianNK VivianNK requested a review from apop5 March 3, 2025 18:42
@apop5
Copy link
Contributor

apop5 commented Mar 3, 2025

@VivianNK

Does a MockBaseMemoryLib actually make sense?

The functions inside of base memory lib all operate on pointers that are passed into the functions. They don't independently access memory outside of what is passed in.

Given the host based unit test functions this is targeting, I don't see why the actual basememory lib instance cannot be used?

Am I missing anything?

@VivianNK
Copy link
Contributor

VivianNK commented Mar 3, 2025

@VivianNK

Does a MockBaseMemoryLib actually make sense?

The functions inside of base memory lib all operate on pointers that are passed into the functions. They don't independently access memory outside of what is passed in.

Given the host based unit test functions this is targeting, I don't see why the actual basememory lib instance cannot be used?

Am I missing anything?

@SeolforHsieh can you confirm that the test you are writing can use the BaseMemoryLib?

@VivianNK VivianNK self-requested a review March 3, 2025 20:45
@SeolforHsieh
Copy link
Contributor Author

@VivianNK and @apop5,

After modifying my google test to consume BaseMemoryLib (MdePkg/Library/BaseMemoryLib/BaseMemoryLib.inf), the test passed w/o errors.
I'll close this PR, thank you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
impact:non-functional Does not have a functional impact type:backport Backport changes in a dev branch PR to its release branch.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants