Allow to communicate with DUT via standard input (project-chip#36687)

arkq · restyled-commits · web-flow · commit 38ad07d8742d · 2024-12-04T16:21:20.000Z
* Allow to communicate with DUT via standard input

* Use fabric-sync-app.py stdin instead dedicated pipe

* Drop pipe stdin forwarder in fabric-sync-app.py

* Restyled by autopep8

* Wait for thread to stop

* Fix referencing not-created variable

---------

Co-authored-by: Restyled.io &lt;commits@restyled.io&gt;
diff --git a/docs/testing/python.md b/docs/testing/python.md
@@ -722,6 +722,11 @@ for that run, e.g.:
 
     -   Example: `"Manual pairing code: \\[\\d+\\]"`
 
+-   `app-stdin-pipe`: Specifies the path to the named pipe that the test runner
+    might use to send input to the application.
+
+    -   Example: `/tmp/app-fifo`
+
 -   `script-args`: Specifies the arguments to be passed to the test script.
 
     -   Example:
diff --git a/examples/fabric-admin/scripts/fabric-sync-app.py b/examples/fabric-admin/scripts/fabric-sync-app.py
@@ -16,7 +16,6 @@
 
 import asyncio
 import contextlib
-import os
 import shutil
 import signal
 import sys
@@ -41,26 +40,6 @@ async def forward_f(prefix: bytes, f_in: asyncio.StreamReader,
         f_out.flush()
 
 
-async def forward_pipe(pipe_path: str, f_out: asyncio.StreamWriter):
-    """Forward named pipe to f_out.
-
-    Unfortunately, Python does not support async file I/O on named pipes. This
-    function performs busy waiting with a short asyncio-friendly sleep to read
-    from the pipe.
-    """
-    fd = os.open(pipe_path, os.O_RDONLY | os.O_NONBLOCK)
-    while True:
-        try:
-            data = os.read(fd, 1024)
-            if data:
-                f_out.write(data)
-                await f_out.drain()
-            if not data:
-                await asyncio.sleep(0.1)
-        except BlockingIOError:
-            await asyncio.sleep(0.1)
-
-
 async def forward_stdin(f_out: asyncio.StreamWriter):
     """Forward stdin to f_out."""
     loop = asyncio.get_event_loop()
@@ -175,9 +154,6 @@ async def main(args):
         storage = TemporaryDirectory(prefix="fabric-sync-app")
         storage_dir = Path(storage.name)
 
-    if args.stdin_pipe and not args.stdin_pipe.exists():
-        os.mkfifo(args.stdin_pipe)
-
     admin, bridge = await asyncio.gather(
         run_admin(
             args.app_admin,
@@ -206,8 +182,6 @@ def terminate():
             admin.terminate()
         with contextlib.suppress(ProcessLookupError):
             bridge.terminate()
-        if args.stdin_pipe:
-            args.stdin_pipe.unlink(missing_ok=True)
         loop.remove_signal_handler(signal.SIGINT)
         loop.remove_signal_handler(signal.SIGTERM)
 
@@ -249,17 +223,12 @@ def terminate():
     await admin.send(f"pairing open-commissioning-window {bridge_node_id} {cw_endpoint_id}"
                      f" {cw_option} {cw_timeout} {cw_iteration} {cw_discriminator}")
 
-    def get_input_forwarder():
-        if args.stdin_pipe:
-            return forward_pipe(args.stdin_pipe, admin.p.stdin)
-        return forward_stdin(admin.p.stdin)
-
     try:
         # Wait for any of the tasks to complete.
         _, pending = await asyncio.wait([
             asyncio.create_task(admin.wait()),
             asyncio.create_task(bridge.wait()),
-            asyncio.create_task(get_input_forwarder()),
+            asyncio.create_task(forward_stdin(admin.p.stdin)),
         ], return_when=asyncio.FIRST_COMPLETED)
         # Cancel the remaining tasks.
         for task in pending:
@@ -285,8 +254,6 @@ def get_input_forwarder():
                         help="fabric-admin RPC server port")
     parser.add_argument("--app-bridge-rpc-port", metavar="PORT", type=int,
                         help="fabric-bridge RPC server port")
-    parser.add_argument("--stdin-pipe", metavar="PATH", type=Path,
-                        help="read input from a named pipe instead of stdin")
     parser.add_argument("--storage-dir", metavar="PATH", type=Path,
                         help=("directory to place storage files in; by default "
                               "volatile storage is used"))
@@ -309,7 +276,5 @@ def get_input_forwarder():
         parser.error("fabric-admin executable not found in PATH. Use '--app-admin' argument to provide it.")
     if args.app_bridge is None or not args.app_bridge.exists():
         parser.error("fabric-bridge-app executable not found in PATH. Use '--app-bridge' argument to provide it.")
-    if args.stdin_pipe and args.stdin_pipe.exists() and not args.stdin_pipe.is_fifo():
-        parser.error("given stdin pipe exists and is not a named pipe")
     with contextlib.suppress(KeyboardInterrupt):
         asyncio.run(main(args))
diff --git a/scripts/tests/run_python_test.py b/scripts/tests/run_python_test.py
@@ -14,6 +14,7 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
+import contextlib
 import datetime
 import glob
 import io
@@ -22,9 +23,12 @@
 import os.path
 import pathlib
 import re
+import select
 import shlex
 import sys
+import threading
 import time
+import typing
 
 import click
 import coloredlogs
@@ -68,6 +72,23 @@ def process_test_script_output(line, is_stderr):
     return process_chip_output(line, is_stderr, TAG_PROCESS_TEST)
 
 
+def forward_fifo(path: str, f_out: typing.BinaryIO, stop_event: threading.Event):
+    """Forward the content of a named pipe to a file-like object."""
+    if not os.path.exists(path):
+        with contextlib.suppress(OSError):
+            os.mkfifo(path)
+    with open(os.open(path, os.O_RDONLY | os.O_NONBLOCK), 'rb') as f_in:
+        while not stop_event.is_set():
+            if select.select([f_in], [], [], 0.5)[0]:
+                line = f_in.readline()
+                if not line:
+                    break
+                f_out.write(line)
+                f_out.flush()
+    with contextlib.suppress(OSError):
+        os.unlink(path)
+
+
 @click.command()
 @click.option("--app", type=click.Path(exists=True), default=None,
               help='Path to local application to use, omit to use external apps.')
@@ -79,6 +100,8 @@ def process_test_script_output(line, is_stderr):
               help='The extra arguments passed to the device. Can use placeholders like {SCRIPT_BASE_NAME}')
 @click.option("--app-ready-pattern", type=str, default=None,
               help='Delay test script start until given regular expression pattern is found in the application output.')
+@click.option("--app-stdin-pipe", type=str, default=None,
+              help='Path for a standard input redirection named pipe to be used by the test script.')
 @click.option("--script", type=click.Path(exists=True), default=os.path.join(DEFAULT_CHIP_ROOT,
                                                                              'src',
                                                                              'controller',
@@ -94,7 +117,8 @@ def process_test_script_output(line, is_stderr):
               help="Do not print output from passing tests. Use this flag in CI to keep GitHub log size manageable.")
 @click.option("--load-from-env", default=None, help="YAML file that contains values for environment variables.")
 def main(app: str, factory_reset: bool, factory_reset_app_only: bool, app_args: str,
-         app_ready_pattern: str, script: str, script_args: str, script_gdb: bool, quiet: bool, load_from_env):
+         app_ready_pattern: str, app_stdin_pipe: str, script: str, script_args: str,
+         script_gdb: bool, quiet: bool, load_from_env):
     if load_from_env:
         reader = MetadataReader(load_from_env)
         runs = reader.parse_script(script)
@@ -106,6 +130,7 @@ def main(app: str, factory_reset: bool, factory_reset_app_only: bool, app_args:
                 app=app,
                 app_args=app_args,
                 app_ready_pattern=app_ready_pattern,
+                app_stdin_pipe=app_stdin_pipe,
                 script_args=script_args,
                 script_gdb=script_gdb,
             )
@@ -128,11 +153,13 @@ def main(app: str, factory_reset: bool, factory_reset_app_only: bool, app_args:
     for run in runs:
         logging.info("Executing %s %s", run.py_script_path.split('/')[-1], run.run)
         main_impl(run.app, run.factory_reset, run.factory_reset_app_only, run.app_args or "",
-                  run.app_ready_pattern, run.py_script_path, run.script_args or "", run.script_gdb, run.quiet)
+                  run.app_ready_pattern, run.app_stdin_pipe, run.py_script_path,
+                  run.script_args or "", run.script_gdb, run.quiet)
 
 
 def main_impl(app: str, factory_reset: bool, factory_reset_app_only: bool, app_args: str,
-              app_ready_pattern: str, script: str, script_args: str, script_gdb: bool, quiet: bool):
+              app_ready_pattern: str, app_stdin_pipe: str, script: str, script_args: str,
+              script_gdb: bool, quiet: bool):
 
     app_args = app_args.replace('{SCRIPT_BASE_NAME}', os.path.splitext(os.path.basename(script))[0])
     script_args = script_args.replace('{SCRIPT_BASE_NAME}', os.path.splitext(os.path.basename(script))[0])
@@ -154,6 +181,8 @@ def main_impl(app: str, factory_reset: bool, factory_reset_app_only: bool, app_a
             pathlib.Path(match.group("path")).unlink(missing_ok=True)
 
     app_process = None
+    app_stdin_forwarding_thread = None
+    app_stdin_forwarding_stop_event = threading.Event()
     app_exit_code = 0
     app_pid = 0
 
@@ -172,7 +201,13 @@ def main_impl(app: str, factory_reset: bool, factory_reset_app_only: bool, app_a
                                  f_stdout=stream_output,
                                  f_stderr=stream_output)
         app_process.start(expected_output=app_ready_pattern, timeout=30)
-        app_process.p.stdin.close()
+        if app_stdin_pipe:
+            logging.info("Forwarding stdin from '%s' to app", app_stdin_pipe)
+            app_stdin_forwarding_thread = threading.Thread(
+                target=forward_fifo, args=(app_stdin_pipe, app_process.p.stdin, app_stdin_forwarding_stop_event))
+            app_stdin_forwarding_thread.start()
+        else:
+            app_process.p.stdin.close()
         app_pid = app_process.p.pid
 
     script_command = [script, "--paa-trust-store-path", os.path.join(DEFAULT_CHIP_ROOT, MATTER_DEVELOPMENT_PAA_ROOT_CERTS),
@@ -204,6 +239,9 @@ def main_impl(app: str, factory_reset: bool, factory_reset_app_only: bool, app_a
 
     if app_process:
         logging.info("Stopping app with SIGTERM")
+        if app_stdin_forwarding_thread:
+            app_stdin_forwarding_stop_event.set()
+            app_stdin_forwarding_thread.join()
         app_process.terminate()
         app_exit_code = app_process.returncode
 
diff --git a/src/python_testing/TC_BRBINFO_4_1.py b/src/python_testing/TC_BRBINFO_4_1.py
@@ -22,8 +22,9 @@
 # test-runner-runs:
 #   run1:
 #     app: examples/fabric-admin/scripts/fabric-sync-app.py
-#     app-args: --app-admin=${FABRIC_ADMIN_APP} --app-bridge=${FABRIC_BRIDGE_APP} --stdin-pipe=dut-fsa-stdin --discriminator=1234
+#     app-args: --app-admin=${FABRIC_ADMIN_APP} --app-bridge=${FABRIC_BRIDGE_APP} --discriminator=1234
 #     app-ready-pattern: "Successfully opened pairing window on the device"
+#     app-stdin-pipe: dut-fsa-stdin
 #     script-args: >
 #       --PICS src/app/tests/suites/certification/ci-pics-values
 #       --storage-path admin_storage.json
diff --git a/src/python_testing/TC_CCTRL_2_1.py b/src/python_testing/TC_CCTRL_2_1.py
@@ -22,8 +22,9 @@
 # test-runner-runs:
 #   run1:
 #     app: examples/fabric-admin/scripts/fabric-sync-app.py
-#     app-args: --app-admin=${FABRIC_ADMIN_APP} --app-bridge=${FABRIC_BRIDGE_APP} --stdin-pipe=dut-fsa-stdin --discriminator=1234
+#     app-args: --app-admin=${FABRIC_ADMIN_APP} --app-bridge=${FABRIC_BRIDGE_APP} --discriminator=1234
 #     app-ready-pattern: "Successfully opened pairing window on the device"
+#     app-stdin-pipe: dut-fsa-stdin
 #     script-args: >
 #       --PICS src/app/tests/suites/certification/ci-pics-values
 #       --storage-path admin_storage.json
diff --git a/src/python_testing/TC_CCTRL_2_2.py b/src/python_testing/TC_CCTRL_2_2.py
@@ -22,8 +22,9 @@
 # test-runner-runs:
 #   run1:
 #     app: examples/fabric-admin/scripts/fabric-sync-app.py
-#     app-args: --app-admin=${FABRIC_ADMIN_APP} --app-bridge=${FABRIC_BRIDGE_APP} --stdin-pipe=dut-fsa-stdin --discriminator=1234
+#     app-args: --app-admin=${FABRIC_ADMIN_APP} --app-bridge=${FABRIC_BRIDGE_APP} --discriminator=1234
 #     app-ready-pattern: "Successfully opened pairing window on the device"
+#     app-stdin-pipe: dut-fsa-stdin
 #     script-args: >
 #       --PICS src/app/tests/suites/certification/ci-pics-values
 #       --storage-path admin_storage.json
diff --git a/src/python_testing/TC_CCTRL_2_3.py b/src/python_testing/TC_CCTRL_2_3.py
@@ -22,8 +22,9 @@
 # test-runner-runs:
 #   run1:
 #     app: examples/fabric-admin/scripts/fabric-sync-app.py
-#     app-args: --app-admin=${FABRIC_ADMIN_APP} --app-bridge=${FABRIC_BRIDGE_APP} --stdin-pipe=dut-fsa-stdin --discriminator=1234
+#     app-args: --app-admin=${FABRIC_ADMIN_APP} --app-bridge=${FABRIC_BRIDGE_APP} --discriminator=1234
 #     app-ready-pattern: "Successfully opened pairing window on the device"
+#     app-stdin-pipe: dut-fsa-stdin
 #     script-args: >
 #       --PICS src/app/tests/suites/certification/ci-pics-values
 #       --storage-path admin_storage.json
diff --git a/src/python_testing/TC_ECOINFO_2_1.py b/src/python_testing/TC_ECOINFO_2_1.py
@@ -22,8 +22,9 @@
 # test-runner-runs:
 #   run1:
 #     app: examples/fabric-admin/scripts/fabric-sync-app.py
-#     app-args: --app-admin=${FABRIC_ADMIN_APP} --app-bridge=${FABRIC_BRIDGE_APP} --stdin-pipe=dut-fsa-stdin --discriminator=1234
+#     app-args: --app-admin=${FABRIC_ADMIN_APP} --app-bridge=${FABRIC_BRIDGE_APP} --discriminator=1234
 #     app-ready-pattern: "Successfully opened pairing window on the device"
+#     app-stdin-pipe: dut-fsa-stdin
 #     script-args: >
 #       --PICS src/app/tests/suites/certification/ci-pics-values
 #       --storage-path admin_storage.json
diff --git a/src/python_testing/TC_ECOINFO_2_2.py b/src/python_testing/TC_ECOINFO_2_2.py
@@ -22,8 +22,9 @@
 # test-runner-runs:
 #   run1:
 #     app: examples/fabric-admin/scripts/fabric-sync-app.py
-#     app-args: --app-admin=${FABRIC_ADMIN_APP} --app-bridge=${FABRIC_BRIDGE_APP} --stdin-pipe=dut-fsa-stdin --discriminator=1234
+#     app-args: --app-admin=${FABRIC_ADMIN_APP} --app-bridge=${FABRIC_BRIDGE_APP} --discriminator=1234
 #     app-ready-pattern: "Successfully opened pairing window on the device"
+#     app-stdin-pipe: dut-fsa-stdin
 #     script-args: >
 #       --PICS src/app/tests/suites/certification/ci-pics-values
 #       --storage-path admin_storage.json
diff --git a/src/python_testing/TC_MCORE_FS_1_1.py b/src/python_testing/TC_MCORE_FS_1_1.py
@@ -24,8 +24,9 @@
 # test-runner-runs:
 #   run1:
 #     app: examples/fabric-admin/scripts/fabric-sync-app.py
-#     app-args: --app-admin=${FABRIC_ADMIN_APP} --app-bridge=${FABRIC_BRIDGE_APP} --stdin-pipe=dut-fsa-stdin --discriminator=1234
+#     app-args: --app-admin=${FABRIC_ADMIN_APP} --app-bridge=${FABRIC_BRIDGE_APP} --discriminator=1234
 #     app-ready-pattern: "Successfully opened pairing window on the device"
+#     app-stdin-pipe: dut-fsa-stdin
 #     script-args: >
 #       --PICS src/app/tests/suites/certification/ci-pics-values
 #       --storage-path admin_storage.json
diff --git a/src/python_testing/TC_MCORE_FS_1_2.py b/src/python_testing/TC_MCORE_FS_1_2.py
@@ -22,8 +22,9 @@
 # test-runner-runs:
 #   run1:
 #     app: examples/fabric-admin/scripts/fabric-sync-app.py
-#     app-args: --app-admin=${FABRIC_ADMIN_APP} --app-bridge=${FABRIC_BRIDGE_APP} --stdin-pipe=dut-fsa-stdin --discriminator=1234
+#     app-args: --app-admin=${FABRIC_ADMIN_APP} --app-bridge=${FABRIC_BRIDGE_APP} --discriminator=1234
 #     app-ready-pattern: "Successfully opened pairing window on the device"
+#     app-stdin-pipe: dut-fsa-stdin
 #     script-args: >
 #       --PICS src/app/tests/suites/certification/ci-pics-values
 #       --storage-path admin_storage.json
diff --git a/src/python_testing/TC_MCORE_FS_1_3.py b/src/python_testing/TC_MCORE_FS_1_3.py
@@ -26,8 +26,9 @@
 # test-runner-runs:
 #   run1:
 #     app: examples/fabric-admin/scripts/fabric-sync-app.py
-#     app-args: --app-admin=${FABRIC_ADMIN_APP} --app-bridge=${FABRIC_BRIDGE_APP} --stdin-pipe=dut-fsa-stdin --discriminator=1234
+#     app-args: --app-admin=${FABRIC_ADMIN_APP} --app-bridge=${FABRIC_BRIDGE_APP} --discriminator=1234
 #     app-ready-pattern: "Successfully opened pairing window on the device"
+#     app-stdin-pipe: dut-fsa-stdin
 #     script-args: >
 #       --PICS src/app/tests/suites/certification/ci-pics-values
 #       --storage-path admin_storage.json
@@ -71,11 +72,11 @@ def setup_class(self):
         self.storage = None
 
         # Get the path to the TH_SERVER_NO_UID app from the user params.
-        th_server_app = self.user_params.get("th_server_no_uid_app_path", None)
-        if not th_server_app:
+        th_server_no_uid_app = self.user_params.get("th_server_no_uid_app_path", None)
+        if not th_server_no_uid_app:
             asserts.fail("This test requires a TH_SERVER_NO_UID app. Specify app path with --string-arg th_server_no_uid_app_path:<path_to_app>")
-        if not os.path.exists(th_server_app):
-            asserts.fail(f"The path {th_server_app} does not exist")
+        if not os.path.exists(th_server_no_uid_app):
+            asserts.fail(f"The path {th_server_no_uid_app} does not exist")
 
         # Create a temporary storage directory for keeping KVS files.
         self.storage = tempfile.TemporaryDirectory(prefix=self.__class__.__name__)
@@ -94,7 +95,7 @@ def setup_class(self):
 
         # Start the TH_SERVER_NO_UID app.
         self.th_server = AppServerSubprocess(
-            th_server_app,
+            th_server_no_uid_app,
             storage_dir=self.storage.name,
             port=self.th_server_port,
             discriminator=self.th_server_discriminator,
diff --git a/src/python_testing/TC_MCORE_FS_1_4.py b/src/python_testing/TC_MCORE_FS_1_4.py
@@ -26,8 +26,9 @@
 # test-runner-runs:
 #   run1:
 #     app: examples/fabric-admin/scripts/fabric-sync-app.py
-#     app-args: --app-admin=${FABRIC_ADMIN_APP} --app-bridge=${FABRIC_BRIDGE_APP} --stdin-pipe=dut-fsa-stdin --discriminator=1234
+#     app-args: --app-admin=${FABRIC_ADMIN_APP} --app-bridge=${FABRIC_BRIDGE_APP} --discriminator=1234
 #     app-ready-pattern: "Successfully opened pairing window on the device"
+#     app-stdin-pipe: dut-fsa-stdin
 #     script-args: >
 #       --PICS src/app/tests/suites/certification/ci-pics-values
 #       --storage-path admin_storage.json
@@ -129,11 +130,11 @@ def setup_class(self):
             asserts.fail(f"The path {th_fsa_bridge_path} does not exist")
 
         # Get the path to the TH_SERVER_NO_UID app from the user params.
-        th_server_app = self.user_params.get("th_server_no_uid_app_path", None)
-        if not th_server_app:
+        th_server_no_uid_app = self.user_params.get("th_server_no_uid_app_path", None)
+        if not th_server_no_uid_app:
             asserts.fail("This test requires a TH_SERVER_NO_UID app. Specify app path with --string-arg th_server_no_uid_app_path:<path_to_app>")
-        if not os.path.exists(th_server_app):
-            asserts.fail(f"The path {th_server_app} does not exist")
+        if not os.path.exists(th_server_no_uid_app):
+            asserts.fail(f"The path {th_server_no_uid_app} does not exist")
 
         # Create a temporary storage directory for keeping KVS files.
         self.storage = tempfile.TemporaryDirectory(prefix=self.__class__.__name__)
@@ -171,7 +172,7 @@ def setup_class(self):
 
         # Start the TH_SERVER_NO_UID app.
         self.th_server = AppServerSubprocess(
-            th_server_app,
+            th_server_no_uid_app,
             storage_dir=self.storage.name,
             port=self.th_server_port,
             discriminator=self.th_server_discriminator,
diff --git a/src/python_testing/TC_MCORE_FS_1_5.py b/src/python_testing/TC_MCORE_FS_1_5.py
@@ -22,8 +22,9 @@
 # test-runner-runs:
 #   run1:
 #     app: examples/fabric-admin/scripts/fabric-sync-app.py
-#     app-args: --app-admin=${FABRIC_ADMIN_APP} --app-bridge=${FABRIC_BRIDGE_APP} --stdin-pipe=dut-fsa-stdin --discriminator=1234
+#     app-args: --app-admin=${FABRIC_ADMIN_APP} --app-bridge=${FABRIC_BRIDGE_APP} --discriminator=1234
 #     app-ready-pattern: "Successfully opened pairing window on the device"
+#     app-stdin-pipe: dut-fsa-stdin
 #     script-args: >
 #       --PICS src/app/tests/suites/certification/ci-pics-values
 #       --storage-path admin_storage.json
diff --git a/src/python_testing/matter_testing_infrastructure/chip/testing/metadata.py b/src/python_testing/matter_testing_infrastructure/chip/testing/metadata.py
