Re: [edk2-devel] [edk2-stable202402][Patch v4 0/7] EDK II CI misses UnitTestFrameworkPkg failures

["Leif Lindholm" <quic_llindhol@quicinc.com>]

This series look safe, as in if the only change that affects the output 
image was buggy, it would blow up spectacularly at build time.
I'm happy for it to go in the stable tag.

On 2024-02-13 17:26, Michael D Kinney wrote:
> The main bug is that EDK II CI can miss a unit test failure when
> a unit test generates an exception. Addressing this one issue
> required the following chain of changes to the UnitTestFrameworkPkg
> that are included in this patch series.
> 
> New in v4
> ==========
> * Moved GoogleTestLib.inf /EHsc -> EHs from Patch #2 ->Patch #3
>    so the /EHsc change is not mixed with /MT changes in Patch #2.
> * Fixed typo in Patch #1 commit message
> 
> New in V3
> =========
> * Add EXPECT_THROW_MESSAGE() and ASSERT_THROW_MESSAGE() to
>    GoogleTestLib.h to simplify unit test case statements testing for
>    an ASSERT() for a specific trigger expressiom. Uncrustify format
>    of EXPECT_THAT() with [](){stmt} make code harder to read. Adding
>    macros makes the source content in unit test cases match the
>    source style of EXPECT_THROW() statements.
> * Update GoogTest samples to use EXPECT_THROW_MESSAGE() instead of
>    EXPECT_THAT().
> 
> New in V2
> =========
> * Include Throws() APIs in GoogleTestLib.h
> * Update GoogleTest samples to demonstrate use of EXPECT_THROW()
>    and EXPECT_THAT() to test for more specific ASSERT() conditions
>    including the ability to verify the specific ASSERT() expression
>    that was triggered.
> =========
> 
> * For host-base VX20xx builds, use release builds by default to
>    make sure unexpected exceptions are caught as a test failure.
>    Using debug libraries may generate a popup window to prompt
>    running a debugger, which is value for debug, but that mode
>    does not generate an error in CI.
> * Update UNIT_TEST_DEBUG to enable use of VS20xx debug libraries
>    to enable popup windows to run debugger.
> * Remove DebugLib.h internal macro name collision with windows.h
>    that was introduced when release libraries are used by default.
> * Update MSFT and GCC flags to consistently enable structured
>    exception handling so Google Test host-based unit tests that
>    check for expected ASSERT() conditions can be properly detected.
>    The additional benefit of this bug fix is that catching thrown
>    exceptions is much faster than EXPECT_DEATH() tests. The gtest
>    sample unit tests are updated to use ASSERT_ANY_THROW() instead
>    of EXPECT_DEATH().
> * In order to catch C++ exceptions for google test, the
>    UnitTestDebugAssertLib has to be split into a target version
>    and a host version and had to be able to detect the difference
>    between cmock host-based tests and gtest host-based tests. This
>    required minor changes to the UnitTestLib to be able to use the
>    UnitTestLib GetFrameworkHandle() API to make that determination.
>    It is NULL for gtest and non-NULL for cmocka.
> * In order to test the unit test failing conditions, unit tests
>    the fail on purpose and unit tests that generate exceptions are
>    added.  These are not added to CI because that would cause CI to
>    always fail. Instead, they can be run to make sure that reports
>    generated provide the right level of detail for a developer to
>    quickly identify the source of the unit test failure or the
>    source of the exception.
> 
> Cc: Michael Kubacki mikuback@linux.microsoft.com
> Cc: Sean Brogan sean.brogan@microsoft.com
> Cc: Liming Gao gaoliming@byosoft.com.cn
> Cc: Zhiguang Liu zhiguang.liu@intel.com
> Cc: Andrew Fish afish@apple.com
> Cc: Leif Lindholm quic_llindhol@quicinc.com
> Signed-off-by: Michael D Kinney michael.d.kinney@intel.com
> 
> Michael D Kinney (7):
>    MdePkg/Include: Rename _DEBUG() to address name collision
>    UnitTestFrameworkPkg: MSFT CC_FLAGS add /MT to for host builds
>    UnitTestFrameworkPkg: Expand host-based exception handling and gcov
>    UnitTestFrameworkPkg/UnitTestLib: GetActiveFrameworkHandle() no
>      ASSERT()
>    UnitTestFrameworkPkg/UnitTestDebugAssertLib: Add GoogleTest support
>    UnitTestFrameworkPkg/SampleGoogleTest: Use EXPECT_ANY_THROW()
>    UnitTestFrameworkPkg: Add DSC and host tests that always fail
> 
>   MdePkg/Include/Library/DebugLib.h             |   6 +-
>   .../Include/Library/GoogleTestLib.h           |  20 +
>   .../Library/GoogleTestLib/GoogleTestLib.inf   |   6 +-
>   .../UnitTestDebugAssertLibHost.cpp            |  63 ++
>   .../UnitTestDebugAssertLibHost.inf            |  36 +
>   .../UnitTestDebugAssertLibHost.uni            |  11 +
>   .../Library/UnitTestLib/Assert.c              |   4 +
>   .../Library/UnitTestLib/Log.c                 |   4 +
>   .../Library/UnitTestLib/RunTests.c            |   1 -
>   .../Library/UnitTestLib/RunTestsCmocka.c      |   1 -
>   .../Library/UnitTestLib/UnitTestLib.c         |   4 +
>   UnitTestFrameworkPkg/ReadMe.md                |   2 +-
>   .../SampleGoogleTest/SampleGoogleTest.cpp     |  36 +-
>   .../SampleGoogleTestExpectFail.cpp}           | 125 ++-
>   .../SampleGoogleTestHostExpectFail.inf        |  36 +
>   .../SampleGoogleTestGenerateException.cpp     |  54 ++
>   .../SampleGoogleTestHostGenerateException.inf |  39 +
>   .../SampleUnitTestDxeExpectFail.inf           |  41 +
>   .../SampleUnitTestExpectFail.c                | 861 ++++++++++++++++++
>   .../SampleUnitTestHostExpectFail.inf          |  35 +
>   .../SampleUnitTestPeiExpectFail.inf           |  41 +
>   .../SampleUnitTestSmmExpectFail.inf           |  42 +
>   .../SampleUnitTestUefiShellExpectFail.inf     |  38 +
>   .../SampleUnitTestDxeGenerateException.inf    |  43 +
>   .../SampleUnitTestGenerateException.c         | 204 +++++
>   .../SampleUnitTestHostGenerateException.inf   |  37 +
>   .../SampleUnitTestPeiGenerateException.inf    |  43 +
>   .../SampleUnitTestSmmGenerateException.inf    |  44 +
>   ...mpleUnitTestUefiShellGenerateException.inf |  40 +
>   .../Test/UnitTestFrameworkPkgHostTest.dsc     |   1 +
>   ...UnitTestFrameworkPkgHostTestExpectFail.dsc |  44 +
>   .../UnitTestFrameworkPkg.ci.yaml              |  18 +-
>   UnitTestFrameworkPkg/UnitTestFrameworkPkg.dsc |  26 +
>   .../UnitTestFrameworkPkgHost.dsc.inc          |  11 +-
>   .../UnitTestFrameworkPkgTarget.dsc.inc        |  14 +
>   35 files changed, 1977 insertions(+), 54 deletions(-)
>   create mode 100644 UnitTestFrameworkPkg/Library/UnitTestDebugAssertLib/UnitTestDebugAssertLibHost.cpp
>   create mode 100644 UnitTestFrameworkPkg/Library/UnitTestDebugAssertLib/UnitTestDebugAssertLibHost.inf
>   create mode 100644 UnitTestFrameworkPkg/Library/UnitTestDebugAssertLib/UnitTestDebugAssertLibHost.uni
>   copy UnitTestFrameworkPkg/Test/GoogleTest/Sample/{SampleGoogleTest/SampleGoogleTest.cpp => SampleGoogleTestExpectFail/SampleGoogleTestExpectFail.cpp} (53%)
>   create mode 100644 UnitTestFrameworkPkg/Test/GoogleTest/Sample/SampleGoogleTestExpectFail/SampleGoogleTestHostExpectFail.inf
>   create mode 100644 UnitTestFrameworkPkg/Test/GoogleTest/Sample/SampleGoogleTestGenerateException/SampleGoogleTestGenerateException.cpp
>   create mode 100644 UnitTestFrameworkPkg/Test/GoogleTest/Sample/SampleGoogleTestGenerateException/SampleGoogleTestHostGenerateException.inf
>   create mode 100644 UnitTestFrameworkPkg/Test/UnitTest/Sample/SampleUnitTestExpectFail/SampleUnitTestDxeExpectFail.inf
>   create mode 100644 UnitTestFrameworkPkg/Test/UnitTest/Sample/SampleUnitTestExpectFail/SampleUnitTestExpectFail.c
>   create mode 100644 UnitTestFrameworkPkg/Test/UnitTest/Sample/SampleUnitTestExpectFail/SampleUnitTestHostExpectFail.inf
>   create mode 100644 UnitTestFrameworkPkg/Test/UnitTest/Sample/SampleUnitTestExpectFail/SampleUnitTestPeiExpectFail.inf
>   create mode 100644 UnitTestFrameworkPkg/Test/UnitTest/Sample/SampleUnitTestExpectFail/SampleUnitTestSmmExpectFail.inf
>   create mode 100644 UnitTestFrameworkPkg/Test/UnitTest/Sample/SampleUnitTestExpectFail/SampleUnitTestUefiShellExpectFail.inf
>   create mode 100644 UnitTestFrameworkPkg/Test/UnitTest/Sample/SampleUnitTestGenerateException/SampleUnitTestDxeGenerateException.inf
>   create mode 100644 UnitTestFrameworkPkg/Test/UnitTest/Sample/SampleUnitTestGenerateException/SampleUnitTestGenerateException.c
>   create mode 100644 UnitTestFrameworkPkg/Test/UnitTest/Sample/SampleUnitTestGenerateException/SampleUnitTestHostGenerateException.inf
>   create mode 100644 UnitTestFrameworkPkg/Test/UnitTest/Sample/SampleUnitTestGenerateException/SampleUnitTestPeiGenerateException.inf
>   create mode 100644 UnitTestFrameworkPkg/Test/UnitTest/Sample/SampleUnitTestGenerateException/SampleUnitTestSmmGenerateException.inf
>   create mode 100644 UnitTestFrameworkPkg/Test/UnitTest/Sample/SampleUnitTestGenerateException/SampleUnitTestUefiShellGenerateException.inf
>   create mode 100644 UnitTestFrameworkPkg/Test/UnitTestFrameworkPkgHostTestExpectFail.dsc
> 



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#115403): https://edk2.groups.io/g/devel/message/115403
Mute This Topic: https://groups.io/mt/104336757/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-