Add audio feature sensors to Spotify #128785
Conversation
|
Hey there @frenck, mind taking a look at this pull request as it has been labeled with an integration ( Code owner commandsCode owners of
|
| with patch("homeassistant.components.spotify.PLATFORMS", [Platform.SENSOR]): | ||
| await setup_integration(hass, mock_config_entry) |
There was a problem hiding this comment.
We should really figure out something for this weird patch behavior we put on everywhere. IMHO integration setup should just be a fixture and this patching should not be needed.
There was a problem hiding this comment.
But I also don't find it ideal to have 5-10 extra mocks in the conftest, while 6-7 are only used in one test only.
It also makes it harder to see what the library should behave in one sight, you then are required to open two files to see the full picture.
There was a problem hiding this comment.
But I also don't find it ideal to have 5-10 extra mocks in the conftest, while 6-7 are only used in one test only.
Not sure how that is related. We can just set up the whole integration. As a matter of fact. Setting up the integration partly as done in these constructs may hide side-effects.
There was a problem hiding this comment.
I don't think I understand what you mean
There was a problem hiding this comment.
it is also not for now, I'm just not happy with seeing this pattern everywhere.
|
Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍 |
Proposed change
Add audio feature sensors to Spotify
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: