Replace remaining logging mocks with assertLogs. #19392
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
We were disabling push_notifications logger but weren't enabling it back. This caused failures on porting logging mocks to assertLogs as assertLogs expects a log to be generated. See 9c224cc for why we disable these. (To avoid logs spam from push_notifications_logger)
Left the mocks which are used to assert a logging call isn't made.
|
Hello @chdinesh1089, it seems like you have referenced #15331 in your pull request description, but you have not referenced them in your commit message description(s). When you reference an issue in a commit message, it automatically closes the corresponding issue when the commit is merged. Please run An example of a correctly-formatted commit: Thank you for your contributions to Zulip! |
Set level to 'ERROR' since exceptions create logs with that level.
Of the two other logging mocks left in this file, one checks a logging call isn't made and another makes sure errors aren't allowed by raising an exception as a side_effect to the logger.
timabbott
reviewed
Jul 26, 2021
| @@ -899,6 +898,8 @@ def handle(self, **options: Any) -> None: | |||
| mark_all_messages_as_read() | |||
| self.stdout.write("Successfully populated test database.\n") | |||
|
|
|||
| push_notifications_logger.disabled = False | |||
There was a problem hiding this comment.
I think probably the important part is just moving this out of the top level, so that importing this file doesn't result in that change occurring.
|
This is awesome, merged, thanks @chdinesh1089! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #15331
I think this completes the migration of logging mocks to
assertLogs. The ones left are either used to assert a logging call isn't made, or some sort of strategic mocking like this, or somehow necessary for eg. to checkstack_infois setTrue.