Merge lp:~wgrant/launchpad/secret-query-count-determinism into lp:launchpad

[William Grant]

[r=jml][bug=724039,724232][no-qa] Prepopulate the cookie secret cache, preventing tests from seeing an extra query when they are the first in a layer.

This branch fixes a spurious failure that I sort of introduced in r12418.

LaunchpadCookieIdClientManager.secret is a manually cached property that is not cleared across transactions -- its state can leak between tests. This means that the first test in a layer can see an extra query as that cache is populated, causing query count to fluctuate depending on test order. Most tests already have a safety buffer of one query, so they tolerate this fluctuation.

r12418 caused transaction lifecycle events to be logged, adding an extra 'query' to most browser requests. This pushed some tests over their limit, but only when run as the first test in a layer. This branch causes the secret cache to be populated at layer setup time, removing the state leak and making query counts static again.

To see the difference, try these two test runs:

 bin/test -cvvvt test_binary_query_counts -t test_source_query_counts
 bin/test -cvvvt test_source_query_counts

The first invocation should always succeed, while the latter will fail without this branch.

I also fixed an unrelated import that was causing the import fascist to whinge.

[Jonathan Lange (jml)]

Personally, I'd prefer you didn't assign to 'unused', since that makes pyflakes upset. But that's a personal thing.

I also wish there were a better solution.

Either way, good to land.

review: Approve

[Jonathan Lange (jml)]

Looks good. Thanks for making pyflakes happy.

I wish the plugins change could be tested – I went to some effort to give remote.py good coverage – but I'm not sure how, or whether it would be an effective use of code. The right way to test it would be to check that it unpickled whatever it was that needs unpickling when run in a sub-process (so as to avoid the loaded plugins of the test process).

review: Approve

=== modified file 'lib/canonical/testing/layers.py'
--- lib/canonical/testing/layers.py	2011-02-19 13:50:19 +0000
+++ lib/canonical/testing/layers.py	2011-02-24 14:03:54 +0000
@@ -95,6 +95,7 @@
 from zope.server.logger.pythonlogger import PythonLogger
 from zope.testing.testrunner.runner import FakeInputContinueGenerator
 
+import canonical.launchpad.webapp.session
 from canonical.launchpad.webapp.vhosts import allvhosts
 from canonical.lazr import pidfile
 from canonical.config import CanonicalConfig, config, dbconfig
@@ -1054,6 +1055,9 @@
         if not is_ca_available():
             raise LayerInvariantError("Component architecture failed to load")
 
+        # Access the cookie manager's secret to get the cache populated.
+        # If we don't, it may issue extra queries depending on test order.
+        canonical.launchpad.webapp.session.idmanager.secret
         # If our request publication factories were defined using ZCML,
         # they'd be set up by FunctionalTestSetup().setUp(). Since
         # they're defined by Python code, we need to call that code
 
=== modified file 'lib/devscripts/ec2test/remote.py'
--- lib/devscripts/ec2test/remote.py	2011-02-23 11:41:12 +0000
+++ lib/devscripts/ec2test/remote.py	2011-02-24 14:03:54 +0000
@@ -36,20 +36,20 @@
 import unittest
 from xml.sax.saxutils import escape
 
-import simplejson
-
 import bzrlib.branch
 import bzrlib.config
+from bzrlib.email_message import EmailMessage
 import bzrlib.errors
+from bzrlib.smtp_connection import SMTPConnection
 import bzrlib.workingtree
-
-from bzrlib.email_message import EmailMessage
-from bzrlib.smtp_connection import SMTPConnection
-
+import simplejson
 import subunit
-
 from testtools import MultiTestResult
 
+# We need to be able to unpickle objects from bzr-pqm, so make sure we
+# can import it.
+bzrlib.plugin.load_plugins()
+
 
 class NonZeroExitCode(Exception):
     """Raised when the child process exits with a non-zero exit code."""
 
=== modified file 'lib/lp/bugs/scripts/checkwatches/remotebugupdater.py'
--- lib/lp/bugs/scripts/checkwatches/remotebugupdater.py	2011-02-23 08:57:39 +0000
+++ lib/lp/bugs/scripts/checkwatches/remotebugupdater.py	2011-02-24 14:03:54 +0000
@@ -13,7 +13,7 @@
 from zope.component import getUtility
 
 from canonical.database.constants import UTC_NOW
-from lp.bugs.externalbugtracker import (
+from lp.bugs.externalbugtracker.base import (
     BugNotFound,
     InvalidBugId,
     PrivateRemoteBug,