[MERGE][Bug #162469] log displayers for custom revision

[Aaron Bentley]

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

Guillermo Gonzalez wrote:
> On Thu, Mar 13, 2008 at 9:56 PM, Robert Collins
>>  Lastly you should test error cases - what happens if there is a bad
>>  plugin function, will bzr blow up/degrade gracefully/undefined? Does log
>>  work correctly with no plugins present (you have to explicitly test this
>>  because selftest loads plugins).
>>
> 
> I added two test cases, one for a broken handler (which raise an
> error) and another with no handlers registered.

> +    def show_properties(self, properties):
> +        """Displays the custom properties returned by each registered handler.
> +        
> +        If a registered handler raise an error, it's silently logged and the
> +        next handler in the registry is executed.
> +        """

Shouldn't that be considered a bug in the handler?  What kinds of errors
are you saying should be ignored?

Also, your docstring should have "raises", not "raise".

> +            try:
> +                for key, value in handler(filtered_props).items():
> +                    self.to_file.write(key + ': ' + value + '\n')
> +            except:
> +                mutter('custom property handler: %s raised an error', 
> +                        key)
> +                trace.log_exception_quietly()

Bare except clauses are almost always a bad idea.  For instance, this
will ignore a KeyboardInterrupt or a pipe error.

> === modified file 'bzrlib/tests/blackbox/test_log.py'
> --- bzrlib/tests/blackbox/test_log.py	2008-01-10 22:34:09 +0000
> +++ bzrlib/tests/blackbox/test_log.py	2008-03-31 03:56:49 +0000
> @@ -184,6 +184,56 @@
>          self.assertNotContainsRe(log, r'revno: 1\n')
>          self.assertContainsRe(log, r'revno: 2\n')
>          self.assertContainsRe(log, r'revno: 3\n')
> +        

These don't look like UI tests to me.  They should probably be in
bzrlib/tests/test_log, instead.

> +    def test_log_without_custom_properties_handler(self):
> +        tree = self._prepare()
> +        tree.commit(message='', revprops={'first_prop':'first_value'})
> +        log = self.run_bzr("log --limit 1")[0]
> +        self.assertNotContainsRe(log, r'first_prop: first_value\n')
> +        log = self.run_bzr("log -r1")[0]
> +        self.assertNotContainsRe(log, r'first_prop: first_value\n')

^^^ How does this ensure that there are no custom properties handlers?

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

iD8DBQFH8OJ60F+nu1YWqI0RAhyYAJ9Sd1BMMzbQqxtSwspS6i5RqCFILwCcCdeq
WIgTmeEWOoASN5xSzWgeBb4=
=X6qm
-----END PGP SIGNATURE-----