-
-
Notifications
You must be signed in to change notification settings - Fork 34.1k
Use non-autospec mock for Reolink's button tests #146969
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
Conversation
This is an initial PR to completely drop autospec'ed mocks from Reolink. The reason is that autospec made the tests too slow, so the fixture scope was changed to 'module' in #125215 Having the fixture scope as 'module' makes the test methods spill over each other, which makes them fragile and more complex since they have to clean up the mock's state. I will update the remaining platform tests in followup PRs.
Hey there @starkillerOG, mind taking a look at this pull request as it has been labeled with an integration ( Code owner commandsCode owners of
|
@@ -62,6 +62,100 @@ def mock_setup_entry() -> Generator[AsyncMock]: | |||
yield mock_setup_entry | |||
|
|||
|
|||
def _init_host_mock(host_mock: MagicMock) -> None: |
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.
in the final PR, after removing everything that depends on autospec, I'll remove reolink_connect_class()
and just move what's on this method to reolink_host()
.
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.
seems good
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.
LGTM.
Did you check how long it took to run the test before and after this PR?
Running only the |
Perfect, lets wait for the CLI and then we can merge |
Proposed change
This is an initial PR to completely drop autospec'ed mocks from Reolink.
The reason is that autospec made the tests too slow, so the fixture scope was changed to 'module' in #125215
Having the fixture scope as 'module' makes the test methods spill over each other, which makes them fragile and more complex since they have to clean up the mock's state.
I will update the remaining platform tests in followup PRs.
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: