Skip to content

Move mqtt_mock to tests/components/mqtt/conftest.py#20621

Merged
MartinHjelmare merged 4 commits into
home-assistant:devfrom
awarecan:cleanup-mqtt-test-fixture
Jan 31, 2019
Merged

Move mqtt_mock to tests/components/mqtt/conftest.py#20621
MartinHjelmare merged 4 commits into
home-assistant:devfrom
awarecan:cleanup-mqtt-test-fixture

Conversation

@awarecan
Copy link
Copy Markdown
Contributor

Description:

mqtt_mock test fixture should only be used in mqtt component tests.

Related issue (if applicable): fixes #

Pull request in home-assistant.io with documentation (if applicable): home-assistant/home-assistant.io#<home-assistant.io PR number goes here>

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 the code does not interact with devices:

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

@ghost ghost assigned awarecan Jan 31, 2019
@ghost ghost added the in progress label Jan 31, 2019
Comment thread tests/components/test_snips.py Outdated
@awarecan awarecan force-pushed the cleanup-mqtt-test-fixture branch from 24b3b9d to c68c671 Compare January 31, 2019 08:47
Copy link
Copy Markdown
Member

@MartinHjelmare MartinHjelmare left a comment

Choose a reason for hiding this comment

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

Right, there are still mqtt platforms not moved under the mqtt component. So this one could get more complicated.

It would be a good move though.

@MartinHjelmare
Copy link
Copy Markdown
Member

Should we really move the tests without moving the corresponding source modules?

@awarecan
Copy link
Copy Markdown
Contributor Author

Maybe we should move source code as well. All other mqtt switch, light, etc, even sensor.mqtt are under mqtt component folder. However this sensor.mqtt_room is under sensor folder.

@MartinHjelmare
Copy link
Copy Markdown
Member

MartinHjelmare commented Jan 31, 2019

I think it's harder to move that due to the name. We can't name that platform mqtt.sensor too, that's already taken. The loader.py is limited in the naming schema it searches for.

@awarecan
Copy link
Copy Markdown
Contributor Author

Revert the moving, fix the mqtt room presence sensor test instead.

@MartinHjelmare MartinHjelmare merged commit d7b61f7 into home-assistant:dev Jan 31, 2019
@ghost ghost removed the in progress label Jan 31, 2019
@balloob balloob mentioned this pull request Feb 20, 2019
@awarecan awarecan deleted the cleanup-mqtt-test-fixture branch March 12, 2019 05:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants