Skip to content

Remove the deprecated callback_handler - #1037

Open
xpconanfan wants to merge 5 commits into
masterfrom
remove-deprecated-callback-handler
Open

xpconanfan wants to merge 5 commits into
masterfrom
remove-deprecated-callback-handler

Conversation

@xpconanfan

@xpconanfan xpconanfan commented Sep 26, 2026 •

Copy link
Copy Markdown
Collaborator

Remove the deprecated callback_handler code for good, and fix the defect in unit tests that manifest due to the removal

@xpconanfan xpconanfan added this to the Mobly Release 1.14 milestone Sep 26, 2026
@xpconanfan
xpconanfan requested a review from ko1in1u September 26, 2026 23:47
@xpconanfan xpconanfan self-assigned this Sep 26, 2026
@xpconanfan
xpconanfan requested review from mhaoli and removed request for ko1in1u September 26, 2026 23:48
- Migrate JsonRpcClientBase._rpc() from the deprecated v1 callback_handler to
  callback_handler_v2.CallbackHandlerV2.
- Define _SOCKET_READ_TIMEOUT and _CALLBACK_DEFAULT_TIMEOUT_SEC directly in
  jsonrpc_client_base.py instead of importing callback_handler.
@xpconanfan
xpconanfan force-pushed the remove-deprecated-callback-handler branch 2 times, most recently from a7ab001 to 6d941e0 Compare September 29, 2026 12:59
@xpconanfan
xpconanfan force-pushed the remove-deprecated-callback-handler branch from 6d941e0 to 0e215be Compare September 29, 2026 16:35
Unblocks the hanging `windows-latest, 3.14` CI job on this PR.

Tests in `snippet_client_v2_test.py` leave `SnippetClientV2` objects with
`host_port` set. When those objects are garbage collected later, after the
test's method-scoped adb mocks are gone, `ClientBase.__del__` ->
`close_connection()` -> `_stop_port_forwarding()` runs a real
`adb forward --list`. Removing `callback_handler_test.py` shifts the
allocation pattern and thus where those finalizers fire, which is enough
to wedge the Windows + Python 3.14 runner mid-session.

* `snippet_client_v2_test.py`: add a cleanup that clears `host_port` on the
  client and its event client so the finalizer is a no-op.
* `android_device_test.py`: mock the standing subprocess in
  `test_AndroidDevice_snippet_cleanup` so it stops starting a real
  `adb logcat`, and stop the services at the end of the test.

Real `adb` launches during a full `pytest tests` run: 18 -> 0 (measured
with a temporary `subprocess.Popen` tracer, 3/3 runs).

This is a targeted mitigation; the broader test-isolation guard and the
library-side finalizer behavior will be addressed in a follow-up PR.
`JsonRpcClientBase` was the 2016 base class for `Sl4aClient` and the v1
`SnippetClient`. SL4A was removed in #939 and the v1 snippet client in
#1021, so nothing in Mobly has subclassed it since. `SnippetClientV2` is
built on `mobly.snippet.client_base.ClientBase` instead.

The error names it re-exported (`Error`, `ApiError`, `ProtocolError`,
`AppStartError`, `AppRestoreConnectionError`) were plain aliases of the
classes in `mobly.snippet.errors`; callers should import those directly.

Also removes its test and the `tests/lib/jsonrpc_client_test_base.py`
helper that only it used, and fixes the copy-pasted class name in
`jsonrpc_shell_base_test.py`.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants