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

> On Sun, Oct 30, 2011 at 1:55 PM, Gary Poster <gary.pos...@canonical.com> 
> 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
_______________________________________________
Mailing list: https://launchpad.net/~launchpad-dev
Post to     : launchpad-dev@lists.launchpad.net
Unsubscribe : https://launchpad.net/~launchpad-dev
More help   : https://help.launchpad.net/ListHelp

Reply via email to