From f60df0f3a04cfc0b985eada1fcadf4d9c364daeb Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Fri, 7 Feb 2025 10:43:14 -0500 Subject: [PATCH 01/36] Add coverage support flags (but no save yet for the flag) --- scripts/build/build/targets.py | 3 +- scripts/tests/local.py | 340 ++++++++++++++++++++------------- 2 files changed, 204 insertions(+), 139 deletions(-) diff --git a/scripts/build/build/targets.py b/scripts/build/build/targets.py index 3611e7372cb3c9..4f74d0fe9e603f 100755 --- a/scripts/build/build/targets.py +++ b/scripts/build/build/targets.py @@ -186,8 +186,7 @@ def BuildHostTarget(): "-clang").ExceptIfRe('-libfuzzer') target.AppendModifier("pw-fuzztest", fuzzing_type=HostFuzzingType.PW_FUZZTEST).OnlyIfRe( "-clang").ExceptIfRe('-(libfuzzer|ossfuzz|asan)') - target.AppendModifier('coverage', use_coverage=True).OnlyIfRe( - '-(chip-tool|all-clusters|tests)') + target.AppendModifier('coverage', use_coverage=True) target.AppendModifier('dmalloc', use_dmalloc=True) target.AppendModifier('clang', use_clang=True) target.AppendModifier('test', extra_tests=True) diff --git a/scripts/tests/local.py b/scripts/tests/local.py index 7321c1b1bead4c..a906d7a5a7a2b5 100755 --- a/scripts/tests/local.py +++ b/scripts/tests/local.py @@ -27,7 +27,7 @@ import textwrap import time from dataclasses import dataclass -from typing import List +from typing import List, Optional import alive_progress import click @@ -36,8 +36,6 @@ import tabulate import yaml -# We compile for the local architecture. Figure out what platform we need - def _get_native_machine_target(): """ @@ -55,6 +53,152 @@ def _get_native_machine_target(): return f"{current_system_info.system.lower()}-{arch}" +def _get_variants(coverage: Optional[bool]): + """ + compute the build variant suffixes for the given options + """ + variants = ["no-ble", "clang", "boringssl"] + + if coverage is None: + # FIXME: figure out previous runs... + pass + + if coverage: + variants.append("coverage") + return "-".join(variants) + + +@dataclass +class ApplicationTarget: + key: str # key for test_env running in python + target: str # target name for build_examples (and directory in out) + binary: str # elf binary to run after it is built + + +def _get_targets(coverage: Optional[bool]) -> list[ApplicationTarget]: + target_prefix = _get_native_machine_target() + suffix = _get_variants(coverage) + + targets = [] + + targets.append( + ApplicationTarget( + key="CHIP_TOOL", + target=f"{target_prefix}-chip-tool-{suffix}", + binary="chip-tool", + ) + ) + targets.append( + ApplicationTarget( + key="ALL_CLUSTERS_APP", + target=f"{target_prefix}-all-clusters-{suffix}", + binary="chip-all-clusters-app", + ) + ) + targets.append( + ApplicationTarget( + key="CHIP_LOCK_APP", + target=f"{target_prefix}-lock-{suffix}", + binary="chip-lock-app", + ) + ) + targets.append( + ApplicationTarget( + key="ENERGY_MANAGEMENT_APP", + target=f"{target_prefix}-energy-management-{suffix}", + binary="chip-energy-management-app", + ) + ) + targets.append( + ApplicationTarget( + key="LIT_ICD_APP", + target=f"{target_prefix}-lit-icd-{suffix}", + binary="lit-icd-app", + ) + ) + targets.append( + ApplicationTarget( + key="CHIP_MICROWAVE_OVEN_APP", + target=f"{target_prefix}-microwave-oven-{suffix}", + binary="chip-microwave-oven-app", + ) + ) + targets.append( + ApplicationTarget( + key="CHIP_RVC_APP", + target=f"{target_prefix}-rvc-{suffix}", + binary="chip-rvc-app", + ) + ) + targets.append( + ApplicationTarget( + key="NETWORK_MANAGEMENT_APP", + target=f"{target_prefix}-network-manager-ipv6only-{suffix}", + binary="matter-network-manager-app", + ) + ) + targets.append( + ApplicationTarget( + key="FABRIC_ADMIN_APP", + target=f"{target_prefix}-fabric-admin-no-wifi-rpc-ipv6only-{suffix}", + binary="fabric-admin", + ) + ) + targets.append( + ApplicationTarget( + key="FABRIC_BRIDGE_APP", + target=f"{target_prefix}-fabric-bridge-no-wifi-rpc-ipv6only-{suffix}", + binary="fabric-bridge-app", + ) + ) + targets.append( + ApplicationTarget( + key="FABRIC_SYNC_APP", + target=f"{target_prefix}-fabric-sync-no-wifi-ipv6only-{suffix}", + binary="fabric-sync", + ) + ) + targets.append( + ApplicationTarget( + key="LIGHTING_APP_NO_UNIQUE_ID", + target=f"{target_prefix}-light-data-model-no-unique-id-ipv6only-no-wifi-{suffix}", + binary="chip-lighting-app", + ) + ) + + # These are needed for chip tool tests + targets.append( + ApplicationTarget( + key="OTA_PROVIDER_APP", + target=f"{target_prefix}-ota-provider-{suffix}", + binary="chip-ota-provider-app", + ) + ) + targets.append( + ApplicationTarget( + key="OTA_REQUESTOR_APP", + target=f"{target_prefix}-ota-requestor-{suffix}", + binary="chip-ota-requestor-app", + ) + ) + targets.append( + ApplicationTarget( + key="TV_APP", + target=f"{target_prefix}-tv-app-{suffix}", + binary="chip-tv-app", + ) + ) + targets.append( + ApplicationTarget( + key="BRIDGE_APP", + target=f"{target_prefix}-bridge-{suffix}", + binary="chip-bridge-app", + ) + ) + + return targets + + class BinaryRunner(enum.Enum): """ Enumeration describing a wrapper runner for an application. Useful for debugging @@ -154,7 +298,7 @@ def _do_build_python(): ) -def _do_build_apps(): +def _do_build_apps(coverage: Optional[bool]): """ Builds example python apps suitable for running all python_tests. @@ -162,25 +306,7 @@ def _do_build_apps(): """ logging.info("Building example apps...") - target_prefix = _get_native_machine_target() - targets = [ - f"{target_prefix}-chip-tool-no-ble-clang-boringssl", - f"{target_prefix}-all-clusters-no-ble-clang-boringssl", - f"{target_prefix}-bridge-no-ble-clang-boringssl", - f"{target_prefix}-energy-management-no-ble-clang-boringssl", - f"{target_prefix}-fabric-admin-no-ble-no-wifi-rpc-ipv6only-clang-boringssl", - f"{target_prefix}-fabric-bridge-no-ble-no-wifi-rpc-ipv6only-clang-boringssl", - f"{target_prefix}-fabric-sync-no-ble-no-wifi-ipv6only-clang-boringssl", - f"{target_prefix}-light-data-model-no-unique-id-ipv6only-no-ble-no-wifi-clang", - f"{target_prefix}-lit-icd-no-ble-clang-boringssl", - f"{target_prefix}-lock-no-ble-clang-boringssl", - f"{target_prefix}-microwave-oven-no-ble-clang-boringssl", - f"{target_prefix}-network-manager-ipv6only-no-ble-clang-boringssl", - f"{target_prefix}-ota-provider-no-ble-clang-boringssl", - f"{target_prefix}-ota-requestor-no-ble-clang-boringssl", - f"{target_prefix}-rvc-no-ble-clang-boringssl", - f"{target_prefix}-tv-app-no-ble-clang-boringssl", - ] + targets = [t.target for t in _get_targets(coverage)] cmd = ["./scripts/build/build_examples.py"] for target in targets: @@ -191,16 +317,17 @@ def _do_build_apps(): subprocess.run(_with_activate(cmd), check=True) -def _do_build_basic_apps(): +def _do_build_basic_apps(coverage: Optional[bool]): """ Builds a minimal subset of test applications, specifically all-clusters and chip-tool only, for basic tests. """ logging.info("Building example apps...") - target_prefix = _get_native_machine_target() + + all_targets = dict([(t.key, t) for t in _get_targets(coverage)]) targets = [ - f"{target_prefix}-chip-tool-no-ble-clang-boringssl", - f"{target_prefix}-all-clusters-no-ble-clang-boringssl", + all_targets["CHIP_TOOL"].target, + all_targets["ALL_CLUSTERS_APP"].target, ] cmd = ["./scripts/build/build_examples.py"] @@ -213,9 +340,10 @@ def _do_build_basic_apps(): @cli.command() -def build_basic_apps(): +@click.option("--coverage/--no-coverage", default=None) +def build_basic_apps(coverage): """Builds chip-tool and all-clusters app.""" - _do_build_basic_apps() + _do_build_basic_apps(coverage) @cli.command() @@ -231,7 +359,8 @@ def build_python(): @cli.command() -def build_apps(): +@click.option("--coverage/--no-coverage", default=None) +def build_apps(coverage): """ Builds MANY apps used by python-tests. @@ -239,18 +368,19 @@ def build_apps(): To re-build the python environment use `build-python`. To re-build both python and apps, use `build` """ - _do_build_apps() + _do_build_apps(coverage) @cli.command() -def build(): +@click.option("--coverage/--no-coverage", default=None) +def build(coverage): """ Builds both python and apps (same as build-python + build-apps) Generally used together with `python-tests`. """ _do_build_python() - _do_build_apps() + _do_build_apps(coverage) def _maybe_with_runner(script_name: str, path: str, runner: BinaryRunner): @@ -337,6 +467,7 @@ def _add_target_to_cmd(cmd, flag, path, runner): help="Save failure logs into the specified directory instead of logging (as logging can be noisy/slow)", type=click.Path(exists=True, file_okay=False, dir_okay=True), ) +@click.option("--coverage/--no-coverage", default=None) @click.option( "--runner", default="none", @@ -351,6 +482,7 @@ def python_tests( no_show_timings, runner, keep_going, + coverage, fail_log_dir, ): """ @@ -365,34 +497,13 @@ def as_runner(path): return _maybe_with_runner(os.path.basename(path), path, runner) # create an env file - target_prefix = _get_native_machine_target() with open("out/test_env.yaml", "wt") as f: - f.write( - textwrap.dedent( - f"""\ - ALL_CLUSTERS_APP: {as_runner(f'out/{target_prefix}-all-clusters-no-ble-clang-boringssl/chip-all-clusters-app')} - CHIP_LOCK_APP: {as_runner(f'out/{target_prefix}-lock-no-ble-clang-boringssl/chip-lock-app')} - ENERGY_MANAGEMENT_APP: { - as_runner(f'out/{target_prefix}-energy-management-no-ble-clang-boringssl/chip-energy-management-app')} - LIT_ICD_APP: {as_runner(f'out/{target_prefix}-lit-icd-no-ble-clang-boringssl/lit-icd-app')} - CHIP_MICROWAVE_OVEN_APP: { - as_runner(f'out/{target_prefix}-microwave-oven-no-ble-clang-boringssl/chip-microwave-oven-app')} - CHIP_RVC_APP: {as_runner(f'out/{target_prefix}-rvc-no-ble-clang-boringssl/chip-rvc-app')} - NETWORK_MANAGEMENT_APP: { - as_runner(f'out/{target_prefix}-network-manager-ipv6only-no-ble-clang-boringssl/matter-network-manager-app')} - FABRIC_ADMIN_APP: { - as_runner(f'out/{target_prefix}-fabric-admin-no-ble-no-wifi-rpc-ipv6only-clang-boringssl/fabric-admin')} - FABRIC_BRIDGE_APP: { - as_runner(f'out/{target_prefix}-fabric-bridge-no-ble-no-wifi-rpc-ipv6only-clang-boringssl/fabric-bridge-app')} - FABRIC_SYNC_APP: { - as_runner(f'out/{target_prefix}-fabric-sync-no-ble-no-wifi-ipv6only-clang-boringssl/fabric-sync')} - LIGHTING_APP_NO_UNIQUE_ID: {as_runner(f'out/{target_prefix}-light-data-model-no-unique-id-ipv6only-no-ble-no-wifi-clang/chip-lighting-app')} - TRACE_APP: out/trace_data/app-{{SCRIPT_BASE_NAME}} - TRACE_TEST_JSON: out/trace_data/test-{{SCRIPT_BASE_NAME}} - TRACE_TEST_PERFETTO: out/trace_data/test-{{SCRIPT_BASE_NAME}} - """ - ) - ) + for target in _get_targets(coverage): + run_path = as_runner(f"out/{target.target}/{target.binary}") + f.write(f"{target.key}: {run_path}\n") + f.write("TRACE_APP: out/trace_data/app-{SCRIPT_BASE_NAME}\n") + f.write("TRACE_TEST_JSON: out/trace_data/test-{SCRIPT_BASE_NAME}") + f.write("TRACE_TEST_PERFETTO: out/trace_data/test-{SCRIPT_BASE_NAME}") if not test_filter.startswith("*"): test_filter = "*" + test_filter @@ -474,13 +585,6 @@ def as_runner(path): base_name, slow_test_duration[base_name], ) - elif base_name == "TC_EEVSE_2_3.py": - # TODO: this should be fixed ... - # for now just note that a `TZ=UTC` makes this pass - logging.warning( - "Test %s is TIMEZONE dependent. Passes with UTC but fails on EST. If this fails set 'TZ=UTC' for running the test.", - base_name, - ) tstart = time.time() result = subprocess.run(cmd, capture_output=True) @@ -546,15 +650,16 @@ def as_runner(path): sys.exit(1) -def _do_build_fabric_sync_apps(): +def _do_build_fabric_sync_apps(coverage: Optional[bool]): """ Build applications used for fabric sync tests """ target_prefix = _get_native_machine_target() + suffix = _get_variants(coverage) targets = [ - f"{target_prefix}-fabric-admin-no-ble-no-wifi-rpc-ipv6only-clang-boringssl", - f"{target_prefix}-fabric-bridge-no-ble-no-wifi-rpc-ipv6only-clang-boringssl", - f"{target_prefix}-all-clusters-boringssl-no-ble", + f"{target_prefix}-fabric-admin-no-wifi-rpc-ipv6only-{suffix}", + f"{target_prefix}-fabric-bridge-no-wifi-rpc-ipv6only-{suffix}", + f"{target_prefix}-all-clusters-{suffix}", ] build_cmd = ["./scripts/build/build_examples.py"] @@ -567,21 +672,23 @@ def _do_build_fabric_sync_apps(): @cli.command() -def build_fabric_sync_apps(): +@click.option("--coverage/--no-coverage", default=None) +def build_fabric_sync_apps(coverage): """ Build fabric synchronizatio applications. """ - _do_build_fabric_sync_apps() + _do_build_fabric_sync_apps(coverage) @cli.command() -def build_fabric_sync(): +@click.option("--coverage/--no-coverage", default=None) +def build_fabric_sync(coverage): """ Builds both python environment and fabric sync applications """ # fabric sync interfaces with python for convenience, so do that _do_build_python() - _do_build_fabric_sync_apps() + _do_build_fabric_sync_apps(coverage) @cli.command() @@ -668,13 +775,16 @@ def casting_test(test, log_directory, tv_app, tv_casting_app, runner): @click.option("--target-glob", default=None) @click.option("--include-tags", default=None) @click.option("--expected-failures", default=None) +@click.option("--coverage/--no-coverage", default=None) @click.option( "--runner", default="none", type=click.Choice(list(__RUNNERS__.keys()), case_sensitive=False), help="Determines the verbosity of script output", ) -def chip_tool_tests(target, target_glob, include_tags, expected_failures, runner): +def chip_tool_tests( + target, target_glob, include_tags, expected_failures, coverage, runner +): """ Run integration tests using chip-tool. @@ -694,13 +804,9 @@ def chip_tool_tests(target, target_glob, include_tags, expected_failures, runner "chip_tool_python", ] - target_prefix = _get_native_machine_target() - cmd.extend( - [ - "--chip-tool", - f"./out/{target_prefix}-chip-tool-no-ble-clang-boringssl/chip-tool", - ] - ) + paths = dict([(t.key, f"./out/{t.target}/{t.binary}") for t in _get_targets(coverage)]) + + cmd.extend(["--chip-tool", paths["CHIP_TOOL"]]) if target is not None: cmd.extend(["--target", target]) @@ -718,60 +824,20 @@ def chip_tool_tests(target, target_glob, include_tags, expected_failures, runner if expected_failures is not None: cmd.extend(["--expected-failures", expected_failures, "--keep-going"]) - _add_target_to_cmd( - cmd, - "--all-clusters-app", - f"./out/{target_prefix}-all-clusters-no-ble-clang-boringssl/chip-all-clusters-app", - runner, - ) - _add_target_to_cmd( - cmd, - "--lock-app", - f"./out/{target_prefix}-lock-no-ble-clang-boringssl/chip-lock-app", - runner, - ) - _add_target_to_cmd( - cmd, - "--ota-provider-app", - f"./out/{target_prefix}-ota-provider-no-ble-clang-boringssl/chip-ota-provider-app", - runner, - ) - _add_target_to_cmd( - cmd, - "--ota-requestor-app", - f"./out/{target_prefix}-ota-requestor-no-ble-clang-boringssl/chip-ota-requestor-app", - runner, - ) - _add_target_to_cmd( - cmd, - "--tv-app", - f"./out/{target_prefix}-tv-app-no-ble-clang-boringssl/chip-tv-app", - runner, - ) - _add_target_to_cmd( - cmd, - "--bridge-app", - f"./out/{target_prefix}-bridge-no-ble-clang-boringssl/chip-bridge-app", - runner, - ) - _add_target_to_cmd( - cmd, - "--lit-icd-app", - f"./out/{target_prefix}-lit-icd-no-ble-clang-boringssl/lit-icd-app", - runner, - ) - _add_target_to_cmd( - cmd, - "--microwave-oven-app", - f"./out/{target_prefix}-microwave-oven-no-ble-clang-boringssl/chip-microwave-oven-app", - runner, - ) - _add_target_to_cmd( - cmd, - "--rvc-app", - f"./out/{target_prefix}-rvc-no-ble-clang-boringssl/chip-rvc-app", - runner, - ) + target_flags = [ + ("--all-clusters-app", "ALL_CLUSTERS_APP"), + ("--lock-app", "CHIP_LOCK_APP"), + ("--ota-provider-app", "OTA_PROVIDER_APP"), + ("--ota-requestor-app", "OTA_REQUESTOR_APP"), + ("--tv-app", "TV_APP"), + ("--bridge-app", "BRIDGE_APP"), + ("--lit-icd-app", "LIT_ICD_APP"), + ("--microwave-oven-app", "CHIP_MICROWAVE_OVEN_APP"), + ("--rvc-app", "CHIP_RVC_APP"), + ] + + for flag, path_key in target_flags: + _add_target_to_cmd(cmd, flag, paths[path_key], runner) subprocess.run(_with_activate(cmd), check=True) From 7b366f5424f76e687ce472f96a7d8fa37e8f4617 Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Fri, 7 Feb 2025 10:53:54 -0500 Subject: [PATCH 02/36] Minor cleanup, save config options when loading variants, so build flags transcend running --- scripts/tests/local.py | 28 +++++++++++++++++++++++++--- 1 file changed, 25 insertions(+), 3 deletions(-) diff --git a/scripts/tests/local.py b/scripts/tests/local.py index a906d7a5a7a2b5..4656a648461680 100755 --- a/scripts/tests/local.py +++ b/scripts/tests/local.py @@ -26,6 +26,7 @@ import sys import textwrap import time +import configparser from dataclasses import dataclass from typing import List, Optional @@ -59,12 +60,31 @@ def _get_variants(coverage: Optional[bool]): """ variants = ["no-ble", "clang", "boringssl"] + config_path = 'out/local_py.ini' + + config = configparser.ConfigParser() + config['OPTIONS'] = {} + try: + config.read(config_path) + logging.info("Defaults read from '%s'", config_path) + except Exception: + config["OPTIONS"]["coverage"] = "true" + if coverage is None: - # FIXME: figure out previous runs... - pass + # Coverage is NOT passed in as an explicit flag, so try to + # resume it from whatever last `build` flag was used + coverage = config["OPTIONS"].getboolean("coverage") + logging.info("Coverage setting not provided via command line. Will use: %s", coverage) if coverage: variants.append("coverage") + config["OPTIONS"]["coverage"] = "true" + else: + config["OPTIONS"]["coverage"] = "false" + + with open(config_path, "w") as f: + config.write(f) + return "-".join(variants) @@ -804,7 +824,9 @@ def chip_tool_tests( "chip_tool_python", ] - paths = dict([(t.key, f"./out/{t.target}/{t.binary}") for t in _get_targets(coverage)]) + paths = dict( + [(t.key, f"./out/{t.target}/{t.binary}") for t in _get_targets(coverage)] + ) cmd.extend(["--chip-tool", paths["CHIP_TOOL"]]) From c9fdf31a1ca623e710255a4586fa472d629e224f Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Fri, 7 Feb 2025 11:19:31 -0500 Subject: [PATCH 03/36] Fix yaml content --- scripts/tests/local.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/scripts/tests/local.py b/scripts/tests/local.py index 4656a648461680..c8dcfb7acb8b4e 100755 --- a/scripts/tests/local.py +++ b/scripts/tests/local.py @@ -522,8 +522,8 @@ def as_runner(path): run_path = as_runner(f"out/{target.target}/{target.binary}") f.write(f"{target.key}: {run_path}\n") f.write("TRACE_APP: out/trace_data/app-{SCRIPT_BASE_NAME}\n") - f.write("TRACE_TEST_JSON: out/trace_data/test-{SCRIPT_BASE_NAME}") - f.write("TRACE_TEST_PERFETTO: out/trace_data/test-{SCRIPT_BASE_NAME}") + f.write("TRACE_TEST_JSON: out/trace_data/test-{SCRIPT_BASE_NAME}\n") + f.write("TRACE_TEST_PERFETTO: out/trace_data/test-{SCRIPT_BASE_NAME}\n") if not test_filter.startswith("*"): test_filter = "*" + test_filter From 678832f83b20af94b185da2df3be760bc3f9b704 Mon Sep 17 00:00:00 2001 From: "Restyled.io" Date: Fri, 7 Feb 2025 16:22:44 +0000 Subject: [PATCH 04/36] Restyled by isort --- scripts/tests/local.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/tests/local.py b/scripts/tests/local.py index c8dcfb7acb8b4e..955dd4d3631fcc 100755 --- a/scripts/tests/local.py +++ b/scripts/tests/local.py @@ -14,6 +14,7 @@ # See the License for the specific language governing permissions and # limitations under the License. +import configparser import enum import fnmatch import glob @@ -26,7 +27,6 @@ import sys import textwrap import time -import configparser from dataclasses import dataclass from typing import List, Optional From 43f94d6878de33aa33dd8d79a843f8d51cb150e0 Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Fri, 7 Feb 2025 12:04:17 -0500 Subject: [PATCH 05/36] Switch coverage to clang, ensure out dir for config exists --- build/config/compiler/BUILD.gn | 11 ++++++++++- scripts/tests/local.py | 4 +++- 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/build/config/compiler/BUILD.gn b/build/config/compiler/BUILD.gn index 1796f02861c18e..6254b2d9a4a227 100644 --- a/build/config/compiler/BUILD.gn +++ b/build/config/compiler/BUILD.gn @@ -536,7 +536,16 @@ config("fuzzing_default") { } config("coverage") { - cflags = [ "--coverage" ] + if (is_clang) { + cflags = [ + # CLANG builds use the clang-specific coverage, so that versions of lcov/gcov + # do not have to match and `llvm-cov export -format=lcov` can be used + "-fprofile-instr-generate", + "-fcoverage-mapping", + ] + } else { + cflags = [ "--coverage" ] + } ldflags = cflags } diff --git a/scripts/tests/local.py b/scripts/tests/local.py index 955dd4d3631fcc..44a49a5002d7bf 100755 --- a/scripts/tests/local.py +++ b/scripts/tests/local.py @@ -82,6 +82,8 @@ def _get_variants(coverage: Optional[bool]): else: config["OPTIONS"]["coverage"] = "false" + if not os.path.exists("./out"): + os.mkdir("./out") with open(config_path, "w") as f: config.write(f) @@ -517,7 +519,7 @@ def as_runner(path): return _maybe_with_runner(os.path.basename(path), path, runner) # create an env file - with open("out/test_env.yaml", "wt") as f: + with open("./out/test_env.yaml", "wt") as f: for target in _get_targets(coverage): run_path = as_runner(f"out/{target.target}/{target.binary}") f.write(f"{target.key}: {run_path}\n") From b839967a68bf86c0f6a8675a2a02f5490902ce4a Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Fri, 7 Feb 2025 12:49:26 -0500 Subject: [PATCH 06/36] Slight doc update, allow local.py to use ccache --- build/config/compiler/BUILD.gn | 2 ++ scripts/tests/local.py | 16 +++++++++++----- 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/build/config/compiler/BUILD.gn b/build/config/compiler/BUILD.gn index 6254b2d9a4a227..909219fcffd24d 100644 --- a/build/config/compiler/BUILD.gn +++ b/build/config/compiler/BUILD.gn @@ -540,6 +540,8 @@ config("coverage") { cflags = [ # CLANG builds use the clang-specific coverage, so that versions of lcov/gcov # do not have to match and `llvm-cov export -format=lcov` can be used + # + # Documentation: https://clang.llvm.org/docs/SourceBasedCodeCoverage.html "-fprofile-instr-generate", "-fcoverage-mapping", ] diff --git a/scripts/tests/local.py b/scripts/tests/local.py index 44a49a5002d7bf..5d24c742e438c3 100755 --- a/scripts/tests/local.py +++ b/scripts/tests/local.py @@ -320,7 +320,7 @@ def _do_build_python(): ) -def _do_build_apps(coverage: Optional[bool]): +def _do_build_apps(coverage: Optional[bool], ccache: bool): """ Builds example python apps suitable for running all python_tests. @@ -334,6 +334,10 @@ def _do_build_apps(coverage: Optional[bool]): for target in targets: cmd.append("--target") cmd.append(target) + + if ccache: + cmd.append("--pw-command-launcher=ccache") + cmd.append("build") subprocess.run(_with_activate(cmd), check=True) @@ -382,7 +386,8 @@ def build_python(): @cli.command() @click.option("--coverage/--no-coverage", default=None) -def build_apps(coverage): +@click.option("--ccache/--no-ccache", default=False) +def build_apps(coverage, ccache): """ Builds MANY apps used by python-tests. @@ -390,19 +395,20 @@ def build_apps(coverage): To re-build the python environment use `build-python`. To re-build both python and apps, use `build` """ - _do_build_apps(coverage) + _do_build_apps(coverage, ccache) @cli.command() @click.option("--coverage/--no-coverage", default=None) -def build(coverage): +@click.option("--ccache/--no-ccache", default=False) +def build(coverage, ccache): """ Builds both python and apps (same as build-python + build-apps) Generally used together with `python-tests`. """ _do_build_python() - _do_build_apps(coverage) + _do_build_apps(coverage, ccache) def _maybe_with_runner(script_name: str, path: str, runner: BinaryRunner): From 9ab32d0837a097a2728ef2e89881e59ea2acee86 Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Fri, 7 Feb 2025 15:13:14 -0500 Subject: [PATCH 07/36] Start adding filter support and comma separation support to local.py --- scripts/tests/local.py | 94 ++++++++++++++++++++++++++++++++++++------ 1 file changed, 81 insertions(+), 13 deletions(-) diff --git a/scripts/tests/local.py b/scripts/tests/local.py index 5d24c742e438c3..b5787824a4126f 100755 --- a/scripts/tests/local.py +++ b/scripts/tests/local.py @@ -54,19 +54,31 @@ def _get_native_machine_target(): return f"{current_system_info.system.lower()}-{arch}" +_CONFIG_PATH = "out/local_py.ini" + + +def get_coverage_default(coverage: Optional[bool]) -> bool: + if coverage is not None: + return coverage + config = configparser.ConfigParser() + try: + config.read(_CONFIG_PATH) + return config["OPTIONS"].getboolean("coverage") + except Exception: + return False + + def _get_variants(coverage: Optional[bool]): """ compute the build variant suffixes for the given options """ variants = ["no-ble", "clang", "boringssl"] - config_path = 'out/local_py.ini' - config = configparser.ConfigParser() - config['OPTIONS'] = {} + config["OPTIONS"] = {} try: - config.read(config_path) - logging.info("Defaults read from '%s'", config_path) + config.read(_CONFIG_PATH) + logging.info("Defaults read from '%s'", _CONFIG_PATH) except Exception: config["OPTIONS"]["coverage"] = "true" @@ -74,7 +86,9 @@ def _get_variants(coverage: Optional[bool]): # Coverage is NOT passed in as an explicit flag, so try to # resume it from whatever last `build` flag was used coverage = config["OPTIONS"].getboolean("coverage") - logging.info("Coverage setting not provided via command line. Will use: %s", coverage) + logging.info( + "Coverage setting not provided via command line. Will use: %s", coverage + ) if coverage: variants.append("coverage") @@ -84,7 +98,7 @@ def _get_variants(coverage: Optional[bool]): if not os.path.exists("./out"): os.mkdir("./out") - with open(config_path, "w") as f: + with open(_CONFIG_PATH, "w") as f: config.write(f) return "-".join(variants) @@ -452,12 +466,54 @@ def _add_target_to_cmd(cmd, flag, path, runner): cmd.append(_maybe_with_runner(flag[2:].replace("-", "_"), path, runner)) +@dataclass +class GlobFilter: + pattern: str + + def matches(self, txt: str) -> bool: + return fnmatch.fnmatch(txt, self.pattern) + + +@dataclass +class FilterList: + filters: list[GlobFilter] + + def any_matches(self, txt: str) -> bool: + return any([f.matches(txt) for f in self.filters]) + + +def _parse_filters(entry: str) -> FilterList: + if not entry: + entry = "*.*" + + if "," in entry: + entry_list = entry.split(",") + else: + entry_list = [entry] + + filters = [] + for f in entry_list: + if not f.startswith("*"): + f = "*" + f + if not f.endswith("*"): + f = f + "*" + filters.append(GlobFilter(pattern=f)) + + return FilterList(filters=filters) + + @cli.command() @click.option( "--test-filter", default="*", show_default=True, - help="Run only tests that match the given glob filter.", + help="Run only tests that match the given glob filter(s). Comma separated list of filters", +) +@click.option( + "--skip", + default="", + show_default=True, + help="Skip the tests matching the given glob. Comma separated list of filters. Empty for no skipping.", ) @click.option( "--from-filter", @@ -504,6 +560,7 @@ def _add_target_to_cmd(cmd, flag, path, runner): ) def python_tests( test_filter, + skip, from_filter, from_skip_filter, dry_run, @@ -533,10 +590,13 @@ def as_runner(path): f.write("TRACE_TEST_JSON: out/trace_data/test-{SCRIPT_BASE_NAME}\n") f.write("TRACE_TEST_PERFETTO: out/trace_data/test-{SCRIPT_BASE_NAME}\n") - if not test_filter.startswith("*"): - test_filter = "*" + test_filter - if not test_filter.endswith("*"): - test_filter = test_filter + "*" + if not test_filter: + test_filter = "*" + test_filter = _parse_filters(test_filter) + + if skip: + print("SKIP IS %r" % skip) + skip = _parse_filters(skip) if from_filter: if not from_filter.startswith("*"): @@ -575,11 +635,14 @@ def as_runner(path): test_scripts.append("src/controller/python/test/test_scripts/mobile-device-test.py") test_scripts.sort() # order consistent + # make sure we are fully aware if running with or without coverage + coverage = get_coverage_default(coverage) + execution_times = [] failed_tests = [] try: to_run = [] - for script in fnmatch.filter(test_scripts, test_filter or "*.*"): + for script in [t for t in test_scripts if test_filter.any_matches(t)]: if from_filter: if not fnmatch.fnmatch(script, from_filter): logging.info("From-filter SKIP %s", script) @@ -591,6 +654,11 @@ def as_runner(path): from_skip_filter = None logging.info("From-skip-filter SKIP %s", script) continue + if skip: + if skip.any_matches(script): + logging.info("EXPLICIT SKIP %s", script) + continue + to_run.append(script) with alive_progress.alive_bar(len(to_run), title="Running tests") as bar: From fffb0daaba7440b54e0e16c6c7b91ea9af1625ac Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Fri, 7 Feb 2025 16:13:46 -0500 Subject: [PATCH 08/36] Fix the runner termination logic --- scripts/tests/local.py | 110 +++++++++++++++++++++++++++++++++++++++-- 1 file changed, 105 insertions(+), 5 deletions(-) diff --git a/scripts/tests/local.py b/scripts/tests/local.py index b5787824a4126f..145879844ffd70 100755 --- a/scripts/tests/local.py +++ b/scripts/tests/local.py @@ -244,6 +244,7 @@ class BinaryRunner(enum.Enum): NONE = enum.auto() RR = enum.auto() VALGRIND = enum.auto() + COVERAGE = enum.auto() def execute_str(self, path: str): if self == BinaryRunner.NONE: @@ -252,6 +253,10 @@ def execute_str(self, path: str): return f"rr record {path}" elif self == BinaryRunner.VALGRIND: return f"valgrind {path}" + elif self == BinaryRunner.COVERAGE: + # Expected path is like "out//" + rawname = path[: path.rindex("/")] + ".profraw" + return f'LLVM_PROFILE_FILE="{rawname}" {path}' __RUNNERS__ = { @@ -434,14 +439,33 @@ def _maybe_with_runner(script_name: str, path: str, runner: BinaryRunner): return path # create a separate runner script based on the app - script_name = f"out/{script_name}.sh" + if not os.path.exists("out/runners"): + os.mkdir("out/runners") + + script_name = f"out/runners/{script_name}.sh" with open(script_name, "wt") as f: f.write( textwrap.dedent( f"""\ #!/usr/bin/env bash - {runner.execute_str(path)} + _term() {{ + kill -TERM "$child" + }} + trap _term SIGTERM + + _int() {{ + kill -INT "$child" + }} + trap _int SIGINT + + {runner.execute_str(path)} $* & + child=$! + wait "$child" + + # TODO: this is awkward, we claim success because otherwise + # we exist with the child exit error code (SIGTERM and such) + exit 0 """ ) ) @@ -502,6 +526,74 @@ def _parse_filters(entry: str) -> FilterList: return FilterList(filters=filters) +@cli.command() +def gen_coverage(): + """ + Generate coverage from tests run with "--coverage" + """ + # This assumes default.profraw exists, so it tries to + # generate coverage out of it + # + # Each target gets its own profile + trace_files = [] + for t in _get_targets(coverage=True): + path = os.path.join("./out", f"{t.target}.profraw") + + if not os.path.exists(path): + logging.warning("No profile file '%s'. Skipping.", path) + continue + + cmd = [ + "llvm-cov", + "export", + "-format=lcov", + "--instr-profile", + path, + os.path.join("./out", t.target, t.binary), + ] + p = subprocess.run(_with_activate(cmd), check=True, capture_output=True) + info_path = os.path.join("./out", f"{t.target}.info") + with open(info_path, "wb") as f: + f.write(p.stdout) + trace_files.append(info_path) + logging.info("Generated %s", info_path) + + if not trace_files: + logging.error( + "Could not find any trace files. Did you run tests with coverage enabled?" + ) + return + + cmd = ["lcov"] + for t in trace_files: + cmd.append("--add-tracefile") + cmd.append(t) + + cmd.append("--output-file") + cmd.append("out/merged.info") + + if os.path.exists("out/merged.info"): + os.unlink("out/merged.info") + + subprocess.run(cmd, check=True) + + logging.info("Generating HTML...") + subprocess.run( + [ + "genhtml", + "--ignore-errors", + "inconsistent", + "--ignore-errors", + "range", + "--output-directory", + "out/coverage", + "out/merged.info", + ], + check=True, + ) + logging.info("Coverage HTML should be available in out/coverage/index.html") + + @cli.command() @click.option( "--test-filter", @@ -578,6 +670,17 @@ def python_tests( """ runner = __RUNNERS__[runner] + # make sure we are fully aware if running with or without coverage + coverage = get_coverage_default(coverage) + if coverage: + if runner != BinaryRunner.NONE: + logging.error("Runner for coverage is implict") + sys.exit(1) + + # wrap around so we get a good LLVM_PROFILE_FILE + runner = BinaryRunner.COVERAGE + pass + def as_runner(path): return _maybe_with_runner(os.path.basename(path), path, runner) @@ -635,9 +738,6 @@ def as_runner(path): test_scripts.append("src/controller/python/test/test_scripts/mobile-device-test.py") test_scripts.sort() # order consistent - # make sure we are fully aware if running with or without coverage - coverage = get_coverage_default(coverage) - execution_times = [] failed_tests = [] try: From 4009d042ef83b7eee4d7343918a0edc5f34302b1 Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Fri, 7 Feb 2025 16:34:28 -0500 Subject: [PATCH 09/36] gen-coverage works --- scripts/tests/local.py | 31 +++++++++++++++++++++++++------ 1 file changed, 25 insertions(+), 6 deletions(-) diff --git a/scripts/tests/local.py b/scripts/tests/local.py index 145879844ffd70..f655d23e14801b 100755 --- a/scripts/tests/local.py +++ b/scripts/tests/local.py @@ -317,15 +317,19 @@ def cli(log_level): ) -def _with_activate(build_cmd: List[str]) -> List[str]: +def _with_activate(build_cmd: List[str], output_path=None) -> List[str]: """ Given a bash command list, will generate a new command suitable for subprocess with an execution of `scripts/activate.sh` prepended to it """ + cmd = shlex.join(build_cmd); + if output_path: + cmd = cmd + f" >{output_path}" + return [ "bash", "-c", - ";".join(["set -e", "source scripts/activate.sh", shlex.join(build_cmd)]), + ";".join(["set -e", "source scripts/activate.sh", cmd]) ] @@ -543,18 +547,27 @@ def gen_coverage(): logging.warning("No profile file '%s'. Skipping.", path) continue + data_path = os.path.join("./out", f"{t.target}.profdata") + cmd = [ + "llvm-profdata", + "merge", + "-sparse", + path, + "-o", + data_path + ] + p = subprocess.run(_with_activate(cmd), check=True, capture_output=True) + cmd = [ "llvm-cov", "export", "-format=lcov", "--instr-profile", - path, + data_path, os.path.join("./out", t.target, t.binary), ] - p = subprocess.run(_with_activate(cmd), check=True, capture_output=True) info_path = os.path.join("./out", f"{t.target}.info") - with open(info_path, "wb") as f: - f.write(p.stdout) + p = subprocess.run(_with_activate(cmd, output_path=info_path), check=True) trace_files.append(info_path) logging.info("Generated %s", info_path) @@ -571,6 +584,12 @@ def gen_coverage(): cmd.append("--output-file") cmd.append("out/merged.info") + cmd.append("--ignore-errors") + cmd.append("inconsistent") + cmd.append("--ignore-errors") + cmd.append("range") + cmd.append("--ignore-errors") + cmd.append("corrupt") if os.path.exists("out/merged.info"): os.unlink("out/merged.info") From a18015d0cd524eabf90162404f366e777577d49c Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Fri, 7 Feb 2025 16:34:55 -0500 Subject: [PATCH 10/36] Restyle --- scripts/tests/local.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/tests/local.py b/scripts/tests/local.py index f655d23e14801b..221bf9608515c2 100755 --- a/scripts/tests/local.py +++ b/scripts/tests/local.py @@ -322,7 +322,7 @@ def _with_activate(build_cmd: List[str], output_path=None) -> List[str]: Given a bash command list, will generate a new command suitable for subprocess with an execution of `scripts/activate.sh` prepended to it """ - cmd = shlex.join(build_cmd); + cmd = shlex.join(build_cmd) if output_path: cmd = cmd + f" >{output_path}" From 4c507c4b3027d5b453a76eed55dae6d939d996bc Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Fri, 7 Feb 2025 19:29:28 -0500 Subject: [PATCH 11/36] make linter happy --- scripts/tests/local.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/tests/local.py b/scripts/tests/local.py index 221bf9608515c2..0ca2b2ddf0cbd8 100755 --- a/scripts/tests/local.py +++ b/scripts/tests/local.py @@ -567,7 +567,7 @@ def gen_coverage(): os.path.join("./out", t.target, t.binary), ] info_path = os.path.join("./out", f"{t.target}.info") - p = subprocess.run(_with_activate(cmd, output_path=info_path), check=True) + subprocess.run(_with_activate(cmd, output_path=info_path), check=True) trace_files.append(info_path) logging.info("Generated %s", info_path) From f4e1946b427f1c8fdafa3a521c596a256e6c07df Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Fri, 7 Feb 2025 19:51:35 -0500 Subject: [PATCH 12/36] Make it clearer what errors we ignore, add more error logic to make this run on more machines --- scripts/tests/local.py | 36 +++++++++++++++++------------------- 1 file changed, 17 insertions(+), 19 deletions(-) diff --git a/scripts/tests/local.py b/scripts/tests/local.py index 0ca2b2ddf0cbd8..9935a22b59fea2 100755 --- a/scripts/tests/local.py +++ b/scripts/tests/local.py @@ -582,14 +582,15 @@ def gen_coverage(): cmd.append("--add-tracefile") cmd.append(t) + errors_to_ignore = [ + "inconsistent", "range", "corrupt", "category" + ] + cmd.append("--output-file") cmd.append("out/merged.info") - cmd.append("--ignore-errors") - cmd.append("inconsistent") - cmd.append("--ignore-errors") - cmd.append("range") - cmd.append("--ignore-errors") - cmd.append("corrupt") + for e in errors_to_ignore: + cmd.append("--ignore-errors") + cmd.append(e) if os.path.exists("out/merged.info"): os.unlink("out/merged.info") @@ -597,19 +598,16 @@ def gen_coverage(): subprocess.run(cmd, check=True) logging.info("Generating HTML...") - subprocess.run( - [ - "genhtml", - "--ignore-errors", - "inconsistent", - "--ignore-errors", - "range", - "--output-directory", - "out/coverage", - "out/merged.info", - ], - check=True, - ) + cmd = [ "genhtml" ] + for e in errors_to_ignore: + cmd.append("--ignore-errors") + cmd.append(e) + + cmd.append("--output-directory") + cmd.append("out/coverage") + cmd.append("out/merged.info") + + subprocess.run(cmd, check=True) logging.info("Coverage HTML should be available in out/coverage/index.html") From f24932cb87e7a2328cc6335bd2ac314c184a345d Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Fri, 7 Feb 2025 19:51:52 -0500 Subject: [PATCH 13/36] Restyle --- scripts/tests/local.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/tests/local.py b/scripts/tests/local.py index 9935a22b59fea2..9f17ca642f5310 100755 --- a/scripts/tests/local.py +++ b/scripts/tests/local.py @@ -598,7 +598,7 @@ def gen_coverage(): subprocess.run(cmd, check=True) logging.info("Generating HTML...") - cmd = [ "genhtml" ] + cmd = ["genhtml"] for e in errors_to_ignore: cmd.append("--ignore-errors") cmd.append(e) From 467647f31c1bb02cb0abf5c9507a29f36dc3c4d5 Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Fri, 7 Feb 2025 19:57:04 -0500 Subject: [PATCH 14/36] Add more ignores for cleaner results --- scripts/tests/local.py | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/scripts/tests/local.py b/scripts/tests/local.py index 9f17ca642f5310..851e3c1eac6847 100755 --- a/scripts/tests/local.py +++ b/scripts/tests/local.py @@ -539,6 +539,16 @@ def gen_coverage(): # generate coverage out of it # # Each target gets its own profile + + #for -ignore-filename-regex + ignore_paths = [ + "third_party/boringssl/.*", + "third_party/perfetto/.*", + "third_party/jsoncpp/.*", + "third_party/nl.*", + "/usr/include/.*", + ] + trace_files = [] for t in _get_targets(coverage=True): path = os.path.join("./out", f"{t.target}.profraw") @@ -566,6 +576,10 @@ def gen_coverage(): data_path, os.path.join("./out", t.target, t.binary), ] + for p in ignore_paths: + cmd.append("-ignore-filename-regex") + cmd.append(p) + info_path = os.path.join("./out", f"{t.target}.info") subprocess.run(_with_activate(cmd, output_path=info_path), check=True) trace_files.append(info_path) From 72e42aba4b596e9c0601e852bc1c7ead48837a45 Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Fri, 7 Feb 2025 19:57:46 -0500 Subject: [PATCH 15/36] Restyle --- scripts/tests/local.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/tests/local.py b/scripts/tests/local.py index 851e3c1eac6847..d9f25308045684 100755 --- a/scripts/tests/local.py +++ b/scripts/tests/local.py @@ -540,7 +540,7 @@ def gen_coverage(): # # Each target gets its own profile - #for -ignore-filename-regex + # for -ignore-filename-regex ignore_paths = [ "third_party/boringssl/.*", "third_party/perfetto/.*", From 7317c8d047f3b1acd47446148465a2e52008281c Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Fri, 7 Feb 2025 19:58:36 -0500 Subject: [PATCH 16/36] One more ignore --- scripts/tests/local.py | 1 + 1 file changed, 1 insertion(+) diff --git a/scripts/tests/local.py b/scripts/tests/local.py index d9f25308045684..0d40d878bcf470 100755 --- a/scripts/tests/local.py +++ b/scripts/tests/local.py @@ -547,6 +547,7 @@ def gen_coverage(): "third_party/jsoncpp/.*", "third_party/nl.*", "/usr/include/.*", + "/usr/lib/.*", ] trace_files = [] From aa6a7e82bb93df5f79eb3dab7eedfd1fdc68eb4b Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Fri, 7 Feb 2025 20:42:57 -0500 Subject: [PATCH 17/36] Better organization, make paths consistent --- scripts/tests/local.py | 46 +++++++++++++++++++++++++++++++++--------- 1 file changed, 36 insertions(+), 10 deletions(-) diff --git a/scripts/tests/local.py b/scripts/tests/local.py index 0d40d878bcf470..60d4bef8e4455b 100755 --- a/scripts/tests/local.py +++ b/scripts/tests/local.py @@ -255,8 +255,8 @@ def execute_str(self, path: str): return f"valgrind {path}" elif self == BinaryRunner.COVERAGE: # Expected path is like "out//" - rawname = path[: path.rindex("/")] + ".profraw" - return f'LLVM_PROFILE_FILE="{rawname}" {path}' + rawname = os.path.basename(path[: path.rindex("/")] + ".profraw") + return f'LLVM_PROFILE_FILE="out/profiling/{rawname}" {path}' __RUNNERS__ = { @@ -552,13 +552,13 @@ def gen_coverage(): trace_files = [] for t in _get_targets(coverage=True): - path = os.path.join("./out", f"{t.target}.profraw") + path = os.path.join("./out", "profiling", f"{t.target}.profraw") if not os.path.exists(path): logging.warning("No profile file '%s'. Skipping.", path) continue - data_path = os.path.join("./out", f"{t.target}.profdata") + data_path = os.path.join("./out", "profiling", f"{t.target}.profdata") cmd = [ "llvm-profdata", "merge", @@ -581,11 +581,34 @@ def gen_coverage(): cmd.append("-ignore-filename-regex") cmd.append(p) - info_path = os.path.join("./out", f"{t.target}.info") + info_path = os.path.join("./out", "profiling", f"{t.target}.info") subprocess.run(_with_activate(cmd, output_path=info_path), check=True) trace_files.append(info_path) logging.info("Generated %s", info_path) + # !!!!! HACK ALERT !!!!! + # + # The paths for our examples are generally including CHIP as + # examples//third_party/connectedhomeip/.... + # So we will replace every occurence of these to remove the extra indirection into third_party + # + # Generally we will replace every path (Shown as SF:...) with the corresponding item + # + # We assume that the info lines fit in RAM + lines = [] + with open(info_path, 'rt') as f: + for line in f.readlines(): + if line.startswith("SF:"): + # This is a source file line: "SF:..." + path = line[3:] + lines.append(f"SF:{os.path.realpath(path)}") + lines.append(line) + + # re-write it. + with open(info_path, 'wt') as f: + f.write("\n".join(lines)) + + if not trace_files: logging.error( "Could not find any trace files. Did you run tests with coverage enabled?" @@ -602,13 +625,13 @@ def gen_coverage(): ] cmd.append("--output-file") - cmd.append("out/merged.info") + cmd.append("out/profiling/merged.info") for e in errors_to_ignore: cmd.append("--ignore-errors") cmd.append(e) - if os.path.exists("out/merged.info"): - os.unlink("out/merged.info") + if os.path.exists("out/profiling/merged.info"): + os.unlink("out/profiling/merged.info") subprocess.run(cmd, check=True) @@ -620,7 +643,7 @@ def gen_coverage(): cmd.append("--output-directory") cmd.append("out/coverage") - cmd.append("out/merged.info") + cmd.append("out/profiling/merged.info") subprocess.run(cmd, check=True) logging.info("Coverage HTML should be available in out/coverage/index.html") @@ -673,7 +696,7 @@ def gen_coverage(): "--fail-log-dir", default=None, help="Save failure logs into the specified directory instead of logging (as logging can be noisy/slow)", - type=click.Path(exists=True, file_okay=False, dir_okay=True), + type=click.Path(file_okay=False, dir_okay=True), ) @click.option("--coverage/--no-coverage", default=None) @click.option( @@ -749,6 +772,9 @@ def as_runner(path): if not os.path.exists("out/trace_data"): os.mkdir("out/trace_data") + if not os.path.exists(fail_log_dir): + os.mkdir(fail_log_dir) + metadata = yaml.full_load(open("src/python_testing/test_metadata.yaml")) excluded_patterns = set([item["name"] for item in metadata["not_automated"]]) From 69dec39849272f9d9e91fb1565a7341118f550b6 Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Fri, 7 Feb 2025 20:53:34 -0500 Subject: [PATCH 18/36] Fix logic. Merged reports now work! --- scripts/tests/local.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/scripts/tests/local.py b/scripts/tests/local.py index 60d4bef8e4455b..0ab889e77fc00b 100755 --- a/scripts/tests/local.py +++ b/scripts/tests/local.py @@ -598,10 +598,11 @@ def gen_coverage(): lines = [] with open(info_path, 'rt') as f: for line in f.readlines(): + line = line.rstrip() if line.startswith("SF:"): # This is a source file line: "SF:..." path = line[3:] - lines.append(f"SF:{os.path.realpath(path)}") + line = f"SF:{os.path.realpath(path)}\n" lines.append(line) # re-write it. From 6c8219e6a343d3f252f3d30c943fe62aacae1aee Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Fri, 7 Feb 2025 20:53:53 -0500 Subject: [PATCH 19/36] Restyle --- scripts/tests/local.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/scripts/tests/local.py b/scripts/tests/local.py index 0ab889e77fc00b..f05e623b827458 100755 --- a/scripts/tests/local.py +++ b/scripts/tests/local.py @@ -607,8 +607,7 @@ def gen_coverage(): # re-write it. with open(info_path, 'wt') as f: - f.write("\n".join(lines)) - + f.write("\n".join(lines)) if not trace_files: logging.error( From 96f1443cc2881a9aedd57a4c37332fec40128e23 Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Sat, 8 Feb 2025 01:59:55 -0500 Subject: [PATCH 20/36] fix stderr logging --- scripts/tests/local.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/tests/local.py b/scripts/tests/local.py index f05e623b827458..fec8cd13dae2bb 100755 --- a/scripts/tests/local.py +++ b/scripts/tests/local.py @@ -856,7 +856,7 @@ def as_runner(path): with open(out_name, "wb") as f: f.write(result.stdout) with open(err_name, "wb") as f: - f.write(result.stdout) + f.write(result.stderr) else: logging.info("STDOUT:\n%s", result.stdout.decode("utf8")) From 9adddfc40620e88c1a46b46602447129ce653d29 Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Sat, 8 Feb 2025 12:28:56 -0500 Subject: [PATCH 21/36] fix fail dir creation --- scripts/tests/local.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/tests/local.py b/scripts/tests/local.py index fec8cd13dae2bb..a0524680625ea2 100755 --- a/scripts/tests/local.py +++ b/scripts/tests/local.py @@ -772,7 +772,7 @@ def as_runner(path): if not os.path.exists("out/trace_data"): os.mkdir("out/trace_data") - if not os.path.exists(fail_log_dir): + if fail_log_dir and not os.path.exists(fail_log_dir): os.mkdir(fail_log_dir) metadata = yaml.full_load(open("src/python_testing/test_metadata.yaml")) From 1d15fed81aee609aa37815b6356e09d41ca53c68 Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Sat, 8 Feb 2025 13:14:38 -0500 Subject: [PATCH 22/36] Make mismatched FIFO pid much easier to investigate --- .../chip/testing/matter_testing.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/python_testing/matter_testing_infrastructure/chip/testing/matter_testing.py b/src/python_testing/matter_testing_infrastructure/chip/testing/matter_testing.py index bac18a7d408414..567d02f03c39df 100644 --- a/src/python_testing/matter_testing_infrastructure/chip/testing/matter_testing.py +++ b/src/python_testing/matter_testing_infrastructure/chip/testing/matter_testing.py @@ -1079,6 +1079,11 @@ def write_to_app_pipe(self, command_dict: dict, app_pipe_name: Optional[str] = N dut_ip = os.getenv('LINUX_DUT_IP') if dut_ip is None: + if not os.path.exists(app_pipe_name): + # Named pipes are unique, so we MUST have consistent PID/paths + # set up for them to work. + logging.error("Named pipe %r does NOT exist" % app_pipe_name) + raise FileNotFoundError("CANNOT FIND %r" % app_pipe_name) with open(app_pipe_name, "w") as app_pipe: app_pipe.write(command + "\n") # TODO(#31239): remove the need for sleep From 4babb192bc7f3850a5eda2d1f7dd32da4bfd729e Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Mon, 10 Feb 2025 09:22:33 -0500 Subject: [PATCH 23/36] Add hierarchical view by default - it seems easier for us to narrow down coverage --- scripts/tests/local.py | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/scripts/tests/local.py b/scripts/tests/local.py index a0524680625ea2..be16b550235a29 100755 --- a/scripts/tests/local.py +++ b/scripts/tests/local.py @@ -531,7 +531,14 @@ def _parse_filters(entry: str) -> FilterList: @cli.command() -def gen_coverage(): +@click.option( + "--flat", + default=False, + is_flag=True, + show_default=True, + help="Use a flat (1-directory) layout rather than hierarchical.", +) +def gen_coverage(flat): """ Generate coverage from tests run with "--coverage" """ @@ -641,6 +648,7 @@ def gen_coverage(): cmd.append("--ignore-errors") cmd.append(e) + cmd.append("--flat" if flat else "--hierarchical") cmd.append("--output-directory") cmd.append("out/coverage") cmd.append("out/profiling/merged.info") From 206efcac9cc9372f17bd6fbf9ee7a4a8a1c54bed Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Mon, 10 Feb 2025 10:20:53 -0500 Subject: [PATCH 24/36] Add coverage support for yaml --- scripts/tests/local.py | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/scripts/tests/local.py b/scripts/tests/local.py index be16b550235a29..37e03f6112148e 100755 --- a/scripts/tests/local.py +++ b/scripts/tests/local.py @@ -742,7 +742,6 @@ def python_tests( # wrap around so we get a good LLVM_PROFILE_FILE runner = BinaryRunner.COVERAGE - pass def as_runner(path): return _maybe_with_runner(os.path.basename(path), path, runner) @@ -1060,6 +1059,16 @@ def chip_tool_tests( # docker run --rm -it -v ~/devel/connectedhomeip:/workspace --privileged ghcr.io/project-chip/chip-build-vscode:64 runner = __RUNNERS__[runner] + # make sure we are fully aware if running with or without coverage + coverage = get_coverage_default(coverage) + if coverage: + if runner != BinaryRunner.NONE: + logging.error("Runner for coverage is implict") + sys.exit(1) + + # wrap around so we get a good LLVM_PROFILE_FILE + runner = BinaryRunner.COVERAGE + cmd = [ "./scripts/tests/run_test_suite.py", "--runner", From dc0ebdc5db938cfbec2f84842ffee4fb47f47811 Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Mon, 10 Feb 2025 10:57:52 -0500 Subject: [PATCH 25/36] Use exec to execute the sub-program --- scripts/tests/local.py | 25 ++++--------------------- 1 file changed, 4 insertions(+), 21 deletions(-) diff --git a/scripts/tests/local.py b/scripts/tests/local.py index 37e03f6112148e..9414b4aca1f1fe 100755 --- a/scripts/tests/local.py +++ b/scripts/tests/local.py @@ -250,13 +250,13 @@ def execute_str(self, path: str): if self == BinaryRunner.NONE: return path elif self == BinaryRunner.RR: - return f"rr record {path}" + return f"exec rr record {path}" elif self == BinaryRunner.VALGRIND: - return f"valgrind {path}" + return f"exec valgrind {path}" elif self == BinaryRunner.COVERAGE: # Expected path is like "out//" rawname = os.path.basename(path[: path.rindex("/")] + ".profraw") - return f'LLVM_PROFILE_FILE="out/profiling/{rawname}" {path}' + return f'export LLVM_PROFILE_FILE="out/profiling/{rawname}"; exec {path}' __RUNNERS__ = { @@ -452,24 +452,7 @@ def _maybe_with_runner(script_name: str, path: str, runner: BinaryRunner): textwrap.dedent( f"""\ #!/usr/bin/env bash - - _term() {{ - kill -TERM "$child" - }} - trap _term SIGTERM - - _int() {{ - kill -INT "$child" - }} - trap _int SIGINT - - {runner.execute_str(path)} $* & - child=$! - wait "$child" - - # TODO: this is awkward, we claim success because otherwise - # we exist with the child exit error code (SIGTERM and such) - exit 0 + {runner.execute_str(path)} $* """ ) ) From bdb8395c5fa961a781db15b6df4ccc3eff1cb907 Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Mon, 10 Feb 2025 11:12:20 -0500 Subject: [PATCH 26/36] Fix up arguments in local.py to support propper quoting --- scripts/tests/local.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/tests/local.py b/scripts/tests/local.py index 9414b4aca1f1fe..013f0f87db6345 100755 --- a/scripts/tests/local.py +++ b/scripts/tests/local.py @@ -452,7 +452,7 @@ def _maybe_with_runner(script_name: str, path: str, runner: BinaryRunner): textwrap.dedent( f"""\ #!/usr/bin/env bash - {runner.execute_str(path)} $* + {runner.execute_str(path)} "$@" """ ) ) From 43d9be2b4666819f6d55abab2cf6904b5bdcf955 Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Mon, 10 Feb 2025 11:32:41 -0500 Subject: [PATCH 27/36] For chip tool tests also run chip-tool with coverage support --- scripts/tests/local.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/scripts/tests/local.py b/scripts/tests/local.py index 013f0f87db6345..30efd5f3a4ae48 100755 --- a/scripts/tests/local.py +++ b/scripts/tests/local.py @@ -1062,7 +1062,11 @@ def chip_tool_tests( [(t.key, f"./out/{t.target}/{t.binary}") for t in _get_targets(coverage)] ) - cmd.extend(["--chip-tool", paths["CHIP_TOOL"]]) + if runner == BinaryRunner.COVERAGE: + # when running with coveage, chip-tool also is covered + cmd.extend(["--chip-tool", _maybe_with_runner("chip-tool", paths["CHIP_TOOL"], runner)]) + else: + cmd.extend(["--chip-tool", paths["CHIP_TOOL"]]) if target is not None: cmd.extend(["--target", target]) From 8ec302f25221cff262a97307c7a814c64b65d547 Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Mon, 10 Feb 2025 12:15:31 -0500 Subject: [PATCH 28/36] Fix the path-variance runs of unit test, so I can run unit tests with coverage --- scripts/tests/chiptest/test_definition.py | 32 ++++++++++++++++++++--- 1 file changed, 29 insertions(+), 3 deletions(-) diff --git a/scripts/tests/chiptest/test_definition.py b/scripts/tests/chiptest/test_definition.py index bdb0680010c5b1..b6f99aec9c887c 100644 --- a/scripts/tests/chiptest/test_definition.py +++ b/scripts/tests/chiptest/test_definition.py @@ -210,6 +210,34 @@ def items(self): self.microwave_oven_app, self.chip_repl_yaml_tester_cmd, self.chip_tool_with_python_cmd, self.rvc_app, self.network_manager_app] + def items_with_key(self): + """ + Returns all path items and also the corresponding "Application Key" which + is the typical application name. + + This is to provider scripts a consistent way to reference a path + """ + return [ + (self.chip_tool, "chip-tool"), + (self.all_clusters_app, "chip-all-clusters-app"), + (self.lock_app, "chip-lock-app"), + (self.fabric_bridge_app, "fabric-bridge-app"), + (self.ota_provider_app, "chip-ota-provider-app"), + (self.ota_requestor_app, "chip-ota-requestor-app"), + (self.tv_app, "chip-tv-app"), + (self.bridge_app, "chip-bridge-app"), + (self.lit_icd_app, "lit-icd-app"), + (self.microwave_oven_app, "chip-microwave-oven-app"), + (self.chip_repl_yaml_tester_cmd, "yamltest_with_chip_repl_tester.py"), + ( + # This path varies, however it is a fixed python tool so it may be ok + self.chip_tool_with_python_cmd, + os.path.basename(self.chip_tool_with_python_cmd[-1]), + ), + (self.rvc_app, "chip-rvc-app"), + (self.network_manager_app, "matter-network-manager-app"), + ] + @dataclass class CaptureLine: @@ -330,7 +358,7 @@ def Run(self, runner, apps_register, paths: ApplicationPaths, pics_file: str, "don't know which application to run") if not dry_run: - for path in paths.items(): + for path, key in paths.items_with_key(): # Do not add chip-tool or chip-repl-yaml-tester-cmd to the register if path == paths.chip_tool or path == paths.chip_repl_yaml_tester_cmd or path == paths.chip_tool_with_python_cmd: continue @@ -345,8 +373,6 @@ def Run(self, runner, apps_register, paths: ApplicationPaths, pics_file: str, # For the app indicated by self.target, give it the 'default' key to add to the register if path == target_app: key = 'default' - else: - key = os.path.basename(path[-1]) app = App(runner, path) # Add the App to the register immediately, so if it fails during From ef5cda666ce33dbda40e6541369f4bb914f204fd Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Mon, 10 Feb 2025 12:17:48 -0500 Subject: [PATCH 29/36] Updated comment a bit --- scripts/tests/chiptest/test_definition.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/scripts/tests/chiptest/test_definition.py b/scripts/tests/chiptest/test_definition.py index b6f99aec9c887c..6531701a1b2f50 100644 --- a/scripts/tests/chiptest/test_definition.py +++ b/scripts/tests/chiptest/test_definition.py @@ -215,7 +215,9 @@ def items_with_key(self): Returns all path items and also the corresponding "Application Key" which is the typical application name. - This is to provider scripts a consistent way to reference a path + This is to provide scripts a consistent way to reference a path, even if + the paths used for individual appplications contain different names + (e.g. they could be wrapper scripts). """ return [ (self.chip_tool, "chip-tool"), From c4b12abfa4e79f2f79c08ab7c2d8972decb70cfe Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Mon, 10 Feb 2025 12:42:52 -0500 Subject: [PATCH 30/36] Increase timeout for test running to cover slow tests, skip slow tests or tests that we know will fail --- scripts/tests/local.py | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/scripts/tests/local.py b/scripts/tests/local.py index 30efd5f3a4ae48..81bb491dc568ef 100755 --- a/scripts/tests/local.py +++ b/scripts/tests/local.py @@ -1058,6 +1058,10 @@ def chip_tool_tests( "chip_tool_python", ] + cmd.extend(["--exclude-tags", "EXTRA_SLOW"]) + cmd.extend(["--exclude-tags", "PURPOSEFUL_FAILURE"]) + cmd.extend(["--exclude-tags", "IN_DEVELOPMENT"]) + paths = dict( [(t.key, f"./out/{t.target}/{t.binary}") for t in _get_targets(coverage)] ) @@ -1079,7 +1083,10 @@ def chip_tool_tests( cmd.append("run") cmd.extend(["--iterations", "1"]) - cmd.extend(["--test-timeout-seconds", "60"]) + + # NOTE: we allow all runs here except extra slow + # This means our timeout is quite large + cmd.extend(["--test-timeout-seconds", "300"]) if expected_failures is not None: cmd.extend(["--expected-failures", expected_failures, "--keep-going"]) From a41b6a15fa08649b178d16692083bc5177c89620 Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Mon, 10 Feb 2025 13:00:01 -0500 Subject: [PATCH 31/36] Also exclude manual tests by default from local runs --- scripts/tests/local.py | 1 + 1 file changed, 1 insertion(+) diff --git a/scripts/tests/local.py b/scripts/tests/local.py index 81bb491dc568ef..a0a5824f99dabb 100755 --- a/scripts/tests/local.py +++ b/scripts/tests/local.py @@ -1061,6 +1061,7 @@ def chip_tool_tests( cmd.extend(["--exclude-tags", "EXTRA_SLOW"]) cmd.extend(["--exclude-tags", "PURPOSEFUL_FAILURE"]) cmd.extend(["--exclude-tags", "IN_DEVELOPMENT"]) + cmd.extend(["--exclude-tags", "MANUAL"]) paths = dict( [(t.key, f"./out/{t.target}/{t.binary}") for t in _get_targets(coverage)] From e905ae06811a8dd9a46f7a19221e8e257564e349 Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Mon, 10 Feb 2025 13:01:07 -0500 Subject: [PATCH 32/36] match default exclusions ... also flaky excluded --- scripts/tests/local.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/scripts/tests/local.py b/scripts/tests/local.py index a0a5824f99dabb..0bf8dcc0969dd4 100755 --- a/scripts/tests/local.py +++ b/scripts/tests/local.py @@ -1058,10 +1058,11 @@ def chip_tool_tests( "chip_tool_python", ] + cmd.extend(["--exclude-tags", "MANUAL"]) + cmd.extend(["--exclude-tags", "IN_DEVELOPMENT"]) + cmd.extend(["--exclude-tags", "FLAKY"]) cmd.extend(["--exclude-tags", "EXTRA_SLOW"]) cmd.extend(["--exclude-tags", "PURPOSEFUL_FAILURE"]) - cmd.extend(["--exclude-tags", "IN_DEVELOPMENT"]) - cmd.extend(["--exclude-tags", "MANUAL"]) paths = dict( [(t.key, f"./out/{t.target}/{t.binary}") for t in _get_targets(coverage)] From 75c81f44ef5c127c8777eb13156485e70553f1b6 Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Mon, 10 Feb 2025 14:10:47 -0500 Subject: [PATCH 33/36] Support keep going for yaml tests, add more ignores --- scripts/tests/local.py | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/scripts/tests/local.py b/scripts/tests/local.py index 0bf8dcc0969dd4..2a84ac7a345b5e 100755 --- a/scripts/tests/local.py +++ b/scripts/tests/local.py @@ -535,6 +535,11 @@ def gen_coverage(flat): "third_party/boringssl/.*", "third_party/perfetto/.*", "third_party/jsoncpp/.*", + "third_party/editline/.*", + "third_party/initpp/.*", + "third_party/libwebsockets/.*", + "third_party/pigweed/.*", + "third_party/nanopb/.*", "third_party/nl.*", "/usr/include/.*", "/usr/lib/.*", @@ -1020,6 +1025,13 @@ def casting_test(test, log_directory, tv_app, tv_casting_app, runner): @click.option("--include-tags", default=None) @click.option("--expected-failures", default=None) @click.option("--coverage/--no-coverage", default=None) +@click.option( + "--keep-going", + default=False, + is_flag=True, + show_default=True, + help="Keep going on errors. Will report all failed tests at the end.", +) @click.option( "--runner", default="none", @@ -1027,7 +1039,7 @@ def casting_test(test, log_directory, tv_app, tv_casting_app, runner): help="Determines the verbosity of script output", ) def chip_tool_tests( - target, target_glob, include_tags, expected_failures, coverage, runner + target, target_glob, include_tags, expected_failures, coverage, keep_going, runner ): """ Run integration tests using chip-tool. @@ -1091,7 +1103,11 @@ def chip_tool_tests( cmd.extend(["--test-timeout-seconds", "300"]) if expected_failures is not None: - cmd.extend(["--expected-failures", expected_failures, "--keep-going"]) + cmd.extend(["--expected-failures", expected_failures]) + keep_going = True # if we expect failures, we have to keep going + + if keep_going: + cmd.append("--keep-going") target_flags = [ ("--all-clusters-app", "ALL_CLUSTERS_APP"), From 9c345d6584ce884ce8678e448f88071252cec0fd Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Mon, 10 Feb 2025 16:21:58 -0500 Subject: [PATCH 34/36] Multiprocessing coverage, coverage now works --- scripts/build/build/targets.py | 2 +- scripts/build/builders/host.py | 5 +- scripts/tests/local.py | 176 +++++++++++++++++++-------------- 3 files changed, 106 insertions(+), 77 deletions(-) diff --git a/scripts/build/build/targets.py b/scripts/build/build/targets.py index 4f74d0fe9e603f..fd7a609cf70a97 100755 --- a/scripts/build/build/targets.py +++ b/scripts/build/build/targets.py @@ -80,7 +80,7 @@ def BuildHostFakeTarget(): target.AppendModifier("pw-fuzztest", fuzzing_type=HostFuzzingType.PW_FUZZTEST).OnlyIfRe( "-clang").ExceptIfRe('-(libfuzzer|ossfuzz|asan)') target.AppendModifier('coverage', use_coverage=True).OnlyIfRe( - '-(chip-tool|all-clusters)') + '-(chip-tool|all-clusters)').ExceptIfRe('-clang') target.AppendModifier('dmalloc', use_dmalloc=True) target.AppendModifier('clang', use_clang=True) diff --git a/scripts/build/builders/host.py b/scripts/build/builders/host.py index 4df8719def5596..04a5f10e12445d 100644 --- a/scripts/build/builders/host.py +++ b/scripts/build/builders/host.py @@ -586,7 +586,7 @@ def generate(self): self._Execute(['mkdir', '-p', self.coverage_dir], title="Create coverage output location") def PreBuildCommand(self): - if self.app == HostApp.TESTS and self.use_coverage: + if self.app == HostApp.TESTS and self.use_coverage and not self.use_clang: cmd = ['ninja', '-C', self.output_dir] if self.ninja_jobs is not None: @@ -604,7 +604,8 @@ def PreBuildCommand(self): '--output-file', os.path.join(self.coverage_dir, 'lcov_base.info')], title="Initial coverage baseline") def PostBuildCommand(self): - if self.app == HostApp.TESTS and self.use_coverage: + # TODO: CLANG coverage is not yet implemented, requires different tooling + if self.app == HostApp.TESTS and self.use_coverage and not self.use_clang: self._Execute(['lcov', '--capture', '--directory', os.path.join(self.output_dir, 'obj'), '--exclude', os.path.join(self.chip_dir, '**/tests/*'), '--exclude', os.path.join(self.chip_dir, 'zzz_generated/*'), diff --git a/scripts/tests/local.py b/scripts/tests/local.py index 2a84ac7a345b5e..8d1334820e7f33 100755 --- a/scripts/tests/local.py +++ b/scripts/tests/local.py @@ -19,6 +19,7 @@ import fnmatch import glob import logging +import multiprocessing import os import platform import shlex @@ -255,7 +256,10 @@ def execute_str(self, path: str): return f"exec valgrind {path}" elif self == BinaryRunner.COVERAGE: # Expected path is like "out//" - rawname = os.path.basename(path[: path.rindex("/")] + ".profraw") + # + # We output up to 10K of separate profiles that will be merged as a + # final step. + rawname = os.path.basename(path[: path.rindex("/")] + "-run-%10000m.profraw") return f'export LLVM_PROFILE_FILE="out/profiling/{rawname}"; exec {path}' @@ -513,22 +517,36 @@ def _parse_filters(entry: str) -> FilterList: return FilterList(filters=filters) -@cli.command() -@click.option( - "--flat", - default=False, - is_flag=True, - show_default=True, - help="Use a flat (1-directory) layout rather than hierarchical.", -) -def gen_coverage(flat): - """ - Generate coverage from tests run with "--coverage" - """ - # This assumes default.profraw exists, so it tries to - # generate coverage out of it - # - # Each target gets its own profile +@dataclass +class RawProfile: + raw_profile_paths: list[str] + binary_path: str + + +def _raw_profile_to_info(profile: RawProfile): + path = profile.raw_profile_paths[0] + + # Merge all per-app profiles into a single large profile + data_path = path[:path.find("-run-")] + ".profdata" + cmd = [ + "llvm-profdata", + "merge", + "-sparse", + ] + cmd.extend(["-o", data_path]) + cmd.extend(profile.raw_profile_paths) + + p = subprocess.run(_with_activate(cmd), check=True, capture_output=True) + + # Export to something lcov likes + cmd = [ + "llvm-cov", + "export", + "-format=lcov", + "--instr-profile", + data_path, + profile.binary_path + ] # for -ignore-filename-regex ignore_paths = [ @@ -545,64 +563,74 @@ def gen_coverage(flat): "/usr/lib/.*", ] - trace_files = [] - for t in _get_targets(coverage=True): - path = os.path.join("./out", "profiling", f"{t.target}.profraw") + for p in ignore_paths: + cmd.append("-ignore-filename-regex") + cmd.append(p) - if not os.path.exists(path): - logging.warning("No profile file '%s'. Skipping.", path) - continue + info_path = path.replace(".profraw", ".info") + subprocess.run(_with_activate(cmd, output_path=info_path), check=True) + logging.info("Generated %s", info_path) - data_path = os.path.join("./out", "profiling", f"{t.target}.profdata") - cmd = [ - "llvm-profdata", - "merge", - "-sparse", - path, - "-o", - data_path - ] - p = subprocess.run(_with_activate(cmd), check=True, capture_output=True) - - cmd = [ - "llvm-cov", - "export", - "-format=lcov", - "--instr-profile", - data_path, - os.path.join("./out", t.target, t.binary), - ] - for p in ignore_paths: - cmd.append("-ignore-filename-regex") - cmd.append(p) - - info_path = os.path.join("./out", "profiling", f"{t.target}.info") - subprocess.run(_with_activate(cmd, output_path=info_path), check=True) - trace_files.append(info_path) - logging.info("Generated %s", info_path) - - # !!!!! HACK ALERT !!!!! - # - # The paths for our examples are generally including CHIP as - # examples//third_party/connectedhomeip/.... - # So we will replace every occurence of these to remove the extra indirection into third_party - # - # Generally we will replace every path (Shown as SF:...) with the corresponding item - # - # We assume that the info lines fit in RAM - lines = [] - with open(info_path, 'rt') as f: - for line in f.readlines(): - line = line.rstrip() - if line.startswith("SF:"): - # This is a source file line: "SF:..." - path = line[3:] - line = f"SF:{os.path.realpath(path)}\n" - lines.append(line) - - # re-write it. - with open(info_path, 'wt') as f: - f.write("\n".join(lines)) + # !!!!! HACK ALERT !!!!! + # + # The paths for our examples are generally including CHIP as + # examples//third_party/connectedhomeip/.... + # So we will replace every occurence of these to remove the extra indirection into third_party + # + # Generally we will replace every path (Shown as SF:...) with the corresponding item + # + # We assume that the info lines fit in RAM + lines = [] + with open(info_path, 'rt') as f: + for line in f.readlines(): + line = line.rstrip() + if line.startswith("SF:"): + # This is a source file line: "SF:..." + path = line[3:] + line = f"SF:{os.path.realpath(path)}\n" + lines.append(line) + + # re-write it. + with open(info_path, 'wt') as f: + f.write("\n".join(lines)) + + return info_path + + +@cli.command() +@click.option( + "--flat", + default=False, + is_flag=True, + show_default=True, + help="Use a flat (1-directory) layout rather than hierarchical.", +) +def gen_coverage(flat): + """ + Generate coverage from tests run with "--coverage" + """ + # This assumes default.profraw exists, so it tries to + # generate coverage out of it + # + # Each target gets its own profile + + raw_profiles = [] + for t in _get_targets(coverage=True): + glob_str = os.path.join("./out", "profiling", f"{t.target}*.profraw") + app_profiles = [] + for path in glob.glob(glob_str): + app_profiles.append(path) + + if app_profiles: + raw_profiles.append(RawProfile( + raw_profile_paths=app_profiles, + binary_path=os.path.join("./out", t.target, t.binary) + )) + else: + logging.warning("No profiles for %s", t.target) + + with multiprocessing.Pool() as p: + trace_files = p.map(_raw_profile_to_info, raw_profiles) if not trace_files: logging.error( @@ -1104,7 +1132,7 @@ def chip_tool_tests( if expected_failures is not None: cmd.extend(["--expected-failures", expected_failures]) - keep_going = True # if we expect failures, we have to keep going + keep_going = True # if we expect failures, we have to keep going if keep_going: cmd.append("--keep-going") From ec5af5900e3ed264d1c3d630c9469888ad8e2b72 Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Mon, 10 Feb 2025 16:27:40 -0500 Subject: [PATCH 35/36] Add missing assignment --- scripts/build/builders/host.py | 1 + 1 file changed, 1 insertion(+) diff --git a/scripts/build/builders/host.py b/scripts/build/builders/host.py index 04a5f10e12445d..e488b52485310a 100644 --- a/scripts/build/builders/host.py +++ b/scripts/build/builders/host.py @@ -411,6 +411,7 @@ def __init__(self, root, runner, app: HostApp, board=HostBoard.NATIVE, if use_coverage: self.extra_gn_options.append('use_coverage=true') + self.use_clang = use_clang # for usage in other commands if use_clang: self.extra_gn_options.append('is_clang=true') From b5d22f2a6cdd589ab30bd2c165bf4244d8eb7191 Mon Sep 17 00:00:00 2001 From: "Restyled.io" Date: Mon, 10 Feb 2025 21:42:57 +0000 Subject: [PATCH 36/36] Restyled by autopep8 --- scripts/build/builders/host.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/build/builders/host.py b/scripts/build/builders/host.py index 9a2fa2af7873b9..8dd1cf3e93dd98 100644 --- a/scripts/build/builders/host.py +++ b/scripts/build/builders/host.py @@ -411,7 +411,7 @@ def __init__(self, root, runner, app: HostApp, board=HostBoard.NATIVE, if use_coverage: self.extra_gn_options.append('use_coverage=true') - self.use_clang = use_clang # for usage in other commands + self.use_clang = use_clang # for usage in other commands if use_clang: self.extra_gn_options.append('is_clang=true')