Skip to content
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

Upgrade pytest-aiohttp #82475

Merged
merged 10 commits into from
Nov 29, 2022
Merged

Upgrade pytest-aiohttp #82475

merged 10 commits into from
Nov 29, 2022

Conversation

elupus
Copy link
Contributor

@elupus elupus commented Nov 21, 2022

Proposed change

Attempt to bump pytest-aiohttp

Type of change

  • Dependency upgrade
  • Bugfix (non-breaking change which fixes an issue)
  • New integration (thank you!)
  • New feature (which adds functionality to an existing integration)
  • Deprecation (breaking change to happen in the future)
  • Breaking change (fix/feature causing existing functionality to break)
  • Code quality improvements to existing code or addition of tests

Additional information

Checklist

  • The code change is tested and works locally.
  • Local tests pass. Your PR cannot be merged unless tests pass
  • There is no commented out code in this PR.
  • I have followed the development checklist
  • The code has been formatted using Black (black --fast homeassistant tests)
  • Tests have been added to verify that the new code works.

If user exposed functionality or configuration variables are added/changed:

If the code communicates with devices, web services, or third-party tools:

  • The manifest file has all fields filled out correctly.
    Updated and included derived files by running: python3 -m script.hassfest.
  • New or updated dependencies have been added to requirements_all.txt.
    Updated by running python3 -m script.gen_requirements_all.
  • For the updated dependencies - a link to the changelog, or at minimum a diff between library versions is added to the PR description.
  • Untested files have been added to .coveragerc.

To help with the load of incoming pull requests:

Copy link
Member

@janiversen janiversen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

"""Freeze clock for tests."""
yield


Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A bit unclear if this is needed after the garbage collect change. But i still think it make sense.

yield

event_loop.run_until_complete(event_loop.shutdown_default_executor())

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we could ignore the executor threads easily in the below threads check, we dont need to stop these here.

Copy link
Member

@frenck frenck left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@elupus A merge conflict appeared; otherwise, it looks good to me (and I don't have an answer for excluding the executor threads above).

Some test will trigger warnings on garbage collect, these warnings
spills over into next test.

Some test trigger tasks that raise errors on shutdown, these spill
over into next test.

This is to mimic older pytest-aiohttp and it's behaviour on test
cleanup.

Discussions on similar changes for pytest-aiohttp are here:
pytest-dev/pytest-asyncio#309
  /home-assistant/homeassistant/helpers/template.py:2082: RuntimeWarning: coroutine 'AsyncMockMixin._execute_mock_call' was never awaited
    def wrapper(*args, **kwargs):
  Enable tracemalloc to get traceback where the object was allocated.
  See https://docs.pytest.org/en/stable/how-to/capture-warnings.html#resource-warnings for more info.
The times are simulated anyway, and we can't stop the normal
event from occuring.
tests/components/motioneye/test_camera.py::test_get_still_image_from_camera
tests/components/motioneye/test_camera.py::test_get_still_image_from_camera
tests/components/motioneye/test_camera.py::test_get_stream_from_camera
tests/components/motioneye/test_camera.py::test_get_stream_from_camera
tests/components/motioneye/test_camera.py::test_camera_option_stream_url_template
tests/components/motioneye/test_camera.py::test_camera_option_stream_url_template
  /Users/joakim/src/hass/home-assistant/venv/lib/python3.9/site-packages/aiohttp/web_urldispatcher.py:189: DeprecationWarning: Bare functions are deprecated, use async ones
    warnings.warn(
The tests allowed clock to tick in between steps
Old tests would trigger attempts to post to could services:

```
DEBUG:aioskybell:HTTP post https://cloud.myskybell.com/api/v3/login/ Request with headers: {'content-type': 'application/json', 'accept': '*/*', 'x-skybell-app-id': 'd2b542c7-a7e4-4e1e-b77d-2b76911c7c46', 'x-skybell-client-id': '1f36a3c0-6dee-4997-a6db-4e1c67338e57'}
```
@elupus elupus merged commit c576a68 into home-assistant:dev Nov 29, 2022
@elupus elupus deleted the pytest/asyncio branch November 29, 2022 21:36
# before moving on to the next test.
tasks = asyncio.all_tasks(event_loop) - tasks_before
for task in tasks:
warnings.warn(f"Linger task after test {task}")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lingering task

handle.cancel()

# Make sure garbage collect run in same test as allocation
# this is to mimic the behavior of pytest-aiohttp, and is
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Start a new sentence on this line.

@github-actions github-actions bot locked and limited conversation to collaborators Dec 1, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants