New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Replace/restructure HomeWizard device fixtures to reflect reality #103311
Conversation
Hey there @DCSBL, mind taking a look at this pull request as it has been labeled with an integration ( Code owner commandsCode owners of
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! Only one small remark but I don't think it is an issue.
@@ -26,7 +26,7 @@ | |||
] | |||
|
|||
|
|||
@pytest.mark.parametrize("device_fixture", ["device-HWE-SKT.json"]) | |||
@pytest.mark.parametrize("device_fixture", ["HWE-SKT"]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure if this adds anything useful, but HWE-P1, SDM230/SDM630 also support cloud_enabled
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tried to not to broaden the scope of this PR. As in, the fixtures can be extended, and the tests as well. But the changeset is already quite large :)
37c065f
to
318a34c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me, noticed one small addition I just wanted to point out.
@pytest.mark.parametrize( | ||
"device_fixture", ["device-HWE-P1.json", "device-HWE-SKT.json"] | ||
) | ||
@pytest.mark.parametrize("device_fixture", ["HWE-P1", "HWE-SKT", "SDM230"]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
SDM230 wasn't part of this before.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That is fine, it was more added to put the fixture to use :)
The output is almost the same as HWE-SKT (which is now more pure and thus misses a case), thus, the SDM230 was added here to keep test coverage at full.
Previously the fixture haven't been actually reflecting their devices, now they do.
Proposed change
Adjusts the device fixtures structure of HomeWizard, to allow a correct set of fixtures to be provided for each specific device.
This is done by introducing a set of fixtures per device/model/situation. This move is needed to be able to test entities/situations for specific devices in follow-up PRs.
Type of change
Additional information
Checklist
black --fast 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
..coveragerc
.To help with the load of incoming pull requests: