Skip to content

Update Z-Wave to use async_get_integration#23014

Merged
cgarwood merged 3 commits intohome-assistant:devfrom
cgarwood:zwave_get_integration
Apr 12, 2019
Merged

Update Z-Wave to use async_get_integration#23014
cgarwood merged 3 commits intohome-assistant:devfrom
cgarwood:zwave_get_integration

Conversation

@cgarwood
Copy link
Copy Markdown
Member

@cgarwood cgarwood commented Apr 11, 2019

Description:

Update Z-Wave component to use async_get_integration instead of get_platform

Related issue (if applicable): fixes #23009

Checklist:

  • The code change is tested and works locally.
  • Local tests pass with tox. Your PR cannot be merged unless tests pass
  • There is no commented out code in this PR.

@cgarwood cgarwood requested a review from a team as a code owner April 11, 2019 19:14
@ghost ghost assigned cgarwood Apr 11, 2019
@ghost ghost added the in progress label Apr 11, 2019
@github-actions
Copy link
Copy Markdown

Hey there @home-assistant/z-wave, mind taking a look at this pull request as its been labeled with a integration (zwave) you are listed as a codeowner for? Thanks!

self.primary.enable_poll(polling_intensity)

platform = get_platform(self._hass, component, DOMAIN)
integration = asyncio.run_coroutine_threadsafe(
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

That is not good, we probably need provide loader.get_integration()

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I may have bit off more than I can chew currently, there's a lot of get_platform mocking in tests that I've never worked with, so it may be better for someone else to take this on

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Nah, we should just replace this code with:

        platform = importlib.import_module('.{}'.format(conf[CONF_PLATFORM]),
                                           __name__)

Copy link
Copy Markdown
Member

@balloob balloob Apr 11, 2019

Choose a reason for hiding this comment

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

Because integration code shouldn't load an integration object to load its own code.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

And then everywhere in the tests that mock get_platform, mock importlib.import_module

@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 12, 2019

Codecov Report

Merging #23014 into dev will decrease coverage by 29.45%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff             @@
##              dev   #23014       +/-   ##
===========================================
- Coverage   93.95%    64.5%   -29.46%     
===========================================
  Files         448      448               
  Lines       36750    36750               
===========================================
- Hits        34529    23704    -10825     
- Misses       2221    13046    +10825
Impacted Files Coverage Δ
homeassistant/components/worldclock/sensor.py 0% <0%> (-100%) ⬇️
homeassistant/components/smhi/weather.py 0% <0%> (-100%) ⬇️
homeassistant/components/demo/water_heater.py 0% <0%> (-100%) ⬇️
homeassistant/components/rflink/binary_sensor.py 0% <0%> (-100%) ⬇️
homeassistant/components/yandextts/tts.py 0% <0%> (-100%) ⬇️
homeassistant/components/fido/sensor.py 0% <0%> (-100%) ⬇️
homeassistant/components/automation/sun.py 0% <0%> (-100%) ⬇️
...omeassistant/components/soundtouch/media_player.py 0% <0%> (-100%) ⬇️
homeassistant/components/ps4/const.py 0% <0%> (-100%) ⬇️
homeassistant/components/demo/switch.py 0% <0%> (-100%) ⬇️
... and 203 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 02347df...9395ce0. Read the comment docs.

@cgarwood cgarwood merged commit c8375be into home-assistant:dev Apr 12, 2019
@ghost ghost removed the in progress label Apr 12, 2019
@cgarwood cgarwood deleted the zwave_get_integration branch April 12, 2019 16:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Convert Z-Wave integration to use get_integration

4 participants