Add tests for join and unjoin service calls in Sonos#145602
Conversation
home-assistant
Bot
added
cla-signed
code-quality
has-tests
integration: sonos
small-pr
|
Hey there @jjlawren, mind taking a look at this pull request as it has been labeled with an integration ( Code owner commandsCode owners of
|
erwindouna
left a comment
erwindouna
left a comment
There was a problem hiding this comment.
Looks good to me, @PeteRager!
| soco: MockSoCo, | ||
| service: SonosMockService, | ||
| variables: dict[str, str], | ||
| uui_ds: str | None = None, |
There was a problem hiding this comment.
uui_ds is an optional XML element that may be present in a Sonos zone topology event. Sonos chose the name.
There was a problem hiding this comment.
Is there a better name to pick? Because I can see many people checking this and thinking its a typo
There was a problem hiding this comment.
An option is to update it to use the full name that Sonos sends in the event which is
zone_player_uui_ds_in_group
That's probably the best so that searching for this attribute will also return this mock function parameter.
| with caplog.at_level(logging.WARNING): | ||
| caplog.clear() | ||
| await hass.services.async_call( | ||
| MP_DOMAIN, | ||
| SERVICE_JOIN, | ||
| { | ||
| "entity_id": "media_player.living_room", | ||
| "group_members": ["media_player.bedroom"], | ||
| }, | ||
| blocking=False, | ||
| ) | ||
| await join_complete_event.wait() | ||
| # Fire the ZGS event to update the speaker grouping as the join method is waiting | ||
| # for the speakers to be regrouped. | ||
| group_speakers(soco_living_room, soco_bedroom) | ||
| await hass.async_block_till_done(wait_background_tasks=True) |
There was a problem hiding this comment.
If the joining failed I think we should raise a HomeAssistantError instead so we can tell the user that without them having to look at the logs
There was a problem hiding this comment.
yes, that is a good change.
|
Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍 |
4 participants
Proposed change
Improve test coverage for media_player.join and add test coverage for media_player.unjoin in the Sonos Integration.
Type of change
Additional information
Checklist
ruff format homeassistant tests)If user exposed functionality or configuration variables are added/changed:
If the code communicates with devices, web services, or third-party tools:
Updated and included derived files by running:
python3 -m script.hassfest.requirements_all.txt.Updated by running
python3 -m script.gen_requirements_all.To help with the load of incoming pull requests: