launchpad-dev team mailing list archive

[Gary Poster <gary.poster@xxxxxxxxxxxxx>]

On Oct 30, 2011, at 5:48 PM, Robert Collins wrote:

> On Sun, Oct 30, 2011 at 1:55 PM, Gary Poster <gary.poster@xxxxxxxxxxxxx> wrote:
>> I didn't look at the which specific tests were triggering the problem, but in general, yeah, this suite tests the testing machinery itself, so it wants to trigger problems and see how they are reported and so on.
>> 
>> You might be right about the race condition.  Some of your diagnosis doesn't quite click with me yet, but I like the plan you sent on the follow-up email: file a bug, add a workaround, I'll try to get my branches landed in spare moments, and I'll follow up with you in two weeks. Thank you for offering such a considerate option.  :-)
> 
> tl;dr: not yuixhr's fault at all, its just the victim.
> 
> Bug 883980. I've pinned it down exactly. This is what happens:
> Test runner sets up rabbit etc because of the layer the test is in.
> This updates the testrunner-appserver-XXXX config.
> Test runner starts slave appserver.
> Slave appserver parses ZCML and gets the *test runner* utility
> configuration (e.g. the dynamically allocated DB etc).
> Slave appserver calls BaseLayer.setUp() which restarts the test
> environment isolation: it reverts back to testrunner-appserver (and
> then allocates a transient layer on top of it).
> This means we can't pass OOPSes *or any other transiently allocated
> resource* from the slave appserver to the testrunner.

Ah, great!  That makes a lot of sense.  

> I've marked the bug high, and I think the fix is just to nuke /all/
> the layer setup calls in the runtestappserver codepath: trust the
> parent testrunner to allocate DB, librarian, rabbit etc, and make the
> test appserver as lightweight as possible. This will make bringing it
> up and down faster, permit assertions in the main testprocess code
> (like 'no oopses were created during this test', or 'the librarian had
> 4 requests made to it').

I have a couple of concerns about that approach (which in fact is what I tried initially with yuixhr).

The first is that the main testprocess has no visibility of JS tests, or even of JS test cases.  It only has visibility into JS test suites as long as we follow the convention of one suite per file.  Therefore, the subprocess is the one that needs to reset the database between tests--it is in charge.  The subprocess is also the one that needs to be able to make per-test assertions about the db, the oopses, the librarian, and so on.  Keeping test configuration in the subprocess is simpler for this use case.  I could (and did) imagine changes that might allow the main process and the subprocess to share responsibilities and access somehow.  The approaches that I imagined for this seemed unnecessarily tricky and hand-wavy, though, and I don't have any new ideas in this regard.

The second concern is that I would like the interactive approach ("make run-testapp") and the non-interactive test-suite-integration approach to be as similar as possible so that it is easier to diagnose problems.  Indeed, one of the few differences between them (the test setup change caused by the INTERACTIVE_TESTS flag) added to the confusion in diagnosing this particular problem, and your fix was to eliminate that difference.

> For now, I'm landing a workaround: remove the 'INTERACTIVE_TESTS' flag
> that was used to prevent rabbit starting - my code makes rabbit
> desirable always, and we're going forward on that, not backwards.

This sounds like a great direction for a solution.  I'll verify later that the parent process is not duplicating setup work.  (I plan to reinstate the flag itself in the Makefile so that I can use it for an additional interactive feature in one of my branches, but I won't let it affect the test setup.)

So, I'm happy with where we are now with this, and what you've done.  We can discuss longer-term plans later, if you'd like to, but I think what you've done as a workaround is the right way forward.

Thanks again

Gary