Skip to content

Rewrite recorder unittest tests to pytest style test function#41264

Merged
balloob merged 2 commits intodevfrom
unknown repository
Oct 6, 2020
Merged

Rewrite recorder unittest tests to pytest style test function#41264
balloob merged 2 commits intodevfrom
unknown repository

Conversation

@ghost
Copy link
Copy Markdown

@ghost ghost commented Oct 5, 2020

Proposed change

Rewrite unit tests in the following files to be pytest functions:

  • tests/components/recorder/test_init.py
  • tests/components/recorder/test_models.py
  • tests/components/recorder/test_purge.py

All other tests on the recorder module are already pytest functions.

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)
  • 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.
  • Untested files have been added to .coveragerc.

The integration reached or maintains the following Integration Quality Scale:

  • No score or internal
  • 🥈 Silver
  • 🥇 Gold
  • 🏆 Platinum

To help with the load of incoming pull requests:

Comment thread tests/components/recorder/conftest.py
Comment thread tests/components/recorder/conftest.py
@balloob
Copy link
Copy Markdown
Member

balloob commented Oct 6, 2020

Oh no I broke it, I am so sorry.

@balloob
Copy link
Copy Markdown
Member

balloob commented Oct 6, 2020

I removed my "improvement" and your fixes. I couldn't get it working locally either 🤔
I think it has to do with hass being an async fixture, which I think then somehow ends up running sync tests inside the event loop thread and then never finish a block 🤷

@ghost
Copy link
Copy Markdown
Author

ghost commented Oct 6, 2020

I got it working now. Tests on another component were dependent on the common functions to be synchronous.
I added both sync and async functions for now and once the history and the logbook component tests are refactored then those can be removed.

@balloob
Copy link
Copy Markdown
Member

balloob commented Oct 6, 2020

If you want we can chop off the last 5 commits, have the tests pas (as they before my change) and call it a day.

@ghost
Copy link
Copy Markdown
Author

ghost commented Oct 6, 2020

I would like to investigate the issue but I'm not managing to make it fail locally. Any ideas?

@balloob
Copy link
Copy Markdown
Member

balloob commented Oct 6, 2020

I was able to reproduce it with Python 3.8 locally.

@ghost
Copy link
Copy Markdown
Author

ghost commented Oct 6, 2020

Ok this is a shame. I will remove the last 5 commits. I'm running tox -e py38 -- tests/components/recorder/*.py and is successful so I don't know what can be wrong.

@balloob
Copy link
Copy Markdown
Member

balloob commented Oct 6, 2020

We can always try to solve it in a future PR. Asyncio is offered via various test plugins, last time I checked there was no native asyncio support. That might interfere when we start getting async and sync fixtures mixed up with async tests.

@balloob balloob merged commit 1e9e40b into home-assistant:dev Oct 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Rewrite recorder unittest tests to pytest style test functions

3 participants