Rewrite mochad unittest to pytest style #42164

tim-werner · 2020-10-21T13:53:13Z

Breaking change

Proposed change

Type of change

Dependency upgrade
Bugfix (non-breaking change which fixes an issue)
New integration (thank you!)
New feature (which adds functionality to an existing integration)
Breaking change (fix/feature causing existing functionality to break)
Code quality improvements to existing code or addition of tests

Example entry for `configuration.yaml`:

# Example configuration.yaml

Additional information

This PR fixes or closes issue: fixes Rewrite mochad unittest tests to pytest style test functions #40867
This PR is related to issue:
Link to documentation pull request:

Checklist

The code change is tested and works locally.
Local tests pass. Your PR cannot be merged unless tests pass
There is no commented out code in this PR.
I have followed the development checklist
The code has been formatted using Black (black --fast homeassistant tests)
Tests have been added to verify that the new code works.

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

Documentation added/updated for www.home-assistant.io

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

The manifest file has all fields filled out correctly.
Updated and included derived files by running: python3 -m script.hassfest.
New or updated dependencies have been added to requirements_all.txt.
Updated by running python3 -m script.gen_requirements_all.
Untested files have been added to .coveragerc.

The integration reached or maintains the following Integration Quality Scale:

No score or internal
🥈 Silver
🥇 Gold
🏆 Platinum

To help with the load of incoming pull requests:

I have reviewed two other open pull requests in this repository.

balloob · 2020-10-22T07:31:40Z

tests/components/mochad/test_light.py

+    assert await async_setup_component(hass, light.DOMAIN, good_config)
+
+
+@pytest.mark.parametrize("brightness", [32, 256, 64])


Why is this parametrized ?

Suggested change

@pytest.mark.parametrize("brightness", [32, 256, 64])

Yeah. Parameterizing this test makes no sense. I will remove the test anyway (see below).

balloob · 2020-10-22T07:32:07Z

tests/components/mochad/test_light.py

+
+
+@pytest.mark.parametrize("brightness", [32, 256, 64])
+async def test_name(light_mock):


This test is 🤷 I think that we can remove it.

Yes I think we can remove this test. I did keep all the test that have been there before. Did not want to remove any test

balloob · 2020-10-22T07:32:54Z

tests/components/mochad/test_light.py

+    assert "fake_light" == light_mock.name
+
+
+@pytest.mark.parametrize(


This makes no sense either?

I have combined the 3 old tests that have tested different light levels into one parametrized. The 3 old test have basically tested the same thing with different light levels. If this does not make sense we can replace these tests with more meaningful tests but I did not want to remove any test that was there before

I am also confused why the test returns on with light level 32 and xdim with different light levels. However, this was also the case for the old tests. I would expect a more consistence behavior ..

Interesting, I didn't realize that you could parametrize stuff in fixtures. TIL!

balloob

These tests and the parametrizations don't seem to match up .

tim-werner

Thanks for reviewing. In general I did not want to remove any test that have been there before. Not sure if the tests make that much sense ...

tim-werner · 2020-10-22T08:11:48Z

tests/components/mochad/test_light.py

+
+
+@pytest.mark.parametrize("brightness", [32, 256, 64])
+async def test_name(light_mock):


Yes I think we can remove this test. I did keep all the test that have been there before. Did not want to remove any test

tim-werner · 2020-10-22T08:13:54Z

tests/components/mochad/test_light.py

+    assert "fake_light" == light_mock.name
+
+
+@pytest.mark.parametrize(


I have combined the 3 old tests that have tested different light levels into one parametrized. The 3 old test have basically tested the same thing with different light levels. If this does not make sense we can replace these tests with more meaningful tests but I did not want to remove any test that was there before

tim-werner · 2020-10-22T08:14:31Z

tests/components/mochad/test_light.py

+    assert await async_setup_component(hass, light.DOMAIN, good_config)
+
+
+@pytest.mark.parametrize("brightness", [32, 256, 64])


Yeah. Parameterizing this test makes no sense. I will remove the test anyway (see below).

tim-werner · 2020-10-22T10:05:36Z

These tests and the parametrizations don't seem to match up .

Hi, I think the confusions comes due to the way how it is parameterized. The brightness in the @pytest.mark.parametrize will affect the brightness in the light_mock fixture. It would be nicer to parameterize the brigthness in the fixture itself however I do not know a good way to combine the expected values (because it will create a test matrix combining each expected with each brightness parameter)

rewrite mochad unittest to pytest style

7582403

probot-home-assistant bot added the Hacktoberfest label Oct 21, 2020

homeassistant added the cla-signed label Oct 21, 2020

probot-home-assistant bot added code-quality has-tests integration: mochad small-pr PRs with less than 30 lines. labels Oct 21, 2020

project-bot bot added this to Needs review in Dev Oct 21, 2020

balloob reviewed Oct 22, 2020

View reviewed changes

Dev automation moved this from Needs review to Review in progress Oct 22, 2020

balloob requested changes Oct 22, 2020

View reviewed changes

MartinHjelmare changed the title ~~rewrite mochad unittest to pytest style~~ Rewrite mochad unittest to pytest style Oct 22, 2020

tim-werner commented Oct 22, 2020

View reviewed changes

removed name test and run turn off test just once

c5580ce

balloob approved these changes Oct 22, 2020

View reviewed changes

Dev automation moved this from Review in progress to Reviewer approved Oct 22, 2020

balloob merged commit fcc895f into home-assistant:dev Oct 22, 2020

Dev automation moved this from Reviewer approved to Done Oct 22, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Rewrite mochad unittest to pytest style #42164

Rewrite mochad unittest to pytest style #42164

tim-werner commented Oct 21, 2020

balloob Oct 22, 2020

tim-werner Oct 22, 2020

balloob Oct 22, 2020

tim-werner Oct 22, 2020

balloob Oct 22, 2020

tim-werner Oct 22, 2020

tim-werner Oct 22, 2020

balloob Oct 22, 2020

balloob left a comment

tim-werner left a comment

tim-werner Oct 22, 2020

tim-werner Oct 22, 2020

tim-werner Oct 22, 2020

tim-werner commented Oct 22, 2020

		assert await async_setup_component(hass, light.DOMAIN, good_config)


		@pytest.mark.parametrize("brightness", [32, 256, 64])



		@pytest.mark.parametrize("brightness", [32, 256, 64])
		async def test_name(light_mock):

		assert "fake_light" == light_mock.name


		@pytest.mark.parametrize(

Navigation Menu

Rewrite mochad unittest to pytest style #42164

Rewrite mochad unittest to pytest style #42164

Conversation

tim-werner commented Oct 21, 2020

Breaking change

Proposed change

Type of change

Example entry for configuration.yaml:

Additional information

Checklist

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

balloob left a comment

Choose a reason for hiding this comment

tim-werner left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tim-werner commented Oct 22, 2020

Example entry for `configuration.yaml`: