Code review comment for lp:~mbp/rabbitfixture/rabbit-startup

On 25 October 2011 21:21, Gavin Panella <email address hidden> wrote:
> Review: Approve
>
> This looks like it'll work, but it's ugly:
>
> - It relies upon the fact that fixture clean-ups are not run when
>  setUp fails.
>
> - When these errors appear as part of a test run they're not
>  particularly useful (useless in ec2 if the instance has already
>  terminated) and could be misleading (for example, someone might
>  believe that they have to capture the log when its contents are
>  actually already in a test detail).
>
> I still think that it would be much better to fix runlaunchpad.py
> rather than muddy things in rabbitfixture. So, approved, but it's a
> +0.1 rather than a +1, and I'd want to revert these changes when
> runlaunchpad is fixed.

I don't understand. With this patch, it raises an exception in just
the same cases as before, but now the exception has a more useful
message. How does that change the interaction with tearDown, and why
would you ever want to revert it?

I take your point about ec2 and the whole disk disappearing, and
perhaps there printing the log would be better.