-
Notifications
You must be signed in to change notification settings - Fork 167
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 unit tests for notify.py #1409
Conversation
Nice, always glad to see more tests :). Is this ready for review? |
I forgot about this PR, yes, let's review. I'll assign. |
# Test that email is not sent when EMAIL_PASSWORD is not defined | ||
mock_smtp.reset_mock() | ||
self.config.EMAIL_PASSWORD = '' | ||
with self.assertLogs('turbinia', level='INFO') as turbinia_info_log: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
woah, cool, I didn't know assertLogs
was a thing, nice.
pass | ||
self.assertEqual( | ||
turbinia_error_log.output[1], | ||
'ERROR:turbinia:Email failed to send, SMTP has raised an error, ' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Normally I might say this is a little brittle since it will fail if the logging message or formatting changes, but this notify module hasn't changed in quite a while so I think it's probably just fine to leave it for now, and if things change we can update as needed (e.g. have a different check, or maybe just check a substring).
* Write missing notify.py test. 100% coverage reported. * yapf
* Write missing notify.py test. 100% coverage reported. * yapf
Description of the change
Adds unit tests for notify.py as it lacks any unit tests.
Applicable issues
None
Checklist