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

Fix racy homekit_controller platform setup caused by #22368 #22655

Merged
merged 1 commit into from
Apr 2, 2019

Conversation

Jc2k
Copy link
Member

@Jc2k Jc2k commented Apr 2, 2019

Description:

#22194 introduced some new homekit_controller tests and @balloob and @awarecan pointed out at least one of them was flaky. This doesn't happen in my config entry branch and i've now traced its root cause to #22368.

Essentially, HKDevice.__init__ on dev has for some time triggered the setup_platform for its sub-domains like light/climate/etc. But there is a race where this code hasn't run yet:

hass.data[KNOWN_DEVICES][hkid] = device

If the setup_platform runs before that happens you get the error in the test. This fix moves where we store the current HKDevice into hass.data so that it is always set before any further setup_platform is called.

This race is probably a real bug, not just a flaky test. So if #22368 is already in a tag or in an rc branch it might be worth applying the fix there too.

This is something of a temporary band aid - the buggy code is actually removed in my config entry branch and doesn't have this problem. The config entry branch won't trigger side effects in HKDevice.__init__ any more. So the config entry work will be the 'real' fix, but this will do until then.

**Related issue (if applicable): #22650

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.

@codecov
Copy link

codecov bot commented Apr 2, 2019

Codecov Report

Merging #22655 into dev will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##              dev   #22655   +/-   ##
=======================================
  Coverage   92.57%   92.57%           
=======================================
  Files         457      457           
  Lines       36977    36977           
=======================================
  Hits        34232    34232           
  Misses       2745     2745

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 48189dd...11c00e9. Read the comment docs.

@MartinHjelmare MartinHjelmare added this to the 0.91.0 milestone Apr 2, 2019
@MartinHjelmare MartinHjelmare merged commit 16e0953 into home-assistant:dev Apr 2, 2019
@ghost ghost removed the in progress label Apr 2, 2019
@balloob balloob mentioned this pull request Apr 3, 2019
@Jc2k Jc2k deleted the homekit_fix_flaky_test branch April 17, 2019 14:41
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.

4 participants