diff --git a/scripts/pylib/twister/twisterlib/environment.py b/scripts/pylib/twister/twisterlib/environment.py index 2b99450ee6c..43c05750bf8 100644 --- a/scripts/pylib/twister/twisterlib/environment.py +++ b/scripts/pylib/twister/twisterlib/environment.py @@ -68,8 +68,6 @@ Artificially long but functional example: __/fifo_api/testcase.yaml """) - compare_group_option = parser.add_mutually_exclusive_group() - platform_group_option = parser.add_mutually_exclusive_group() run_group_option = parser.add_mutually_exclusive_group() @@ -84,6 +82,10 @@ Artificially long but functional example: valgrind_asan_group = parser.add_mutually_exclusive_group() + footprint_group = parser.add_argument_group( + title="Memory footprint", + description="Collect and report ROM/RAM size footprint for the test instance images built.") + case_select.add_argument( "-E", "--save-tests", @@ -124,14 +126,6 @@ Artificially long but functional example: case_select.add_argument("--test-tree", action="store_true", help="""Output the test plan in a tree form""") - compare_group_option.add_argument("--compare-report", - help="Use this report file for size comparison") - - compare_group_option.add_argument( - "-m", "--last-metrics", action="store_true", - help="Compare with the results of the previous twister " - "invocation") - platform_group_option.add_argument( "-G", "--integration", @@ -332,20 +326,10 @@ structure in the main Zephyr tree: boards///""") help="Test level to be used. By default, no levels are used for filtering" "and do the selection based on existing filters.") - parser.add_argument( - "-D", "--all-deltas", action="store_true", - help="Show all footprint deltas, positive or negative. Implies " - "--footprint-threshold=0") - parser.add_argument( "--device-serial-baud", action="store", default=None, help="Serial device baud rate (default 115200)") - parser.add_argument( - "--disable-unrecognized-section-test", action="store_true", - default=False, - help="Skip the 'unrecognized section' test.") - parser.add_argument( "--disable-suite-name-check", action="store_true", default=False, help="Disable extended test suite name verification at the beginning " @@ -375,14 +359,6 @@ structure in the main Zephyr tree: boards///""") binaries such as those generated for the native_sim configuration. """) - parser.add_argument("--enable-size-report", action="store_true", - help="Enable expensive computation of RAM/ROM segment sizes.") - - parser.add_argument("--create-rom-ram-report", action="store_true", - help="Generate detailed ram/rom json reports for " - "each build, via cmake build calls with the " - "`--target footprint` argument") - parser.add_argument( "--filter", choices=['buildable', 'runnable'], default='buildable', @@ -403,12 +379,73 @@ structure in the main Zephyr tree: boards///""") help="Path to the gcov tool to use for code coverage " "reports") + footprint_group.add_argument( + "--create-rom-ram-report", + action="store_true", + help="Generate detailed json reports with ROM/RAM symbol sizes for each test image built " + "using additional build option `--target footprint`.") + + footprint_group.add_argument( + "--enable-size-report", + action="store_true", + help="Collect and report ROM/RAM section sizes for each test image built.") + parser.add_argument( - "-H", "--footprint-threshold", type=float, default=5, - help="When checking test case footprint sizes, warn the user if " - "the new app size is greater then the specified percentage " - "from the last release. Default is 5. 0 to warn on any " - "increase on app size.") + "--disable-unrecognized-section-test", + action="store_true", + default=False, + help="Don't error on unrecognized sections in the binary images.") + + footprint_group.add_argument( + "--footprint-from-buildlog", + action = "store_true", + help="Take ROM/RAM sections footprint summary values from the 'build.log' " + "instead of 'objdump' results used otherwise." + "Requires --enable-size-report or one of the baseline comparison modes.") + + compare_group_option = footprint_group.add_mutually_exclusive_group() + + compare_group_option.add_argument( + "-m", "--last-metrics", + action="store_true", + help="Compare footprints to the previous twister invocation as a baseline " + "running in the same output directory. " + "Implies --enable-size-report option.") + + compare_group_option.add_argument( + "--compare-report", + help="Use this report file as a baseline for footprint comparison. " + "The file should be of 'twister.json' schema. " + "Implies --enable-size-report option.") + + footprint_group.add_argument( + "--show-footprint", + action="store_true", + help="With footprint comparison to a baseline, log ROM/RAM section deltas. ") + + footprint_group.add_argument( + "-H", "--footprint-threshold", + type=float, + default=5.0, + help="With footprint comparison to a baseline, " + "warn the user for any of the footprint metric change which is greater or equal " + "to the specified percentage value. " + "Default is %(default)s for %(default)s%% delta from the new footprint value. " + "Use zero to warn on any footprint metric increase.") + + footprint_group.add_argument( + "-D", "--all-deltas", + action="store_true", + help="With footprint comparison to a baseline, " + "warn on any footprint change, increase or decrease. " + "Implies --footprint-threshold=0") + + footprint_group.add_argument( + "-z", "--size", + action="append", + metavar='FILENAME', + help="Ignore all other command line options and just produce a report to " + "stdout with ROM/RAM section sizes on the specified binary images.") parser.add_argument( "-i", "--inline-logs", action="store_true", @@ -608,13 +645,6 @@ structure in the main Zephyr tree: boards///""") "example on Windows OS. This option can be used only with " "'--ninja' argument (to use Ninja build generator).") - parser.add_argument( - "--show-footprint", - action="store_true", - required = "--footprint-from-buildlog" in sys.argv, - help="Show footprint statistics and deltas since last release." - ) - parser.add_argument( "-t", "--tag", action="append", help="Specify tags to restrict which tests to run by tag value. " @@ -695,18 +725,6 @@ structure in the main Zephyr tree: boards///""") directory (testplan.json). """) - parser.add_argument( - "-z", "--size", action="append", - help="Don't run twister. Instead, produce a report to " - "stdout detailing RAM/ROM sizes on the specified filenames. " - "All other command line arguments ignored.") - - parser.add_argument( - "--footprint-from-buildlog", - action = "store_true", - help="Get information about memory footprint from generated build.log. " - "Requires using --show-footprint option.") - parser.add_argument("extra_test_args", nargs=argparse.REMAINDER, help="Additional args following a '--' are passed to the test binary") @@ -755,7 +773,7 @@ def parse_arguments(parser, args, options = None): options.testsuite_root = [os.path.join(ZEPHYR_BASE, "tests"), os.path.join(ZEPHYR_BASE, "samples")] - if options.show_footprint or options.compare_report: + if options.last_metrics or options.compare_report: options.enable_size_report = True if options.aggressive_no_clean: @@ -811,6 +829,10 @@ def parse_arguments(parser, args, options = None): sc.size_report() sys.exit(0) + if options.footprint_from_buildlog and not options.enable_size_report: + logger.error("--footprint-from-buildlog requires --enable-size-report") + sys.exit(1) + if len(options.extra_test_args) > 0: # extra_test_args is a list of CLI args that Twister did not recognize # and are intended to be passed through to the ztest executable. This diff --git a/scripts/pylib/twister/twisterlib/reports.py b/scripts/pylib/twister/twisterlib/reports.py index 001ee7d5092..e03d754ee8c 100644 --- a/scripts/pylib/twister/twisterlib/reports.py +++ b/scripts/pylib/twister/twisterlib/reports.py @@ -404,7 +404,7 @@ class Reporting: logger.debug("running footprint_reports") deltas = self.compare_metrics(report) warnings = 0 - if deltas and show_footprint: + if deltas: for i, metric, value, delta, lower_better in deltas: if not all_deltas and ((delta < 0 and lower_better) or (delta > 0 and not lower_better)): @@ -417,15 +417,19 @@ class Reporting: if not all_deltas and (percentage < (footprint_threshold / 100.0)): continue - logger.info("{:<25} {:<60} {}{}{}: {} {:<+4}, is now {:6} {:+.2%}".format( - i.platform.name, i.testsuite.name, Fore.YELLOW, - "INFO" if all_deltas else "WARNING", Fore.RESET, - metric, delta, value, percentage)) + if show_footprint: + logger.log( + logging.INFO if all_deltas else logging.WARNING, + "{:<25} {:<60} {} {:<+4}, is now {:6} {:+.2%}".format( + i.platform.name, i.testsuite.name, + metric, delta, value, percentage)) + warnings += 1 if warnings: - logger.warning("Deltas based on metrics from last %s" % - ("release" if not last_metrics else "run")) + logger.warning("Found {} footprint deltas to {} as a baseline.".format( + warnings, + (report if not last_metrics else "the last twister run."))) def synopsis(self): cnt = 0 @@ -456,13 +460,13 @@ class Reporting: f"{example_instance.testsuite.source_dir_rel} -T {example_instance.testsuite.id}") logger.info("-+" * 40) - def summary(self, results, unrecognized_sections, duration): + def summary(self, results, ignore_unrecognized_sections, duration): failed = 0 run = 0 for instance in self.instances.values(): if instance.status == "failed": failed += 1 - elif instance.metrics.get("unrecognized") and not unrecognized_sections: + elif not ignore_unrecognized_sections and instance.metrics.get("unrecognized"): logger.error("%sFAILED%s: %s has unrecognized binary sections: %s" % (Fore.RED, Fore.RESET, instance.name, str(instance.metrics.get("unrecognized", [])))) diff --git a/scripts/tests/twister/test_environment.py b/scripts/tests/twister/test_environment.py index 4623fad0bc4..d30f7fdad00 100644 --- a/scripts/tests/twister/test_environment.py +++ b/scripts/tests/twister/test_environment.py @@ -225,7 +225,7 @@ def test_parse_arguments_warnings(caplog): TESTDATA_2 = [ - (['--show-footprint']), + (['--enable-size-report']), (['--compare-report', 'dummy']), ] diff --git a/scripts/tests/twister_blackbox/test_footprint.py b/scripts/tests/twister_blackbox/test_footprint.py index 67aee1ea448..d3a3bea1f4b 100644 --- a/scripts/tests/twister_blackbox/test_footprint.py +++ b/scripts/tests/twister_blackbox/test_footprint.py @@ -12,6 +12,7 @@ import mock import os import pytest import sys +import re from conftest import ZEPHYR_BASE, TEST_DATA, testsuite_filename_mock, clear_log_in_test from twisterlib.testplan import TestPlan @@ -24,11 +25,14 @@ class TestFootprint: # These warnings notify us that deltas were shown in log. # Coupled with the code under test. - DELTA_WARNING_RELEASE = 'Deltas based on metrics from last release' - DELTA_WARNING_RUN = 'Deltas based on metrics from last run' + DELTA_WARNING_COMPARE = re.compile( + r'Found [1-9]+[0-9]* footprint deltas to .*blackbox-out\.[0-9]+/twister.json as a baseline' + ) + DELTA_WARNING_RUN = re.compile(r'Found [1-9]+[0-9]* footprint deltas to the last twister run') # Size report key we modify to control for deltas RAM_KEY = 'used_ram' + DELTA_DETAIL = re.compile(RAM_KEY + r' \+[0-9]+, is now +[0-9]+ \+[0-9.]+%') @classmethod def setup_class(cls): @@ -103,11 +107,11 @@ class TestFootprint: if expect_delta_log: assert self.RAM_KEY in caplog.text - assert self.DELTA_WARNING_RELEASE in caplog.text, \ + assert re.search(self.DELTA_WARNING_COMPARE, caplog.text), \ 'Expected footprint deltas not logged.' else: assert self.RAM_KEY not in caplog.text - assert self.DELTA_WARNING_RELEASE not in caplog.text, \ + assert not re.search(self.DELTA_WARNING_COMPARE, caplog.text), \ 'Unexpected footprint deltas logged.' def test_footprint_from_buildlog(self, out_path): @@ -116,7 +120,7 @@ class TestFootprint: path = os.path.join(TEST_DATA, 'tests', 'dummy', 'device', 'group') args = ['-i', '--outdir', out_path, '-T', path] + \ [] + \ - ['--show-footprint'] + \ + ['--enable-size-report'] + \ [val for pair in zip( ['-p'] * len(test_platforms), test_platforms ) for val in pair] @@ -141,7 +145,7 @@ class TestFootprint: path = os.path.join(TEST_DATA, 'tests', 'dummy', 'device', 'group') args = ['-i', '--outdir', out_path, '-T', path] + \ ['--footprint-from-buildlog'] + \ - ['--show-footprint'] + \ + ['--enable-size-report'] + \ [val for pair in zip( ['-p'] * len(test_platforms), test_platforms ) for val in pair] @@ -229,11 +233,11 @@ class TestFootprint: if expect_delta_log: assert self.RAM_KEY in caplog.text - assert self.DELTA_WARNING_RELEASE in caplog.text, \ + assert re.search(self.DELTA_WARNING_COMPARE, caplog.text), \ 'Expected footprint deltas not logged.' else: assert self.RAM_KEY not in caplog.text - assert self.DELTA_WARNING_RELEASE not in caplog.text, \ + assert not re.search(self.DELTA_WARNING_COMPARE, caplog.text), \ 'Unexpected footprint deltas logged.' @pytest.mark.parametrize( @@ -298,12 +302,16 @@ class TestFootprint: if expect_delta_log: assert self.RAM_KEY in caplog.text - assert self.DELTA_WARNING_RELEASE in caplog.text, \ + assert re.search(self.DELTA_DETAIL, caplog.text), \ + 'Expected footprint delta not logged.' + assert re.search(self.DELTA_WARNING_COMPARE, caplog.text), \ 'Expected footprint deltas not logged.' else: assert self.RAM_KEY not in caplog.text - assert self.DELTA_WARNING_RELEASE not in caplog.text, \ - 'Unexpected footprint deltas logged.' + assert not re.search(self.DELTA_DETAIL, caplog.text), \ + 'Expected footprint delta not logged.' + assert re.search(self.DELTA_WARNING_COMPARE, caplog.text), \ + 'Expected footprint deltas logged.' @pytest.mark.parametrize( 'old_ram_multiplier, expect_delta_log', @@ -367,11 +375,11 @@ class TestFootprint: if expect_delta_log: assert self.RAM_KEY in caplog.text - assert self.DELTA_WARNING_RUN in caplog.text, \ + assert re.search(self.DELTA_WARNING_RUN, caplog.text), \ 'Expected footprint deltas not logged.' else: assert self.RAM_KEY not in caplog.text - assert self.DELTA_WARNING_RUN not in caplog.text, \ + assert not re.search(self.DELTA_WARNING_RUN, caplog.text), \ 'Unexpected footprint deltas logged.' second_logs = caplog.records @@ -464,9 +472,9 @@ class TestFootprint: if expect_delta_log: assert self.RAM_KEY in caplog.text - assert self.DELTA_WARNING_RELEASE in caplog.text, \ + assert re.search(self.DELTA_WARNING_COMPARE, caplog.text), \ 'Expected footprint deltas not logged.' else: assert self.RAM_KEY not in caplog.text - assert self.DELTA_WARNING_RELEASE not in caplog.text, \ + assert not re.search(self.DELTA_WARNING_COMPARE, caplog.text), \ 'Unexpected footprint deltas logged.'