Skip to content

Commit a627b91

Browse files
arkqandy31415
andauthored
Fail python tests run on unhandled asyncio exceptions (project-chip#41976)
* Improve typing in the matter/clusters/Command.py * Replace get_event_loop with get_running_loop to ensure loop was created elsewhere * Fail the test run on unhandled exceptions * Refactor _handleError() logic to be more readable * Update according to LLM suggestions --------- Co-authored-by: Andrei Litvin <[email protected]>
1 parent 6a456c3 commit a627b91

File tree

9 files changed

+83
-93
lines changed

9 files changed

+83
-93
lines changed

examples/fabric-admin/scripts/fabric-sync-app.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ async def forward_f(prefix: bytes, f_in: asyncio.StreamReader,
4242

4343
async def forward_stdin(f_out: asyncio.StreamWriter):
4444
"""Forward stdin to f_out."""
45-
loop = asyncio.get_event_loop()
45+
loop = asyncio.get_running_loop()
4646
reader = asyncio.StreamReader()
4747
protocol = asyncio.StreamReaderProtocol(reader)
4848
await loop.connect_read_pipe(lambda: protocol, sys.stdin)
@@ -175,7 +175,7 @@ async def main(args):
175175
passcode=args.passcode,
176176
))
177177

178-
loop = asyncio.get_event_loop()
178+
loop = asyncio.get_running_loop()
179179

180180
def terminate():
181181
with contextlib.suppress(ProcessLookupError):

scripts/tests/chipyaml/runner.py

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -280,8 +280,7 @@ def parse(parser_group: ParserGroup):
280280
runner_config = None
281281

282282
runner = TestRunner()
283-
loop = asyncio.get_event_loop()
284-
return loop.run_until_complete(runner.run(parser_group.builder_config, runner_config))
283+
return asyncio.run(runner.run(parser_group.builder_config, runner_config))
285284

286285

287286
@runner_base.command()
@@ -291,8 +290,7 @@ def dry_run(parser_group: ParserGroup):
291290
runner_config = TestRunnerConfig(hooks=TestRunnerLogger())
292291

293292
runner = TestRunner()
294-
loop = asyncio.get_event_loop()
295-
return loop.run_until_complete(runner.run(parser_group.builder_config, runner_config))
293+
return asyncio.run(runner.run(parser_group.builder_config, runner_config))
296294

297295

298296
@runner_base.command()
@@ -306,8 +304,7 @@ def run(parser_group: ParserGroup, adapter: str, stop_on_error: bool, stop_on_wa
306304
runner_config = TestRunnerConfig(adapter, parser_group.pseudo_clusters, runner_options, runner_hooks)
307305

308306
runner = TestRunner()
309-
loop = asyncio.get_event_loop()
310-
return loop.run_until_complete(runner.run(parser_group.builder_config, runner_config))
307+
return asyncio.run(runner.run(parser_group.builder_config, runner_config))
311308

312309

313310
@runner_base.command()
@@ -330,8 +327,7 @@ def websocket(parser_group: ParserGroup, adapter: str, stop_on_error: bool, stop
330327
server_address, server_port, server_path, server_arguments, websocket_runner_hooks)
331328

332329
runner = WebSocketRunner(websocket_runner_config)
333-
loop = asyncio.get_event_loop()
334-
return loop.run_until_complete(runner.run(parser_group.builder_config, runner_config))
330+
return asyncio.run(runner.run(parser_group.builder_config, runner_config))
335331

336332

337333
@runner_base.command()
@@ -349,8 +345,7 @@ def matter_repl(parser_group: ParserGroup, adapter: str, stop_on_error: bool, st
349345
if commission_on_network_dut:
350346
node_id_to_commission = parser_group.builder_config.parser_config.config_override['nodeId']
351347
runner = __import__(runner, fromlist=[None]).Runner(repl_storage_path, node_id_to_commission=node_id_to_commission)
352-
loop = asyncio.get_event_loop()
353-
return loop.run_until_complete(runner.run(parser_group.builder_config, runner_config))
348+
return asyncio.run(runner.run(parser_group.builder_config, runner_config))
354349

355350

356351
@runner_base.command()

src/controller/python/matter/ChipStack.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,7 @@ class AsyncioCallableHandle:
110110

111111
def __init__(self, callback):
112112
self._callback = callback
113-
self._loop = asyncio.get_event_loop()
113+
self._loop = asyncio.get_running_loop()
114114
self._future = self._loop.create_future()
115115
self._result = None
116116
self._exception = None

src/controller/python/matter/clusters/Command.py

Lines changed: 48 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -79,26 +79,24 @@ def FindCommandClusterObject(isClientSideCommand: bool, path: CommandPath):
7979

8080

8181
class AsyncCommandTransaction:
82-
def __init__(self, future: Future, eventLoop, expectType: Type):
82+
def __init__(self, future: Future, eventLoop, expectType: type[ClusterCommand] | None):
8383
self._event_loop = eventLoop
8484
self._future = future
8585
self._expect_type = expectType
8686

8787
def _handleResponse(self, path: CommandPath, status: Status, response: bytes):
88-
if (len(response) == 0):
88+
if len(response) == 0:
8989
self._future.set_result(None)
9090
else:
9191
# If a type hasn't been assigned, let's auto-deduce it.
92-
if (self._expect_type is None):
92+
if self._expect_type is None:
9393
self._expect_type = FindCommandClusterObject(False, path)
9494

9595
if self._expect_type:
9696
try:
97-
self._future.set_result(
98-
self._expect_type.FromTLV(response))
97+
self._future.set_result(self._expect_type.FromTLV(response))
9998
except Exception as ex:
100-
self._handleError(
101-
status, 0, ex)
99+
self._handleError(status, PyChipError.from_code(0), ex)
102100
else:
103101
self._future.set_result(None)
104102

@@ -107,101 +105,89 @@ def handleResponse(self, path: CommandPath, index: int, status: Status, response
107105
# checking `index`. We just share a callback API with batch commands. If we ever get a
108106
# second call to `handleResponse` we will see a different error on trying to set future
109107
# that has already been set.
110-
self._event_loop.call_soon_threadsafe(
111-
self._handleResponse, path, status, response)
112-
113-
def _handleError(self, imError: Status, chipError: PyChipError, exception: Exception):
114-
if exception:
115-
self._future.set_exception(exception)
116-
elif chipError != 0:
117-
self._future.set_exception(chipError.to_exception())
118-
else:
108+
self._event_loop.call_soon_threadsafe(self._handleResponse, path, status, response)
109+
110+
def _handleError(self, imError: Status, chipError: PyChipError, exception: Exception | None):
111+
if chipError != 0:
112+
exception = chipError.to_exception()
113+
if exception is None:
119114
try:
120115
# If you got an exception from this call other than AttributeError please
121116
# add it to the except block below. We changed Exception->AttributeError as
122117
# that is what we thought we are trying to catch here.
123-
self._future.set_exception(
124-
InteractionModelError(InteractionModelStatus(imError.IMStatus), imError.ClusterStatus))
118+
exception = InteractionModelError(InteractionModelStatus(imError.IMStatus), imError.ClusterStatus)
125119
except AttributeError:
126-
logger.exception("Failed to map interaction model status received: %s. Remapping to Failure." % imError)
127-
self._future.set_exception(InteractionModelError(
128-
InteractionModelStatus.Failure, imError.ClusterStatus))
120+
logger.exception("Failed to map interaction model status received: %s. Remapping to Failure.", imError)
121+
exception = InteractionModelError(InteractionModelStatus.Failure, imError.ClusterStatus)
122+
self._future.set_exception(exception)
129123

130124
def handleError(self, status: Status, chipError: PyChipError):
131-
self._event_loop.call_soon_threadsafe(
132-
self._handleError, status, chipError, None
133-
)
125+
self._event_loop.call_soon_threadsafe(self._handleError, status, chipError, None)
134126

135127
def handleDone(self):
136128
ctypes.pythonapi.Py_DecRef(ctypes.py_object(self))
137129

138130

139131
class AsyncBatchCommandsTransaction:
140-
def __init__(self, future: Future, eventLoop, expectTypes: List[Type]):
132+
def __init__(self, future: Future, eventLoop, expectTypes: list[type[ClusterCommand] | None]):
141133
self._event_loop = eventLoop
142134
self._future = future
143135
self._expect_types = expectTypes
144-
default_im_failure = InteractionModelError(
145-
InteractionModelStatus.NoCommandResponse)
146-
self._responses = [default_im_failure] * len(expectTypes)
136+
default_im_failure = InteractionModelError(InteractionModelStatus.NoCommandResponse)
137+
self._responses: list[ClusterCommand | InteractionModelError | None] = [default_im_failure] * len(expectTypes)
147138

148139
def _handleResponse(self, path: CommandPath, index: int, status: Status, response: bytes):
149140
if index > len(self._responses):
150-
self._handleError(status, 0, IndexError(f"CommandSenderCallback has given us an unexpected index value {index}"))
141+
self._handleError(status, PyChipError.from_code(0),
142+
IndexError(f"CommandSenderCallback has given us an unexpected index value {index}"))
151143
return
152144

153145
if status.IMStatus != InteractionModelStatus.Success:
154146
try:
155147
self._responses[index] = InteractionModelError(
156148
InteractionModelStatus(status.IMStatus), status.ClusterStatus)
157149
except AttributeError as ex:
158-
self._handleError(status, 0, ex)
159-
elif (len(response) == 0):
150+
self._handleError(status, PyChipError.from_code(0), ex)
151+
elif len(response) == 0:
160152
self._responses[index] = None
161153
else:
162154
# If a type hasn't been assigned, let's auto-deduce it.
163-
if (self._expect_types[index] is None):
155+
if self._expect_types[index] is None:
164156
self._expect_types[index] = FindCommandClusterObject(False, path)
165157

166-
if self._expect_types[index]:
158+
if expectType := self._expect_types[index]:
167159
try:
168160
# If you got an exception from this call other than AttributeError please
169161
# add it to the except block below. We changed Exception->AttributeError as
170162
# that is what we thought we are trying to catch here.
171-
self._responses[index] = self._expect_types[index].FromTLV(response)
163+
self._responses[index] = expectType.FromTLV(response)
172164
except AttributeError as ex:
173-
self._handleError(status, 0, ex)
165+
self._handleError(status, PyChipError.from_code(0), ex)
174166
else:
175167
self._responses[index] = None
176168

177169
def handleResponse(self, path: CommandPath, index: int, status: Status, response: bytes):
178-
self._event_loop.call_soon_threadsafe(
179-
self._handleResponse, path, index, status, response)
170+
self._event_loop.call_soon_threadsafe(self._handleResponse, path, index, status, response)
180171

181-
def _handleError(self, imError: Status, chipError: PyChipError, exception: Exception):
172+
def _handleError(self, imError: Status, chipError: PyChipError, exception: Exception | None):
182173
if self._future.done():
183-
logger.exception(f"Recieved another error. Only expecting one error. imError:{imError}, chipError {chipError}")
174+
logger.exception(f"Received another error. Only expecting one error. imError:{imError}, chipError {chipError}")
184175
return
185-
if exception:
186-
self._future.set_exception(exception)
187-
elif chipError != 0:
188-
self._future.set_exception(chipError.to_exception())
189-
else:
176+
if chipError != 0:
177+
exception = chipError.to_exception()
178+
if exception is None:
190179
try:
191180
# If you got an exception from this call other than AttributeError please
192181
# add it to the except block below. We changed Exception->AttributeError as
193182
# that is what we thought we are trying to catch here.
194-
self._future.set_exception(
195-
InteractionModelError(InteractionModelStatus(imError.IMStatus), imError.ClusterStatus))
183+
exception = InteractionModelError(InteractionModelStatus(imError.IMStatus), imError.ClusterStatus)
196184
except AttributeError:
197-
logger.exception("Failed to map interaction model status received: %s. Remapping to Failure." % imError)
198-
self._future.set_exception(InteractionModelError(
199-
InteractionModelStatus.Failure, imError.ClusterStatus))
185+
logger.exception("Failed to map interaction model status received: %s. Remapping to Failure.", imError)
186+
exception = InteractionModelError(InteractionModelStatus.Failure, imError.ClusterStatus)
187+
self._future.set_exception(exception)
200188

201189
def handleError(self, status: Status, chipError: PyChipError):
202-
self._event_loop.call_soon_threadsafe(
203-
self._handleError, status, chipError, None
204-
)
190+
self._event_loop.call_soon_threadsafe(self._handleError, status, chipError, None)
205191

206192
def _handleDone(self):
207193
# Future might already be set with exception from `handleError`
@@ -210,13 +196,11 @@ def _handleDone(self):
210196
ctypes.pythonapi.Py_DecRef(ctypes.py_object(self))
211197

212198
def handleDone(self):
213-
self._event_loop.call_soon_threadsafe(
214-
self._handleDone
215-
)
199+
self._event_loop.call_soon_threadsafe(self._handleDone)
216200

217201

218202
class TestOnlyAsyncBatchCommandsTransaction(AsyncBatchCommandsTransaction):
219-
def __init__(self, future: Future, eventLoop, expectTypes: List[Type]):
203+
def __init__(self, future: Future, eventLoop, expectTypes: list[type[ClusterCommand] | None]):
220204
self._responseMessageCount = 0
221205
super().__init__(future, eventLoop, expectTypes)
222206

@@ -264,7 +248,8 @@ def _TestOnlyOnCommandSenderDoneCallback(closure, testOnlyDoneInfo: TestOnlyPyOn
264248
closure.handleDone()
265249

266250

267-
def TestOnlySendCommandTimedRequestFlagWithNoTimedInvoke(future: Future, eventLoop, responseType, device, commandPath, payload):
251+
def TestOnlySendCommandTimedRequestFlagWithNoTimedInvoke(future: Future, eventLoop,
252+
responseType: type[ClusterCommand] | None, device, commandPath, payload):
268253
''' ONLY TO BE USED FOR TEST: Sends the payload with a TimedRequest flag but no TimedInvoke transaction
269254
'''
270255
if (responseType is not None) and (not issubclass(responseType, ClusterCommand)):
@@ -285,7 +270,8 @@ def TestOnlySendCommandTimedRequestFlagWithNoTimedInvoke(future: Future, eventLo
285270
))
286271

287272

288-
async def SendCommand(future: Future, eventLoop, responseType: Type, device, commandPath: CommandPath, payload: ClusterCommand,
273+
async def SendCommand(future: Future, eventLoop, responseType: type[ClusterCommand] | None,
274+
device, commandPath: CommandPath, payload: ClusterCommand,
289275
timedRequestTimeoutMs: Union[None, int] = None, interactionTimeoutMs: Union[None, int] = None,
290276
busyWaitMs: Union[None, int] = None, suppressResponse: Union[None, bool] = None, allowLargePayload: Union[None, bool] = None) -> PyChipError:
291277
''' Send a cluster-object encapsulated command to a device and does the following:
@@ -322,7 +308,8 @@ async def SendCommand(future: Future, eventLoop, responseType: Type, device, com
322308
))
323309

324310

325-
def _BuildPyInvokeRequestData(commands: List[InvokeRequestInfo], timedRequestTimeoutMs: Optional[int], responseTypes, suppressTimedRequestMessage: bool = False) -> List[PyInvokeRequestData]:
311+
def _BuildPyInvokeRequestData(commands: List[InvokeRequestInfo], timedRequestTimeoutMs: Optional[int],
312+
responseTypes: list[type[ClusterCommand] | None], suppressTimedRequestMessage: bool = False) -> List[PyInvokeRequestData]:
326313
numberOfCommands = len(commands)
327314
pyBatchCommandsDataArrayType = PyInvokeRequestData * numberOfCommands
328315
pyBatchCommandsData = pyBatchCommandsDataArrayType()
@@ -377,7 +364,7 @@ async def SendBatchCommands(future: Future, eventLoop, device, commands: List[In
377364
'''
378365
handle = GetLibraryHandle()
379366

380-
responseTypes: List[Type] = []
367+
responseTypes: list[type[ClusterCommand] | None] = []
381368
pyBatchCommandsData = _BuildPyInvokeRequestData(commands, timedRequestTimeoutMs, responseTypes)
382369

383370
transaction = AsyncBatchCommandsTransaction(future, eventLoop, responseTypes)
@@ -412,7 +399,7 @@ def TestOnlySendBatchCommands(future: Future, eventLoop, device, commands: List[
412399

413400
handle = GetLibraryHandle()
414401

415-
responseTypes: List[Type] = []
402+
responseTypes: list[type[ClusterCommand] | None] = []
416403
pyBatchCommandsData = _BuildPyInvokeRequestData(commands, timedRequestTimeoutMs,
417404
responseTypes, suppressTimedRequestMessage=suppressTimedRequestMessage)
418405

src/controller/python/matter/webrtc/browser_webrtc_client.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ def __init__(self, ws_client: AsyncWebSocketClient, id: int):
2727
self.event_callbacks: dict[str, Callable] = {}
2828
self.ws_client = ws_client
2929
self.pending_cmd_responses: dict[str, asyncio.Future] = {}
30-
self.event_loop = asyncio.get_event_loop()
30+
self.event_loop = asyncio.get_running_loop()
3131
self._event_callbacks_without_parameter = ("GATHERING_STATE_COMPLETE",)
3232
self.id = id
3333

src/controller/python/tests/scripts/commissioning_window_test.py

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -116,9 +116,7 @@ async def main():
116116

117117
if __name__ == "__main__":
118118
try:
119-
loop = asyncio.get_event_loop()
120-
loop.run_until_complete(main())
121-
loop.close()
119+
asyncio.run(main())
122120
except Exception as ex:
123121
logger.exception(ex)
124122
TestFail("Exception occurred when running tests.")

src/controller/python/tests/scripts/icd_device_test.py

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -145,9 +145,7 @@ async def main():
145145

146146
if __name__ == "__main__":
147147
try:
148-
loop = asyncio.get_event_loop()
149-
loop.run_until_complete(main())
150-
loop.close()
148+
asyncio.run(main())
151149
except Exception as ex:
152150
logger.exception(ex)
153151
TestFail("Exception occurred when running tests.")

src/python_testing/matter_testing_infrastructure/matter/testing/runner.py

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -405,8 +405,7 @@ def run_tests_no_exit(
405405

406406
# NOTE: It's not possible to pass event loop via Mobly TestRunConfig user params, because the
407407
# Mobly deep copies the user params before passing them to the test class and the event
408-
# loop is not serializable. So, we are setting the event loop as a test
409-
# class member.
408+
# loop is not serializable. So, we are setting the event loop as a test class member.
410409
CommissionDeviceTest.event_loop = event_loop
411410
test_class.event_loop = event_loop
412411

@@ -455,6 +454,15 @@ def run_tests_no_exit(
455454
# Execute the test class with the config
456455
ok = True
457456

457+
def _handler(loop, context):
458+
loop.default_exception_handler(context)
459+
nonlocal ok
460+
# Fail the test run on unhandled exceptions.
461+
ok = False
462+
463+
# Set custom exception handler to catch unhandled exceptions.
464+
event_loop.set_exception_handler(_handler)
465+
458466
runner = TestRunner(log_dir=test_config.log_path,
459467
testbed_name=test_config.testbed_name)
460468

0 commit comments

Comments
 (0)