Skip to content

[recorder] Include & Exclude domain fix & unit tests#5213

Merged
pvizeli merged 2 commits into
home-assistant:devfrom
kellerza:recorder_test
Jan 9, 2017
Merged

[recorder] Include & Exclude domain fix & unit tests#5213
pvizeli merged 2 commits into
home-assistant:devfrom
kellerza:recorder_test

Conversation

@kellerza
Copy link
Copy Markdown
Member

@kellerza kellerza commented Jan 7, 2017

Description:
Unit tests for include & exclude

Related issue (if applicable): fixes #4733

Example entry for configuration.yaml (if applicable):

recorder:
  include:
  exclude:

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
Copy link
Copy Markdown

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

Comment thread tests/components/recorder/test_init.py Outdated
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

expected 2 blank lines, found 1

Copy link
Copy Markdown
Member

@pvizeli pvizeli left a comment

Choose a reason for hiding this comment

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

I like it


entity_id = event.data.get(ATTR_ENTITY_ID)
domain = event.data.get(ATTR_DOMAIN)
entity_id = str(event.data.get(ATTR_ENTITY_ID))
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.

This is always a string? If not, we should error out instead of trying to "fix" it because it means malformed state changed events are being sent.

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.

👍 Events that are not STATE_CHANGE dont have entity_ids.

New commit should handle this better

@Maxr1998
Copy link
Copy Markdown

Maxr1998 commented Jan 8, 2017

Looking good!

@kellerza kellerza added this to the 0.36 milestone Jan 9, 2017
@pvizeli pvizeli merged commit 1f31dfe into home-assistant:dev Jan 9, 2017
@kellerza kellerza deleted the recorder_test branch January 9, 2017 20:54
@home-assistant home-assistant locked and limited conversation to collaborators Apr 30, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants