Harden blueman/main/Sendto.py: reliability, UX, portability, tests - #3319
Open
geraldo-netto wants to merge 9 commits into
Open
Harden blueman/main/Sendto.py: reliability, UX, portability, tests#3319geraldo-netto wants to merge 9 commits into
geraldo-netto wants to merge 9 commits into
Conversation
on_transfer_progress throttled speed/ETA updates with time.time(); a backward wall-clock step (NTP or manual) stalled all updates until real time caught up, and a forward step fired every call. Switch the throttle to time.monotonic(), which never steps. Add test/main/test_sendto.py with a Sender built via __new__ (no GTK/D-Bus init) covering the throttle window and progress accumulation. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The ETA computation relied on catching ZeroDivisionError to handle a zero speed, and the progress-bar fraction divided by total_bytes with no guard at all — a transfer of only empty files (total_bytes == 0) raised an uncaught ZeroDivisionError. Replace the exception flow with an explicit `if spd > 0` guard (logging when ETA can't be estimated) and only update the fraction when total_bytes > 0. Add tests for zero speed, zero total_bytes, and the normal ETA path. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Replace the explicit per-UUID loop with a single any() over the device's UUIDs checking for the OBEX Object Push service class. Same behaviour, less boilerplate. Add tests for present / absent / empty UUID lists. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
SendTo.__init__ repeated the connect_signal boilerplate six times across the manager and the any-adapter/any-device watchers. Add a _setup_signal_handlers(source, handlers) helper that connects each signal to its (callback, *args) and route all six wirings through it. connect_signal is an alias of GObject.connect, so behaviour is unchanged. Add tests for multi-handler wiring and the empty case. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
PyGObject deprecated GLib.Error.message. In the obex client-start error path, switch both the StartServiceByName check and the debug log to str(e), which yields the same message without the deprecated attribute. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
A mistyped -s/--source previously only logged "Unknown adapter, trying first available" to the console before silently falling back, so a CLI user never learned their choice was ignored. Also print a clear notice to stderr naming the unknown adapter and the fallback. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Launched from a file manager's "Send To", sendto previously aborted with only a console log when no Bluetooth adapter was present, leaving the user with no on-screen feedback. Show an ErrorDialog explaining that an adapter must be connected/enabled (mirroring check_bluetooth_status) before exiting. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
After stopping discovery, Sender.__init__ called time.sleep(1) on the UI thread to let the adapter settle, freezing the dialog for a second. Replace it with GLib.timeout_add_seconds scheduling a one-shot _create_session_timeout, so the UI stays responsive and the session is created after the delay. Add a test that the timer callback creates the session once and returns False. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add headless tests (Sender/SendTo built via __new__) for the manager and property-changed signal dispatch, _start_discovery, the progress-bar text formatter, and the send queue (process_queue / on_transfer_completed), raising in-scope coverage of the testable logic to ~99%. The GTK/D-Bus __init__ and modal dialog paths remain outside headless reach. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Works through the open
blueman/main/Sendto.pyfindings. One commit per fix; adds the first test file for this module (test/main/test_sendto.py).Reliability
time.monotonic()so a wall-clock step can't stall or spam the UI.ZeroDivisionErrorflow inon_transfer_progresswith an explicitspd > 0guard, and only update the progress fraction whentotal_bytes > 0(a transfer of only empty files previously raised an uncaughtZeroDivisionError).UX
time.sleep(1)after stopping discovery — defer session creation via a one-shotGLib.timeout_add_seconds.--sourcenames an unknown adapter instead of silently falling back.Cleanup / portability
_has_objpushto a singleany()._setup_signal_handlersfor the repeatedconnect_signalwiring.str(e)instead of the deprecatedGLib.Error.message.Tests
test/main/test_sendto.py(26 tests) usingSender/SendTobuilt via__new__to exercise the logic methods without GTK/D-Bus init.__init__and modal-dialog paths are not headless-reachable, so whole-file coverage is lower by nature.mypy -p blueman --strictclean; flake8 (core codes) clean.Deferred
dialog.run()/.destroy()→ async response handlers): three of the four sites (select_files,select_device, the obex-start error dialog) are synchronous startup gates in__init__whose return values drive control flow before the GTK main loop runs. Converting to async response-signals requires restructuring both__init__s into continuation-based flows — not headless-testable and high regression risk on the core send path. Left for a dedicated GTK4-era change.🤖 Generated with Claude Code