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

Added AppDaemon command. #49

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Conversation

andyinno
Copy link

select_option is a AppDaemon command that was not mocked.

in issue #48 I used the select_option AppDaemon method. It seems that it was not implemented. Adding it to the list of the mocked function allowed me to make my tests work.

I don't know if this change is the only needed, in the case, can I be instructed on other changes to do?

select_option is a AppDaemon command that was not mocked.
Copy link
Owner

@HelloThisIsFlo HelloThisIsFlo left a comment

Choose a reason for hiding this comment

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

That's all good. Thanks to the refactor from @mcampbellizo it's just that simple to add new functions now 😁

If you want a smoother interaction you could add an entry to assert_that. Something like assert_that(ENTITY).was_set_to_option(OPTION). But your current version works for sparse needs :)

appdaemontestframework/hass_mocks.py Show resolved Hide resolved
@@ -120,6 +124,23 @@ def called_with(self, **kwargs):
self.hass_functions['call_service'].assert_any_call(
service_full_name, **kwargs)

def set_to_option(self, newvalue, **service_specific_parameters):
Copy link
Author

Choose a reason for hiding this comment

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

@FlorianKempenich I think I need some help here. I don't understand how it works. Never used lambdas and all this part seems to obscure to me.

Copy link
Owner

Choose a reason for hiding this comment

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

Yes I agree that part is a bit obscure. I tried to make it as readable as possible, but even for someone familiar w/ lambda it's not the most self-explanatory code . . .

The part with the lambda is for the turned on/off assertions. For turning on/off a light, there are 2 ways to do so, from the automation we can call self.turn_on(ENTITY) or self.call_service('turn_on', {'entity_id': ENTITY}). What the helper does when we call assert_that(ENTITY).was.turned_on(): it checks both (turn_on and call_service) and see if at least one was called. If none was called, it'll collect errors from both and merge them into 1. _capture_assert_failure_exception runs the lambda and capture any failure or None if there was no failure. Then we check in if service_not_called and turn_on_helper_not_called: if any of the 2 calls returned an error,, and if so we raise a merged error

raise EitherOrAssertionError(
                service_not_called, turn_on_helper_not_called)

In the case of set_option, if there is a way to call set_option via a service call, then we should be able to replicate a similar behavior indeed. I'll now look into why your test is failing :)

Copy link
Owner

@HelloThisIsFlo HelloThisIsFlo left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution 😃
The thing blocking you was just a tini-tiny minor omission, it was so small that I added it and its works now. There's just one test scenario missing, I've added a comment explaining why. All around good job 😃

test/test_assert_that.py Show resolved Hide resolved
def test_option_is_set(self, assert_that, automation):
assert_that(INPUT_SELECT).was_not.set_to_option('new_option')
automation.select_option('new_option')
assert_that(INPUT_SELECT).was.set_to_option('new_option')
Copy link
Owner

Choose a reason for hiding this comment

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

Just one thing I noticed after answering your question above. Could you please add a test for the case when the option is selected via a service call instead of via the helper? :)

@andyinno
Copy link
Author

andyinno commented Jun 7, 2020

Updated the test.
I am unsure if this is really what you was intending.

By the way, I can see that the test is passing if I set the right option with the call_service method call.

If you could review it would be great
thank you
Andrea

@HelloThisIsFlo
Copy link
Owner

Hey 👋

For now, you can consider this project unmaintained, unfortunately.

I haven't had time to maintain this repo in the last few years, in good parts because I moved home and I haven't had a HomeAssistant setup for a very long time. So I'm a bit disconnected with the latest updates, and it's a huge overhead to start everything again, get context and understand how to fix issues on this project

However, I've finally taken the task to set up my server at home again, so I'll probably use this project for my own use again soon.

This means I may bring this project up-to-date in the next few months. No guarantees, obviously, but I may.

Best,
Flo

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants