launchpad-reviewers team mailing list archive

[Colin Watson <mp+428901@xxxxxxxxxxxxxxxxxx>]

Colin Watson has proposed merging ~cjwatson/launchpad:avoid-system-bzr into launchpad:master.

Commit message:
Fix two tests to avoid relying on system bzr

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/+git/launchpad/+merge/428901

A couple of tests in `lp.codehosting.scanner.tests.test_email.TestViaCelery` called out to `bzr whoami`, which failed if the system `bzr` package isn't installed.  The same effect can be achieved by temporarily setting an environment variable instead.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:avoid-system-bzr into launchpad:master.

diff --git a/lib/lp/codehosting/scanner/tests/test_email.py b/lib/lp/codehosting/scanner/tests/test_email.py
index f5d2021..0291fc6 100644
--- a/lib/lp/codehosting/scanner/tests/test_email.py
+++ b/lib/lp/codehosting/scanner/tests/test_email.py
@@ -4,7 +4,6 @@
 """Tests for the scanner's email generation."""
 
 import email
-import os
 
 from breezy.uncommit import uncommit
 from zope.component import getUtility
@@ -29,6 +28,7 @@ from lp.services.features.testing import FeatureFixture
 from lp.services.job.runner import JobRunner
 from lp.services.job.tests import block_on_job, pop_remote_notifications
 from lp.services.mail import stub
+from lp.services.osutils import override_environ
 from lp.testing import TestCaseWithFactory
 from lp.testing.dbuser import switch_dbuser
 from lp.testing.layers import CeleryJobLayer, LaunchpadZopelessLayer
@@ -166,10 +166,6 @@ class TestViaCelery(TestCaseWithFactory):
         switch_dbuser("branchscanner")
         # Needed for feature flag teardown
         self.addCleanup(switch_dbuser, config.launchpad.dbuser)
-        # Set 'bzr whoami' for proper test isolation.  (See bug 981114).
-        # This setting is done in an isolated bzr environment so it does not
-        # affect the environment of the person running the tests.
-        os.system("bzr whoami 'Nobody Knows <nobody@xxxxxxxxxxx>'")
         return db_branch, tree
 
     def test_empty_branch(self):
@@ -182,7 +178,8 @@ class TestViaCelery(TestCaseWithFactory):
     def test_uncommit_branch(self):
         """RevisionMailJob for removed revisions runs via Celery."""
         db_branch, tree = self.prepare("RevisionMailJob")
-        tree.commit("message")
+        with override_environ(BRZ_EMAIL="nobody@xxxxxxxxxxx"):
+            tree.commit("message")
         bzr_sync = BzrSync(db_branch)
         with block_on_job():
             bzr_sync.syncBranchAndClose(tree.branch)
@@ -197,12 +194,14 @@ class TestViaCelery(TestCaseWithFactory):
         # Enable RevisionMailJob to let celery activate a new connection
         # before trying to flush sent emails calling pop_remote_notifications.
         db_branch, tree = self.prepare("RevisionMailJob RevisionsAddedJob")
-        tree.commit("message")
+        with override_environ(BRZ_EMAIL="nobody@xxxxxxxxxxx"):
+            tree.commit("message")
         bzr_sync = BzrSync(db_branch)
         with block_on_job():
             bzr_sync.syncBranchAndClose(tree.branch)
         pop_remote_notifications()
-        tree.commit("message2")
+        with override_environ(BRZ_EMAIL="nobody@xxxxxxxxxxx"):
+            tree.commit("message2")
         with block_on_job():
             bzr_sync.syncBranchAndClose(tree.branch)
         self.assertEqual(1, len(pop_remote_notifications()))