Merge lp:~jml/launchpad/branch-sample-data-unit-tests into lp:launchpad

[Jonathan Lange]

Fix some unit tests to not need sample data. Add "sampledata" module and tests for Launchpad object factory.

This branch fixes up a bunch of unit tests to not use sample data.

The tests that I've changed are tests that would fail if all branch-related sample data were removed.

There's also a new "lp.testing.sampledata" module. The idea is that rather than using magic literal strings or numbers, we define constants in the file. Doing so also makes it much easier to statically analyze the code-base to find tests that use sampledata.

I have also used this branch as an opportunity to start testing the Launchpad object factory. I haven't written complete tests, but I have written tests to cover the changes I made. Now that tests exists, I suggest that we insist on tests for any changes made to the factory.

There are some incidental cleanups:
  * Change the sampledata header to have the correct copyright date, and change the Makefile to generate the correct copyright date.
  * Many tests changed to use DatabaseFunctionalLayer instead of LaunchpadFunctionalLayer
  * Removed "verifyObject" from the lp.testing namespace. I have no idea why it was there in the first place.

[Robert Collins (lifeless)]

I think this is useful; thanks for doing it.

review: Approve

=== modified file 'database/schema/Makefile'
--- database/schema/Makefile	2010-03-26 07:37:50 +0000
+++ database/schema/Makefile	2010-07-13 10:02:50 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009 Canonical Ltd.  This software is licensed under the
+# Copyright 2010 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 #
 # Quick hack makefile to (re)create the Launchpad database.
@@ -42,7 +42,7 @@
 # The command we use to drop (if exists) and recreate a database.
 CREATEDB=${PYTHON} ../../utilities/pgmassacre.py -t
 
-HEADER="-- Copyright 2009 Canonical Ltd.  This software is licensed under \
+HEADER="-- Copyright 2010 Canonical Ltd.  This software is licensed under \
 	the\n-- GNU Affero General Public License version 3 (see the file \
 	LICENSE)."
 

=== modified file 'lib/canonical/launchpad/scripts/tests/test_garbo.py'
--- lib/canonical/launchpad/scripts/tests/test_garbo.py	2010-05-26 01:48:12 +0000
+++ lib/canonical/launchpad/scripts/tests/test_garbo.py	2010-07-13 10:02:50 +0000
@@ -175,11 +175,18 @@
         results_to_keep_count = (
             config.codeimport.consecutive_failure_limit - 1)
 
+        LaunchpadZopelessLayer.switchDbUser('testadmin')
+        code_import_id = self.factory.makeCodeImport().id
+        machine_id = self.factory.makeCodeImportMachine().id
+        requester_id = self.factory.makePerson().id
+        transaction.commit()
+
         def new_code_import_result(timestamp):
             LaunchpadZopelessLayer.switchDbUser('testadmin')
             CodeImportResult(
                 date_created=timestamp,
-                code_importID=1, machineID=1, requesting_userID=1,
+                code_importID=code_import_id, machineID=machine_id,
+                requesting_userID=requester_id,
                 status=CodeImportResultStatus.FAILURE,
                 date_job_started=timestamp)
             transaction.commit()

=== modified file 'lib/canonical/launchpad/tests/test_launchpadlib.py'
--- lib/canonical/launchpad/tests/test_launchpadlib.py	2009-06-25 05:30:52 +0000
+++ lib/canonical/launchpad/tests/test_launchpadlib.py	2010-07-13 10:02:50 +0000
@@ -5,22 +5,22 @@
 
 import unittest
 
+import transaction
+
 from launchpadlib.testing.helpers import salgado_with_full_permissions
 from canonical.testing import AppServerLayer
-
-
-class TestLaunchpadLib(unittest.TestCase):
+from lp.testing import TestCaseWithFactory
+
+
+class TestLaunchpadLib(TestCaseWithFactory):
     """Tests for the launchpadlib client for the REST API."""
 
     layer = AppServerLayer
