Restore for automation entities by kellerza · Pull Request #6254 · home-assistant/core

kellerza · 2017-02-26T20:11:47Z

Description:

Restore for automation entities #4614

Example entry for `configuration.yaml` (if applicable):

recorder:

automation:

Checklist:

If the code does not interact with devices:

Local tests with tox run successfully. Your PR cannot be merged unless tests pass
Tests have been added to verify that the new code works.

mention-bot · 2017-02-26T20:11:48Z

@kellerza, thanks for your PR! By analyzing the history of the files in this pull request, we identified @balloob, @fabaff and @lwis to be potential reviewers.

balloob

Can you also extend your test to actually fire an event to make sure that not just the state in the state machine is correct but also the handlers are enabled/disabled.

balloob · 2017-02-26T22:02:51Z

Please check booleans with !=.

houndci-bot · 2017-02-28T18:54:48Z

too many blank lines (2)

kellerza · 2017-02-28T18:58:25Z

So why would this pass?

I see also strange thing with assert in last time... I don't trust assert anymore

pvizeli · 2017-03-01T09:01:09Z

Need to be rebased. I change the handling a bit with new async bootstrap.

emlove · 2017-03-01T14:31:14Z

Not very important, but it would be cool to pull in last_triggered here as well.

kellerza · 2017-03-02T18:02:18Z

+
+    hass.bus.fire('test_event_hello')
+    yield from hass.async_block_till_done()
+    assert len(calls) == 1


This seems to fail for py3.5 & 3.6, but always passes on 3.4

@pvizeli any ideas?

balloob · 2017-03-03T06:35:19Z


-    # pylint: disable=unnecessary-lambda
-    hass.services.register(domain, service, mock_service)
+    if hass.loop.__dict__.get("_thread_ident", 0) == threading.get_ident():


I prefer if we make an async_mock_service and have mock_service call that

balloob · 2017-03-03T06:37:13Z

I think the test is failing because the automation is not properly enabled: there is no log entry that the automation was triggered.

ps, the sleep would never have worked because the event loop is 1 thread. So you just postponed executing all the further tasks, no race conditions could be resolved.

I would take a look but I don't have time right now

balloob · 2017-03-03T06:38:38Z

We should make sure that there is a test that enabling it works when there is no state to restore. (there might already be one)

houndci-bot · 2017-03-03T16:28:37Z

comparison to False should be 'if cond is False:' or 'if not cond:'

kellerza · 2017-03-03T17:03:50Z

💥 🐬 🎉

balloob · 2017-03-04T23:18:47Z

Very cool 🐬

homeassistant added cla-signed integration: automation labels Feb 26, 2017

kellerza mentioned this pull request Feb 26, 2017

Restore state tracking #6255

Closed

10 tasks

balloob requested changes Feb 26, 2017

View reviewed changes

Comment thread homeassistant/components/automation/__init__.py Outdated

Copy link
Copy Markdown

Member

balloob Feb 26, 2017

Choose a reason for hiding this comment

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

Please check booleans with !=.

kellerza added the cla-recheck label Feb 28, 2017

homeassistant added the cla-signed label Feb 28, 2017

houndci-bot reviewed Feb 28, 2017

View reviewed changes

Comment thread tests/components/automation/test_init.py Outdated

Copy link
Copy Markdown

houndci-bot Feb 28, 2017

Choose a reason for hiding this comment

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

too many blank lines (2)

homeassistant added cla-signed and removed cla-recheck labels Feb 28, 2017

kellerza commented Feb 28, 2017

View reviewed changes

kellerza force-pushed the restore_automation branch from 0ca7cdf to 8001308 Compare February 28, 2017 19:54

homeassistant added the cla-signed label Feb 28, 2017

emlove reviewed Mar 1, 2017

View reviewed changes

kellerza force-pushed the restore_automation branch from 8001308 to b12e453 Compare March 2, 2017 04:25

homeassistant added the cla-signed label Mar 2, 2017

Restore for automation entities

55f8668

kellerza force-pushed the restore_automation branch from b12e453 to 55f8668 Compare March 2, 2017 16:56

homeassistant added the cla-signed label Mar 2, 2017

kellerza commented Mar 2, 2017

View reviewed changes

coroutine

ff94797

homeassistant added the cla-signed label Mar 2, 2017

kellerza added 2 commits March 3, 2017 06:45

no clue what i'm doing now

1034eff

Still passes nicely in py 3.4

06ca47e

balloob reviewed Mar 3, 2017

View reviewed changes

houndci-bot reviewed Mar 3, 2017

View reviewed changes

kellerza force-pushed the restore_automation branch from d4ce06f to 06ca47e Compare March 3, 2017 16:29

homeassistant added the cla-signed label Mar 3, 2017

balloob approved these changes Mar 4, 2017

View reviewed changes

balloob merged commit 1522e67 into home-assistant:dev Mar 4, 2017

home-assistant locked and limited conversation to collaborators Jul 17, 2017

kellerza deleted the restore_automation branch March 18, 2018 20:07

Uh oh!

Conversation

kellerza commented Feb 26, 2017

Description:

Example entry for configuration.yaml (if applicable):

Checklist:

Uh oh!

mention-bot commented Feb 26, 2017

Uh oh!

balloob left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pvizeli commented Mar 1, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

balloob commented Mar 3, 2017

Uh oh!

balloob commented Mar 3, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kellerza commented Mar 3, 2017

Uh oh!

balloob commented Mar 4, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

Example entry for `configuration.yaml` (if applicable):