[PING] [PATCH] ignore log to stdout in benchmarks

Martin Pool mbp at canonical.com
Thu Sep 21 05:03:51 BST 2006


On  7 Sep 2006, Carl Friedrich Bolz <cfbolz at gmx.de> wrote:
> Carl Friedrich Bolz wrote:
> > Carl Friedrich Bolz wrote:
> >> Robert Collins wrote:
> > [snip]
> >>> I was suggesting the following:
> >>> By default:
> >>>  - successful or skipping tests delete the log from disk and throw away
> >>> the contents
> >>>  - failing or erroring tests put the log on disk until get_log_content()
> >>> is called, when they read the file into memory and delete the log from
> >>> disk.
> >>>
> >>> Tests that need to can request that the log on disk be explicitly kept.
> >> I tried to implement that, see attached patch.
> > 
> > Could somebody look at it again? see patch, the branch is also
> > registered at launchpad as:
> > 
> > http://launchpad.net/people/cfbolz/+branch/bzr/throw-test-log-away
> > 
> Oops, here's the patch.

+1 from me.  Any other comments?

> === modified file 'bzrlib/tests/__init__.py'
> --- bzrlib/tests/__init__.py	2006-09-06 12:05:59 +0000
> +++ bzrlib/tests/__init__.py	2006-09-06 21:59:56 +0000
> @@ -233,6 +233,7 @@
>          if isinstance(err[1], TestSkipped):
>              return self.addSkipped(test, err)    
>          unittest.TestResult.addError(self, test, err)
> +        test.setKeepLogfile()
>          self.extractBenchmarkTime(test)
>          if self.showAll:
>              self.stream.writeln("ERROR %s" % self._testTimeString())
> @@ -249,6 +250,7 @@
>  
>      def addFailure(self, test, err):
>          unittest.TestResult.addFailure(self, test, err)
> +        test.setKeepLogfile()
>          self.extractBenchmarkTime(test)
>          if self.showAll:
>              self.stream.writeln(" FAIL %s" % self._testTimeString())
> @@ -475,6 +477,7 @@
>  
>      _log_file_name = None
>      _log_contents = ''
> +    _keep_log_file = False
>      # record lsprof data when performing benchmark calls.
>      _gather_lsprof_in_benchmarks = False
>  
> @@ -655,16 +658,20 @@
>      def _finishLogFile(self):
>          """Finished with the log file.
>  
> -        Read contents into memory, close, and delete.
> +        Close the file and delete it, unless setKeepLogfile was called.
>          """
>          if self._log_file is None:
>              return
>          bzrlib.trace.disable_test_log(self._log_nonce)
> -        self._log_file.seek(0)
> -        self._log_contents = self._log_file.read()
>          self._log_file.close()
> -        os.remove(self._log_file_name)
> -        self._log_file = self._log_file_name = None
> +        self._log_file = None
> +        if not self._keep_log_file:
> +            os.remove(self._log_file_name)
> +            self._log_file_name = None
> +
> +    def setKeepLogfile(self):
> +        """Make the logfile not be deleted when _finishLogFile is called."""
> +        self._keep_log_file = True
>  
>      def addCleanup(self, callable):
>          """Arrange to run a callable when this case is torn down.
> @@ -751,13 +758,27 @@
>      def log(self, *args):
>          mutter(*args)
>  
> -    def _get_log(self):
> -        """Return as a string the log for this test"""
> -        if self._log_file_name:
> -            return open(self._log_file_name).read()
> -        else:
> +    def _get_log(self, keep_log_file=False):
> +        """Return as a string the log for this test. If the file is still
> +        on disk and keep_log_file=False, delete the log file and store the
> +        content in self._log_contents."""
> +        # flush the log file, to get all content
> +        import bzrlib.trace
> +        bzrlib.trace._trace_file.flush()
> +        if self._log_contents:
>              return self._log_contents
> -        # TODO: Delete the log after it's been read in
> +        if self._log_file_name is not None:
> +            logfile = open(self._log_file_name)
> +            try:
> +                log_contents = logfile.read()
> +            finally:
> +                logfile.close()
> +            if not keep_log_file:
> +                self._log_contents = log_contents
> +                os.remove(self._log_file_name)
> +            return log_contents
> +        else:
> +            return "DELETED log file to reduce memory footprint"
>  
>      def capture(self, cmd, retcode=0):
>          """Shortcut that splits cmd into words, runs, and returns stdout"""
> 
> === modified file 'bzrlib/tests/blackbox/test_exceptions.py'
> --- bzrlib/tests/blackbox/test_exceptions.py	2006-08-15 16:26:01 +0000
> +++ bzrlib/tests/blackbox/test_exceptions.py	2006-08-17 10:40:35 +0000
> @@ -49,7 +49,8 @@
>              os.mkdir('foo')
>              bzrdir.BzrDirFormat5().initialize('foo')
>              out, err = self.run_bzr("status", "foo")
> -            self.assertContainsRe(self._get_log(), "bzr upgrade")
> +            self.assertContainsRe(self._get_log(keep_log_file=True),
> +                                  "bzr upgrade")
>          finally:
>              repository._deprecation_warning_done = True
>  
> 
> === modified file 'bzrlib/tests/test_merge.py'
> --- bzrlib/tests/test_merge.py	2006-08-30 19:34:45 +0000
> +++ bzrlib/tests/test_merge.py	2006-08-31 11:45:57 +0000
> @@ -135,12 +135,12 @@
>          log = StringIO()
>          merge_inner(tree_b.branch, tree_a, tree_b.basis_tree(), 
>                      this_tree=tree_b, ignore_zero=True)
> -        log = self._get_log()
> +        log = self._get_log(keep_log_file=True)
>          self.failUnless('All changes applied successfully.\n' not in log)
>          tree_b.revert([])
>          merge_inner(tree_b.branch, tree_a, tree_b.basis_tree(), 
>                      this_tree=tree_b, ignore_zero=False)
> -        log = self._get_log()
> +        log = self._get_log(keep_log_file=True)
>          self.failUnless('All changes applied successfully.\n' in log)
>  
>      def test_merge_inner_conflicts(self):
> 
> === modified file 'bzrlib/tests/test_selftest.py'
> --- bzrlib/tests/test_selftest.py	2006-09-06 12:05:59 +0000
> +++ bzrlib/tests/test_selftest.py	2006-09-06 21:59:56 +0000
> @@ -60,7 +60,8 @@
>          """Test logs are captured when a test fails."""
>          self.log('a test message')
>          self._log_file.flush()
> -        self.assertContainsRe(self._get_log(), 'a test message\n')
> +        self.assertContainsRe(self._get_log(keep_log_file=True),
> +                              'a test message\n')
>  
>  
>  class TestTreeShape(TestCaseInTempDir):
> 
> === modified file 'bzrlib/tests/test_trace.py'
> --- bzrlib/tests/test_trace.py	2006-08-30 19:34:43 +0000
> +++ bzrlib/tests/test_trace.py	2006-09-06 22:00:13 +0000
> @@ -87,22 +87,22 @@
>  
>      def test_trace_unicode(self):
>          """Write Unicode to trace log"""
> -        mutter(u'the unicode character for benzene is \N{BENZENE RING}')
> -        self._log_file.flush()
> -        self.assertContainsRe(self._get_log(), 'the unicode character',)
> +        self.log(u'the unicode character for benzene is \N{BENZENE RING}')
> +        self.assertContainsRe(self._get_log(keep_log_file=True),
> +                              "the unicode character for benzene is")
>      
>      def test_trace_argument_unicode(self):
>          """Write a Unicode argument to the trace log"""
>          mutter(u'the unicode character for benzene is %s', u'\N{BENZENE RING}')
> -        self._log_file.flush()
> -        self.assertContainsRe(self._get_log(), 'the unicode character')
> +        self.assertContainsRe(self._get_log(keep_log_file=True),
> +                              'the unicode character')
>  
>      def test_trace_argument_utf8(self):
>          """Write a Unicode argument to the trace log"""
>          mutter(u'the unicode character for benzene is %s',
>                 u'\N{BENZENE RING}'.encode('utf-8'))
> -        self._log_file.flush()
> -        self.assertContainsRe(self._get_log(), 'the unicode character')
> +        self.assertContainsRe(self._get_log(keep_log_file=True),
> +                              'the unicode character')
>  
>      def test_report_broken_pipe(self):
>          try:
> @@ -119,11 +119,7 @@
>          mutter(u'Writing a greek mu (\xb5) works in a unicode string')
>          mutter('But fails in an ascii string \xb5')
>          mutter('and in an ascii argument: %s', '\xb5')
> -        # TODO: jam 20051227 mutter() doesn't flush the log file, and
> -        #       self._get_log() opens the file directly and reads it.
> -        #       So we need to manually flush the log file
> -        self._log_file.flush()
> -        log = self._get_log()
> +        log = self._get_log(keep_log_file=True)
>          self.assertContainsRe(log, 'Writing a greek mu')
>          self.assertContainsRe(log, "But fails in an ascii string")
>          self.assertContainsRe(log, u"ascii argument: \xb5")
> 

-- 
Martin




More information about the bazaar mailing list