From 8b847a42e21ebb249d244db024199e89a0e683ba Mon Sep 17 00:00:00 2001 From: Joey Vagedes Date: Wed, 1 May 2024 09:04:39 -0700 Subject: [PATCH 1/4] Host Based Unit Test updates Updates the host based unit test runner to fail if a unit test executable returns successfully, but has no test results, or if a test suite generated from a unit test executable does not contain any tests. The issues above indicate configuration errors in the unit test source code itself and indicates to the developer that changes to the unit test need to be made. Updates the README.md file for the UnitTestFrameworkPkg to correct inaccurate information regarding Code Coverage, and provide information on how to consolidate and generate unit test html reports. --- .../HostBasedUnitTestRunner.py | 15 ++++++++++- UnitTestFrameworkPkg/ReadMe.md | 25 ++++++++++++++++--- .../UnitTestFrameworkPkg.ci.yaml | 3 ++- 3 files changed, 38 insertions(+), 5 deletions(-) diff --git a/BaseTools/Plugin/HostBasedUnitTestRunner/HostBasedUnitTestRunner.py b/BaseTools/Plugin/HostBasedUnitTestRunner/HostBasedUnitTestRunner.py index 8bd3e37c27..41a634dcdc 100644 --- a/BaseTools/Plugin/HostBasedUnitTestRunner/HostBasedUnitTestRunner.py +++ b/BaseTools/Plugin/HostBasedUnitTestRunner/HostBasedUnitTestRunner.py @@ -117,10 +117,23 @@ def do_post_build(self, thebuilder): os.path.basename(test)) file_match_pattern = test + ".*." + arch + ".result.xml" xml_results_list = glob.glob(file_match_pattern) + # MU_CHANGE [BEGIN] - Check for invalid tests + if len(xml_results_list) == 0: + logging.error(f'{os.path.basename(test)} did not generate any test suites.') + logging.error('Review Code to ensure Test suites are created and tests are registered!') + failure_count += 1 + # MU_CHANGE [END] - Check for invalid tests for xml_result_file in xml_results_list: root = xml.etree.ElementTree.parse( xml_result_file).getroot() for suite in root: + # MU_CHANGE [BEGIN] - Check for invalid tests + if len(suite) == 0: + logging.error(f'TestSuite [{suite.attrib["name"]}] for test {test} did ' + 'not contain any test case.') + logging.error('Review Code to ensure test cases are registered to the suite!') + failure_count += 1 + # MU_CHANGE [END] - Check for invalid tests for case in suite: for result in case: if result.tag == 'failure': @@ -130,7 +143,7 @@ def do_post_build(self, thebuilder): " %s - %s" % (case.attrib['name'], result.text)) failure_count += 1 - if thebuilder.env.GetValue("CODE_COVERAGE") != "FALSE": + if thebuilder.env.GetValue("CODE_COVERAGE", "FALSE") == "TRUE": # MU_CHANGE if thebuilder.env.GetValue("TOOL_CHAIN_TAG") == "GCC5": ret = self.gen_code_coverage_gcc(thebuilder) if ret != 0: diff --git a/UnitTestFrameworkPkg/ReadMe.md b/UnitTestFrameworkPkg/ReadMe.md index dd497617a9..4e05f44223 100644 --- a/UnitTestFrameworkPkg/ReadMe.md +++ b/UnitTestFrameworkPkg/ReadMe.md @@ -1491,6 +1491,18 @@ GTEST_OUTPUT=xml: This mode is used by the test running plugin to aggregate the results for CI test status reporting in the web view. +### XML Reporting Test Consolidation + +There exists multiple tools for consolidating and generating consolidated test results from the +test xml files that are generated. The arguably most convenient tool available is the +`xunit-viewer` node package, installed via `npm install -g xunit-viewer`. This tool can +consolidate all generated xml reports and create an html or cli summary of test results. + +The following command will generate a consolidated report at `.html` and +also print summary overview of the test results to the command line: + +`xunit-viewer --results Build --output .html --console` + ### Code Coverage Code coverage can be enabled for Host based Unit Tests with `CODE_COVERAGE=TRUE`, which generates a cobertura report @@ -1498,10 +1510,17 @@ per package tested, and combined cobertura report for all packages tested. The p present at `Build//HostTest//_coverage.xml`. The overall cobertura report will be present at `Build/coverage.xml` -Code coverage generation has two config knobs: +Code coverage generation has three config knobs. Each can be turned on/off by setting it to TRUE +or FALSE e.g. `CC_REORGANIZE=TRUE`: + +1. `CC_REORGANIZE`: Controls if code coverage results are re-formatted into a "by-inf" folder + structure rather than the default "by-test" folder structure. Default: `TRUE` +1. `CC_FULL`: Generates zero'd out coverage data for untested source files in the package. + Default: `FALSE` +2. `CC_FLATTEN`: Groups all source files together, rather than by INF. Default: `FALSE` -1. `CC_FULL`: If set to `TRUE`, will generate zero'd out coverage data for untested source files in the package. -2. `CC_FLATTEN`: If Set to `TRUE`, will group all source files together, rather than by INF. +** NOTE: `CC_FULL` and `CC_FLATTEN` values only matter if `CC_REORGANIZE=TRUE`, as they only +effect how the coverage report is reorganized. **TIP: `CC_FLATTEN=TRUE/FALSE` will produce different coverage percentage results as `TRUE` de-duplicates source files that are consumed by multiple INFs. diff --git a/UnitTestFrameworkPkg/UnitTestFrameworkPkg.ci.yaml b/UnitTestFrameworkPkg/UnitTestFrameworkPkg.ci.yaml index d17f6e094e..4774c6cb32 100644 --- a/UnitTestFrameworkPkg/UnitTestFrameworkPkg.ci.yaml +++ b/UnitTestFrameworkPkg/UnitTestFrameworkPkg.ci.yaml @@ -115,7 +115,8 @@ "cobertura", # tool for code coverage "pycobertura", # tool for code coverage "loongarch", - "loongson" + "loongson", + "xunit", ], "IgnoreStandardPaths": [], # Standard Plugin defined paths that should be ignore "AdditionalIncludePaths": [] # Additional paths to spell check (wildcards supported) From e6f792e11e770dd42e379c3608f7891e39c5e621 Mon Sep 17 00:00:00 2001 From: Joey Vagedes Date: Thu, 2 May 2024 07:51:50 -0700 Subject: [PATCH 2/4] Update --- .../HostBasedUnitTestRunner.py | 26 +++++++++++-------- UnitTestFrameworkPkg/ReadMe.md | 4 +-- 2 files changed, 17 insertions(+), 13 deletions(-) diff --git a/BaseTools/Plugin/HostBasedUnitTestRunner/HostBasedUnitTestRunner.py b/BaseTools/Plugin/HostBasedUnitTestRunner/HostBasedUnitTestRunner.py index 41a634dcdc..a9f7a353aa 100644 --- a/BaseTools/Plugin/HostBasedUnitTestRunner/HostBasedUnitTestRunner.py +++ b/BaseTools/Plugin/HostBasedUnitTestRunner/HostBasedUnitTestRunner.py @@ -99,6 +99,7 @@ def do_post_build(self, thebuilder): """).strip()) return 0 + error_messages = [] # MU_CHANGE- Check for invalid tests for test in testList: # Configure output name if test uses cmocka. shell_env.set_shell_var( @@ -117,23 +118,22 @@ def do_post_build(self, thebuilder): os.path.basename(test)) file_match_pattern = test + ".*." + arch + ".result.xml" xml_results_list = glob.glob(file_match_pattern) - # MU_CHANGE [BEGIN] - Check for invalid tests - if len(xml_results_list) == 0: - logging.error(f'{os.path.basename(test)} did not generate any test suites.') - logging.error('Review Code to ensure Test suites are created and tests are registered!') - failure_count += 1 - # MU_CHANGE [END] - Check for invalid tests + for xml_result_file in xml_results_list: root = xml.etree.ElementTree.parse( xml_result_file).getroot() + # MU_CHANGE [BEGIN] - Check for invalid tests + if len(root) == 0: + error_messages.append(f'{os.path.basename(test)} did not generate a test suite(s).') + error_messages.append(' Review source code to ensure Test suites are created and tests are registered!') + failure_count += 1 for suite in root: - # MU_CHANGE [BEGIN] - Check for invalid tests if len(suite) == 0: - logging.error(f'TestSuite [{suite.attrib["name"]}] for test {test} did ' - 'not contain any test case.') - logging.error('Review Code to ensure test cases are registered to the suite!') + error_messages.append(f'TestSuite [{suite.attrib["name"]}] for test {test} did ' + 'not contain a test case(s).') + error_messages.append(' Review source code to ensure test cases are registered to the suite!') failure_count += 1 - # MU_CHANGE [END] - Check for invalid tests + # MU_CHANGE [END] - Check for invalid tests for case in suite: for result in case: if result.tag == 'failure': @@ -164,6 +164,10 @@ def do_post_build(self, thebuilder): return -1 # MU_CHANGE end - reformat coverage data + # MU_CHANGE [BEGIN] - Check for invalid tests + for error in error_messages: + logging.error(error) + # MU_CHANGE [END] - Check for invalid tests return failure_count def gen_code_coverage_gcc(self, thebuilder): diff --git a/UnitTestFrameworkPkg/ReadMe.md b/UnitTestFrameworkPkg/ReadMe.md index 4e05f44223..e534e8c6d5 100644 --- a/UnitTestFrameworkPkg/ReadMe.md +++ b/UnitTestFrameworkPkg/ReadMe.md @@ -1515,9 +1515,9 @@ or FALSE e.g. `CC_REORGANIZE=TRUE`: 1. `CC_REORGANIZE`: Controls if code coverage results are re-formatted into a "by-inf" folder structure rather than the default "by-test" folder structure. Default: `TRUE` -1. `CC_FULL`: Generates zero'd out coverage data for untested source files in the package. +1. `CC_FULL`: Generates zero'd out coverage data for untested source files in the package. Default: `FALSE` -2. `CC_FLATTEN`: Groups all source files together, rather than by INF. Default: `FALSE` +1. `CC_FLATTEN`: Groups all source files together, rather than by INF. Default: `FALSE` ** NOTE: `CC_FULL` and `CC_FLATTEN` values only matter if `CC_REORGANIZE=TRUE`, as they only effect how the coverage report is reorganized. From c74b993b2747a32c7c94af93936394e7cd2960c2 Mon Sep 17 00:00:00 2001 From: Joey Vagedes Date: Thu, 2 May 2024 07:54:12 -0700 Subject: [PATCH 3/4] formatting --- .../HostBasedUnitTestRunner/HostBasedUnitTestRunner.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/BaseTools/Plugin/HostBasedUnitTestRunner/HostBasedUnitTestRunner.py b/BaseTools/Plugin/HostBasedUnitTestRunner/HostBasedUnitTestRunner.py index a9f7a353aa..627474e8db 100644 --- a/BaseTools/Plugin/HostBasedUnitTestRunner/HostBasedUnitTestRunner.py +++ b/BaseTools/Plugin/HostBasedUnitTestRunner/HostBasedUnitTestRunner.py @@ -125,13 +125,15 @@ def do_post_build(self, thebuilder): # MU_CHANGE [BEGIN] - Check for invalid tests if len(root) == 0: error_messages.append(f'{os.path.basename(test)} did not generate a test suite(s).') - error_messages.append(' Review source code to ensure Test suites are created and tests are registered!') + error_messages.append(' Review source code to ensure Test suites are created and tests ' + ' are registered!') failure_count += 1 for suite in root: if len(suite) == 0: - error_messages.append(f'TestSuite [{suite.attrib["name"]}] for test {test} did ' - 'not contain a test case(s).') - error_messages.append(' Review source code to ensure test cases are registered to the suite!') + error_messages.append(f'TestSuite [{suite.attrib["name"]}] for test {test} did not ' + 'contain a test case(s).') + error_messages.append(' Review source code to ensure test cases are registered to ' + 'the suite!') failure_count += 1 # MU_CHANGE [END] - Check for invalid tests for case in suite: From 960e4e47c1d79425d3311b8f6db535436a08889e Mon Sep 17 00:00:00 2001 From: Joey Vagedes Date: Mon, 3 Jun 2024 12:36:36 -0700 Subject: [PATCH 4/4] Update BaseTools/Plugin/HostBasedUnitTestRunner/HostBasedUnitTestRunner.py Signed-off-by: Joey Vagedes --- .../Plugin/HostBasedUnitTestRunner/HostBasedUnitTestRunner.py | 1 - 1 file changed, 1 deletion(-) diff --git a/BaseTools/Plugin/HostBasedUnitTestRunner/HostBasedUnitTestRunner.py b/BaseTools/Plugin/HostBasedUnitTestRunner/HostBasedUnitTestRunner.py index 627474e8db..9d70dfbd54 100644 --- a/BaseTools/Plugin/HostBasedUnitTestRunner/HostBasedUnitTestRunner.py +++ b/BaseTools/Plugin/HostBasedUnitTestRunner/HostBasedUnitTestRunner.py @@ -118,7 +118,6 @@ def do_post_build(self, thebuilder): os.path.basename(test)) file_match_pattern = test + ".*." + arch + ".result.xml" xml_results_list = glob.glob(file_match_pattern) - for xml_result_file in xml_results_list: root = xml.etree.ElementTree.parse( xml_result_file).getroot()