Python framework - write_to_app_pipe updates. #37637
Conversation
…eter app-pid to function. Added log info, added fail when app-pid is not present. Updated doc string. Naming fix
…. Moved pid definition into derived class.
|
Review changes with Changed Files
|
|
PR #37637: Size comparison from 08535fd to 6b17a4d Full report (7 builds for cc32xx, qpg, stm32, tizen)
|
| @@ -968,6 +968,8 @@ def __init__(self, *args): | |||
| self.is_commissioning = False | |||
| # The named pipe name must be set in the derived classes | |||
| self.app_pipe = None | |||
| # The app_pipe_pid pid must be set in the derived class | |||
| self.app_pipe_pid = None | |||
There was a problem hiding this comment.
This comes from the command line, so you an use self.matter_test_config.app_pid as the default directly, without having the tests override this individually.
There was a problem hiding this comment.
Removed those as they wont be needed.
src/python_testing/TC_OCC_3_2.py
Outdated
| @async_test_body | ||
| async def test_TC_OCC_3_2(self): | ||
| endpoint_id = self.get_endpoint() | ||
| node_id = self.dut_node_id | ||
| dev_ctrl = self.default_controller | ||
| self.app_pipe = "/tmp/chip_all_clusters_fifo_" |
There was a problem hiding this comment.
This is also something that should come in on the command line rather than being hard coded. This makes it so that you can only run this test against the one app specified in the CI config.
There was a problem hiding this comment.
If this is going to be sent over an argument , should it be sent using --string-arg or should it use a dedicated argument like is the case for --app-pid? . Also this implementation raises a different question that if is possible to have more than one app to pipe commands to?
There was a problem hiding this comment.
Udpated to use to arguments -app-pid and --app-pipe from the args.
No need to use hardcoded values in the code.
…he command line to build the app file to send the out of band command. Implemetation the same for CI and SSH
…name to use the values from arguments while on CI
| 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: | ||
| logger.info(f"Sending out-of-band command: {command} to file: {app_pipe_name}") | ||
| app_pipe.write(command + "\n") |
There was a problem hiding this comment.
adding app_pipe.flush() should ensure immediate write and we can remove the sleep then
There was a problem hiding this comment.
The sleep is used to emulate the linux app Acknowledgement , but this needs to be implemented in the app and then implement the ack check on this side.
There was a problem hiding this comment.
Generally it looks ok to me, but I have one concern, we have parallel activity to extract commonly used functions/classes to separate modules project-chip/matter-test-scripts#443 . My understanding is that after split complete we would have only Base class in matter_testing.py that should handle only test-related stuff like steps, init, teardown, test config, stack management and so on and this function looks like really utility one that should be places in some separate module.
FYI @cecille
Fixes #32139
Description
Improve the base function
write_to_app_pipefrommatter_testig.py.Update the implementations for
write_to_app_pipein the test cases.Ensure the test cases are working as expected on CI.
Ack is not implemented in this task. As it should be implemented by the app.
Found files using
write_to_app_pipe:Excluded: src/python_testing/TC_OpstateCommon.py
Updated test cases:
Testing
Commands executed to test the updated files: