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

Add tests using custom-components/integration_blueprint#50 as base #112

Merged
merged 23 commits into from
Jan 13, 2021

Conversation

raman325
Copy link
Contributor

@raman325 raman325 commented Jan 6, 2021

I just pulled the files over and replaced references to integration_blueprint with {{cookiecutter.project_name}} and IntegrationBlueprint/Blueprint with {{cookiecutter.class_name_prefix}}.

What might be missing is instructions on how to run the tests, check for coverage, etc but figured someone else could tackle that.

Resolves #10

@oncleben31
Copy link
Owner

Thank you very much for your contribution. Some general comments you can already process during my code review:

  • The test folder is not at the right place. When generating a project with cookiecutter, only the files in {{cookiecutter.project_name}} will be processed for replacing Jinja tags and copied to the destination.
  • Could you add some explanations about how to use the test suite in the {{cookiecutter.project_name}}/CONTRIBUTIN.mdfile ?
  • It will be perfect if you can update ./docs/guide.rst to explain what is doing the test suite and how to do use it. Could be in another PR.

@oncleben31 oncleben31 self-assigned this Jan 6, 2021
@raman325
Copy link
Contributor Author

raman325 commented Jan 7, 2021

🤦🏾‍♂️ clearly I moved too quickly on this - I have resolved bullet 1.

I will add an item for bullet 2 tomorrow, and bullet 3 sometime this weekend. The code has a lot of comments so I will likely just be pulling those together and making them into a single doc

Copy link
Owner

@oncleben31 oncleben31 left a comment

Choose a reason for hiding this comment

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

On top of minor updates or questions, you need to review the tag used from the cookiecutter setting to make it works:

  • Friendly name for human readable (ex: My awesome Custom Component)
  • Project name with "-" as GitHub request it (ex: my-awesome-custom-component)
  • Domain name with "_" as Python package expect it (ex: my_awesome_custom_component)

I've tried my best to suggest modifications but I have perhaps miss some.

docs/guide.rst Show resolved Hide resolved
docs/guide.rst Outdated Show resolved Hide resolved
{{cookiecutter.project_name}}/CONTRIBUTING.md Outdated Show resolved Hide resolved
{{cookiecutter.project_name}}/tests/conftest.py Outdated Show resolved Hide resolved
{{cookiecutter.project_name}}/tests/conftest.py Outdated Show resolved Hide resolved
{{cookiecutter.project_name}}/tests/test_init.py Outdated Show resolved Hide resolved
{{cookiecutter.project_name}}/tests/test_init.py Outdated Show resolved Hide resolved
{{cookiecutter.project_name}}/tests/test_switch.py Outdated Show resolved Hide resolved
{{cookiecutter.project_name}}/tests/test_switch.py Outdated Show resolved Hide resolved
{{cookiecutter.project_name}}/tests/test_switch.py Outdated Show resolved Hide resolved
raman325 and others added 7 commits January 8, 2021 14:20
Co-authored-by: Oncleben31 <oncleben31@users.noreply.github.com>
Co-authored-by: Oncleben31 <oncleben31@users.noreply.github.com>
Co-authored-by: Oncleben31 <oncleben31@users.noreply.github.com>
Copy link
Owner

@oncleben31 oncleben31 left a comment

Choose a reason for hiding this comment

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

Thank you for the modifications. I'm trying to run the test suite with a generated project.

I've identified some improvement to make it works.

To solve an error, we need to add a dummy __init__.py file in custom_components folder.

If you want to check the modification ? (you can test the generated project using this branch

{{cookiecutter.project_name}}/CONTRIBUTING.md Outdated Show resolved Hide resolved
{{cookiecutter.project_name}}/CONTRIBUTING.md Outdated Show resolved Hide resolved
{{cookiecutter.project_name}}/CONTRIBUTING.md Show resolved Hide resolved
@oncleben31
Copy link
Owner

Now the project generated by the cookiecutter template succeed to pass the test suite with 100% coverage. Houra !

There are some warnings with config_flow.py tests about some mocking function not awaited.
I'm not in my confort zone but I made some investigations and took inspiration from standard tests suite in Home Assistant core code and other custom component using pytest-homeassistant-custom-component

It seem's to be a best practice and make sens in config_flow test suite to avoid calling the async_setup and async_setup_entry coroutines. So I implement a patch and warnings are gone.

@raman325 Can you check the modification I suggest in this commit ?

@raman325
Copy link
Contributor Author

good find and good fix. I will submit the same change here

@raman325
Copy link
Contributor Author

Only thing to note is that you do not need to pass the fixture in as a parameter to test_successful_config_flow because you have already set autouse to True for the fixture

Copy link
Owner

@oncleben31 oncleben31 left a comment

Choose a reason for hiding this comment

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

Thank you for your contribution

@oncleben31 oncleben31 merged commit b5c30b5 into oncleben31:main Jan 13, 2021
@oncleben31 oncleben31 added the testing Adding missing tests or correcting existing tests label Jan 13, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
testing Adding missing tests or correcting existing tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add tests in template
2 participants