launchpad-reviewers team mailing list archive

[William Grant <me@xxxxxxxxxxxxxxxxxx>]

William Grant has proposed merging lp:~wgrant/launchpad/secret-query-count-determinism into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  #724039 test_source_query_counts failing spuriously
  https://bugs.launchpad.net/bugs/724039

For more details, see:
https://code.launchpad.net/~wgrant/launchpad/secret-query-count-determinism/+merge/51086

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.
-- 
https://code.launchpad.net/~wgrant/launchpad/secret-query-count-determinism/+merge/51086
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wgrant/launchpad/secret-query-count-determinism into lp:launchpad.

=== 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 08:56:22 +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.
+        unused = 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/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 08:56:22 +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,