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

Bugfix/delete settings meta #2254

Merged
merged 3 commits into from Aug 11, 2019

Conversation

@forslund
Copy link
Member

commented Aug 10, 2019

Description

Fixes the incorrect variable name reported by @cclauss
This PR also adds a couple of simple unittests for these methods and does a bit of simplification of the api tests.

How to test

Check that the unittests passes.

Contributor license agreement signed?

CLA [ Yes ]

forslund added 2 commits Aug 10, 2019
change from the old uuid to the newer skill_gid fixing the invalid
reference to skill_gid.
Copy link
Contributor

left a comment

LGTM... Some optional optimizations.

mock_identity = mock.MagicMock()
mock_identity.is_expired.return_value = False
mock_identity.uuid = '1234'
mock_identity_get.return_value = mock_identity

This comment has been minimized.

Copy link
@cclauss

cclauss Aug 11, 2019

Contributor

4 lines —> 1 line: mock_identity_get.return_value = create_identity('1234')

mock_identity = mock.MagicMock()
mock_identity.is_expired.return_value = False
mock_identity.uuid = '1234'
mock_identity_get.return_value = mock_identity

This comment has been minimized.

Copy link
@cclauss

cclauss Aug 11, 2019

Contributor

4 lines —> 1 line: mock_identity_get.return_value = create_identity('1234')

mock_identity = mock.MagicMock()
mock_identity.is_expired.return_value = False
mock_identity.uuid = '1234'
mock_identity_get.return_value = mock_identity

This comment has been minimized.

Copy link
@cclauss

cclauss Aug 11, 2019

Contributor

4 lines —> 1 line: mock_identity_get.return_value = create_identity('1234')

This comment has been minimized.

Copy link
@forslund

forslund Aug 11, 2019

Author Member

Good catch, totally missed replacing in the new tests.

- remove duplicated code for creating identity mock
- separate the Api class tests from the rest of the tests
@forslund forslund force-pushed the forslund:bugfix/delete-settings-meta branch from c920d6a to b68fff3 Aug 11, 2019
@forslund forslund merged commit bbd8bf9 into MycroftAI:dev Aug 11, 2019
3 checks passed
3 checks passed
:-) Build finished.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage increased (+0.04%) to 51.691%
Details
@forslund forslund deleted the forslund:bugfix/delete-settings-meta branch Aug 11, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.