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

Clean up remaining rflink tests #19551

Merged
merged 6 commits into from Dec 26, 2018

Conversation

Projects
None yet
3 participants
@javicalle
Copy link
Contributor

javicalle commented Dec 23, 2018

Description:

After various modifications, the test_rflink.py class was the last to one left from refactor.
The objective of this PR is to homogenize the class with the rest of the tests of the RFLink components.

Related issue (if applicable): N/A

Pull request in home-assistant.io with documentation (if applicable): N/A

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.

If user exposed functionality or configuration variables are added/changed:

If the code communicates with devices, web services, or third-party tools:

  • New dependencies have been added to the REQUIREMENTS variable (example). N/A
  • New dependencies are only imported inside functions that use them (example). N/A
  • New or updated dependencies have been added to requirements_all.txt by running script/gen_requirements_all.py. N/A
  • New files were added to .coveragerc. N/A

If the code does not interact with devices:

  • Tests have been added to verify that the new code works. N/A

javicalle added some commits Dec 21, 2018

@javicalle

This comment has been minimized.

Copy link
Contributor

javicalle commented Dec 23, 2018

I need some help to fix the errors and doubts generated by the change.

Specifically:

  1. How to refactor the sleep block (line 282)?
await asyncio.sleep(0, loop=hass.loop)
  1. How to solve errors in the tests?
E  RuntimeError: 1 Runtime Warning,
E  /home-assistant/homeassistant/components/rflink.py:497:coroutine 'send_command' was never awaited

In the case of (1), it is the last reference to the asyncio class that prevents the import block from being deleted. I do not know if it is possible to avoid it.

In the case of (2) I'm not sure how to approach it.
The two failing tests intend to validate the correct operation even if the module is configured in no_wait_for_ack mode. The error message seems to indicate that the error is related to the desired behavior ('send_command' was never awaited), but here I enter into speculations and I am not sure if it is an error or a functionality.
And worse, I do not know how to correct the test to make it work.

@javicalle javicalle referenced this pull request Dec 23, 2018

Merged

Clean up RFLink tests and add two tests #19511

4 of 4 tasks complete
Show resolved Hide resolved tests/components/test_rflink.py Outdated
@MartinHjelmare

This comment has been minimized.

Copy link
Member

MartinHjelmare commented Dec 26, 2018

Remove WIP and ping me when ready.

@javicalle javicalle changed the title [WIP] components/test_rflink.py refactor components/test_rflink.py refactor Dec 26, 2018

@javicalle

This comment has been minimized.

Copy link
Contributor

javicalle commented Dec 26, 2018

Done.
@MartinHjelmare thank you very much for the support and for your patience.

@MartinHjelmare MartinHjelmare changed the title components/test_rflink.py refactor Clean up remaining rflink tests Dec 26, 2018

@MartinHjelmare MartinHjelmare merged commit 70fff26 into home-assistant:dev Dec 26, 2018

5 checks passed

Hound No violations found. Woof!
WIP Legacy commit status override — see details
Details
cla-bot Everyone involved has signed the CLA
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage remained the same at 93.057%
Details

@wafflebot wafflebot bot removed the in progress label Dec 26, 2018

@javicalle javicalle deleted the javicalle:base_test_rflink_refactor branch Dec 26, 2018

@balloob balloob referenced this pull request Jan 10, 2019

Merged

0.85.0 #19897

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment