[MERGE][bug #125784] Don't suppress original errors when .unlock() fails

[Andrew Bennetts]

John Arbash Meinel wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> Martin Pool wrote:
> | Martin Pool has voted tweak.
> | Status is now: Conditionally approved
> | Comment:
> | +    except:
> | +        import sys
> |
> | You already imported 'sys' at the top level; it's certainly already in
> | memory.
>
> No, because this is in a string that is being compiled. Don't ask me why, but
> without this, it borks. Though I might have copied that to one of the locations
> that wouldn't have borked. I do know that I needed to add 'import sys' for the
> "pretty" functions.

See file:///usr/share/doc/python2.4/html/ref/exec.html (assuming you have
python2.4-doc installed on your Ubuntu system).

It's because we do “exec func_def in locals()”, but there is no sys in locals.
So a simple alternative would be to move the import sys into
_pretty_needs_write_lock rather than the template string.  (We don't want to add
globals() to the exec statement, because we don't want the resulting functions
to be added to the decorators module!)

> | As we mentioned in the bug it might have been nice to log or tell the
> | user about any errors occurring while unlocking.
> |
> | +class TestFastDecoratorActions(TestDecoratorActions):
> | +
> | +    _decorator_style = 'fast'
> | +
> | +
> | +class TestPrettyDecoratorActions(TestDecoratorActions):
> | +
> | +    _decorator_style = 'pretty'
> |
> | (comment) I would kind of like if these could be written using the
> | scenario multiplier code rather than subclassing, so that we can move
> | towards consistently using that.  But I guess it can wait until someone
> | goes to update all of them; it's not particularly cumbersome like this.
> |
>
> It is quite a bit more effort to multiply rather than subclass. Because the
> multiplier is designed for adapting all tests in a module, so adapting just 1
> class XX is kind of hard. 'load_tests' would let you do it, but you have to
> 'iter_suite_tests(basic_tests)' and then figure out which ones need to be adapted.
>
> Or have them in a separate module, and adapt everything, etc.
>
> I'm not saying I wouldn't do it if pushed, just that in this case it would be a
> lot more effort.

I've had the same experience, although I admit I haven't checked with the most
recent version of the multiplication code.  I would love for it to be as easy as
subclassing.

-Andrew.