Code review comment for lp:~wallyworld/launchpad/codehost-error-trackback-leak

On 06/12/10 14:41, Tim Penhey wrote:
> Review: Needs Fixing
> +1 on Graham's comments.

Done

>
> I found the variable name 'tb' a little jarring. It caused me to pause to
> work out what it might be. Longer more explicit names are good. Perhaps
> 'traceback'?
>

Done

> Someone told me we had a monkeypatch helper method that added the cleanup
> code. If that isn't the case, you could still to the cleanup with addCleanup
> rather than adding a tearDown method.
>
> # Before changing the stderr, do this.
> self.addCleanup(setattr, sys, 'stderr', sys.stderr)
>

Done

> The test class has self.oopses that records oopses generated by the global
> error utility, and should be used insetad of getLastOopsReport().
> getLastOoopsReport can fail in unexpected ways if the oops was generated on
> one side of midnight, and the method is called just after midnight. This was
> a cause of quite some pain and debugging, and is the reason that the testcase
> has self.oopses and records them based on generated events.
>

Done