-    launchpad = None
-    project = None
 
     def setUp(self):
-        if self.launchpad is None:
-            self.launchpad = salgado_with_full_permissions.login()
-        if self.project is None:
-            self.project = self.launchpad.projects['firefox']
+        super(TestLaunchpadLib, self).setUp()
+        self.launchpad = salgado_with_full_permissions.login()
+        self.project = self.launchpad.projects['firefox']
 
     def verifyAttributes(self, element):
         """Verify that launchpadlib can parse the element's attributes."""
@@ -43,7 +43,10 @@
 
     def test_branch(self):
         """Test branch attributes."""
-        branch = self.project.getBranches()[0]
+        branch_name = self.factory.makeBranch().unique_name
+        transaction.commit()
+        branch = self.launchpad.branches.getByUniqueName(
+            unique_name=branch_name)
         self.verifyAttributes(branch)
 
     def test_milestone(self):

=== modified file 'lib/lp/code/browser/tests/test_branch.py'
--- lib/lp/code/browser/tests/test_branch.py	2010-06-14 02:37:06 +0000
+++ lib/lp/code/browser/tests/test_branch.py	2010-07-13 10:02:50 +0000
@@ -6,7 +6,6 @@
 from __future__ import with_statement
 
 __metaclass__ = type
-__all__ = ['TestBranchView', 'test_suite']
 
 from datetime import datetime, timedelta
 from textwrap import dedent
@@ -16,7 +15,6 @@
 import simplejson
 
 
-from zope.component import getUtility
 from zope.security.proxy import removeSecurityProxy
 
 from canonical.config import config
@@ -32,9 +30,6 @@
 from lp.code.interfaces.branchtarget import IBranchTarget
 from canonical.launchpad.helpers import truncate_text
 from lp.code.enums import BranchLifecycleStatus, BranchType
-from lp.registry.interfaces.person import IPersonSet
-from lp.registry.interfaces.product import IProductSet
-from lp.code.interfaces.branchlookup import IBranchLookup
 from lp.testing import (
     login, login_person, logout, person_logged_in, ANONYMOUS,
     TestCaseWithFactory)
@@ -122,20 +117,18 @@
 
 class TestBranchView(TestCaseWithFactory):
 
-    layer = LaunchpadFunctionalLayer
+    layer = DatabaseFunctionalLayer
 
     def setUp(self):
         super(TestBranchView, self).setUp()
-        login(ANONYMOUS)
         self.request = LaunchpadTestRequest()
 
-    def tearDown(self):
-        logout()
-        super(TestBranchView, self).tearDown()
-
     def testMirrorStatusMessageIsTruncated(self):
         """mirror_status_message is truncated if the text is overly long."""
-        branch = getUtility(IBranchLookup).get(28)
+        branch = self.factory.makeBranch()
+        branch.mirrorFailed(
+            "on quick brown fox the dog jumps to" *
+            BranchMirrorStatusView.MAXIMUM_STATUS_MESSAGE_LENGTH)
         branch_view = BranchMirrorStatusView(branch, self.request)
         self.assertEqual(
             truncate_text(branch.mirror_status_message,
@@ -144,7 +137,7 @@
 
     def testMirrorStatusMessage(self):
         """mirror_status_message on the view is the same as on the branch."""
-        branch = getUtility(IBranchLookup).get(5)
+        branch = self.factory.makeBranch()
         branch.mirrorFailed("This is a short error message.")
         branch_view = BranchMirrorStatusView(branch, self.request)
         self.assertTrue(
@@ -160,8 +153,8 @@
 
     def testBranchAddRequestsMirror(self):
         """Registering a mirrored branch requests a mirror."""
-        arbitrary_person = getUtility(IPersonSet).get(1)
-        arbitrary_product = getUtility(IProductSet).get(1)
+        arbitrary_person = self.factory.makePerson()
+        arbitrary_product = self.factory.makeProduct()
         login(arbitrary_person.preferredemail.email)
         try:
             add_view = BranchAddView(arbitrary_person, self.request)
@@ -195,7 +188,7 @@
         # The merge links are shown on projects that have multiple branches.
         product = self.factory.makeProduct(name='super-awesome-project')
         branch1 = self.factory.makeAnyBranch(product=product)
-        branch2 = self.factory.makeAnyBranch(product=product)
+        self.factory.makeAnyBranch(product=product)
         view = BranchView(branch1, self.request)
         view.initialize()
         self.assertTrue(view.show_merge_links)

=== modified file 'lib/lp/code/interfaces/codeimportjob.py'
--- lib/lp/code/interfaces/codeimportjob.py	2010-03-16 01:34:38 +0000
+++ lib/lp/code/interfaces/codeimportjob.py	2010-07-13 10:02:50 +0000
@@ -16,8 +16,6 @@
     'ICodeImportJobWorkflow',
     ]
 
-import datetime
-
 from zope.interface import Interface
 from zope.schema import Choice, Datetime, Int, Object, Text
 

=== modified file 'lib/lp/code/model/tests/test_codeimportjob.py'
--- lib/lp/code/model/tests/test_codeimportjob.py	2010-07-13 10:02:48 +0000
+++ lib/lp/code/model/tests/test_codeimportjob.py	2010-07-13 10:02:50 +0000
@@ -5,7 +5,9 @@
 
 __metaclass__ = type
 
-__all__ = ['NewEvents', 'test_suite']
+__all__ = [
+    'NewEvents',
+    ]
 
 from datetime import datetime
 from pytz import UTC
@@ -24,7 +26,6 @@
 from canonical.config import config
 from canonical.database.constants import UTC_NOW
 from lp.code.model.codeimportjob import CodeImportJob
-from lp.code.model.codeimportmachine import CodeImportMachine
 from lp.code.model.codeimportresult import CodeImportResult
 from canonical.launchpad.interfaces.librarian import ILibraryFileAliasSet
 from lp.code.enums import (
@@ -38,12 +39,14 @@
 from lp.registry.interfaces.person import IPersonSet
 from lp.testing import (
     ANONYMOUS, login, login_celebrity, logout, TestCaseWithFactory)
+from lp.testing.sampledata import NO_PRIVILEGE_EMAIL, VCS_IMPORTS_MEMBER_EMAIL
 from canonical.launchpad.testing.codeimporthelpers import (
     make_finished_import, make_running_import)
 from canonical.launchpad.testing.pages import get_feedback_messages
 from canonical.launchpad.webapp import canonical_url
 from canonical.librarian.interfaces import ILibrarianClient
-from canonical.testing import LaunchpadFunctionalLayer
+from canonical.testing import (
+    DatabaseFunctionalLayer, LaunchpadFunctionalLayer)
 
 
 def login_for_code_imports():
@@ -52,28 +55,29 @@
     CodeImports are currently hidden from regular users currently. Members of
     the vcs-imports team and can access the objects freely.
     """
-    login_celebrity('vcs_imports')
-
-
-class TestCodeImportJobSet(unittest.TestCase):
+    return login_celebrity('vcs_imports')
+
+
+class TestCodeImportJobSet(TestCaseWithFactory):
     """Unit tests for the CodeImportJobSet utility."""
 
-    layer = LaunchpadFunctionalLayer
+    layer = DatabaseFunctionalLayer
 
     def setUp(self):
+        super(TestCodeImportJobSet, self).setUp()
         login_for_code_imports()
 
     def test_getByIdExisting(self):
         # CodeImportJobSet.getById retrieves a CodeImportJob by database id.
-        job = getUtility(ICodeImportJobSet).getById(1)
-        self.assertNotEqual(job, None)
-        self.assertEqual(job.id, 1)
+        made_job = self.factory.makeCodeImportJob()
+        found_job = getUtility(ICodeImportJobSet).getById(made_job.id)
+        self.assertEqual(made_job, found_job)
 
     def test_getByIdNotExisting(self):
         # CodeImportJobSet.getById returns None if there is not CodeImportJob
         # with the specified id.
         no_job = getUtility(ICodeImportJobSet).getById(-1)
-        self.assertEqual(no_job, None)
+        self.assertIs(None, no_job)
 
 
 class TestCodeImportJobSetGetJobForMachine(TestCaseWithFactory):
@@ -88,7 +92,7 @@
     method makeJob() creates actual CodeImportJob objects from these specs.
     """
 
-    layer = LaunchpadFunctionalLayer
+    layer = DatabaseFunctionalLayer
 
     def setUp(self):
         # Login so we can access the code import system, delete all jobs in
@@ -115,7 +119,7 @@
     def assertJobIsSelected(self, desired_job):
         """Assert that the expected job is chosen by getJobForMachine."""
         observed_job = getUtility(ICodeImportJobSet).getJobForMachine(
-            self.machine.hostname, 10)
+            self.machine.hostname, worker_limit=10)
         self.assert_(observed_job is not None, "No job was selected.")
         self.assertEqual(desired_job, observed_job,
                          "Expected job not selected.")
@@ -123,7 +127,7 @@
     def assertNoJobSelected(self):
         """Assert that no job is selected."""
         observed_job = getUtility(ICodeImportJobSet).getJobForMachine(
-            'machine', 10)
+            'machine', worker_limit=10)
         self.assert_(observed_job is None, "Job unexpectedly selected.")
 
     def test_nothingSelectedIfNothingCreated(self):
@@ -231,7 +235,7 @@
 class TestCodeImportJobSetGetReclaimableJobs(ReclaimableJobTests):
     """Tests for the CodeImportJobSet.getReclaimableJobs method."""
 
-    layer = LaunchpadFunctionalLayer
+    layer = DatabaseFunctionalLayer
 
     def test_upToDateJob(self):
         # A job that was updated recently is not considered reclaimable.
@@ -264,7 +268,7 @@
 class TestCodeImportJobSetGetJobForMachineGardening(ReclaimableJobTests):
     """Test that getJobForMachine gardens stale code import jobs."""
 
-    layer = LaunchpadFunctionalLayer
+    layer = DatabaseFunctionalLayer
 
     def test_getJobForMachineGardens(self):
         # getJobForMachine reclaims all reclaimable jobs each time it is
@@ -275,7 +279,7 @@
         machine = self.factory.makeCodeImportMachine(set_online=True)
         login(ANONYMOUS)
         getUtility(ICodeImportJobSet).getJobForMachine(
-            machine.hostname, 10)
+            machine.hostname, worker_limit=10)
         login_for_code_imports()
         # Now there are no reclaimable jobs.
         self.assertReclaimableJobs([])
@@ -364,7 +368,7 @@
     AssertFailureMixin):
     """Unit tests for the CodeImportJobWorkflow.newJob method."""
 
-    layer = LaunchpadFunctionalLayer
+    layer = DatabaseFunctionalLayer
 
     def setUp(self):
         super(TestCodeImportJobWorkflowNewJob, self).setUp()
@@ -373,50 +377,31 @@
     def test_wrongReviewStatus(self):
         # CodeImportJobWorkflow.newJob fails if the CodeImport review_status
         # is different from REVIEWED.
-        new_import = getUtility(ICodeImportSet).get(2)
-        # Checking sampledata expectations.
-        self.assertEqual(new_import.branch.unique_name,
-                         '~vcs-imports/evolution/import')
-        NEW = CodeImportReviewStatus.NEW
-        self.assertEqual(new_import.review_status, NEW)
+        new_import = self.factory.makeCodeImport()
+        branch_name = new_import.branch.unique_name
         # Testing newJob failure.
         self.assertFailure(
-            "Review status of ~vcs-imports/evolution/import "
-            "is not REVIEWED: NEW",
+            "Review status of %s is not REVIEWED: NEW" % (branch_name,),
             getUtility(ICodeImportJobWorkflow).newJob, new_import)
 
     def test_existingJob(self):
         # CodeImportJobWorkflow.newJob fails if the CodeImport is already
         # associated to a CodeImportJob.
-        reviewed_import = getUtility(ICodeImportSet).get(1)
-        # Checking sampledata expectations.
-        self.assertEqual(reviewed_import.branch.unique_name,
-                         '~vcs-imports/gnome-terminal/import')
-        REVIEWED = CodeImportReviewStatus.REVIEWED
-        self.assertEqual(reviewed_import.review_status, REVIEWED)
-        self.assertNotEqual(reviewed_import.import_job, None)
-        # Testing newJob failure.
+        job = self.factory.makeCodeImportJob()
+        reviewed_import = job.code_import
+        branch_name = reviewed_import.branch.unique_name
         self.assertFailure(
-            "Already associated to a CodeImportJob: "
-            "~vcs-imports/gnome-terminal/import",
+            "Already associated to a CodeImportJob: %s" % (branch_name,),
             getUtility(ICodeImportJobWorkflow).newJob, reviewed_import)
 
     def getCodeImportForDateDueTest(self):
         """Return a `CodeImport` object for testing how date_due is set.
 
-        We check that it is not associated to any `CodeImportJob` or
-        `CodeImportResult`, and we ensure its review_status is REVIEWED.
+        It is not associated to any `CodeImportJob` or `CodeImportResult`, and
+        its review_status is REVIEWED.
         """
-        new_import = getUtility(ICodeImportSet).get(2)
-        # Checking sampledata expectations.
-        self.assertEqual(new_import.import_job, None)
-        self.assertEqual(
-            CodeImportResult.selectBy(code_importID=new_import.id).count(), 0)
-        # We need to set review_status to REVIEWED before calling newJob, and
-        # the interface marks review_status as read-only.
-        REVIEWED = CodeImportReviewStatus.REVIEWED
-        removeSecurityProxy(new_import).review_status = REVIEWED
-        return new_import
+        return self.factory.makeCodeImport(
+            review_status=CodeImportReviewStatus.REVIEWED)
 
     def test_dateDueNoPreviousResult(self):
         # If there is no CodeImportResult for the CodeImport, then the new
@@ -432,7 +417,7 @@
         code_import = self.getCodeImportForDateDueTest()
         # Create a CodeImportResult that started a long time ago. This one
         # must be superseded by the more recent one created below.
-        machine = CodeImportMachine.get(1)
+        machine = self.factory.makeCodeImportMachine()
         FAILURE = CodeImportResultStatus.FAILURE
         CodeImportResult(
             code_import=code_import, machine=machine, status=FAILURE,
@@ -467,7 +452,7 @@
         # set to UTC_NOW.
         code_import = self.getCodeImportForDateDueTest()
         # Create a CodeImportResult that started a long time ago.
-        machine = CodeImportMachine.get(1)
+        machine = self.factory.makeCodeImportMachine()
         FAILURE = CodeImportResultStatus.FAILURE
         CodeImportResult(
             code_import=code_import, machine=machine, status=FAILURE,
@@ -478,78 +463,66 @@
         self.assertSqlAttributeEqualsDate(job, 'date_due', UTC_NOW)
 
 
-class TestCodeImportJobWorkflowDeletePendingJob(unittest.TestCase,
+class TestCodeImportJobWorkflowDeletePendingJob(TestCaseWithFactory,
         AssertFailureMixin):
     """Unit tests for CodeImportJobWorkflow.deletePendingJob."""
 
-    layer = LaunchpadFunctionalLayer
+    layer = DatabaseFunctionalLayer
 
     def setUp(self):
         super(TestCodeImportJobWorkflowDeletePendingJob, self).setUp()
-        login_for_code_imports()
+        self.import_admin = login_for_code_imports()
 
     def test_wrongReviewStatus(self):
         # CodeImportJobWorkflow.deletePendingJob fails if the
         # CodeImport review_status is equal to REVIEWED.
-        reviewed_import = getUtility(ICodeImportSet).get(1)
-        # Checking sampledata expectations.
-        self.assertEqual(reviewed_import.branch.unique_name,
-                         '~vcs-imports/gnome-terminal/import')
-        REVIEWED = CodeImportReviewStatus.REVIEWED
-        self.assertEqual(reviewed_import.review_status, REVIEWED)
+        reviewed_import = self.factory.makeCodeImport()
+        reviewed_import.updateFromData(
+            {'review_status': CodeImportReviewStatus.REVIEWED},
+            self.import_admin)
+        branch_name = reviewed_import.branch.unique_name
         # Testing deletePendingJob failure.
         self.assertFailure(
-            "The review status of ~vcs-imports/gnome-terminal/import "
-            "is REVIEWED.",
+            "The review status of %s is REVIEWED." % (branch_name,),
             getUtility(ICodeImportJobWorkflow).deletePendingJob,
             reviewed_import)
 
     def test_noJob(self):
         # CodeImportJobWorkflow.deletePendingJob fails if the
         # CodeImport is not associated to a CodeImportJob.
-        new_import = getUtility(ICodeImportSet).get(2)
-        # Checking sampledata expectations.
-        self.assertEqual(new_import.branch.unique_name,
-                         '~vcs-imports/evolution/import')
-        NEW = CodeImportReviewStatus.NEW
-        self.assertEqual(new_import.review_status, NEW)
-        self.assertEqual(new_import.import_job, None)
+        new_import = self.factory.makeCodeImport()
+        branch_name = new_import.branch.unique_name
         # Testing deletePendingJob failure.
         self.assertFailure(
-            "Not associated to a CodeImportJob: "
-            "~vcs-imports/evolution/import",
+            "Not associated to a CodeImportJob: %s" % (branch_name,),
             getUtility(ICodeImportJobWorkflow).deletePendingJob,
             new_import)
 
     def test_wrongJobState(self):
         # CodeImportJobWorkflow.deletePendingJob fails if the state of
         # the CodeImportJob is different from PENDING.
-        reviewed_import = getUtility(ICodeImportSet).get(1)
-        # Checking sampledata expectations.
-        self.assertEqual(reviewed_import.branch.unique_name,
-                         '~vcs-imports/gnome-terminal/import')
-        # ICodeImport does not allow setting any attribute, so we need to use
-        # removeSecurityProxy to set the review_status attribute.
+        job = self.factory.makeCodeImportJob()
+        code_import = job.code_import
+        branch_name = job.code_import.branch.unique_name
+        # ICodeImport does not allow setting 'review_status', so we must use
+        # removeSecurityProxy.
         INVALID = CodeImportReviewStatus.INVALID
-        removeSecurityProxy(reviewed_import).review_status = INVALID
-        self.assertNotEqual(reviewed_import.import_job, None)
+        removeSecurityProxy(code_import).review_status = INVALID
         # ICodeImportJob does not allow setting 'state', so we must
         # use removeSecurityProxy.
         RUNNING = CodeImportJobState.RUNNING
-        removeSecurityProxy(reviewed_import.import_job).state = RUNNING
+        removeSecurityProxy(code_import.import_job).state = RUNNING
         # Testing deletePendingJob failure.
         self.assertFailure(
-            "The CodeImportJob associated to "
-            "~vcs-imports/gnome-terminal/import is RUNNING.",
-            getUtility(ICodeImportJobWorkflow).deletePendingJob,
-            reviewed_import)
+            "The CodeImportJob associated to %s is RUNNING." % (branch_name,),
+            getUtility(ICodeImportJobWorkflow).deletePendingJob, code_import)
 
 
 class TestCodeImportJobWorkflowRequestJob(TestCaseWithFactory,
         AssertFailureMixin, AssertEventMixin):
     """Unit tests for CodeImportJobWorkflow.requestJob."""
 
-    layer = LaunchpadFunctionalLayer
+    layer = DatabaseFunctionalLayer
 
     def setUp(self):
         super(TestCodeImportJobWorkflowRequestJob, self).setUp()
@@ -641,7 +614,7 @@
         AssertFailureMixin, AssertEventMixin):
     """Unit tests for CodeImportJobWorkflow.startJob."""
 
-    layer = LaunchpadFunctionalLayer
+    layer = DatabaseFunctionalLayer
 
     def setUp(self):
         super(TestCodeImportJobWorkflowStartJob, self).setUp()
@@ -679,7 +652,7 @@
         AssertFailureMixin, AssertEventMixin):
     """Unit tests for CodeImportJobWorkflow.updateHeartbeat."""
 
-    layer = LaunchpadFunctionalLayer
+    layer = DatabaseFunctionalLayer
 
     def setUp(self):
         super(TestCodeImportJobWorkflowUpdateHeartbeat, self).setUp()
@@ -705,7 +678,7 @@
 
     def setUp(self):
         super(TestCodeImportJobWorkflowFinishJob, self).setUp()
-        login_for_code_imports()
+        self.vcs_imports = login_for_code_imports()
         self.machine = self.factory.makeCodeImportMachine(set_online=True)
 
     def makeRunningJob(self, code_import=None):
@@ -804,10 +777,9 @@
         # unless the CodeImport has been suspended or marked invalid.
         running_job = self.makeRunningJob()
         code_import = running_job.code_import
-        ddaa = getUtility(IPersonSet).getByEmail(
-            '[e-mail]')
         code_import.updateFromData(
-            {'review_status': CodeImportReviewStatus.SUSPENDED}, ddaa)
+            {'review_status': CodeImportReviewStatus.SUSPENDED},
+            self.vcs_imports)
         getUtility(ICodeImportJobWorkflow).finishJob(
             running_job, CodeImportResultStatus.SUCCESS, None)
         self.assertTrue(code_import.import_job is None)
@@ -920,8 +892,7 @@
         log_alias = getUtility(ILibraryFileAliasSet)[log_alias_id]
         result = self.getResultForJob(job, log_alias=log_alias)
 
-        self.assertEqual(
-            result.log_file.read(), log_data)
+        self.assertEqual(result.log_file.read(), log_data)
 
     def test_createsFinishCodeImportEvent(self):
         # finishJob() creates a FINISH CodeImportEvent.
@@ -1005,7 +976,7 @@
 
 # This is a dependence on the sample data: David Allouche is a member of the
 # ~vcs-imports celebrity team.
-logged_in_for_code_imports = logged_in_as('[e-mail]')
+logged_in_for_code_imports = logged_in_as(VCS_IMPORTS_MEMBER_EMAIL)
 
 
 class TestRequestJobUIRaces(TestCaseWithFactory):
@@ -1016,7 +987,7 @@
     the button and check that appropriate notifications are displayed.
     """
 
-    layer = LaunchpadFunctionalLayer
+    layer = DatabaseFunctionalLayer
 
     @logged_in_for_code_imports
     def getNewCodeImportIDAndBranchURL(self):
@@ -1027,7 +998,7 @@
         code_import_id = code_import.id
         return code_import_id, branch_url
 
-    @logged_in_as('[e-mail]')
+    @logged_in_as(NO_PRIVILEGE_EMAIL)
     def requestJobByUserWithDisplayName(self, code_import_id, displayname):
         """Record a request for the job by a user with the given name."""
         getUtility(ICodeImportJobWorkflow).requestJob(

=== modified file 'lib/lp/testing/factory.py'
--- lib/lp/testing/factory.py	2010-07-08 11:54:57 +0000
+++ lib/lp/testing/factory.py	2010-07-13 10:02:50 +0000
@@ -1436,7 +1436,7 @@
     def makeCodeImport(self, svn_branch_url=None, cvs_root=None,
                        cvs_module=None, target=None, branch_name=None,
                        git_repo_url=None, hg_repo_url=None, registrant=None,
-                       rcs_type=None):
+                       rcs_type=None, review_status=None):
         """Create and return a new, arbitrary code import.
 
         The type of code import will be inferred from the source details
