Fixes for monitored logger #485
Conversation
azawlocki
force-pushed
the
az/fix-monitored-logger
branch
from
92fd497 to
38d4baf
Compare
| logger.removeFilter(filter) | ||
| logger_to_monitor.removeFilter(filter) |
There was a problem hiding this comment.
The bug in #484 was caused by using logger instead of logger_ here.
| await super().stop() | ||
| if self._buffer_task: | ||
| self._buffer_task.stop(StopThreadException) | ||
| await super().stop() |
There was a problem hiding this comment.
This change is to fix #468, in which _buffer_task tries to add some more events to a monitor that is already stopped.
| @@ -324,6 +325,9 @@ async def run_command_on_host( | |||
| ) | |||
| yield cmd_task, cmd_monitor | |||
|
|
|||
| await cmd_task | |||
There was a problem hiding this comment.
It's important to wait until cmd_task finishes here, in the monitored_logger's scope. Otherwise we're risking that the monitor gets disconnected from the task's output (which happens when exiting the monitored_logger context manager) before seeing all lines of the output (this happened in the unit test for run_command_on_host added in this PR, before adding this change).
| for assertion in cmd_monitor.failed: | ||
| raise TemporalAssertionError(assertion.name) |
There was a problem hiding this comment.
Let's not forget to report assertion failures that occurred at the "end of events".
Resolves #484
Resolves #468