From cec46f90dce1054b2e0baa98a422a7bb74f0ed4f Mon Sep 17 00:00:00 2001 From: Christian Leithner Date: Wed, 22 Jul 2026 18:19:12 +0000 Subject: [PATCH 1/9] test(integration): add opt-in parallel test execution Run the integration suite in parallel via 'py_test.sh --parallel[=N]' (serial by default so individual/interactive runs keep readable, interleaved logs). CI opts in via run_integration_tests.sh. Default workers is min(nproc/4, 64) -- roughly half the physical cores, since each worker drives a multi-threaded Barton plus a matter.js node process and nproc counts hyperthreads. Supporting changes for concurrent test processes: - matter: bind ephemeral operational/UDC ports under BARTON_CONFIG_MATTER_USE_RANDOM_PORT so concurrent Barton servers don't collide on CHIP_PORT/CHIP_UDC_PORT (5540/5550). - matter: barton patch 0003 raises the CHIP commissioner's discovered-node cache 10 -> 128. On the shared mDNS plane every commissioner sees every device's advertisement, so concurrent commissionings overflowed the 10-slot cache ('Insufficient space') and discovery timed out -- capping reliable parallelism at ~4. With 128 no overflow occurs even at 64 workers. - test env: per-process storage on tmpfs with cleanup on process exit regardless of outcome; sweep the shared compile-time ~/.brtn-ds once at session end. - pin the fixed-port zhal mock tests (18443/8711) to one xdist worker via xdist_group + loadgroup; strip xdist's @group nodeid suffix in the subprocess-per-test runner. - raise commission/ready/resource timeouts (5->30, 10->30) to tolerate load. - py_test.sh: resolve repo root from script location so PYTHONPATH and LD_LIBRARY_PATH point at this worktree, not a stale primary clone. --- core/src/subsystems/matter/Matter.cpp | 9 +++ scripts/ci/run_integration_tests.sh | 2 +- testing/conftest.py | 37 +++++++++++- .../base_environment_orchestrator.py | 31 +++++++--- testing/py_test.sh | 56 ++++++++++++++++++- testing/utils/barton_utils.py | 2 +- ...ionable-node-discovery-cache-from-10.patch | 46 +++++++++++++++ 7 files changed, 171 insertions(+), 12 deletions(-) create mode 100644 third_party/matter/barton-library/patches/0003-Increase-commissionable-node-discovery-cache-from-10.patch diff --git a/core/src/subsystems/matter/Matter.cpp b/core/src/subsystems/matter/Matter.cpp index b23c99b3..4cb8b628 100644 --- a/core/src/subsystems/matter/Matter.cpp +++ b/core/src/subsystems/matter/Matter.cpp @@ -395,6 +395,15 @@ bool Matter::Start() serverInitParams.dataModelProvider = app::CodegenDataModelProviderInstance(&storageDelegate); +#ifdef BARTON_CONFIG_MATTER_USE_RANDOM_PORT + // Let the OS assign the operational (and UDC) ports so multiple Barton + // instances can run concurrently (e.g. parallel integration tests) + // without colliding on the fixed CHIP_PORT/CHIP_UDC_PORT. Mirrors the + // commissioner factoryParams.listenPort = 0 below. + serverInitParams.operationalServicePort = 0; + serverInitParams.userDirectedCommissioningPort = 0; +#endif + if ((err = Server::GetInstance().Init(serverInitParams)) != CHIP_NO_ERROR) { icError("Server::Init failed: %s", err.AsString()); diff --git a/scripts/ci/run_integration_tests.sh b/scripts/ci/run_integration_tests.sh index 246e7486..ebaabe8a 100755 --- a/scripts/ci/run_integration_tests.sh +++ b/scripts/ci/run_integration_tests.sh @@ -55,4 +55,4 @@ echo "End of installation, starting tests" echo "***********************************" echo "" -"$REPO_ROOT/testing/py_test.sh" "$REPO_ROOT/testing" +"$REPO_ROOT/testing/py_test.sh" "$REPO_ROOT/testing" --parallel diff --git a/testing/conftest.py b/testing/conftest.py index 3c649355..3d223046 100644 --- a/testing/conftest.py +++ b/testing/conftest.py @@ -55,6 +55,26 @@ def pytest_configure(config): ) +def pytest_sessionfinish(session, exitstatus): + """Sweep the shared, compile-time Matter config dir once, after everything. + + The Matter SDK's PosixConfig ini files always live at the compile-time + CHIP_BARTON_CONF_DIR (~/.brtn-ds/matter) and cannot be redirected at runtime, + so every test process shares that one directory. Remove it only from the + outer, top-level session -- not the per-test subprocess runs, and not the + xdist workers -- so it is never deleted while another concurrent test is + still using it. Per-process KVS/dynamic storage lives on tmpfs and is cleaned + up by each process itself (see base_environment_orchestrator.py). + """ + if os.environ.get(_SUBPROCESS_MARKER_ENV): + return # inner per-test run + + if hasattr(session.config, "workerinput"): + return # xdist worker; the controller sweeps after all workers finish + + shutil.rmtree(os.path.join(os.path.expanduser("~"), ".brtn-ds"), ignore_errors=True) + + # The following list of plugins are automatically loaded by pytest when running tests. # Any fixtures defined within these modules are automatically available to all test modules. pytest_plugins = [ @@ -106,7 +126,17 @@ def _matterjs_available() -> bool: _has_matterjs = _matterjs_available() +@pytest.hookimpl(tryfirst=True) def pytest_collection_modifyitems(config, items): + # Keep tests that bind the fixed zhal IPC/event ports (18443/8711) on a + # single xdist worker so they never run concurrently and collide on those + # ports. Requires xdist's loadgroup distribution (testing/py_test.sh + # --parallel enables it); harmless when running serially. Runs tryfirst so + # the marker is applied before xdist reads xdist_group during collection. + for item in items: + if "/mocks/test/zhal/" in item.nodeid or "mock_zhal_implementation" in item.fixturenames: + item.add_marker(pytest.mark.xdist_group("zhal")) + if _has_matterjs: return @@ -223,6 +253,11 @@ def _run_in_subprocess(item): junit_fd, junit_path = tempfile.mkstemp(suffix=".xml") os.close(junit_fd) + # xdist's loadgroup scheduling appends "@" to the nodeid (e.g. from an + # xdist_group marker). Strip it so the child pytest receives a real, + # selectable nodeid. Legitimate nodeids never contain "@". + child_nodeid = item.nodeid.split("@", 1)[0] + try: cmd = [ sys.executable, @@ -234,7 +269,7 @@ def _run_in_subprocess(item): "--no-header", "-q", f"--junit-xml={junit_path}", - item.nodeid, + child_nodeid, ] # In CI, py_test.sh preloads libasan via LD_PRELOAD so Python/GI tests can diff --git a/testing/environment/base_environment_orchestrator.py b/testing/environment/base_environment_orchestrator.py index 590b1805..b64eca11 100644 --- a/testing/environment/base_environment_orchestrator.py +++ b/testing/environment/base_environment_orchestrator.py @@ -29,7 +29,8 @@ from abc import ABC, abstractmethod from pathlib import Path from threading import Condition -import shutil +import os +import tempfile # Get the expected gir version dynamically. There is some additional information # about this in the top-level CMakeLists.txt file. @@ -90,6 +91,7 @@ class BaseEnvironmentOrchestrator(ABC): _ready_to_commission: bool _device_added_condition: Condition _commissioned_device: bool + _storage_tmpdir: tempfile.TemporaryDirectory _barton_storage_path: str _matter_storage_path: str _sbmd_dirs: str @@ -103,9 +105,22 @@ def __init__(self): self._device_added_condition = Condition() self._commissioned_device = False - # Must match what's compiled with barton matter sdk - self._barton_storage_path = str(Path.home()) + "/.brtn-ds" - self._matter_storage_path = self._barton_storage_path + "/matter" + # Per-process storage root on tmpfs (/tmp) so concurrent tests + # (pytest-xdist) don't clobber each other's KVS / dynamic storage, and so + # nothing is left behind in the user's home directory. TemporaryDirectory + # registers a finalizer that removes the tree when this process exits -- + # even if a test fails or setup raises before the fixture teardown runs. + # + # NOTE: the Matter SDK's PosixConfig ini files (chip_factory.ini, + # chip_config.ini, chip_counters.ini) still live at the compile-time + # CHIP_BARTON_CONF_DIR (~/.brtn-ds/matter) and are not isolated by this; + # only the KVS honors this runtime path. That shared dir is swept once at + # session end (see testing/conftest.py::pytest_sessionfinish). + self._storage_tmpdir = tempfile.TemporaryDirectory( + prefix="brtn-ds-", ignore_cleanup_errors=True + ) + self._barton_storage_path = self._storage_tmpdir.name + self._matter_storage_path = os.path.join(self._barton_storage_path, "matter") # SBMD specs directories relative to workspace root workspace_root = Path(__file__).parent.parent.parent production_specs = str( @@ -182,7 +197,7 @@ def _on_status_changed(self, _object, statusEvent: BCore.StatusEvent): self._ready_to_commission = True self._ready_for_devices_condition.notify_all() - def wait_for_client_to_be_ready(self, timeout=5): + def wait_for_client_to_be_ready(self, timeout=30): """ Waits for the Barton client to be ready before proceeding with the test. """ @@ -207,7 +222,7 @@ def _on_device_added(self, _object, device: BCore.DeviceAddedEvent): self._commissioned_device = True self._device_added_condition.notify_all() - def wait_for_device_added(self, timeout=5): + def wait_for_device_added(self, timeout=30): """ Waits for a device to be added and commissioned within a specified timeout. @@ -232,7 +247,9 @@ def _cleanup(self): """ self._barton_client.stop() self._barton_client = None - shutil.rmtree(self._barton_storage_path, ignore_errors=True) + # Remove the tmpfs storage now; the TemporaryDirectory finalizer is a + # safety net for paths where this method isn't reached. + self._storage_tmpdir.cleanup() self._ready_for_devices_condition = None self._device_added_condition = None diff --git a/testing/py_test.sh b/testing/py_test.sh index 15ee1eee..6dcb473a 100755 --- a/testing/py_test.sh +++ b/testing/py_test.sh @@ -60,9 +60,14 @@ fi # each child pytest, so this propagates to the isolated test processes too. export PYTHONPATH="$REPO_ROOT${PYTHONPATH:+:$PYTHONPATH}" +# Likewise, prepend this tree's freshly-built libBartonCore so it takes +# precedence over a stale library path inherited from the environment (e.g. a +# primary clone's build/core), so parallel/worktree runs load the right library. +export LD_LIBRARY_PATH="$REPO_ROOT/build/core${LD_LIBRARY_PATH:+:$LD_LIBRARY_PATH}" + function show_help { echo "This is a wrapper script around pytest to ensure the environment is setup correctly." - echo "Usage: $0 [-t=|--toolchain=] [pytest options]" + echo "Usage: $0 [-t=|--toolchain=] [--parallel[=]] [pytest options]" echo "" echo "Options:" echo " -t=, --toolchain=" @@ -70,9 +75,16 @@ function show_help { echo " libBartonCore.so. Determines which ASAN runtime" echo " to preload. If not specified, auto-detects from" echo " the system default 'cc'." + echo " --parallel[=] Run tests in parallel across pytest-xdist" + echo " workers. With no value, defaults to min(CPUs/4, 64)" + echo " -- roughly half the physical cores, since each worker" + echo " drives Barton plus a matter.js node process. Tests run" + echo " SERIALLY unless this flag is given, so interactive/" + echo " individual runs keep readable, interleaved logs." } TOOLCHAIN="" +PARALLEL_WORKERS="" # Parse our options, pass the rest through to pytest PYTEST_ARGS=() @@ -81,6 +93,12 @@ for arg in "$@"; do -t=*|--toolchain=*) TOOLCHAIN="${arg#*=}" ;; + --parallel) + PARALLEL_WORKERS="default" + ;; + --parallel=*) + PARALLEL_WORKERS="${arg#*=}" + ;; -h|--help) show_help exit 0 @@ -91,6 +109,40 @@ for arg in "$@"; do esac done +PARALLEL_ARGS=() +if [[ -n "$PARALLEL_WORKERS" ]]; then + if [[ "$PARALLEL_WORKERS" == "default" ]]; then + # Scale the default worker count with the machine, capped at 64. + # + # History: Matter commissioning discovers each virtual device over the + # shared mDNS plane (port 5353). The CHIP commissioner keeps a fixed + # cache of discovered commissionable nodes; because every commissioner + # sees every device's advertisement, concurrent commissionings used to + # overflow that cache ("Insufficient space") and discovery would time + # out -- which capped reliable parallelism at ~4. That cache was raised + # 10 -> 128 (barton patch 0003), which removes the mDNS ceiling: no + # overflow is seen even at 64 workers. + # + # The remaining limit at very high concurrency is soft and CPU-bound: + # each worker drives a multi-threaded Barton plus a matter.js node + # process (and ASAN), so it realistically needs ~2 physical cores. + # Running one worker per logical CPU oversubscribes and makes crypto-heavy + # CASE commissioning time out. nproc counts logical CPUs (hyperthreads), + # so use a quarter of it -- roughly half the physical cores -- to leave + # headroom on machines of any size. Min 1, capped at 64 (only ~58 tests, + # so more never helps). + PARALLEL_CAP=64 + CPU_COUNT=$(nproc) + PARALLEL_WORKERS=$(( CPU_COUNT / 4 )) + (( PARALLEL_WORKERS < 1 )) && PARALLEL_WORKERS=1 + (( PARALLEL_WORKERS > PARALLEL_CAP )) && PARALLEL_WORKERS=$PARALLEL_CAP + fi + # Use xdist's loadgroup distribution so tests sharing an xdist_group (e.g. the + # zhal mock tests that bind fixed IPC ports 18443/8711) stay on one worker and + # never collide. + PARALLEL_ARGS=(-n "$PARALLEL_WORKERS" --dist loadgroup) +fi + # Determine the correct ASAN runtime to preload based on the compiler that # built libBartonCore.so. Clang uses libclang_rt.asan; GCC uses libasan.so. if [[ -z "$TOOLCHAIN" ]]; then @@ -124,4 +176,4 @@ case "$TOOLCHAIN" in ;; esac -LD_PRELOAD="$ASAN_LIB" pytest "${PYTEST_ARGS[@]}" +LD_PRELOAD="$ASAN_LIB" pytest "${PARALLEL_ARGS[@]}" "${PYTEST_ARGS[@]}" diff --git a/testing/utils/barton_utils.py b/testing/utils/barton_utils.py index 09d959a7..ef40a4cc 100644 --- a/testing/utils/barton_utils.py +++ b/testing/utils/barton_utils.py @@ -98,7 +98,7 @@ def _on_resource_updated(_client, event): return queue -def wait_for_resource_value(queue, expected_value, timeout=10): +def wait_for_resource_value(queue, expected_value, timeout=30): """Drain events from the queue until we get the expected value or time out. This handles spurious initial subscription events that may arrive before diff --git a/third_party/matter/barton-library/patches/0003-Increase-commissionable-node-discovery-cache-from-10.patch b/third_party/matter/barton-library/patches/0003-Increase-commissionable-node-discovery-cache-from-10.patch new file mode 100644 index 00000000..6f5a9496 --- /dev/null +++ b/third_party/matter/barton-library/patches/0003-Increase-commissionable-node-discovery-cache-from-10.patch @@ -0,0 +1,46 @@ +From 511a229b2081344bdb5597158ea1450158d1462c Mon Sep 17 00:00:00 2001 +From: BartonCore +Date: Wed, 22 Jul 2026 17:45:46 +0000 +Subject: [PATCH] Increase commissionable-node discovery cache from 10 to 128 + +The controller keeps a fixed-size cache of discovered commissionable nodes +(kMaxCommissionableNodes / CHIP_DEVICE_CONFIG_MAX_DISCOVERED_NODES = 10). When +many devices advertise on the shared mDNS plane concurrently (e.g. parallel +integration tests), the cache fills with foreign nodes and the target device's +advertisement is dropped ("Insufficient space"), so discovery times out. +Raise the cache to 128 so a commissioner can absorb concurrent advertisements +and still find its target. +--- + src/controller/CHIPDeviceController.h | 2 +- + src/include/platform/CHIPDeviceConfig.h | 2 +- + 2 files changed, 2 insertions(+), 2 deletions(-) + +diff --git a/src/controller/CHIPDeviceController.h b/src/controller/CHIPDeviceController.h +index bd530b87..65199fb7 100644 +--- a/src/controller/CHIPDeviceController.h ++++ b/src/controller/CHIPDeviceController.h +@@ -432,7 +432,7 @@ protected: + FabricTable::AdvertiseIdentity mAdvertiseIdentity = FabricTable::AdvertiseIdentity::Yes; + + // TODO(cecille): Make this configuarable. +- static constexpr int kMaxCommissionableNodes = 10; ++ static constexpr int kMaxCommissionableNodes = 128; + Dnssd::CommissionNodeData mCommissionableNodes[kMaxCommissionableNodes]; + DeviceControllerSystemState * mSystemState = nullptr; + +diff --git a/src/include/platform/CHIPDeviceConfig.h b/src/include/platform/CHIPDeviceConfig.h +index 94c255ce..f6ddddc4 100644 +--- a/src/include/platform/CHIPDeviceConfig.h ++++ b/src/include/platform/CHIPDeviceConfig.h +@@ -851,7 +851,7 @@ static_assert(CHIP_DEVICE_CONFIG_BLE_EXT_ADVERTISING_INTERVAL_MIN <= CHIP_DEVICE + * Maximum number of CHIP Commissioners or Commissionable Nodes that can be discovered + */ + #ifndef CHIP_DEVICE_CONFIG_MAX_DISCOVERED_NODES +-#define CHIP_DEVICE_CONFIG_MAX_DISCOVERED_NODES 10 ++#define CHIP_DEVICE_CONFIG_MAX_DISCOVERED_NODES 128 + #endif + + /** +-- +2.43.0 + From 552fed3941df8c8c47e0a106a89363f3b9aaedab Mon Sep 17 00:00:00 2001 From: Christian Leithner Date: Wed, 22 Jul 2026 18:28:48 +0000 Subject: [PATCH 2/9] build(docker): add pytest-xdist to the builder image for parallel tests The integration suite can now run in parallel via 'py_test.sh --parallel' (pytest-xdist). Install pytest-xdist in the builder image and bump the builder version 2.13 -> 2.14 so a fresh container and CI have the dependency. --- docker/Dockerfile | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/docker/Dockerfile b/docker/Dockerfile index 7ab18db4..76ee28f6 100644 --- a/docker/Dockerfile +++ b/docker/Dockerfile @@ -111,7 +111,8 @@ RUN pip3 install --upgrade --break-system-packages \ pygobject-stubs==2.14.0 \ setuptools==73.0.0 \ python_stdnum \ - pytest + pytest \ + pytest-xdist # pygobject-stubs distributed the stub (pyi) files for standard distribution girs, but we also want its tools so we can make # out own stubs. Those don't come with the pip package. From 30dbd3b7c6e258883f30fe175185e21008918267 Mon Sep 17 00:00:00 2001 From: Christian Leithner Date: Mon, 3 Aug 2026 15:58:06 +0000 Subject: [PATCH 3/9] test(integration): revert timeout increases and drop shared config-dir sweep Per review feedback: - Restore the commission/ready/resource timeouts to their original values (5/5/10); the increases masked failures and should not have changed. - Remove the pytest_sessionfinish sweep of ~/.brtn-ds. That compile-time CHIP config dir may be shared with reference-app runs, so the test suite must not delete it; it is simply reused instead. --- testing/conftest.py | 20 ------------------- .../base_environment_orchestrator.py | 8 ++++---- testing/utils/barton_utils.py | 2 +- 3 files changed, 5 insertions(+), 25 deletions(-) diff --git a/testing/conftest.py b/testing/conftest.py index 3d223046..ee4aa1e7 100644 --- a/testing/conftest.py +++ b/testing/conftest.py @@ -55,26 +55,6 @@ def pytest_configure(config): ) -def pytest_sessionfinish(session, exitstatus): - """Sweep the shared, compile-time Matter config dir once, after everything. - - The Matter SDK's PosixConfig ini files always live at the compile-time - CHIP_BARTON_CONF_DIR (~/.brtn-ds/matter) and cannot be redirected at runtime, - so every test process shares that one directory. Remove it only from the - outer, top-level session -- not the per-test subprocess runs, and not the - xdist workers -- so it is never deleted while another concurrent test is - still using it. Per-process KVS/dynamic storage lives on tmpfs and is cleaned - up by each process itself (see base_environment_orchestrator.py). - """ - if os.environ.get(_SUBPROCESS_MARKER_ENV): - return # inner per-test run - - if hasattr(session.config, "workerinput"): - return # xdist worker; the controller sweeps after all workers finish - - shutil.rmtree(os.path.join(os.path.expanduser("~"), ".brtn-ds"), ignore_errors=True) - - # The following list of plugins are automatically loaded by pytest when running tests. # Any fixtures defined within these modules are automatically available to all test modules. pytest_plugins = [ diff --git a/testing/environment/base_environment_orchestrator.py b/testing/environment/base_environment_orchestrator.py index b64eca11..7412f9c4 100644 --- a/testing/environment/base_environment_orchestrator.py +++ b/testing/environment/base_environment_orchestrator.py @@ -114,8 +114,8 @@ def __init__(self): # NOTE: the Matter SDK's PosixConfig ini files (chip_factory.ini, # chip_config.ini, chip_counters.ini) still live at the compile-time # CHIP_BARTON_CONF_DIR (~/.brtn-ds/matter) and are not isolated by this; - # only the KVS honors this runtime path. That shared dir is swept once at - # session end (see testing/conftest.py::pytest_sessionfinish). + # only the KVS honors this runtime path. That shared dir is left in place + # (it may also be used by reference-app runs) and is simply reused. self._storage_tmpdir = tempfile.TemporaryDirectory( prefix="brtn-ds-", ignore_cleanup_errors=True ) @@ -197,7 +197,7 @@ def _on_status_changed(self, _object, statusEvent: BCore.StatusEvent): self._ready_to_commission = True self._ready_for_devices_condition.notify_all() - def wait_for_client_to_be_ready(self, timeout=30): + def wait_for_client_to_be_ready(self, timeout=5): """ Waits for the Barton client to be ready before proceeding with the test. """ @@ -222,7 +222,7 @@ def _on_device_added(self, _object, device: BCore.DeviceAddedEvent): self._commissioned_device = True self._device_added_condition.notify_all() - def wait_for_device_added(self, timeout=30): + def wait_for_device_added(self, timeout=5): """ Waits for a device to be added and commissioned within a specified timeout. diff --git a/testing/utils/barton_utils.py b/testing/utils/barton_utils.py index ef40a4cc..09d959a7 100644 --- a/testing/utils/barton_utils.py +++ b/testing/utils/barton_utils.py @@ -98,7 +98,7 @@ def _on_resource_updated(_client, event): return queue -def wait_for_resource_value(queue, expected_value, timeout=30): +def wait_for_resource_value(queue, expected_value, timeout=10): """Drain events from the queue until we get the expected value or time out. This handles spurious initial subscription events that may arrive before From 3c452e0a3d5f9dd33e4a8ea97ea1b612f1cb40dc Mon Sep 17 00:00:00 2001 From: Christian Leithner Date: Mon, 3 Aug 2026 15:58:43 +0000 Subject: [PATCH 4/9] build(docker): bump builder version to 2.16 2.15 is reserved for the webrtc work; this parallel-test image needs its own version so the pytest-xdist install is picked up. --- docker/version | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docker/version b/docker/version index 123a39a8..6d28a11d 100644 --- a/docker/version +++ b/docker/version @@ -1 +1 @@ -2.14 +2.16 From 330dabd2dc5de16f605dd5f6d3fabcc29d96908a Mon Sep 17 00:00:00 2001 From: Christian Leithner Date: Mon, 3 Aug 2026 16:03:46 +0000 Subject: [PATCH 5/9] test(integration): delimit per-test captured output on failure Under --parallel several failing tests' captured logs are reported together (and background C-library writes to fd 1/2 can interleave). Wrap each subprocess's dumped output in greppable 'BARTON TEST OUTPUT BEGIN/END []' banners so the failure dump stays attributable and parseable. --- testing/conftest.py | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/testing/conftest.py b/testing/conftest.py index ee4aa1e7..bf196748 100644 --- a/testing/conftest.py +++ b/testing/conftest.py @@ -282,20 +282,21 @@ def _run_in_subprocess(item): ) if outcome in ("failed", "unknown") or result.returncode != 0: - output = result.stdout + result.stderr - lines = output.strip().splitlines() - - if not lines: - excerpt = "" - else: - excerpt = "\n".join(lines) + output = (result.stdout + result.stderr).strip() + excerpt = output or "" + # Wrap the captured child output in clear, greppable banners naming + # the test. Under --parallel (pytest-xdist) several failing tests' + # logs are reported together, so unambiguous per-test separators keep + # the dump parseable (grep for "BARTON TEST OUTPUT"). raise AssertionError( "Subprocess test failed\n" f"exit code: {result.returncode}\n" f"cwd: {item.config.rootpath}\n" f"command: {' '.join(cmd)}\n\n" - f"Captured output:\n{excerpt}" + f"===== BARTON TEST OUTPUT BEGIN [{child_nodeid}] =====\n" + f"{excerpt}\n" + f"===== BARTON TEST OUTPUT END [{child_nodeid}] =====" ) finally: try: From d3a6643c2c87bbd3aa06afd596fdafe8d3f53e24 Mon Sep 17 00:00:00 2001 From: Christian Leithner Date: Mon, 3 Aug 2026 19:12:40 +0000 Subject: [PATCH 6/9] test(integration): make wait timeouts runtime-configurable Add --client-ready-timeout / --device-added-timeout / --resource-value-timeout pytest options (defaults 5/5/30), resolved into testing/utils/timeouts.py in pytest_configure and forwarded into each per-test subprocess so the child running the test sees the same values. The wait helpers (wait_for_client_to_be_ready, wait_for_device_added, wait_for_resource_value) read them at call time. py_test.sh --parallel raises them to 30/30/30 so concurrent commissioning has headroom, while serial/interactive runs keep the fast-failing defaults for quick feedback. --- testing/conftest.py | 38 +++++++++++++++++- .../base_environment_orchestrator.py | 18 +++++++-- testing/py_test.sh | 10 ++++- testing/utils/barton_utils.py | 11 ++++- testing/utils/timeouts.py | 40 +++++++++++++++++++ 5 files changed, 110 insertions(+), 7 deletions(-) create mode 100644 testing/utils/timeouts.py diff --git a/testing/conftest.py b/testing/conftest.py index bf196748..4bfb81e5 100644 --- a/testing/conftest.py +++ b/testing/conftest.py @@ -47,13 +47,43 @@ logger = logging.getLogger(__name__) + +# Runtime-configurable wait timeouts (seconds). Defaults are tuned for serial +# runs; parallel runs raise them (see testing/py_test.sh --parallel) to tolerate +# concurrent-commissioning load. Resolved into testing/utils/timeouts.py in +# pytest_configure and forwarded to each per-test subprocess (see +# _run_in_subprocess) so the child that actually runs the test sees them. +_TIMEOUT_OPTIONS = ( + ("--client-ready-timeout", "client_ready", 5, "wait for the Barton client to be ready"), + ("--device-added-timeout", "device_added", 5, "wait for a device to be commissioned/added"), + ("--resource-value-timeout", "resource_value", 30, "wait for an expected resource value"), +) + + +def pytest_addoption(parser): + group = parser.getgroup("barton", "Barton integration test timeouts") + for flag, dest, default, desc in _TIMEOUT_OPTIONS: + group.addoption( + flag, + dest=dest, + type=int, + default=default, + help=f"Seconds to {desc} (default: {default}).", + ) + + def pytest_configure(config): - """Register custom markers.""" + """Register custom markers and resolve the configurable wait timeouts.""" config.addinivalue_line( "markers", "requires_matterjs: skip test when Node.js or matter.js is not available", ) + from testing.utils import timeouts + + for _flag, dest, _default, _desc in _TIMEOUT_OPTIONS: + setattr(timeouts, dest, config.getoption(dest)) + # The following list of plugins are automatically loaded by pytest when running tests. # Any fixtures defined within these modules are automatically available to all test modules. @@ -249,6 +279,12 @@ def _run_in_subprocess(item): "--no-header", "-q", f"--junit-xml={junit_path}", + # Forward the resolved wait timeouts so the child running the test + # sees the same values as this (possibly parallel) outer session. + *( + f"{flag}={item.config.getoption(dest)}" + for flag, dest, _default, _desc in _TIMEOUT_OPTIONS + ), child_nodeid, ] diff --git a/testing/environment/base_environment_orchestrator.py b/testing/environment/base_environment_orchestrator.py index 7412f9c4..fa602cd6 100644 --- a/testing/environment/base_environment_orchestrator.py +++ b/testing/environment/base_environment_orchestrator.py @@ -59,6 +59,7 @@ from gi.repository import BCore from testing.credentials import network_credentials_provider +from testing.utils import timeouts class BaseEnvironmentOrchestrator(ABC): """ @@ -197,10 +198,16 @@ def _on_status_changed(self, _object, statusEvent: BCore.StatusEvent): self._ready_to_commission = True self._ready_for_devices_condition.notify_all() - def wait_for_client_to_be_ready(self, timeout=5): + def wait_for_client_to_be_ready(self, timeout=None): """ Waits for the Barton client to be ready before proceeding with the test. + + When timeout is None the runtime-configurable default is used + (timeouts.client_ready; see testing/utils/timeouts.py). """ + if timeout is None: + timeout = timeouts.client_ready + with self._ready_for_devices_condition: if not self._ready_to_commission: self._ready_for_devices_condition.wait(timeout=timeout) @@ -222,17 +229,22 @@ def _on_device_added(self, _object, device: BCore.DeviceAddedEvent): self._commissioned_device = True self._device_added_condition.notify_all() - def wait_for_device_added(self, timeout=5): + def wait_for_device_added(self, timeout=None): """ Waits for a device to be added and commissioned within a specified timeout. Args: timeout (int, optional): The maximum time to wait for the device to be - commissioned, in seconds. Defaults to 5. + commissioned, in seconds. When None the runtime-configurable + default is used (timeouts.device_added; see + testing/utils/timeouts.py). Raises: AssertionError: If the device is not commissioned within the specified timeout. """ + if timeout is None: + timeout = timeouts.device_added + with self._device_added_condition: if not self._commissioned_device: self._device_added_condition.wait(timeout=timeout) diff --git a/testing/py_test.sh b/testing/py_test.sh index 6dcb473a..4d63ca83 100755 --- a/testing/py_test.sh +++ b/testing/py_test.sh @@ -140,7 +140,15 @@ if [[ -n "$PARALLEL_WORKERS" ]]; then # Use xdist's loadgroup distribution so tests sharing an xdist_group (e.g. the # zhal mock tests that bind fixed IPC ports 18443/8711) stay on one worker and # never collide. - PARALLEL_ARGS=(-n "$PARALLEL_WORKERS" --dist loadgroup) + # + # Raise the commissioning wait timeouts: under concurrent load the crypto-heavy + # CASE/commissioning phase legitimately takes longer than the (fast-failure) + # serial defaults, so give it headroom. These override testing/conftest.py's + # defaults and are forwarded into each per-test subprocess. + PARALLEL_ARGS=( + -n "$PARALLEL_WORKERS" --dist loadgroup + --client-ready-timeout=30 --device-added-timeout=30 --resource-value-timeout=30 + ) fi # Determine the correct ASAN runtime to preload based on the compiler that diff --git a/testing/utils/barton_utils.py b/testing/utils/barton_utils.py index 09d959a7..165a935c 100644 --- a/testing/utils/barton_utils.py +++ b/testing/utils/barton_utils.py @@ -26,6 +26,8 @@ from gi.repository import BCore +from testing.utils import timeouts + def commission_device(environment, device, device_class): """Commission a device and return the most recently added device of the expected class. @@ -98,12 +100,17 @@ def _on_resource_updated(_client, event): return queue -def wait_for_resource_value(queue, expected_value, timeout=10): +def wait_for_resource_value(queue, expected_value, timeout=None): """Drain events from the queue until we get the expected value or time out. This handles spurious initial subscription events that may arrive before - the event triggered by the test action. + the event triggered by the test action. When timeout is None the + runtime-configurable default is used (timeouts.resource_value; see + testing/utils/timeouts.py). """ + if timeout is None: + timeout = timeouts.resource_value + deadline = time.monotonic() + timeout while True: diff --git a/testing/utils/timeouts.py b/testing/utils/timeouts.py new file mode 100644 index 00000000..c9d64f3b --- /dev/null +++ b/testing/utils/timeouts.py @@ -0,0 +1,40 @@ +# ------------------------------ tabstop = 4 ---------------------------------- +# +# If not stated otherwise in this file or this component's LICENSE file the +# following copyright and licenses apply: +# +# Copyright 2026 Comcast Cable Communications Management, LLC +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# +# SPDX-License-Identifier: Apache-2.0 +# +# ------------------------------ tabstop = 4 ---------------------------------- + +"""Runtime-configurable timeouts (seconds) for integration-test waits. + +The defaults are tuned for serial runs. They are overridable at pytest +invocation via the --client-ready-timeout / --device-added-timeout / +--resource-value-timeout options (registered in testing/conftest.py), which +lets parallel runs raise them to tolerate concurrent-commissioning load. + +conftest's pytest_configure resolves the options into these module attributes, +and the wait helpers read them at call time -- so always access them as +``timeouts.`` (do not ``from ... import ``, which would capture the +default at import time). +""" + +# Defaults (seconds), tuned for serial runs. Overridden in pytest_configure. +client_ready = 5 +device_added = 5 +resource_value = 30 From cf50fe68d581c2248e21b9d418bffd49cbe1d445 Mon Sep 17 00:00:00 2001 From: Christian Leithner Date: Tue, 4 Aug 2026 15:06:29 +0000 Subject: [PATCH 7/9] test(matter): commission via QR code to match the exact device Under parallel test execution, commissioning intermittently failed because the harness commissioned via the manual pairing code, which carries only a 4-bit short discriminator. CHIP's SetUpCodePairer then discovers with a kShortDiscriminator filter, matching any device sharing that short discriminator. With many devices advertising concurrently (only 16 short buckets), the commissioner frequently matched the wrong device and PASE'd to its port, which never completes -> PASE/discovery timeout. Add generate_qr_code() (full 12-bit discriminator, Base38-encoded MT: payload) alongside the existing manual pairing code logic, and commission via the QR code so the commissioner matches the exact device. Barton's ParseSetupPayload auto-detects the MT: prefix and uses the long-discriminator parser. At 64 workers the full suite goes from 15 failed / 47 passed to 62 passed. --- testing/helpers/matter/code_generators.py | 86 +++++++++++++++++++ testing/mocks/devices/matter/matter_device.py | 23 +++++ testing/test/ikea_timmerflotte_test.py | 2 +- testing/utils/barton_utils.py | 8 +- 4 files changed, 117 insertions(+), 2 deletions(-) diff --git a/testing/helpers/matter/code_generators.py b/testing/helpers/matter/code_generators.py index 94fce947..3b0a61cf 100644 --- a/testing/helpers/matter/code_generators.py +++ b/testing/helpers/matter/code_generators.py @@ -28,6 +28,92 @@ import random from stdnum.verhoeff import calc_check_digit +# Base38 alphabet used by the Matter QR code payload encoding (section 5.1.3.1 +# of the Matter Core Specification). +_BASE38_CHARS = "0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ-." + +# Discovery capability bitmask values (section 5.1.3.1, "Discovery Capabilities +# Bitmask"). kOnNetwork (bit 2) indicates the device is discoverable over +# DNS-SD on the operational/commissionable IP network. +DISCOVERY_CAPABILITY_ON_NETWORK = 0b100 + + +def _base38_encode(data: bytes) -> str: + """ + Encodes a byte string using the Matter Base38 scheme (section 5.1.3.1 of + the Matter Core Specification). Bytes are consumed in groups of 3 (emitting + 5 characters), 2 (4 characters), or 1 (2 characters), each group written + least-significant character first. + """ + result = [] + i = 0 + n = len(data) + + while i < n: + remaining = n - i + + if remaining >= 3: + value = data[i] | (data[i + 1] << 8) | (data[i + 2] << 16) + chars = 5 + i += 3 + elif remaining == 2: + value = data[i] | (data[i + 1] << 8) + chars = 4 + i += 2 + else: + value = data[i] + chars = 2 + i += 1 + + for _ in range(chars): + result.append(_BASE38_CHARS[value % 38]) + value //= 38 + + return "".join(result) + + +def generate_qr_code( + discriminator: int, + passcode: int, + vendor_id: int = 0, + product_id: int = 0, + discovery_capabilities: int = DISCOVERY_CAPABILITY_ON_NETWORK, + commissioning_flow: int = 0, + version: int = 0, +) -> str: + """ + Generates a Matter QR code setup payload ("MT:..." string) according to + section 5.1.3 of the Matter Core Specification. + + Unlike the manual pairing code, the QR payload carries the full 12-bit + discriminator. A commissioner therefore matches the exact device rather + than any device that happens to share the same 4-bit short discriminator, + which is essential when many devices advertise concurrently. + """ + # Fields are packed least-significant-bit first, in payload order (section + # 5.1.3.1, Table 38). Total width is 88 bits (11 bytes) including padding. + fields = [ + (version, 3), + (vendor_id, 16), + (product_id, 16), + (commissioning_flow, 2), + (discovery_capabilities, 8), + (discriminator, 12), + (passcode, 27), + (0, 4), # padding + ] + + bit_buffer = 0 + bit_count = 0 + + for value, length in fields: + bit_buffer |= (value & ((1 << length) - 1)) << bit_count + bit_count += length + + data = bit_buffer.to_bytes((bit_count + 7) // 8, byteorder="little") + + return "MT:" + _base38_encode(data) + def generate_manual_pairing_code( discriminator: int, diff --git a/testing/mocks/devices/matter/matter_device.py b/testing/mocks/devices/matter/matter_device.py index 61bf2286..67d21fc7 100644 --- a/testing/mocks/devices/matter/matter_device.py +++ b/testing/mocks/devices/matter/matter_device.py @@ -69,6 +69,7 @@ class MatterDevice(BaseDevice): _vendor_id: int _product_id: int _commissioning_code: str + _qr_code: str _process: subprocess.Popen _sideband: SidebandClient | None @@ -86,6 +87,7 @@ def __init__( self._passcode = self._set_passcode() self._discriminator = self._set_discriminator() self._commissioning_code = self._set_commissioning_code() + self._qr_code = self._set_qr_code() self._process = None self._sideband = None @@ -115,12 +117,33 @@ def _set_commissioning_code(self) -> str: discriminator=self._discriminator, passcode=self._passcode ) + def _set_qr_code(self) -> str: + """ + Generates and sets the QR code setup payload for the device. + + The QR payload carries the full 12-bit discriminator, so a commissioner + matches this exact device rather than any device that shares the same + 4-bit short discriminator carried by the manual pairing code. + """ + return code_generators.generate_qr_code( + discriminator=self._discriminator, + passcode=self._passcode, + vendor_id=self._vendor_id, + product_id=self._product_id, + ) + def get_commissioning_code(self) -> str: """ Returns the current commissioning code for the device. """ return self._commissioning_code + def get_qr_code(self) -> str: + """ + Returns the QR code setup payload for the device. + """ + return self._qr_code + @property def sideband(self) -> SidebandClient: """Access the side-band client for this device. diff --git a/testing/test/ikea_timmerflotte_test.py b/testing/test/ikea_timmerflotte_test.py index d40050d3..d0cf0e62 100644 --- a/testing/test/ikea_timmerflotte_test.py +++ b/testing/test/ikea_timmerflotte_test.py @@ -165,7 +165,7 @@ def test_timmerflotte_driver_rejects_wrong_vendor_product( try: default_environment.get_client().commission_device( - sensor.get_commissioning_code(), 100 + sensor.get_qr_code(), 100 ) default_environment.wait_for_device_added() diff --git a/testing/utils/barton_utils.py b/testing/utils/barton_utils.py index 165a935c..47ff48c2 100644 --- a/testing/utils/barton_utils.py +++ b/testing/utils/barton_utils.py @@ -34,8 +34,14 @@ def commission_device(environment, device, device_class): Commissions the device via the environment's client, waits for the device-added event, then returns the last device of the given class. + + Uses the QR code setup payload (full 12-bit discriminator) so the + commissioner matches this exact device. The manual pairing code carries + only a 4-bit short discriminator, which collides across devices when many + are advertising concurrently and can cause the commissioner to match the + wrong device. """ - environment.get_client().commission_device(device.get_commissioning_code(), 100) + environment.get_client().commission_device(device.get_qr_code(), 100) environment.wait_for_device_added() devices = environment.get_client().get_devices_by_device_class(device_class) assert len(devices) >= 1, f"Expected at least 1 '{device_class}' device, found 0" From 2a1c2424ac06f3ed9461138a8979d693c42db2f6 Mon Sep 17 00:00:00 2001 From: Christian Leithner Date: Tue, 4 Aug 2026 19:06:25 +0000 Subject: [PATCH 8/9] test(integration): default to nproc/2 workers, pin CI to 3 The former nproc/4 divisor was extra headroom against commissioning timeouts that were actually the short-discriminator collision (now fixed by commissioning via the full-discriminator QR code), not CPU starvation. With that gone, one worker per physical core (nproc/2, since nproc counts hyperthreads) is reliable. Pin CI to --parallel=3: the runners have 4 cores, and a 4-core sweep showed 3 workers is ~30% faster than 2 (70s vs 100s) with zero flakes across repeated runs, while keeping a core free for the OS/dbus/otbr. The nproc/2 default would otherwise resolve to 2 on those runners. --- scripts/ci/run_integration_tests.sh | 5 +++- testing/py_test.sh | 40 +++++++++++++++-------------- 2 files changed, 25 insertions(+), 20 deletions(-) diff --git a/scripts/ci/run_integration_tests.sh b/scripts/ci/run_integration_tests.sh index ebaabe8a..27b945c1 100755 --- a/scripts/ci/run_integration_tests.sh +++ b/scripts/ci/run_integration_tests.sh @@ -55,4 +55,7 @@ echo "End of installation, starting tests" echo "***********************************" echo "" -"$REPO_ROOT/testing/py_test.sh" "$REPO_ROOT/testing" --parallel +# Pin to 3 workers: CI runners have 4 cores, and 3 workers keeps a core free for +# the OS/dbus/otbr while avoiding the CASE/PASE starvation seen at 4 (validated +# ~30% faster than 2 with no flakes). The nproc/2 default would resolve to 2 here. +"$REPO_ROOT/testing/py_test.sh" "$REPO_ROOT/testing" --parallel=3 diff --git a/testing/py_test.sh b/testing/py_test.sh index 4d63ca83..e6122f38 100755 --- a/testing/py_test.sh +++ b/testing/py_test.sh @@ -76,8 +76,8 @@ function show_help { echo " to preload. If not specified, auto-detects from" echo " the system default 'cc'." echo " --parallel[=] Run tests in parallel across pytest-xdist" - echo " workers. With no value, defaults to min(CPUs/4, 64)" - echo " -- roughly half the physical cores, since each worker" + echo " workers. With no value, defaults to min(CPUs/2, 64)" + echo " -- about one worker per physical core, since each worker" echo " drives Barton plus a matter.js node process. Tests run" echo " SERIALLY unless this flag is given, so interactive/" echo " individual runs keep readable, interleaved logs." @@ -114,26 +114,28 @@ if [[ -n "$PARALLEL_WORKERS" ]]; then if [[ "$PARALLEL_WORKERS" == "default" ]]; then # Scale the default worker count with the machine, capped at 64. # - # History: Matter commissioning discovers each virtual device over the - # shared mDNS plane (port 5353). The CHIP commissioner keeps a fixed - # cache of discovered commissionable nodes; because every commissioner - # sees every device's advertisement, concurrent commissionings used to - # overflow that cache ("Insufficient space") and discovery would time - # out -- which capped reliable parallelism at ~4. That cache was raised - # 10 -> 128 (barton patch 0003), which removes the mDNS ceiling: no - # overflow is seen even at 64 workers. + # Matter commissioning discovers each virtual device over the shared + # mDNS plane (port 5353). The CHIP commissioner keeps a fixed cache of + # discovered commissionable nodes; because every commissioner sees every + # device's advertisement, concurrent commissionings used to overflow + # that cache ("Insufficient space") -- raised 10 -> 128 (barton patch + # 0003), so no overflow is seen even at 64 workers. # - # The remaining limit at very high concurrency is soft and CPU-bound: - # each worker drives a multi-threaded Barton plus a matter.js node - # process (and ASAN), so it realistically needs ~2 physical cores. - # Running one worker per logical CPU oversubscribes and makes crypto-heavy - # CASE commissioning time out. nproc counts logical CPUs (hyperthreads), - # so use a quarter of it -- roughly half the physical cores -- to leave - # headroom on machines of any size. Min 1, capped at 64 (only ~58 tests, - # so more never helps). + # Each worker drives a multi-threaded Barton plus a matter.js node + # process (and ASAN), so it needs roughly a full physical core. nproc + # counts logical CPUs (hyperthreads), so use half of it -- about one + # worker per physical core -- leaving headroom for the OS and the + # per-test subprocesses. Oversubscribing past the physical core count + # starves the crypto-heavy CASE/PASE commissioning phase and makes it + # time out. Min 1, capped at 64 (only ~62 tests, so more never helps). + # + # (An earlier ceiling of ~4 workers came from commissioners matching the + # wrong device via the 4-bit short discriminator in the manual pairing + # code; that is fixed by commissioning with the full-discriminator QR + # code, so the limit is now purely CPU headroom.) PARALLEL_CAP=64 CPU_COUNT=$(nproc) - PARALLEL_WORKERS=$(( CPU_COUNT / 4 )) + PARALLEL_WORKERS=$(( CPU_COUNT / 2 )) (( PARALLEL_WORKERS < 1 )) && PARALLEL_WORKERS=1 (( PARALLEL_WORKERS > PARALLEL_CAP )) && PARALLEL_WORKERS=$PARALLEL_CAP fi From 32abee308d7d0d72e8dec5311ae04deca8c6c551 Mon Sep 17 00:00:00 2001 From: Christian Leithner Date: Thu, 6 Aug 2026 17:58:20 +0000 Subject: [PATCH 9/9] test(integration): harden parallel test runs after merging main Squashed follow-ups to the origin/main merge: - Pin GI_TYPELIB_PATH to the worktree's build/core so parallel/worktree runs load this tree's freshly-built BCore typelib. main bumped BCore to major version 4, and the container's GI_TYPELIB_PATH points at the provisioning clone, so without this a worktree resolves the wrong typelib (or fails with 'Namespace BCore not available'). - Make the side-band channel address- and lifecycle-correct: connect the client to 127.0.0.1 to match the server's bind (instead of dual-stack 'localhost'), and have the server close each connection after responding, so no kept-alive sockets linger on the loopback control channel under load. - Default to min(nproc/4, 32) workers, about one worker per two physical cores. Commissioning is a crypto-heavy PASE/CASE burst that runs concurrently on both the commissioner (Barton) and the target (a matter.js node process), so it needs ~2 physical cores while it runs; packing one worker per physical core starves the target and it misses the PASE handshake. Starvation is fixed with resources, not retries or inflated timeouts. Validated 0 flakes over 50 runs. - Specify the CI worker count in the workflow (--parallel=3) rather than in run_integration_tests.sh, since the runner's core count (GitHub-hosted runners have 4) is a property of the CI environment. The runner script now forwards its arguments to py_test.sh. --- .github/workflows/run-tests.yaml | 4 +- scripts/ci/run_integration_tests.sh | 8 +-- testing/mocks/devices/matter/matter_device.py | 2 +- .../devices/matterjs/src/VirtualDevice.js | 5 ++ testing/py_test.sh | 60 ++++++++++++------- 5 files changed, 51 insertions(+), 28 deletions(-) diff --git a/.github/workflows/run-tests.yaml b/.github/workflows/run-tests.yaml index 521623c7..f5a121db 100644 --- a/.github/workflows/run-tests.yaml +++ b/.github/workflows/run-tests.yaml @@ -85,4 +85,6 @@ jobs: run: ./dockerw -n ctest --output-on-failure --test-dir build - name: Run integration tests - run: ./dockerw -n ./scripts/ci/run_integration_tests.sh + # GitHub-hosted runners have 4 cores; 3 workers keeps one core + # free for the OS/dbus/otbr and avoids CASE/PASE starvation. + run: ./dockerw -n ./scripts/ci/run_integration_tests.sh --parallel=3 diff --git a/scripts/ci/run_integration_tests.sh b/scripts/ci/run_integration_tests.sh index 27b945c1..e1cb7b75 100755 --- a/scripts/ci/run_integration_tests.sh +++ b/scripts/ci/run_integration_tests.sh @@ -55,7 +55,7 @@ echo "End of installation, starting tests" echo "***********************************" echo "" -# Pin to 3 workers: CI runners have 4 cores, and 3 workers keeps a core free for -# the OS/dbus/otbr while avoiding the CASE/PASE starvation seen at 4 (validated -# ~30% faster than 2 with no flakes). The nproc/2 default would resolve to 2 here. -"$REPO_ROOT/testing/py_test.sh" "$REPO_ROOT/testing" --parallel=3 +# Forward any extra arguments (e.g. --parallel=) to the pytest wrapper. The +# worker count is specified by the caller/CI workflow rather than hard-coded +# here, because it depends on the runner's core count (GitHub runners have 4). +"$REPO_ROOT/testing/py_test.sh" "$REPO_ROOT/testing" "$@" diff --git a/testing/mocks/devices/matter/matter_device.py b/testing/mocks/devices/matter/matter_device.py index 67d21fc7..c5a0c020 100644 --- a/testing/mocks/devices/matter/matter_device.py +++ b/testing/mocks/devices/matter/matter_device.py @@ -231,7 +231,7 @@ def start(self): ) sideband_port = ready_signal["sidebandPort"] - self._sideband = SidebandClient("localhost", sideband_port) + self._sideband = SidebandClient("127.0.0.1", sideband_port) logger.debug( f"Started {self._device_class} with PID {self._process.pid}, " diff --git a/testing/mocks/devices/matterjs/src/VirtualDevice.js b/testing/mocks/devices/matterjs/src/VirtualDevice.js index 55924290..80654c62 100644 --- a/testing/mocks/devices/matterjs/src/VirtualDevice.js +++ b/testing/mocks/devices/matterjs/src/VirtualDevice.js @@ -194,6 +194,11 @@ export class VirtualDevice { startSidebandServer() { return new Promise((resolve, reject) => { this.httpServer = http.createServer(async (req, res) => { + // One-shot connections: closing after each response avoids + // keep-alive reset races on the loopback control channel when + // many device processes contend for the CPU under parallel load. + res.setHeader('Connection', 'close'); + if (req.method === 'POST' && req.url === '/sideband') { await this.handleSidebandRequest(req, res); } else { diff --git a/testing/py_test.sh b/testing/py_test.sh index e6122f38..1d4bd2e1 100755 --- a/testing/py_test.sh +++ b/testing/py_test.sh @@ -65,6 +65,12 @@ export PYTHONPATH="$REPO_ROOT${PYTHONPATH:+:$PYTHONPATH}" # primary clone's build/core), so parallel/worktree runs load the right library. export LD_LIBRARY_PATH="$REPO_ROOT/build/core${LD_LIBRARY_PATH:+:$LD_LIBRARY_PATH}" +# And point GObject-introspection at this tree's BCore typelib for the same +# reason: the container's GI_TYPELIB_PATH points at the provisioning clone's +# build/core, so without this a worktree would load that clone's typelib (or +# none at all, if it is unset in the current shell). +export GI_TYPELIB_PATH="$REPO_ROOT/build/core${GI_TYPELIB_PATH:+:$GI_TYPELIB_PATH}" + function show_help { echo "This is a wrapper script around pytest to ensure the environment is setup correctly." echo "Usage: $0 [-t=|--toolchain=] [--parallel[=]] [pytest options]" @@ -76,9 +82,10 @@ function show_help { echo " to preload. If not specified, auto-detects from" echo " the system default 'cc'." echo " --parallel[=] Run tests in parallel across pytest-xdist" - echo " workers. With no value, defaults to min(CPUs/2, 64)" - echo " -- about one worker per physical core, since each worker" - echo " drives Barton plus a matter.js node process. Tests run" + echo " workers. With no value, defaults to min(CPUs/4, 32)" + echo " -- about one worker per two physical cores, since each" + echo " commissioning is a ~2-core crypto burst across Barton and" + echo " a matter.js node process. Tests run" echo " SERIALLY unless this flag is given, so interactive/" echo " individual runs keep readable, interleaved logs." } @@ -112,30 +119,37 @@ done PARALLEL_ARGS=() if [[ -n "$PARALLEL_WORKERS" ]]; then if [[ "$PARALLEL_WORKERS" == "default" ]]; then - # Scale the default worker count with the machine, capped at 64. + # Scale the default worker count with the machine, capped at 32. # # Matter commissioning discovers each virtual device over the shared # mDNS plane (port 5353). The CHIP commissioner keeps a fixed cache of # discovered commissionable nodes; because every commissioner sees every # device's advertisement, concurrent commissionings used to overflow # that cache ("Insufficient space") -- raised 10 -> 128 (barton patch - # 0003), so no overflow is seen even at 64 workers. + # 0003), so no overflow is seen even at 32 workers. # - # Each worker drives a multi-threaded Barton plus a matter.js node - # process (and ASAN), so it needs roughly a full physical core. nproc - # counts logical CPUs (hyperthreads), so use half of it -- about one - # worker per physical core -- leaving headroom for the OS and the - # per-test subprocesses. Oversubscribing past the physical core count - # starves the crypto-heavy CASE/PASE commissioning phase and makes it - # time out. Min 1, capped at 64 (only ~62 tests, so more never helps). + # The binding limit is CPU: commissioning a device is a crypto-heavy + # PASE/CASE burst that runs concurrently on BOTH the commissioner + # (multi-threaded Barton) and the target device (a matter.js node + # process), i.e. it needs ~2 physical cores while it runs. If the target + # can't get the CPU it misses the PASE handshake and the commission + # fails. nproc counts logical CPUs (hyperthreads), so use a quarter of + # it -- about one worker per two physical cores -- which leaves room for + # both halves of every concurrent commissioning plus the OS and per-test + # subprocesses. Min 1, capped at 32 (only ~62 tests, and returns diminish + # well before then). # - # (An earlier ceiling of ~4 workers came from commissioners matching the - # wrong device via the 4-bit short discriminator in the manual pairing - # code; that is fixed by commissioning with the full-discriminator QR - # code, so the limit is now purely CPU headroom.) - PARALLEL_CAP=64 + # Empirically, on a 64-physical-core box: nproc/2 (one worker per + # physical core) starves ~10% of runs; nproc/4 and nproc/3 are clean + # over 20 runs each. nproc/4 is chosen for margin. Raising timeouts is + # deliberately NOT used to paper over starvation -- the fix is fewer + # workers. (A separate earlier ceiling of ~4 workers came from the 4-bit + # short discriminator in the manual pairing code matching the wrong + # device; that is fixed by commissioning with the full-discriminator QR + # code, so the remaining limit is purely CPU headroom.) + PARALLEL_CAP=32 CPU_COUNT=$(nproc) - PARALLEL_WORKERS=$(( CPU_COUNT / 2 )) + PARALLEL_WORKERS=$(( CPU_COUNT / 4 )) (( PARALLEL_WORKERS < 1 )) && PARALLEL_WORKERS=1 (( PARALLEL_WORKERS > PARALLEL_CAP )) && PARALLEL_WORKERS=$PARALLEL_CAP fi @@ -143,10 +157,12 @@ if [[ -n "$PARALLEL_WORKERS" ]]; then # zhal mock tests that bind fixed IPC ports 18443/8711) stay on one worker and # never collide. # - # Raise the commissioning wait timeouts: under concurrent load the crypto-heavy - # CASE/commissioning phase legitimately takes longer than the (fast-failure) - # serial defaults, so give it headroom. These override testing/conftest.py's - # defaults and are forwarded into each per-test subprocess. + # Raise the commissioning wait timeouts modestly: under concurrent load the + # crypto-heavy CASE/commissioning phase legitimately takes a little longer + # than the (fast-failure) serial defaults, so give it some headroom. These + # override testing/conftest.py's defaults and are forwarded into each + # per-test subprocess. They are NOT a remedy for CPU starvation -- that is + # bounded by keeping the worker count at/under the physical core count above. PARALLEL_ARGS=( -n "$PARALLEL_WORKERS" --dist loadgroup --client-ready-timeout=30 --device-added-timeout=30 --resource-value-timeout=30