[patch] cleanups to trace/selftest

[Aaron Bentley]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

John Arbash Meinel wrote:
> Martin Pool wrote:
>>.bzr.log is removed.

This doesn't sound like a win to me.  I still see .bzr.log as useful for
1. recording tracebacks for errors with is_user_error True
2. recording tracebacks for plugin load failures

> logging costs about 12ms to load. So there is a gain, but not a huge one.

Also, I believe there was an observable overhead for *doing* logging via
this module.

>>@@ -1912,7 +1915,7 @@
>>         print '%10s: %s' % ('bzr', osutils.realpath(sys.argv[0]))
>>         print '%10s: %s' % ('bzrlib', bzrlib.__path__[0])
>>         print
>>-        info('running tests...')
>>+        print 'running tests...'
> 
> 
> ^- I kind of cringe at this.
> 1) info() used to go to stderr instead of stdout. Which for selftest,
> going to stdout is fine.

Even for selftest, I believe it makes sense for secondary output (info,
progress bars) to go to stderr.  I figure the primary output is the
report of test success/failure.

>>-        except errors.AmbiguousBase, e:
>>-            m = ("sorry, bzr can't determine the right merge base yet\n"
>>-                 "candidates are:\n  "
>>-                 + "\n  ".join(e.bases)
>>-                 + "\n"
>>-                 "please specify an explicit base with -r,\n"
>>-                 "and (if you want) report this to the bzr developers\n")
> 
> 
> ^- So is this just assuming that AmbiguousBase will be reported properly
> by the standard exception handler?

AmbiguousBase has not been used by any code since pre-0.6, I believe.

>>-        log = self._get_log()
>>-        self.failUnless('All changes applied successfully.\n' in log)
>>+        # we used to look in the UI output from merge, but this is a bit of a
>>+        # layering violation - only blackbox tests should look at that --
>>+        # merge should probably give some kind of structured report on what
>>+        # was done -- mbp 20060706
>>+        ## log = self._get_log()
>>+        ## self.failUnless('All changes applied successfully.\n' in log)
> 
> 
> ^- I'm a little concerned that until merge does create a structured
> report, that we might miss stuff. Obviously the test is now not
> asserting the same consistency it used to.

Merge doesn't produce structured output, but it will return 0 iff it
prints 'All changes applied successfully'.

Aaron
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.1 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://enigmail.mozdev.org

iD8DBQFErStJ0F+nu1YWqI0RAl9RAJ4iK4Vza4pd3qKnFpp9ccI5BCzYMQCfTea8
jX85k31k+H7P3oRLIRGE3h4=
=GMEP
-----END PGP SIGNATURE-----