Skip to content

tests: cover existing pair() and unpair() behavior#1993

Open
JPHutchins wants to merge 1 commit into
hbldh:developfrom
JPHutchins:pairing-tests-existing
Open

tests: cover existing pair() and unpair() behavior#1993
JPHutchins wants to merge 1 commit into
hbldh:developfrom
JPHutchins:pairing-tests-existing

Conversation

@JPHutchins

Copy link
Copy Markdown
Contributor

Split out of #1990 — a regression net for the current pair() / unpair() behavior, landed first so the pairing-agent work that follows has coverage to change against.

What

Adds unit and integration coverage for existing behavior, with no production changes:

  • tests/backends/winrt/test_pairing.py, tests/backends/bluezdbus/test_pairing.py — unit coverage of the existing pair() / unpair() paths.
  • tests/integration/test_client_pairing.py — bumble integration tests: unpair() of a connected, never-paired device; CoreBluetooth raises NotImplementedError.

Testing

  • Integration tests run on the BlueZ VHCI leg (bumble) in CI — no hardware required.
  • CoreBluetooth cases are skipif-guarded.

🤖 Generated with Claude Code

Copilot AI review requested due to automatic review settings June 1, 2026 00:33
@codecov

codecov Bot commented Jun 1, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 52.79%. Comparing base (ae2f589) to head (d70bd60).

Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #1993      +/-   ##
===========================================
+ Coverage    52.45%   52.79%   +0.34%     
===========================================
  Files           43       43              
  Lines         4097     4097              
  Branches       504      504              
===========================================
+ Hits          2149     2163      +14     
+ Misses        1817     1801      -16     
- Partials       131      133       +2     
Flag Coverage Δ
bluez-integration-py310 39.49% <ø> (+0.34%) ⬆️
bluez-integration-py311 39.49% <ø> (+0.34%) ⬆️
bluez-integration-py312 39.49% <ø> (+0.34%) ⬆️
bluez-integration-py313 39.49% <ø> (+0.34%) ⬆️
bluez-integration-py314 37.99% <ø> (+0.35%) ⬆️
macos-latest-py310 20.03% <ø> (ø)
macos-latest-py311 20.03% <ø> (ø)
macos-latest-py312 20.03% <ø> (ø)
macos-latest-py313 20.03% <ø> (ø)
macos-latest-py314 19.84% <ø> (ø)
ubuntu-latest-py310 23.94% <ø> (ø)
ubuntu-latest-py311 23.94% <ø> (ø)
ubuntu-latest-py312 23.94% <ø> (ø)
ubuntu-latest-py313 23.94% <ø> (ø)
ubuntu-latest-py314 22.05% <ø> (ø)
windows-latest-py310 18.72% <ø> (ø)
windows-latest-py311 18.72% <ø> (ø)
windows-latest-py312 18.72% <ø> (ø)
windows-latest-py313 18.72% <ø> (ø)
windows-latest-py314 18.44% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@dlech

dlech commented Jun 1, 2026

Copy link
Copy Markdown
Collaborator

I don't consider the tests with mocks very helpful since it is quite difficult mock how OS-specific Bluetooth works and it tends to change over time.

I would rather have just have integration tests for everything.

@dlech

dlech commented Jun 1, 2026

Copy link
Copy Markdown
Collaborator

For example, we should have an integration test where the peripheral returns the error code “Pairing Not Supported” to make sure we have a common exception raised on all platforms.

We might also want to have integrations tests for both peripherals that support "LE legacy pairing" (temporary key + short term key) and ones that support "LE Secure Connections" (long term key) to make sure the OS handles both (parameterized tests). Hopefully the OS handles the difference between these transparently, but one never knows.

I probably need to do some more reading to see if there are any other interesting cases we would want to test.

@JPHutchins

Copy link
Copy Markdown
Contributor Author

I will remove the mocked tests and leave in the integration test.

Lock in the current BlueZ and WinRT pair()/unpair() contract before the
pairing-agent series reworks it, so later refactors cannot silently
regress today's behavior.

- winrt: idempotent already-paired return, can_pair guard, Just Works
  success + handler detach, result-status failure, the deprecated
  protection_level keyword, and unpair status handling.
- bluez: already-paired return, Trusted-then-Pair + resolvable-private-
  address refresh, D-Bus failure propagation (no agent is registered on
  develop, so a real Pair fails, cf. hbldh#1380 / hbldh#1434), and unpair
  RemoveDevice / DoesNotExist mapping.
- integration: a connect/disconnect/unpair smoke test for the existing
  unpair path.

Expands the integration harness from hbldh#1888. Addresses dlech's request on
hbldh#1990 to split the work and add tests for existing pairing functions
first. Part of the pairing series continuing hbldh#1864.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@JPHutchins JPHutchins force-pushed the pairing-tests-existing branch from 471c094 to d70bd60 Compare June 1, 2026 20:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants