← Back to team overview

launchpad-reviewers team mailing list archive

[Merge] ~cjwatson/launchpad:py3-breezy-revids-db into launchpad:master

 

Colin Watson has proposed merging ~cjwatson/launchpad:py3-breezy-revids-db into launchpad:master.

Commit message:
Convert Breezy revision IDs to text for DB purposes

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

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

Breezy's revision IDs are bytes on Python 3, but we store them in the database as text.  Encode or decode as appropriate.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:py3-breezy-revids-db into launchpad:master.
diff --git a/lib/lp/code/bzr.py b/lib/lp/code/bzr.py
index ce51ca4..88d7abb 100644
--- a/lib/lp/code/bzr.py
+++ b/lib/lp/code/bzr.py
@@ -329,7 +329,7 @@ def branch_changed(db_branch, bzr_branch=None):
         stacked_on = bzr_branch.get_stacked_on_url()
     except (NotStacked, UnstackableBranchFormat):
         stacked_on = None
-    last_revision = bzr_branch.last_revision()
+    last_revision = six.ensure_text(bzr_branch.last_revision())
     formats = get_branch_formats(bzr_branch)
     db_branch.branchChanged(stacked_on, last_revision, *formats)
 
diff --git a/lib/lp/code/model/branch.py b/lib/lp/code/model/branch.py
index dac3b4a..947aaf1 100644
--- a/lib/lp/code/model/branch.py
+++ b/lib/lp/code/model/branch.py
@@ -1185,7 +1185,7 @@ class Branch(SQLBase, WebhookTargetMixin, BzrIdentityMixin):
         # in the database, so if the revision_date is a future date, then we
         # use the date created instead.
         if db_revision is None:
-            revision_id = NULL_REVISION
+            revision_id = six.ensure_text(NULL_REVISION)
             revision_date = UTC_NOW
         else:
             revision_id = db_revision.revision_id
@@ -1386,6 +1386,8 @@ class Branch(SQLBase, WebhookTargetMixin, BzrIdentityMixin):
             increment = getUtility(IBranchPuller).MIRROR_TIME_INCREMENT
             self.next_mirror_time = (
                 datetime.now(pytz.timezone('UTC')) + increment)
+        if isinstance(last_revision_id, bytes):
+            last_revision_id = last_revision_id.decode('ASCII')
         self.last_mirrored_id = last_revision_id
         if self.last_scanned_id != last_revision_id:
             from lp.code.model.branchjob import BranchScanJob
diff --git a/lib/lp/code/model/branchjob.py b/lib/lp/code/model/branchjob.py
index 3463db7..e3c1c16 100644
--- a/lib/lp/code/model/branchjob.py
+++ b/lib/lp/code/model/branchjob.py
@@ -533,7 +533,8 @@ class RevisionsAddedJob(BranchJobDerived):
         """Iterate through revisions added to the mainline."""
         repository = self.bzr_branch.repository
         added_revisions = repository.get_graph().find_unique_ancestors(
-            self.last_revision_id, [self.last_scanned_id])
+            six.ensure_binary(self.last_revision_id),
+            [six.ensure_binary(self.last_scanned_id)])
         # Avoid hitting the database since breezy makes it easy to check.
         # There are possibly more efficient ways to get the mainline
         # revisions, but this is simple and it works.
@@ -673,7 +674,8 @@ class RevisionsAddedJob(BranchJobDerived):
             (BranchMergeProposal, Branch),
             BranchMergeProposal.target_branch == self.branch.id,
             BranchMergeProposal.source_branch == Branch.id,
-            Branch.last_scanned_id.is_in(revision_ids),
+            Branch.last_scanned_id.is_in({
+                six.ensure_text(revision_id) for revision_id in revision_ids}),
             (BranchMergeProposal.queue_status !=
              BranchMergeProposalStatus.SUPERSEDED))
 
@@ -799,6 +801,7 @@ class RosettaUploadJob(BranchJobDerived):
 
         if from_revision_id is None:
             from_revision_id = NULL_REVISION
+        from_revision_id = six.ensure_text(from_revision_id)
 
         if force_translations_upload or cls.providesTranslationFiles(branch):
             metadata = cls.getMetadata(from_revision_id,
@@ -874,9 +877,9 @@ class RosettaUploadJob(BranchJobDerived):
 
         bzrbranch = self.branch.getBzrBranch()
         from_tree = bzrbranch.repository.revision_tree(
-            self.from_revision_id)
+            six.ensure_binary(self.from_revision_id))
         to_tree = bzrbranch.repository.revision_tree(
-            self.branch.last_scanned_id)
+            six.ensure_binary(self.branch.last_scanned_id))
 
         importer = TranslationImporter()
 
diff --git a/lib/lp/code/model/diff.py b/lib/lp/code/model/diff.py
index cd973c3..54f1aa1 100644
--- a/lib/lp/code/model/diff.py
+++ b/lib/lp/code/model/diff.py
@@ -308,8 +308,9 @@ class Diff(SQLBase):
             for branch in [source_branch] + ignore_branches:
                 stack.enter_context(read_locked(branch))
             diff_ignore_branches(
-                source_branch, ignore_branches, old_revision.revision_id,
-                new_revision.revision_id, diff_content)
+                source_branch, ignore_branches,
+                six.ensure_binary(old_revision.revision_id),
+                six.ensure_binary(new_revision.revision_id), diff_content)
         return cls.fromFileAtEnd(diff_content)
 
 
diff --git a/lib/lp/code/model/directbranchcommit.py b/lib/lp/code/model/directbranchcommit.py
index 0b7a902..89586b0 100644
--- a/lib/lp/code/model/directbranchcommit.py
+++ b/lib/lp/code/model/directbranchcommit.py
@@ -18,6 +18,7 @@ from breezy.transform import (
     ROOT_PARENT,
     TransformPreview,
     )
+import six
 
 from lp.code.errors import StaleLastMirrored
 from lp.codehosting.bzrutils import (
@@ -88,6 +89,8 @@ class DirectBranchCommit:
         self.db_branch = db_branch
 
         self.last_scanned_id = self.db_branch.last_scanned_id
+        if self.last_scanned_id is not None:
+            self.last_scanned_id = six.ensure_binary(self.last_scanned_id)
 
         if committer is None:
             committer = db_branch.owner
@@ -126,7 +129,7 @@ class DirectBranchCommit:
         if (self.db_branch.last_mirrored_id is None
             and revision_id == NULL_REVISION):
             return True
-        return revision_id == self.db_branch.last_mirrored_id
+        return six.ensure_text(revision_id) == self.db_branch.last_mirrored_id
 
     def _getDir(self, path):
         """Get trans_id for directory "path."  Create if necessary."""
@@ -235,7 +238,8 @@ class DirectBranchCommit:
                     self.bzrbranch, commit_message, self.merge_parents,
                     committer=committer_id)
             IMasterObject(self.db_branch).branchChanged(
-                get_stacked_on_url(self.bzrbranch), new_rev_id,
+                get_stacked_on_url(self.bzrbranch),
+                None if new_rev_id is None else six.ensure_text(new_rev_id),
                 self.db_branch.control_format, self.db_branch.branch_format,
                 self.db_branch.repository_format)
 
diff --git a/lib/lp/code/model/revision.py b/lib/lp/code/model/revision.py
index f6f29fa..5740579 100644
--- a/lib/lp/code/model/revision.py
+++ b/lib/lp/code/model/revision.py
@@ -355,7 +355,7 @@ class RevisionSet:
         # Collect all data for making Revision objects.
         data = []
         for bzr_revision, author_name in zip(revisions, author_names):
-            revision_id = bzr_revision.revision_id
+            revision_id = six.ensure_text(bzr_revision.revision_id)
             revision_date = self._timestampToDatetime(bzr_revision.timestamp)
             revision_author = revision_authors[author_name]
 
@@ -376,7 +376,7 @@ class RevisionSet:
         parent_data = []
         property_data = []
         for bzr_revision in revisions:
-            db_id = revision_db_id[bzr_revision.revision_id]
+            db_id = revision_db_id[six.ensure_text(bzr_revision.revision_id)]
             # Property data: revision DB id, name, value.
             for name, value in six.iteritems(bzr_revision.properties):
                 # pristine-tar properties can be huge, and storing them
@@ -384,7 +384,9 @@ class RevisionSet:
                 if name.startswith('deb-pristine-delta'):
                     continue
                 property_data.append((db_id, name, value))
-            parent_ids = bzr_revision.parent_ids
+            parent_ids = [
+                six.ensure_text(parent_id)
+                for parent_id in bzr_revision.parent_ids]
             # Parent data: revision DB id, sequence, revision_id
             seen_parents = set()
             for sequence, parent_id in enumerate(parent_ids):
diff --git a/lib/lp/code/model/tests/test_branch.py b/lib/lp/code/model/tests/test_branch.py
index a9ddb99..1649dc9 100644
--- a/lib/lp/code/model/tests/test_branch.py
+++ b/lib/lp/code/model/tests/test_branch.py
@@ -18,6 +18,7 @@ from breezy.bzr.bzrdir import BzrDir
 from breezy.revision import NULL_REVISION
 from breezy.url_policy_open import BadUrl
 from pytz import UTC
+import six
 from sqlobject import SQLObjectNotFound
 from storm.exceptions import LostObjectError
 from storm.locals import Store
@@ -2260,7 +2261,8 @@ class TestRevisionHistory(TestCaseWithFactory):
         # breezy.revision.NULL_REVISION.
         branch = self.factory.makeBranch()
         branch.updateScannedDetails(None, 0)
-        self.assertEqual(NULL_REVISION, branch.last_scanned_id)
+        self.assertEqual(
+            six.ensure_text(NULL_REVISION), branch.last_scanned_id)
         self.assertIs(None, branch.getTipRevision())
 
     def test_tip_revision_is_updated(self):
@@ -2388,7 +2390,7 @@ class TestPendingWritesAndUpdates(TestCaseWithFactory):
         # are pending writes but no pending updates.
         self.useBzrBranches(direct_database=True)
         branch, bzr_tree = self.create_branch_and_tree()
-        rev_id = self.factory.getUniqueString(b'rev-id')
+        rev_id = self.factory.getUniqueBytes('rev-id')
         bzr_tree.commit('Commit', committer='me@xxxxxxxxxxx', rev_id=rev_id)
         removeSecurityProxy(branch).branchChanged('', rev_id, None, None, None)
         transaction.commit()
@@ -2406,7 +2408,7 @@ class TestPendingWritesAndUpdates(TestCaseWithFactory):
         # are pending writes and pending updates.
         self.useBzrBranches(direct_database=True)
         branch, bzr_tree = self.create_branch_and_tree()
-        rev_id = self.factory.getUniqueString(b'rev-id')
+        rev_id = self.factory.getUniqueBytes('rev-id')
         bzr_tree.commit('Commit', committer='me@xxxxxxxxxxx', rev_id=rev_id)
         removeSecurityProxy(branch).branchChanged('', rev_id, None, None, None)
         transaction.commit()
@@ -2444,7 +2446,7 @@ class TestPendingWritesAndUpdates(TestCaseWithFactory):
         # pending writes and pending updates.
         branch = self.factory.makeAnyBranch(branch_type=BranchType.MIRRORED)
         branch.startMirroring()
-        rev_id = self.factory.getUniqueString('rev-id')
+        rev_id = self.factory.getUniqueBytes('rev-id')
         removeSecurityProxy(branch).branchChanged(
             '', rev_id, None, None, None)
         self.assertTrue(branch.pending_writes)
@@ -2455,13 +2457,13 @@ class TestPendingWritesAndUpdates(TestCaseWithFactory):
         # writes or pending updates.
         branch = self.factory.makeAnyBranch(branch_type=BranchType.MIRRORED)
         branch.startMirroring()
-        rev_id = self.factory.getUniqueString('rev-id')
+        rev_id = self.factory.getUniqueBytes('rev-id')
         removeSecurityProxy(branch).branchChanged(
             '', rev_id, None, None, None)
         # Cheat! The actual API for marking a branch as scanned is to run
         # the BranchScanJob. That requires a revision in the database
         # though.
-        removeSecurityProxy(branch).last_scanned_id = rev_id
+        removeSecurityProxy(branch).last_scanned_id = six.ensure_text(rev_id)
         [job] = getUtility(IBranchScanJobSource).iterReady()
         removeSecurityProxy(job).job._status = JobStatus.COMPLETED
         self.assertFalse(branch.pending_writes)
@@ -2480,7 +2482,7 @@ class TestPendingWritesAndUpdates(TestCaseWithFactory):
         # pending writes and pending updates.
         branch = self.factory.makeAnyBranch(branch_type=BranchType.MIRRORED)
         branch.startMirroring()
-        rev_id = self.factory.getUniqueString('rev-id')
+        rev_id = self.factory.getUniqueBytes('rev-id')
         removeSecurityProxy(branch).branchChanged(
             '', rev_id, None, None, None)
         # Cheat! We can only tell if mirroring has started if the last
diff --git a/lib/lp/code/model/tests/test_branchjob.py b/lib/lp/code/model/tests/test_branchjob.py
index 2abb78a..b4ef73d 100644
--- a/lib/lp/code/model/tests/test_branchjob.py
+++ b/lib/lp/code/model/tests/test_branchjob.py
@@ -443,11 +443,11 @@ class TestRevisionsAddedJob(TestCaseWithFactory):
         """
         for bzr_revision in bzr_branch.repository.get_revisions(revision_ids):
             existing = branch.getBranchRevision(
-                revision_id=bzr_revision.revision_id)
+                revision_id=six.ensure_text(bzr_revision.revision_id))
             if existing is None:
                 RevisionSet().newFromBazaarRevisions([bzr_revision])
             revision = RevisionSet().getByRevisionId(
-                bzr_revision.revision_id)
+                six.ensure_text(bzr_revision.revision_id))
             try:
                 revno = bzr_branch.revision_id_to_revno(revision.revision_id)
             except bzr_errors.NoSuchRevision:
@@ -983,7 +983,7 @@ class TestRosettaUploadJob(TestCaseWithFactory):
         # XXX: AaronBentley 2010-08-06 bug=614404: a bzr username is
         # required to generate the revision-id.
         with override_environ(BRZ_EMAIL='me@xxxxxxxxxxx'):
-            revision_id = self.tree.commit(commit_message)
+            revision_id = six.ensure_text(self.tree.commit(commit_message))
         self.branch.last_scanned_id = revision_id
         self.branch.last_mirrored_id = revision_id
         return revision_id
@@ -1218,7 +1218,8 @@ class TestRosettaUploadJob(TestCaseWithFactory):
         # iterReady does not return jobs for branches where last_scanned_id
         # and last_mirror_id are different.
         self._makeBranchWithTreeAndFiles([])
-        self.branch.last_scanned_id = NULL_REVISION  # Was not scanned yet.
+        # Was not scanned yet.
+        self.branch.last_scanned_id = six.ensure_text(NULL_REVISION)
         self._makeProductSeries(
             TranslationsBranchImportMode.IMPORT_TEMPLATES)
         # Put the job in ready state.
diff --git a/lib/lp/code/model/tests/test_branchmergeproposaljobs.py b/lib/lp/code/model/tests/test_branchmergeproposaljobs.py
index 7236b18..77a6ead 100644
--- a/lib/lp/code/model/tests/test_branchmergeproposaljobs.py
+++ b/lib/lp/code/model/tests/test_branchmergeproposaljobs.py
@@ -429,7 +429,8 @@ def make_runnable_incremental_diff_job(test_case):
     parent_id = repository.get_revision(source_rev_id).parent_ids[0]
     test_case.factory.makeRevision(rev_id=source_rev_id)
     test_case.factory.makeRevision(rev_id=parent_id)
-    return GenerateIncrementalDiffJob.create(bmp, parent_id, source_rev_id)
+    return GenerateIncrementalDiffJob.create(
+        bmp, six.ensure_text(parent_id), six.ensure_text(source_rev_id))
 
 
 class TestGenerateIncrementalDiffJob(DiffTestCase):
diff --git a/lib/lp/code/model/tests/test_diff.py b/lib/lp/code/model/tests/test_diff.py
index f51497e..0c49746 100644
--- a/lib/lp/code/model/tests/test_diff.py
+++ b/lib/lp/code/model/tests/test_diff.py
@@ -18,6 +18,7 @@ from breezy.patches import (
     parse_patches,
     RemoveLine,
     )
+import six
 from six.moves import reload_module
 from testtools.matchers import (
     Equals,
@@ -547,8 +548,10 @@ class TestPreviewDiff(DiffTestCase):
         # Correctly generates a PreviewDiff from a BranchMergeProposal.
         bmp, source_rev_id, target_rev_id = self.createExampleBzrMerge()
         preview = PreviewDiff.fromBranchMergeProposal(bmp)
-        self.assertEqual(source_rev_id, preview.source_revision_id)
-        self.assertEqual(target_rev_id, preview.target_revision_id)
+        self.assertEqual(
+            six.ensure_text(source_rev_id), preview.source_revision_id)
+        self.assertEqual(
+            six.ensure_text(target_rev_id), preview.target_revision_id)
         transaction.commit()
         self.checkExampleBzrMerge(preview.text)
         self.assertEqual({'foo': (5, 0)}, preview.diffstat)
diff --git a/lib/lp/code/tests/test_directbranchcommit.py b/lib/lp/code/tests/test_directbranchcommit.py
index 9e1ff0b..9930b67 100644
--- a/lib/lp/code/tests/test_directbranchcommit.py
+++ b/lib/lp/code/tests/test_directbranchcommit.py
@@ -7,6 +7,7 @@ from __future__ import absolute_import, print_function, unicode_literals
 
 __metaclass__ = type
 
+import six
 from testtools.testcase import ExpectedException
 from zope.security.proxy import removeSecurityProxy
 
@@ -217,7 +218,8 @@ class TestDirectBranchCommit(DirectBranchCommitTestCase, TestCaseWithFactory):
         # the branch.
         self.committer.writeFile('hi.c', b'main(){puts("hi world");}')
         revid = self.committer.commit('')
-        self.assertEqual(revid, self.db_branch.last_mirrored_id)
+        self.assertEqual(
+            six.ensure_text(revid), self.db_branch.last_mirrored_id)
 
     def test_commit_uses_getBzrCommitterID(self):
         # commit() passes self.getBzrCommitterID() to bzr as the
diff --git a/lib/lp/codehosting/scanner/bzrsync.py b/lib/lp/codehosting/scanner/bzrsync.py
index f452b2c..fc26a28 100755
--- a/lib/lp/codehosting/scanner/bzrsync.py
+++ b/lib/lp/codehosting/scanner/bzrsync.py
@@ -94,7 +94,9 @@ class BzrSync:
         # Get the history and ancestry from the branch first, to fail early
         # if something is wrong with the branch.
         self.logger.info("Retrieving history from breezy.")
-        bzr_history = branch_revision_history(bzr_branch)
+        bzr_history = [
+            six.ensure_text(revid)
+            for revid in branch_revision_history(bzr_branch)]
         # The BranchRevision, Revision and RevisionParent tables are only
         # written to by the branch-scanner, so they are not subject to
         # write-lock contention. Update them all in a single transaction to
@@ -152,7 +154,9 @@ class BzrSync:
                 BranchRevision.branch_id == self.db_branch.id,
                 Revision.id == BranchRevision.revision_id)
         parent_map = dict(
-            (r.revision_id, r.parent_ids) for r in revisions)
+            (six.ensure_binary(r.revision_id),
+             [six.ensure_binary(revid) for revid in r.parent_ids])
+            for r in revisions)
         parents_provider = DictParentsProvider(parent_map)
 
         class PPSource:
@@ -171,10 +175,14 @@ class BzrSync:
             added_ancestry = get_ancestry(bzr_branch.repository, bzr_last)
             removed_ancestry = set()
         else:
+            db_last = six.ensure_binary(db_last)
             graph = self._getRevisionGraph(bzr_branch, db_last)
             added_ancestry, removed_ancestry = (
                 graph.find_difference(bzr_last, db_last))
             added_ancestry.discard(NULL_REVISION)
+        added_ancestry = {six.ensure_text(revid) for revid in added_ancestry}
+        removed_ancestry = {
+            six.ensure_text(revid) for revid in removed_ancestry}
         return added_ancestry, removed_ancestry
 
     def getHistoryDelta(self, bzr_history, db_history):
@@ -245,7 +253,8 @@ class BzrSync:
         :param revisions: the set of Breezy revision IDs to return breezy
             Revision objects for.
         """
-        revisions = bzr_branch.repository.get_parent_map(revisions)
+        revisions = bzr_branch.repository.get_parent_map(
+            [six.ensure_binary(revid) for revid in revisions])
         return bzr_branch.repository.get_revisions(revisions.keys())
 
     def syncRevisions(self, bzr_branch, bzr_revisions, revids_to_insert):
@@ -260,7 +269,8 @@ class BzrSync:
         self.revision_set.newFromBazaarRevisions(bzr_revisions)
         mainline_revisions = []
         for bzr_revision in bzr_revisions:
-            if revids_to_insert[bzr_revision.revision_id] is None:
+            revision_id = six.ensure_text(bzr_revision.revision_id)
+            if revids_to_insert[revision_id] is None:
                 continue
             mainline_revisions.append(bzr_revision)
         notify(events.NewMainlineRevisions(
diff --git a/lib/lp/codehosting/scanner/email.py b/lib/lp/codehosting/scanner/email.py
index 96cc3d8..914b380 100644
--- a/lib/lp/codehosting/scanner/email.py
+++ b/lib/lp/codehosting/scanner/email.py
@@ -11,6 +11,7 @@ __all__ = [
     'queue_tip_changed_email_jobs',
     ]
 
+import six
 from zope.component import getUtility
 
 from lp.code.enums import BranchSubscriptionNotificationLevel
@@ -75,6 +76,6 @@ def queue_tip_changed_email_jobs(tip_changed):
     else:
         job = getUtility(IRevisionsAddedJobSource).create(
             tip_changed.db_branch, tip_changed.db_branch.last_scanned_id,
-            tip_changed.bzr_branch.last_revision(),
+            six.ensure_text(tip_changed.bzr_branch.last_revision()),
             config.canonical.noreply_from_address)
     job.celeryRunOnCommit()
diff --git a/lib/lp/codehosting/scanner/events.py b/lib/lp/codehosting/scanner/events.py
index 02fb140..c5754c9 100644
--- a/lib/lp/codehosting/scanner/events.py
+++ b/lib/lp/codehosting/scanner/events.py
@@ -12,6 +12,7 @@ __all__ = [
     'TipChanged',
     ]
 
+import six
 from zope.component.interfaces import (
     IObjectEvent,
     ObjectEvent,
@@ -48,10 +49,7 @@ class NewMainlineRevisions(ScannerEvent):
 
         :param db_branch: The database branch.
         :param bzr_branch: The Bazaar branch.
-        :param db_revision: An `IRevision` for the new revision.
-        :param bzr_revision: The new Bazaar revision.
-        :param revno: The revision number of the new revision, None if not
-            mainline.
+        :param bzr_revisions: The new Bazaar revisions.
         """
         ScannerEvent.__init__(self, db_branch, bzr_branch)
         self.bzr_revisions = bzr_revisions
@@ -83,7 +81,7 @@ class TipChanged(ScannerEvent):
     @property
     def new_tip_revision_id(self):
         """The new tip revision id from this scan."""
-        return self.bzr_branch.last_revision()
+        return six.ensure_text(self.bzr_branch.last_revision())
 
     @staticmethod
     def composeWebhookPayload(branch, old_revid, new_revid):
diff --git a/lib/lp/codehosting/scanner/mergedetection.py b/lib/lp/codehosting/scanner/mergedetection.py
index 26a23c7..4811b40 100644
--- a/lib/lp/codehosting/scanner/mergedetection.py
+++ b/lib/lp/codehosting/scanner/mergedetection.py
@@ -12,6 +12,7 @@ __all__ = [
     ]
 
 from breezy.revision import NULL_REVISION
+import six
 from zope.component import getUtility
 
 from lp.code.adapters.branch import BranchMergeProposalNoPreviewDiffDelta
@@ -168,7 +169,8 @@ def auto_merge_proposals(scan_completed):
     for proposal in db_branch.landing_candidates:
         tip_rev_id = proposal.source_branch.last_scanned_id
         if tip_rev_id in new_ancestry:
-            merged_revno = find_merged_revno(merge_sorted, tip_rev_id)
+            merged_revno = find_merged_revno(
+                merge_sorted, six.ensure_binary(tip_rev_id))
             # Remember so we can find the merged revision number.
             merge_detected(
                 logger, proposal.source_branch, db_branch, proposal,
diff --git a/lib/lp/codehosting/scanner/tests/test_bzrsync.py b/lib/lp/codehosting/scanner/tests/test_bzrsync.py
index 3c49360..70a4104 100644
--- a/lib/lp/codehosting/scanner/tests/test_bzrsync.py
+++ b/lib/lp/codehosting/scanner/tests/test_bzrsync.py
@@ -22,6 +22,7 @@ from fixtures import (
     TempDir,
     )
 import pytz
+import six
 from storm.locals import Store
 from testtools.matchers import (
     Equals,
@@ -377,12 +378,13 @@ class TestBzrSync(BzrSyncTestCase):
     def test_sync_updates_branch(self):
         # test that the last scanned revision ID is recorded
         self.syncAndCount()
-        self.assertEqual(NULL_REVISION, self.db_branch.last_scanned_id)
+        self.assertEqual(
+            six.ensure_text(NULL_REVISION), self.db_branch.last_scanned_id)
         last_modified = self.db_branch.date_last_modified
         last_scanned = self.db_branch.last_scanned
         self.commitRevision()
         self.syncAndCount(new_revisions=1, new_numbers=1, new_authors=1)
-        self.assertEqual(self.bzr_branch.last_revision(),
+        self.assertEqual(six.ensure_text(self.bzr_branch.last_revision()),
                          self.db_branch.last_scanned_id)
         self.assertTrue(self.db_branch.last_scanned > last_scanned,
                         "last_scanned was not updated")
@@ -413,7 +415,7 @@ class TestBzrSync(BzrSyncTestCase):
             tip.
         """
         (db_branch, bzr_tree), ignored = self.makeBranchWithMerge(
-            'base', 'trunk', 'branch', 'merge')
+            b'base', b'trunk', b'branch', b'merge')
         bzr_branch = bzr_tree.branch
         self.factory.makeBranchRevision(db_branch, 'base', 0)
         self.factory.makeBranchRevision(
@@ -439,12 +441,12 @@ class TestBzrSync(BzrSyncTestCase):
                 delta_branch = bzr_branch
             return sync.getAncestryDelta(delta_branch)
 
-        added_ancestry, removed_ancestry = get_delta('merge', None)
+        added_ancestry, removed_ancestry = get_delta(b'merge', None)
         # All revisions are new for an unscanned branch
         self.assertEqual(
             set(['base', 'trunk', 'branch', 'merge']), added_ancestry)
         self.assertEqual(set(), removed_ancestry)
-        added_ancestry, removed_ancestry = get_delta('merge', 'base')
+        added_ancestry, removed_ancestry = get_delta(b'merge', 'base')
         self.assertEqual(
             set(['trunk', 'branch', 'merge']), added_ancestry)
         self.assertEqual(set(), removed_ancestry)
@@ -453,12 +455,12 @@ class TestBzrSync(BzrSyncTestCase):
             set(), added_ancestry)
         self.assertEqual(
             set(['base', 'trunk', 'branch', 'merge']), removed_ancestry)
-        added_ancestry, removed_ancestry = get_delta('base', 'merge')
+        added_ancestry, removed_ancestry = get_delta(b'base', 'merge')
         self.assertEqual(
             set(), added_ancestry)
         self.assertEqual(
             set(['trunk', 'branch', 'merge']), removed_ancestry)
-        added_ancestry, removed_ancestry = get_delta('trunk', 'branch')
+        added_ancestry, removed_ancestry = get_delta(b'trunk', 'branch')
         self.assertEqual(set(['trunk']), added_ancestry)
         self.assertEqual(set(['branch']), removed_ancestry)
 
@@ -480,7 +482,9 @@ class TestBzrSync(BzrSyncTestCase):
         # yield each revision along with a sequence number, starting at 1.
         self.commitRevision(rev_id=b'rev-1')
         bzrsync = self.makeBzrSync(self.db_branch)
-        bzr_history = branch_revision_history(self.bzr_branch)
+        bzr_history = [
+            six.ensure_text(revid)
+            for revid in branch_revision_history(self.bzr_branch)]
         added_ancestry = bzrsync.getAncestryDelta(self.bzr_branch)[0]
         result = bzrsync.revisionsToInsert(
             bzr_history, self.bzr_branch.revno(), added_ancestry)
@@ -490,9 +494,11 @@ class TestBzrSync(BzrSyncTestCase):
         # Confirm that these revisions are generated by getRevisions with None
         # as the sequence 'number'.
         (db_branch, bzr_tree), ignored = self.makeBranchWithMerge(
-            'base', 'trunk', 'branch', 'merge')
+            b'base', b'trunk', b'branch', b'merge')
         bzrsync = self.makeBzrSync(db_branch)
-        bzr_history = branch_revision_history(bzr_tree.branch)
+        bzr_history = [
+            six.ensure_text(revid)
+            for revid in branch_revision_history(bzr_tree.branch)]
         added_ancestry = bzrsync.getAncestryDelta(bzr_tree.branch)[0]
         expected = {'base': 1, 'trunk': 2, 'merge': 3, 'branch': None}
         self.assertEqual(
@@ -503,7 +509,7 @@ class TestBzrSync(BzrSyncTestCase):
         # Confirm that when we syncHistory, all of the revisions are included
         # correctly in the BranchRevision table.
         (db_branch, branch_tree), ignored = self.makeBranchWithMerge(
-            'r1', 'r2', 'r1.1.1', 'r3')
+            b'r1', b'r2', b'r1.1.1', b'r3')
         self.makeBzrSync(db_branch).syncBranchAndClose()
         expected = set(
             [(1, 'r1'), (2, 'r2'), (3, 'r3'), (None, 'r1.1.1')])
@@ -515,7 +521,7 @@ class TestBzrSync(BzrSyncTestCase):
         # mainline when synced.
 
         (db_trunk, trunk_tree), (db_branch, branch_tree) = (
-            self.makeBranchWithMerge('base', 'trunk', 'branch', 'merge'))
+            self.makeBranchWithMerge(b'base', b'trunk', b'branch', b'merge'))
 
         self.syncBazaarBranchToDatabase(trunk_tree.branch, db_branch)
         self.assertInMainline('trunk', db_branch)
@@ -528,7 +534,7 @@ class TestBzrSync(BzrSyncTestCase):
         # When replacing a branch by one of the branches it merged, the
         # database must be updated appropriately.
         (db_trunk, trunk_tree), (db_branch, branch_tree) = (
-            self.makeBranchWithMerge('base', 'trunk', 'branch', 'merge'))
+            self.makeBranchWithMerge(b'base', b'trunk', b'branch', b'merge'))
         # First, sync with the merging branch.
         self.syncBazaarBranchToDatabase(trunk_tree.branch, db_trunk)
         # Then sync with the merged branch.
@@ -572,15 +578,15 @@ class TestPlanDatabaseChanges(BzrSyncTestCase):
         # If a BranchRevision is being added, and it's already in the DB, but
         # not found through the graph operations, we should schedule it for
         # deletion anyway.
-        rev1_id = self.bzr_tree.commit(
-            'initial commit', committer='me@xxxxxxxxxxx')
+        rev1_id = six.ensure_text(self.bzr_tree.commit(
+            'initial commit', committer='me@xxxxxxxxxxx'))
         merge_tree = self.bzr_tree.controldir.sprout(
             'merge').open_workingtree()
-        merge_id = merge_tree.commit(
-            'mergeable commit', committer='me@xxxxxxxxxxx')
+        merge_id = six.ensure_text(merge_tree.commit(
+            'mergeable commit', committer='me@xxxxxxxxxxx'))
         self.bzr_tree.merge_from_branch(merge_tree.branch)
-        rev2_id = self.bzr_tree.commit(
-            'merge', committer='me@xxxxxxxxxxx')
+        rev2_id = six.ensure_text(self.bzr_tree.commit(
+            'merge', committer='me@xxxxxxxxxxx'))
         self.useContext(read_locked(self.bzr_tree))
         syncer = BzrSync(self.db_branch)
         syncer.syncBranchAndClose(self.bzr_tree.branch)
@@ -662,8 +668,8 @@ class TestBzrTranslationsUploadJob(BzrSyncTestCase):
             TranslationsBranchImportMode.IMPORT_TEMPLATES)
         revision_id = self.commitRevision()
         self.makeBzrSync(self.db_branch).syncBranchAndClose()
-        self.db_branch.last_mirrored_id = revision_id
-        self.db_branch.last_scanned_id = revision_id
+        self.db_branch.last_mirrored_id = six.ensure_text(revision_id)
+        self.db_branch.last_scanned_id = six.ensure_text(revision_id)
         ready_jobs = list(getUtility(IRosettaUploadJobSource).iterReady())
         self.assertEqual(1, len(ready_jobs))
         job = ready_jobs[0]
@@ -706,7 +712,7 @@ class TestGenerateIncrementalDiffJob(BzrSyncTestCase):
         parent_id = commit_file(self.db_branch, 'foo', 'bar')
         self.factory.makeBranchRevision(self.db_branch, parent_id,
                 revision_date=self.factory.getUniqueDate())
-        self.db_branch.last_scanned_id = parent_id
+        self.db_branch.last_scanned_id = six.ensure_text(parent_id)
         # Make sure that the merge proposal is created in the past.
         date_created = (
             datetime.datetime.now(pytz.UTC) - datetime.timedelta(days=7))
@@ -719,8 +725,8 @@ class TestGenerateIncrementalDiffJob(BzrSyncTestCase):
         switch_dbuser("branchscanner")
         self.makeBzrSync(self.db_branch).syncBranchAndClose()
         (job,) = self.getPending()
-        self.assertEqual(revision_id, job.new_revision_id)
-        self.assertEqual(parent_id, job.old_revision_id)
+        self.assertEqual(six.ensure_text(revision_id), job.new_revision_id)
+        self.assertEqual(six.ensure_text(parent_id), job.old_revision_id)
 
 
 class TestSetRecipeStale(BzrSyncTestCase):
@@ -791,7 +797,7 @@ class TestTriggerWebhooks(BzrSyncTestCase):
             hook = self.factory.makeWebhook(
                 target=self.db_branch, event_types=["bzr:push:0.1"])
         self.commitRevision()
-        new_revid = self.bzr_branch.last_revision()
+        new_revid = six.ensure_text(self.bzr_branch.last_revision())
         self.makeBzrSync(self.db_branch).syncBranchAndClose()
         delivery = hook.deliveries.one()
         payload_matcher = MatchesDict({
@@ -825,7 +831,7 @@ class TestRevisionProperty(BzrSyncTestCase):
         self.commitRevision(rev_id=b'rev1', revprops=properties)
         self.makeBzrSync(self.db_branch).syncBranchAndClose()
         # Check that properties were saved to the revision.
-        bzr_revision = self.bzr_branch.repository.get_revision('rev1')
+        bzr_revision = self.bzr_branch.repository.get_revision(b'rev1')
         self.assertEqual(properties, bzr_revision.properties)
         # Check that properties are stored in the database.
         db_revision = getUtility(IRevisionSet).getByRevisionId('rev1')
diff --git a/lib/lp/codehosting/scanner/tests/test_mergedetection.py b/lib/lp/codehosting/scanner/tests/test_mergedetection.py
index 3868865..2268ca8 100644
--- a/lib/lp/codehosting/scanner/tests/test_mergedetection.py
+++ b/lib/lp/codehosting/scanner/tests/test_mergedetection.py
@@ -61,7 +61,7 @@ class TestAutoMergeDetectionForMergeProposals(BzrSyncTestCase):
         # Create two branches where the trunk has the branch as a merge.  Also
         # create a merge proposal from the branch to the trunk.
         (db_trunk, trunk_tree), (db_branch, branch_tree) = (
-            self.makeBranchWithMerge('base', 'trunk', 'branch', 'merge'))
+            self.makeBranchWithMerge(b'base', b'trunk', b'branch', b'merge'))
         trunk_id = db_trunk.id
         branch_id = db_branch.id
         self.createProposal(db_branch, db_trunk)
@@ -246,7 +246,7 @@ class TestMergeDetection(TestCaseWithFactory):
         # of the branch is the NULL_REVISION no merge event is emitted for
         # that branch.
         source = self.factory.makeProductBranch(product=self.product)
-        source.last_scanned_id = NULL_REVISION
+        source.last_scanned_id = six.ensure_text(NULL_REVISION)
         self.autoMergeBranches(self.db_branch, ['revid'])
         self.assertEqual([], self.merges)
 
diff --git a/lib/lp/codehosting/tests/test_upgrade.py b/lib/lp/codehosting/tests/test_upgrade.py
index ae8ef77..e27d1cb 100644
--- a/lib/lp/codehosting/tests/test_upgrade.py
+++ b/lib/lp/codehosting/tests/test_upgrade.py
@@ -155,7 +155,7 @@ class TestUpgrader(TestCaseWithFactory):
         with read_locked(upgrader.bzr_branch):
             upgrader.start_upgrade()
             upgraded = upgrader.add_upgraded_branch().open_branch()
-        self.assertEqual('prepare-commit', upgraded.last_revision())
+        self.assertEqual(b'prepare-commit', upgraded.last_revision())
 
     def test_create_upgraded_repository_preserves_dead_heads(self):
         """Fetch-based upgrade preserves heads in the repository."""
@@ -165,7 +165,7 @@ class TestUpgrader(TestCaseWithFactory):
             upgrader.create_upgraded_repository()
         upgraded = upgrader.get_bzrdir().open_repository()
         self.assertEqual(
-            'foo', upgraded.get_revision('prepare-commit').message)
+            'foo', upgraded.get_revision(b'prepare-commit').message)
 
     def test_create_upgraded_repository_uses_target_subdir(self):
         """The repository is created in the right place."""
@@ -177,11 +177,11 @@ class TestUpgrader(TestCaseWithFactory):
     def test_add_upgraded_branch_preserves_tags(self):
         """Fetch-based upgrade preserves heads in the repository."""
         upgrader = self.prepare('pack-0.92-subtree')
-        upgrader.bzr_branch.tags.set_tag('steve', 'rev-id')
+        upgrader.bzr_branch.tags.set_tag('steve', b'rev-id')
         with read_locked(upgrader.bzr_branch):
             upgrader.start_upgrade()
             upgraded = upgrader.add_upgraded_branch().open_branch()
-        self.assertEqual('rev-id', upgraded.tags.lookup_tag('steve'))
+        self.assertEqual(b'rev-id', upgraded.tags.lookup_tag('steve'))
 
     def test_has_tree_references(self):
         """Detects whether repo contains actual tree references."""
@@ -239,7 +239,7 @@ class TestUpgrader(TestCaseWithFactory):
         upgraded = upgrader.get_bzrdir().open_repository()
         self.assertIs(RepositoryFormat2a, upgraded._format.__class__)
         self.assertEqual(
-            'foo', upgraded.get_revision('prepare-commit').message)
+            'foo', upgraded.get_revision(b'prepare-commit').message)
 
     def test_finish_upgrade_fetches(self):
         """finish_upgrade fetches new changes into the branch."""
@@ -273,7 +273,7 @@ class TestUpgrader(TestCaseWithFactory):
         self.assertIs(RepositoryFormat2a,
             upgraded.repository._format.__class__)
         self.assertEqual(
-            'foo', upgraded.repository.get_revision('prepare-commit').message)
+            'foo', upgraded.repository.get_revision(b'prepare-commit').message)
 
     def test_invalid_stacking(self):
         """Upgrade tolerates branches stacked on different-format branches."""
diff --git a/lib/lp/codehosting/vfs/tests/test_branchfs.py b/lib/lp/codehosting/vfs/tests/test_branchfs.py
index 63fa3ef..dabd4ec 100644
--- a/lib/lp/codehosting/vfs/tests/test_branchfs.py
+++ b/lib/lp/codehosting/vfs/tests/test_branchfs.py
@@ -942,7 +942,7 @@ class TestBranchChangedNotification(TestCaseWithTransport):
         branch.unlock()
         self.assertEqual(1, len(self._branch_changed_log))
         self.assertEqual(
-            revid,
+            six.ensure_text(revid),
             self._branch_changed_log[0]['last_revision'])
 
     def assertStackedOnIsRewritten(self, input, output):
diff --git a/lib/lp/testing/__init__.py b/lib/lp/testing/__init__.py
index 6e16010..a44121d 100644
--- a/lib/lp/testing/__init__.py
+++ b/lib/lp/testing/__init__.py
@@ -915,7 +915,8 @@ class TestCaseWithFactory(TestCase):
         if parent:
             bzr_branch.pull(parent)
             naked_branch = removeSecurityProxy(db_branch)
-            naked_branch.last_scanned_id = bzr_branch.last_revision()
+            naked_branch.last_scanned_id = six.ensure_text(
+                bzr_branch.last_revision())
         return bzr_branch
 
     def useTempBzrHome(self):
diff --git a/lib/lp/testing/factory.py b/lib/lp/testing/factory.py
index c833ddf..a176d48 100644
--- a/lib/lp/testing/factory.py
+++ b/lib/lp/testing/factory.py
@@ -493,16 +493,11 @@ class ObjectFactory(
         string = "%s-%s" % (prefix, self.getUniqueInteger())
         return string
 
-    if sys.version_info[0] >= 3:
-        def getUniqueBytes(self, prefix=None):
-            return six.ensure_binary(self.getUniqueString(prefix=prefix))
+    def getUniqueBytes(self, prefix=None):
+        return six.ensure_binary(self.getUniqueString(prefix=prefix))
 
-        getUniqueUnicode = getUniqueString
-    else:
-        getUniqueBytes = getUniqueString
-
-        def getUniqueUnicode(self, prefix=None):
-            return six.ensure_text(self.getUniqueString(prefix=prefix))
+    def getUniqueUnicode(self, prefix=None):
+        return six.ensure_text(self.getUniqueString(prefix=prefix))
 
     def getUniqueURL(self, scheme=None, host=None):
         """Return a URL unique to this run of the test case."""
@@ -1685,7 +1680,9 @@ class BareLaunchpadObjectFactory(ObjectFactory):
         if parent_ids is None:
             parent_ids = []
         if rev_id is None:
-            rev_id = self.getUniqueString('revision-id')
+            rev_id = self.getUniqueUnicode('revision-id')
+        else:
+            rev_id = six.ensure_text(rev_id)
         if log_body is None:
             log_body = self.getUniqueString('log-body')
         return getUtility(IRevisionSet).new(
diff --git a/lib/lp/translations/scripts/tests/test_translations_to_branch.py b/lib/lp/translations/scripts/tests/test_translations_to_branch.py
index 2b5877e..b0f9b45 100644
--- a/lib/lp/translations/scripts/tests/test_translations_to_branch.py
+++ b/lib/lp/translations/scripts/tests/test_translations_to_branch.py
@@ -160,14 +160,16 @@ class TestExportTranslationsToBranch(TestCaseWithFactory):
         self.becomeDbUser('translationstobranch')
         self.assertFalse(db_branch.pending_writes)
         self.assertNotEqual(
-            db_branch.last_mirrored_id, tree.branch.last_revision())
+            db_branch.last_mirrored_id,
+            six.ensure_text(tree.branch.last_revision()))
         # The export code works on a Branch from the slave store.  It
         # shouldn't stop the scan request.
         slave_series = ISlaveStore(productseries).get(
             ProductSeries, productseries.id)
         exporter._exportToBranch(slave_series)
         self.assertEqual(
-            db_branch.last_mirrored_id, tree.branch.last_revision())
+            db_branch.last_mirrored_id,
+            six.ensure_text(tree.branch.last_revision()))
         self.assertTrue(db_branch.pending_writes)
         matches = MatchesRegex(
             "(.|\n)*WARNING Skipped .* due to stale DB info, and scheduled a "
diff --git a/lib/lp/translations/tests/test_rosetta_branches_script.py b/lib/lp/translations/tests/test_rosetta_branches_script.py
index 1d140cd..947d231 100644
--- a/lib/lp/translations/tests/test_rosetta_branches_script.py
+++ b/lib/lp/translations/tests/test_rosetta_branches_script.py
@@ -10,6 +10,7 @@ provisions to handle Bazaar branches.
 __metaclass__ = type
 
 from breezy.revision import NULL_REVISION
+import six
 import transaction
 from zope.component import getUtility
 
@@ -49,8 +50,8 @@ class TestRosettaBranchesScript(TestCaseWithFactory):
         # required to generate the revision-id.
         with override_environ(BRZ_EMAIL='me@xxxxxxxxxxx'):
             revision_id = tree.commit("first commit")
-        branch.last_scanned_id = revision_id
-        branch.last_mirrored_id = revision_id
+        branch.last_scanned_id = six.ensure_text(revision_id)
+        branch.last_mirrored_id = six.ensure_text(revision_id)
         series = self.factory.makeProductSeries()
         series.branch = branch
         series.translations_autoimport_mode = (