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

Refactor Tests: Replace logmock with self.assertLogs #748

Closed
FabioRosado opened this issue Nov 21, 2018 · 1 comment · Fixed by #752
Closed

Refactor Tests: Replace logmock with self.assertLogs #748

FabioRosado opened this issue Nov 21, 2018 · 1 comment · Fixed by #752

Comments

@FabioRosado
Copy link
Member

Last year I needed to figure out how to assert if a logging level was called. I tried a few things and came up with the solution to mock the _LOGGER.<level> and then assert if logmock was called.

I have noticed that you can use a self.assertLogs(self, logger, level) to check if a message was logged. Since when we are testing we don't really care about what message was logged but if the message was logging we should replace every amock.patch('opsdroid.core._LOGGER.exception') as logmock with the self.assertLogs.

This should make the tests a bit more readable and easier to maintain in the future. This Issue is also great for new contributors since it's quite easy to implement.

For a better explanation check the example:

Old style: test_main.py:132-136

    def test_welcome_message(self):
        config = {"welcome-message": True}
        with mock.patch('opsdroid.__main__._LOGGER.info') as logmock:
            opsdroid.welcome_message(config)
            self.assertTrue(logmock.called)

Refactored test

    def test_welcome_message(self):
        config = {"welcome-message": True}
        opsdroid.welcome_message(config)
        self.assertLogs('_LOGGER', 'info')

If you need any help with this issue make sure to check out gitter channel.

@jacobtomlinson
Copy link
Member

Awesome! Looks like this was added to unittests in 3.4 (back in Feb).

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

Successfully merging a pull request may close this issue.

2 participants