We need a contextmanager which disables logging in a block and renables logging on exit. Currently, OutputCapture only provides functions to disable and then re-enable logging.
<rdar://problem/32546391>
Created attachment 311877 [details] Patch
Comment on attachment 311877 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=311877&action=review > Tools/Scripts/webkitpy/common/system/outputcapture.py:36 > +from contextlib import contextmanager > from StringIO import StringIO These imports should be sorted based on the UNIX sort command. > Tools/Scripts/webkitpy/common/system/outputcapture.py:112 > +class Suppressor(OutputCapture): Maybe a better name for this would be OutputCaptureScope? > Tools/Scripts/webkitpy/common/system/outputcapture.py:116 > + self.last_log = None I suggest that we use the same terminology as used in OutputCapture and call this captured_output. > Tools/Scripts/webkitpy/common/system/outputcapture.py:123 > + @contextmanager > + def suppress_logging(self): > + self.capture_output() > + yield > + self.last_log = self.restore_output() > + Instead of using @contextmanager can we have this class implement __enter__() and __exit__() itself so that we can make use of this convenience class using the following syntax: scope = OutputCaptureScope() with scope: ... scope.captured_output
Comment on attachment 311877 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=311877&action=review >> Tools/Scripts/webkitpy/common/system/outputcapture.py:112 >> +class Suppressor(OutputCapture): > > Maybe a better name for this would be OutputCaptureScope? We may want to have this class take the OutputCapture instance instead of extending it to make it easy to use this class correctly and hard to use it incorrectly or un-idiomatic.
Created attachment 311886 [details] Patch
Comment on attachment 311886 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=311886&action=review > Tools/Scripts/webkitpy/common/system/outputcapture.py:114 > + self.captured_output = None This should return (None, None, None) to match up with how restore_output() behaves. > Tools/Scripts/webkitpy/common/system/outputcapture_unittest.py:61 > + def test_output_capture_scope(self): Please add a test where OutputScope takes an OutputCapture as an argument.
Comment on attachment 311886 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=311886&action=review > Tools/Scripts/webkitpy/common/system/outputcapture.py:120 > + self.captured_output = self._output_capture.restore_output() exc_val => exc_value exc_tb => traceback (To match https://docs.python.org/2/reference/datamodel.html#context-managers)
Created attachment 311927 [details] Patch for landing
Comment on attachment 311927 [details] Patch for landing Clearing flags on attachment: 311927 Committed r217758: <http://trac.webkit.org/changeset/217758>
All reviewed patches have been landed. Closing bug.