launchpad-reviewers team mailing list archive

[Brad Crittenden <bac@xxxxxxxxxxxxx>]

Brad Crittenden has proposed merging lp:~bac/launchpad/bug-987499 into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #987499 in Launchpad itself: "lp.app.browser.tests.test_stringformatter.TestOOPSFormatter.test_doesnt_linkify_for_non_developers fails intermittently in parallel tests"
  https://bugs.launchpad.net/launchpad/+bug/987499

For more details, see:
https://code.launchpad.net/~bac/launchpad/bug-987499/+merge/104607

= Summary =

LaunchBag.developer is being assumed to be unset when it can be left
on by previous tests.  Clear it explicitly. 

== Proposed fix ==

In the test, explicitly turn off the flag.

Alternatively, a tearDown method can be added to TestCase to ensure it
is always reset to False at the end of a run.  

Similarly, a ILoggedOutEvent could be sent.

This flag is rarely used so I chose to not muddy the test
infrastructure to deal with it.

== Pre-implementation notes ==

None

== Implementation details ==

== Tests ==

Set tests.txt to:
lp.registry.browser.tests.test_subscription_links.DistroMilestoneView.test_subscribe_link_admin
lp.app.browser.tests.test_stringformatter.TestOOPSFormatter.test_doesnt_linkify_for_non_developers

bin/test -vv --load-list tests.txt

== Demo and Q/A ==

None

= Launchpad lint =

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/app/browser/tests/test_stringformatter.py
-- 
https://code.launchpad.net/~bac/launchpad/bug-987499/+merge/104607
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~bac/launchpad/bug-987499 into lp:launchpad.

=== modified file 'lib/lp/app/browser/tests/test_stringformatter.py'
--- lib/lp/app/browser/tests/test_stringformatter.py	2012-04-02 05:42:19 +0000
+++ lib/lp/app/browser/tests/test_stringformatter.py	2012-05-03 19:35:23 +0000
@@ -373,10 +373,16 @@
 
     layer = DatabaseFunctionalLayer
 
+    def _setDeveloper(self, value):
+        """Override ILaunchBag.developer for testing purposes."""
+        launch_bag = getUtility(ILaunchBag)
+        launch_bag.setDeveloper(value)
+
     def test_doesnt_linkify_for_non_developers(self):
         # OOPS IDs won't be linkified for non-developers.
         oops_id = 'OOPS-12345TEST'
         formatter = FormattersAPI(oops_id)
+        self._setDeveloper(False)
         formatted_string = formatter.oops_id()
 
         self.assertEqual(
@@ -384,17 +390,11 @@
             "Formatted string should be '%s', was '%s'" % (
                 oops_id, formatted_string))
 
-    def _setDeveloper(self):
-        """Override ILaunchBag.developer for testing purposes."""
-        launch_bag = getUtility(ILaunchBag)
-        launch_bag.setDeveloper(True)
-
     def test_linkifies_for_developers(self):
         # OOPS IDs will be linkified for Launchpad developers.
         oops_id = 'OOPS-12345TEST'
         formatter = FormattersAPI(oops_id)
-
-        self._setDeveloper()
+        self._setDeveloper(True)
         formatted_string = formatter.oops_id()
 
         expected_string = '<a href="%s">%s</a>' % (