@@ -1461,26 +1461,29 @@
             else:
                 assert rcs_type in (RevisionControlSystems.SVN,
                                     RevisionControlSystems.BZR_SVN)
-            return code_import_set.new(
+            code_import = code_import_set.new(
                 registrant, target, branch_name, rcs_type=rcs_type,
                 url=svn_branch_url)
         elif git_repo_url is not None:
             assert rcs_type in (None, RevisionControlSystems.GIT)
-            return code_import_set.new(
+            code_import = code_import_set.new(
                 registrant, target, branch_name,
                 rcs_type=RevisionControlSystems.GIT,
                 url=git_repo_url)
         elif hg_repo_url is not None:
-            return code_import_set.new(
+            code_import = code_import_set.new(
                 registrant, target, branch_name,
                 rcs_type=RevisionControlSystems.HG,
                 url=hg_repo_url)
         else:
             assert rcs_type in (None, RevisionControlSystems.CVS)
-            return code_import_set.new(
+            code_import = code_import_set.new(
                 registrant, target, branch_name,
                 rcs_type=RevisionControlSystems.CVS,
                 cvs_root=cvs_root, cvs_module=cvs_module)
+        if review_status:
+            removeSecurityProxy(code_import).review_status = review_status
+        return code_import
 
     def makeCodeImportEvent(self):
         """Create and return a CodeImportEvent."""

=== added file 'lib/lp/testing/sampledata.py'
--- lib/lp/testing/sampledata.py	1970-01-01 00:00:00 +0000
+++ lib/lp/testing/sampledata.py	2010-07-13 10:02:50 +0000
@@ -0,0 +1,18 @@
+# Copyright 2010 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Constants that refer to values in sampledata.
+
+If ever you use a literal in a test that refers to sample data, even if it's
+just a small number, then you should define it as a constant here.
+"""
+
+__metaclass__ = type
+__all__ = [
+    'NO_PRIVILEGE_EMAIL',
+    'VCS_IMPORTS_MEMBER_EMAIL',
+    ]
+
+
+NO_PRIVILEGE_EMAIL = '[e-mail]'
+VCS_IMPORTS_MEMBER_EMAIL = '[e-mail]'

=== added file 'lib/lp/testing/tests/test_factory.py'
--- lib/lp/testing/tests/test_factory.py	1970-01-01 00:00:00 +0000
+++ lib/lp/testing/tests/test_factory.py	2010-07-13 10:02:50 +0000
@@ -0,0 +1,34 @@
+# Copyright 2010 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Tests for the Launchpad object factory."""
+
+__metaclass__ = type
+
+import unittest
+
+from canonical.testing.layers import DatabaseFunctionalLayer
+from lp.code.enums import CodeImportReviewStatus
+from lp.testing import TestCaseWithFactory
+
+
+class TestFactory(TestCaseWithFactory):
+
+    layer = DatabaseFunctionalLayer
+
+    def test_makeCodeImportNoStatus(self):
+        # If makeCodeImport is not given a review status, it defaults to NEW.
+        code_import = self.factory.makeCodeImport()
+        self.assertEqual(
+            CodeImportReviewStatus.NEW, code_import.review_status)
+
+    def test_makeCodeImportReviewStatus(self):
+        # If makeCodeImport is given a review status, then that is the status
+        # of the created import.
+        status = CodeImportReviewStatus.REVIEWED
+        code_import = self.factory.makeCodeImport(review_status=status)
+        self.assertEqual(status, code_import.review_status)
+
+
+def test_suite():
+    return unittest.TestLoader().loadTestsFromName(__name__)