Merge branch 'main' into avm99963-monorail

Merged commit 34d8229ae2b51fb1a15bd208e6fe6185c94f6266

GitOrigin-RevId: 7ee0917f93a577e475f8e09526dd144d245593f4
diff --git a/api/test/converters_test.py b/api/test/converters_test.py
index e193423..ee060bd 100644
--- a/api/test/converters_test.py
+++ b/api/test/converters_test.py
@@ -1,17 +1,18 @@
-# Copyright 2018 The Chromium Authors. All rights reserved.
-# Use of this source code is governed by a BSD-style
-# license that can be found in the LICENSE file or at
-# https://developers.google.com/open-source/licenses/bsd
+# Copyright 2018 The Chromium Authors
+# Use of this source code is governed by a BSD-style license that can be
+# found in the LICENSE file.
 
 """Tests for converting internal protorpc to external protoc."""
 from __future__ import print_function
 from __future__ import division
 from __future__ import absolute_import
 
-from mock import Mock, patch
+from mock import patch
+import six
 import unittest
 
 from google.protobuf import wrappers_pb2
+import pytest
 
 import settings
 from api import converters
@@ -22,8 +23,8 @@
 from api.api_proto import user_objects_pb2
 from framework import exceptions
 from framework import permissions
-from proto import tracker_pb2
-from proto import user_pb2
+from mrproto import tracker_pb2
+from mrproto import user_pb2
 from testing import fake
 from testing import testing_helpers
 from tracker import tracker_bizobj
@@ -284,28 +285,27 @@
 
     actual = converters.ConvertUsers(
         [user1, user2, user3, user4], users_by_id)
-    self.assertItemsEqual(
-        actual,
-        [user_objects_pb2.User(
-            user_id=1,
-            display_name='user1@example.com'),
-         user_objects_pb2.User(
-            user_id=2,
-            display_name='user2@example.com',
-            is_site_admin=True),
-         user_objects_pb2.User(
-            user_id=3,
-            display_name='user3@example.com',
-            availability='User never visited',
-            linked_child_refs=[common_pb2.UserRef(
-              user_id=4, display_name='user4@example.com')]),
-         user_objects_pb2.User(
-            user_id=4,
-            display_name='user4@example.com',
-            availability='Last visit > 30 days ago',
-            linked_parent_ref=common_pb2.UserRef(
-              user_id=3, display_name='user3@example.com')),
-         ])
+    six.assertCountEqual(
+        self, actual, [
+            user_objects_pb2.User(user_id=1, display_name='user1@example.com'),
+            user_objects_pb2.User(
+                user_id=2, display_name='user2@example.com',
+                is_site_admin=True),
+            user_objects_pb2.User(
+                user_id=3,
+                display_name='user3@example.com',
+                availability='User never visited',
+                linked_child_refs=[
+                    common_pb2.UserRef(
+                        user_id=4, display_name='user4@example.com')
+                ]),
+            user_objects_pb2.User(
+                user_id=4,
+                display_name='user4@example.com',
+                availability='Last visit > 30 days ago',
+                linked_parent_ref=common_pb2.UserRef(
+                    user_id=3, display_name='user3@example.com')),
+        ])
 
   def testConvetPrefValues(self):
     """We can convert a list of UserPrefValues from protorpc to protoc."""
@@ -506,7 +506,7 @@
               field_id=5, field_name='Pre', type=common_pb2.ENUM_TYPE),
           value='label2', is_derived=True),
       ]
-    self.assertItemsEqual(expected, actual)
+    six.assertCountEqual(self, expected, actual)
 
   def testConvertIssue(self):
     """We can convert a protorpc Issue to a protoc Issue."""
@@ -584,6 +584,83 @@
               rank=2)])
     self.assertEqual(expected, actual)
 
+  def testConvertIssue_WithMigratedID(self):
+    """We can convert a protorpc Issue to a protoc Issue."""
+    related_refs_dict = {
+        78901: ('proj', 1),
+        78902: ('proj', 2),
+        }
+    now = 12345678
+    self.config.component_defs = [
+      tracker_pb2.ComponentDef(component_id=1, path='UI'),
+      tracker_pb2.ComponentDef(component_id=2, path='DB'),
+      ]
+    issue = fake.MakeTestIssue(
+      789, 3, 'sum', 'New', 111, labels=['Hot'],
+      derived_labels=['Scalability'], star_count=12, reporter_id=222,
+      opened_timestamp=now, component_ids=[1], project_name='proj',
+      cc_ids=[111], derived_cc_ids=[222])
+    issue.phases = [
+        tracker_pb2.Phase(phase_id=1, name='Dev', rank=1),
+        tracker_pb2.Phase(phase_id=2, name='Beta', rank=2),
+        ]
+    issue.dangling_blocked_on_refs = [
+        tracker_pb2.DanglingIssueRef(project='dangling_proj', issue_id=1234)]
+    issue.dangling_blocking_refs = [
+        tracker_pb2.DanglingIssueRef(project='dangling_proj', issue_id=5678)]
+
+    actual = converters.ConvertIssue(
+        issue, self.users_by_id, related_refs_dict, self.config, '123')
+
+    expected = issue_objects_pb2.Issue(
+        project_name='proj',
+        local_id=3,
+        summary='sum',
+        status_ref=common_pb2.StatusRef(
+            status='New',
+            is_derived=False,
+            means_open=True),
+        owner_ref=common_pb2.UserRef(
+            user_id=111,
+            display_name='one@example.com',
+            is_derived=False),
+        cc_refs=[
+            common_pb2.UserRef(
+                user_id=111,
+                display_name='one@example.com',
+                is_derived=False),
+            common_pb2.UserRef(
+                user_id=222,
+                display_name='two@example.com',
+                is_derived=True)],
+        label_refs=[
+            common_pb2.LabelRef(label='Hot', is_derived=False),
+            common_pb2.LabelRef(label='Scalability', is_derived=True)],
+        component_refs=[common_pb2.ComponentRef(path='UI', is_derived=False)],
+        is_deleted=False,
+        reporter_ref=common_pb2.UserRef(
+            user_id=222, display_name='two@example.com', is_derived=False),
+        opened_timestamp=now,
+        component_modified_timestamp=now,
+        status_modified_timestamp=now,
+        owner_modified_timestamp=now,
+        star_count=12,
+        is_spam=False,
+        attachment_count=0,
+        dangling_blocked_on_refs=[
+            common_pb2.IssueRef(project_name='dangling_proj', local_id=1234)],
+        dangling_blocking_refs=[
+            common_pb2.IssueRef(project_name='dangling_proj', local_id=5678)],
+        phases=[
+            issue_objects_pb2.PhaseDef(
+              phase_ref=issue_objects_pb2.PhaseRef(phase_name='Dev'),
+              rank=1),
+            issue_objects_pb2.PhaseDef(
+              phase_ref=issue_objects_pb2.PhaseRef(phase_name='Beta'),
+              rank=2)],
+        migrated_id='123',)
+    self.assertEqual(expected, actual)
+
   def testConvertIssue_NegativeAttachmentCount(self):
     """We can convert a protorpc Issue to a protoc Issue."""
     related_refs_dict = {
@@ -1423,21 +1500,22 @@
     """Uploading files results in a list of attachments."""
     uploads = [
         issue_objects_pb2.AttachmentUpload(
-            filename='hello.c', content='int main() {}'),
+            filename='hello.c', content=b'int main() {}'),
         issue_objects_pb2.AttachmentUpload(
-            filename='README.md', content='readme content'),
-        ]
+            filename='README.md', content=b'readme content'),
+    ]
     actual = converters.IngestAttachmentUploads(uploads)
     self.assertEqual(
-      [('hello.c', 'int main() {}', 'text/plain'),
-       ('README.md', 'readme content', 'text/plain')],
-      actual)
+        [
+            ('hello.c', b'int main() {}', 'text/plain'),
+            ('README.md', b'readme content', 'text/plain')
+        ], actual)
 
   def testIngestAttachmentUploads_Invalid(self):
     """We reject uploaded files that lack a name or content."""
     with self.assertRaises(exceptions.InputException):
-      converters.IngestAttachmentUploads([
-          issue_objects_pb2.AttachmentUpload(content='name is mssing')])
+      converters.IngestAttachmentUploads(
+          [issue_objects_pb2.AttachmentUpload(content=b'name is mssing')])
 
     with self.assertRaises(exceptions.InputException):
       converters.IngestAttachmentUploads([
@@ -1607,9 +1685,7 @@
       converters.IngestFieldValues(
           self.cnxn, self.services.user, field_values, self.config, [])
 
-    self.assertEqual(
-        'Unparsable value for field SecField',
-        cm.exception.message)
+    self.assertEqual('Unparsable value for field SecField', str(cm.exception))
 
   def testIngestSavedQueries(self):
     self.services.project.TestAddProject('chromium', project_id=1)
@@ -1656,6 +1732,7 @@
         self.cnxn, self.services.user, self.services.features, hotlist_ref)
     self.assertEqual(actual_hotlist_id, hotlist.hotlist_id)
 
+  @pytest.mark.skip(reason='Test is flaky (https://crbug.com/monorail/12052)')
   def testIngestHotlistRef_HotlistID(self):
     self.services.user.TestAddUser('user1@example.com', 111)
     hotlist = self.services.features.CreateHotlist(
@@ -1674,6 +1751,7 @@
       converters.IngestHotlistRef(
           self.cnxn, self.services.user, self.services.features, hotlist_ref)
 
+  @pytest.mark.skip(reason='Test is flaky (https://crbug.com/monorail/12052)')
   def testIngestHotlistRef_InconsistentRequest(self):
     self.services.user.TestAddUser('user1@example.com', 111)
     hotlist1 = self.services.features.CreateHotlist(
diff --git a/api/test/features_servicer_test.py b/api/test/features_servicer_test.py
index 21154de..59d01c6 100644
--- a/api/test/features_servicer_test.py
+++ b/api/test/features_servicer_test.py
@@ -1,13 +1,13 @@
-# Copyright 2018 The Chromium Authors. All rights reserved.
-# Use of this source code is governed by a BSD-style
-# license that can be found in the LICENSE file or at
-# https://developers.google.com/open-source/licenses/bsd
+# Copyright 2018 The Chromium Authors
+# Use of this source code is governed by a BSD-style license that can be
+# found in the LICENSE file.
 
 """Tests for the projects servicer."""
 from __future__ import print_function
 from __future__ import division
 from __future__ import absolute_import
 
+import pytest
 import unittest
 try:
   from mox3 import mox
@@ -407,6 +407,7 @@
 
     self.assertEqual(0, len(response.hotlists))
 
+  @pytest.mark.skip(reason='Test is flaky (https://crbug.com/monorail/12052)')
   def testListHotlistsByIssue_NonProjectHotlists(self):
     hotlist = self.services.features.CreateHotlist(
         self.cnxn,
diff --git a/api/test/issues_servicer_test.py b/api/test/issues_servicer_test.py
index 0ba50ac..ab35a9f 100644
--- a/api/test/issues_servicer_test.py
+++ b/api/test/issues_servicer_test.py
@@ -1,7 +1,6 @@
-# Copyright 2018 The Chromium Authors. All rights reserved.
-# Use of this source code is governed by a BSD-style
-# license that can be found in the LICENSE file or at
-# https://developers.google.com/open-source/licenses/bsd
+# Copyright 2018 The Chromium Authors
+# Use of this source code is governed by a BSD-style license that can be
+# found in the LICENSE file.
 
 """Tests for the issues servicer."""
 from __future__ import print_function
@@ -33,13 +32,12 @@
 from framework import monorailcontext
 from framework import permissions
 from search import frontendsearchpipeline
-from proto import tracker_pb2
-from proto import project_pb2
+from mrproto import tracker_pb2
+from mrproto import project_pb2
 from testing import fake
 from tracker import tracker_bizobj
 from services import service_manager
-from proto import tracker_pb2
-
+from mrproto import tracker_pb2
 
 class IssuesServicerTest(unittest.TestCase):
 
@@ -68,7 +66,7 @@
     self.issue_2 = fake.MakeTestIssue(
         789, 2, 'sum', 'New', 111, project_name='proj', issue_id=1002)
     self.issue_1.blocked_on_iids.append(self.issue_2.issue_id)
-    self.issue_1.blocked_on_ranks.append(sys.maxint)
+    self.issue_1.blocked_on_ranks.append(sys.maxsize)
     self.services.issue.TestAddIssue(self.issue_1)
     self.services.issue.TestAddIssue(self.issue_2)
     self.issues_svcr = issues_servicer.IssuesServicer(
@@ -161,8 +159,10 @@
 
     self.assertEqual('proj', response.project_name)
 
-  def testGetIssue_Normal(self):
+  @patch('businesslogic.work_env.WorkEnv.GetIssueMigratedID')
+  def testGetIssue_Normal(self, mockGetIssueMigratedID):
     """We can get an issue."""
+    mockGetIssueMigratedID.return_value = None
     request = issues_pb2.GetIssueRequest()
     request.issue_ref.project_name = 'proj'
     request.issue_ref.local_id = 1
@@ -179,6 +179,27 @@
     self.assertEqual('proj', actual.blocked_on_issue_refs[0].project_name)
     self.assertEqual(2, actual.blocked_on_issue_refs[0].local_id)
 
+  @patch('businesslogic.work_env.WorkEnv.GetIssueMigratedID')
+  def testGetIssue_WithMigratedID(self, mockGetIssueMigratedID):
+    """We can get an issue."""
+    mockGetIssueMigratedID.return_value = '123'
+    request = issues_pb2.GetIssueRequest()
+    request.issue_ref.project_name = 'proj'
+    request.issue_ref.local_id = 1
+    mc = monorailcontext.MonorailContext(
+        self.services, cnxn=self.cnxn, requester='owner@example.com')
+    mc.LookupLoggedInUserPerms(self.project)
+
+    response = self.CallWrapped(self.issues_svcr.GetIssue, mc, request)
+
+    actual = response.issue
+    self.assertEqual('proj', actual.project_name)
+    self.assertEqual(1, actual.local_id)
+    self.assertEqual(1, len(actual.blocked_on_issue_refs))
+    self.assertEqual('proj', actual.blocked_on_issue_refs[0].project_name)
+    self.assertEqual(2, actual.blocked_on_issue_refs[0].local_id)
+    self.assertEqual('123', actual.migrated_id)
+
   def testGetIssue_Moved(self):
     """We can get a moved issue."""
     self.services.project.TestAddProject(
@@ -560,10 +581,11 @@
     request.issue_ref.project_name = 'proj'
     request.issue_ref.local_id = 1
     request.comment_content = 'test comment'
-    request.uploads.extend([
-          issue_objects_pb2.AttachmentUpload(
-              filename='a.txt',
-              content='aaaaa')])
+    request.uploads.extend(
+        [
+            issue_objects_pb2.AttachmentUpload(
+                filename='a.txt', content=b'aaaaa')
+        ])
     mc = monorailcontext.MonorailContext(
         self.services, cnxn=self.cnxn, requester='owner@example.com')
     mc.LookupLoggedInUserPerms(self.project)
@@ -1176,10 +1198,11 @@
     )
     request.issue_ref.project_name = 'proj'
     request.issue_ref.local_id = 1
-    request.uploads.extend([
-          issue_objects_pb2.AttachmentUpload(
-              filename='a.txt',
-              content='aaaaa')])
+    request.uploads.extend(
+        [
+            issue_objects_pb2.AttachmentUpload(
+                filename='a.txt', content=b'aaaaa')
+        ])
     request.kept_attachments.extend([1, 2, 3])
     request.send_email = True
 
@@ -1222,7 +1245,7 @@
     work_env.WorkEnv(mc, self.services).UpdateIssueApproval.\
     assert_called_once_with(
         self.issue_1.issue_id, 3, ANY, u'Well, actually', False,
-        attachments=[(u'a.txt', 'aaaaa', 'text/plain')], send_email=True,
+        attachments=[(u'a.txt', b'aaaaa', 'text/plain')], send_email=True,
         kept_attachments=[1, 2, 3])
     self.assertEqual(expected, actual)
 
@@ -1511,10 +1534,10 @@
     response = self.CallWrapped(self.issues_svcr.IssueSnapshot, mc, request)
 
     self.assertEqual(2, len(response.snapshot_count))
-    self.assertEqual('Opened', response.snapshot_count[0].dimension)
-    self.assertEqual(100, response.snapshot_count[0].count)
-    self.assertEqual('Closed', response.snapshot_count[1].dimension)
-    self.assertEqual(23, response.snapshot_count[1].count)
+    self.assertEqual('Closed', response.snapshot_count[0].dimension)
+    self.assertEqual(23, response.snapshot_count[0].count)
+    self.assertEqual('Opened', response.snapshot_count[1].dimension)
+    self.assertEqual(100, response.snapshot_count[1].count)
     mockSnapshotCountsQuery.assert_called_once_with(self.project, 1531334109,
         'open', label_prefix='', query=None, canned_query=None, hotlist=None)
 
@@ -1531,10 +1554,10 @@
     response = self.CallWrapped(self.issues_svcr.IssueSnapshot, mc, request)
 
     self.assertEqual(2, len(response.snapshot_count))
-    self.assertEqual('Fixed', response.snapshot_count[0].dimension)
-    self.assertEqual(23, response.snapshot_count[0].count)
-    self.assertEqual('Accepted', response.snapshot_count[1].dimension)
-    self.assertEqual(100, response.snapshot_count[1].count)
+    self.assertEqual('Accepted', response.snapshot_count[0].dimension)
+    self.assertEqual(100, response.snapshot_count[0].count)
+    self.assertEqual('Fixed', response.snapshot_count[1].dimension)
+    self.assertEqual(23, response.snapshot_count[1].count)
     mockSnapshotCountsQuery.assert_called_once_with(self.project, 1531334109,
         'status', label_prefix='', query=None, canned_query=None, hotlist=None)
 
diff --git a/api/test/monorail_servicer_test.py b/api/test/monorail_servicer_test.py
index 3c2d8a6..1253fdb 100644
--- a/api/test/monorail_servicer_test.py
+++ b/api/test/monorail_servicer_test.py
@@ -1,7 +1,6 @@
-# Copyright 2018 The Chromium Authors. All rights reserved.
-# Use of this source code is governed by a BSD-style
-# license that can be found in the LICENSE file or at
-# https://developers.google.com/open-source/licenses/bsd
+# Copyright 2018 The Chromium Authors
+# Use of this source code is governed by a BSD-style license that can be
+# found in the LICENSE file.
 
 """Tests for MonorailServicer."""
 from __future__ import print_function
@@ -75,11 +74,11 @@
   pass
 
 
-class TestableServicer(monorail_servicer.MonorailServicer):
+class _TestableServicer(monorail_servicer.MonorailServicer):
   """Fake servicer class."""
 
   def __init__(self, services):
-    super(TestableServicer, self).__init__(services)
+    super(_TestableServicer, self).__init__(services)
     self.was_called = False
     self.seen_mc = None
     self.seen_request = None
@@ -128,7 +127,7 @@
     self.allowed_domain_user = self.services.user.TestAddUser(
         'chickenchicken@google.com', 333)
     self.test_user = self.services.user.TestAddUser('test@example.com', 420)
-    self.svcr = TestableServicer(self.services)
+    self.svcr = _TestableServicer(self.services)
     self.nonmember_token = xsrf.GenerateToken(222, xsrf.XHR_SERVLET_PATH)
     self.request = UpdateSomethingRequest(exc_class=None)
     self.prpc_context = context.ServicerContext()
diff --git a/api/test/projects_servicer_test.py b/api/test/projects_servicer_test.py
index 683fa90..630d1cf 100644
--- a/api/test/projects_servicer_test.py
+++ b/api/test/projects_servicer_test.py
@@ -1,7 +1,6 @@
-# Copyright 2018 The Chromium Authors. All rights reserved.
-# Use of this source code is governed by a BSD-style
-# license that can be found in the LICENSE file or at
-# https://developers.google.com/open-source/licenses/bsd
+# Copyright 2018 The Chromium Authors
+# Use of this source code is governed by a BSD-style license that can be
+# found in the LICENSE file.
 
 """Tests for the projects servicer."""
 from __future__ import print_function
@@ -22,8 +21,8 @@
 from framework import exceptions
 from framework import monorailcontext
 from framework import permissions
-from proto import tracker_pb2
-from proto import project_pb2
+from mrproto import tracker_pb2
+from mrproto import project_pb2
 from tracker import tracker_bizobj
 from tracker import tracker_constants
 from testing import fake
diff --git a/api/test/resource_name_converters_test.py b/api/test/resource_name_converters_test.py
index e9ca437..eed6957 100644
--- a/api/test/resource_name_converters_test.py
+++ b/api/test/resource_name_converters_test.py
@@ -1,7 +1,6 @@
-# Copyright 2018 The Chromium Authors. All rights reserved.
-# Use of this source code is governed by a BSD-style
-# license that can be found in the LICENSE file or at
-# https://developers.google.com/open-source/licenses/bsd
+# Copyright 2018 The Chromium Authors
+# Use of this source code is governed by a BSD-style license that can be
+# found in the LICENSE file.
 
 """Tests for converting between resource names and external ids."""
 from __future__ import print_function
@@ -16,7 +15,7 @@
 from framework import exceptions
 from testing import fake
 from services import service_manager
-from proto import tracker_pb2
+from mrproto import tracker_pb2
 
 class ResourceNameConverterTest(unittest.TestCase):
 
@@ -173,9 +172,9 @@
         'hotlists/78909/items/proj.1', 'hotlists/78909/items/chicken.2',
         'hotlists/78909/items/cow.3'
     ]
-    with self.assertRaisesRegexp(exceptions.NoSuchProjectException,
-                                 'Project chicken not found.\n' +
-                                 'Project cow not found.'):
+    with self.assertRaisesRegex(exceptions.NoSuchProjectException,
+                                'Project chicken not found.\n' +
+                                'Project cow not found.'):
       rnc.IngestHotlistItemNames(self.cnxn, names, self.services)
 
   def testIngestHotlistItems_IssueNotFound(self):
@@ -183,7 +182,7 @@
     names = [
         'hotlists/78909/items/proj.1',
         'hotlists/78909/items/goose.5']
-    with self.assertRaisesRegexp(exceptions.NoSuchIssueException, '%r' % names):
+    with self.assertRaisesRegex(exceptions.NoSuchIssueException, '%r' % names):
       rnc.IngestHotlistItemNames(self.cnxn, names, self.services)
 
   def testConvertHotlistName(self):
@@ -193,8 +192,10 @@
   def testConvertHotlistItemNames(self):
     """We can get Hotlist items' resource names."""
     expected_dict = {
-        self.hotlist_1.items[0].issue_id: 'hotlists/7739/items/proj.1',
-        self.hotlist_1.items[1].issue_id: 'hotlists/7739/items/goose.2',
+        self.hotlist_1.items[0].issue_id:
+            'hotlists/%d/items/proj.1' % self.hotlist_1.hotlist_id,
+        self.hotlist_1.items[1].issue_id:
+            'hotlists/%d/items/goose.2' % self.hotlist_1.hotlist_id,
     }
     self.assertEqual(
         rnc.ConvertHotlistItemNames(
@@ -214,8 +215,8 @@
           self.cnxn, 'projects/noproj/issues/1/approvalValues/1', self.services)
 
   def testIngestApprovalValueName_IssueDoesNotExist(self):
-    with self.assertRaisesRegexp(exceptions.NoSuchIssueException,
-                                 'projects/proj/issues/404'):
+    with self.assertRaisesRegex(exceptions.NoSuchIssueException,
+                                'projects/proj/issues/404'):
       rnc.IngestApprovalValueName(
           self.cnxn, 'projects/proj/issues/404/approvalValues/1', self.services)
 
@@ -292,7 +293,7 @@
 
   def testIngestIssueNames_WithBadInputs(self):
     """We aggregate input exceptions."""
-    with self.assertRaisesRegexp(
+    with self.assertRaisesRegex(
         exceptions.InputException,
         'Invalid resource name: projects/proj/badformat/1.\n' +
         'Invalid resource name: badformat/proj/issues/1.'):
@@ -312,15 +313,15 @@
   def testIngestIssueNames_ManyDoNotExist(self):
     """We get an exception if one issue name provided does not exist."""
     dne_issues = ['projects/proj/issues/2', 'projects/proj/issues/3']
-    with self.assertRaisesRegexp(exceptions.NoSuchIssueException,
-                                 '%r' % dne_issues):
+    with self.assertRaisesRegex(exceptions.NoSuchIssueException,
+                                '%r' % dne_issues):
       rnc.IngestIssueNames(self.cnxn, dne_issues, self.services)
 
   def testIngestIssueNames_ProjectsNotExist(self):
     """Aggregated exceptions raised if projects are not found."""
-    with self.assertRaisesRegexp(exceptions.NoSuchProjectException,
-                                 'Project chicken not found.\n' +
-                                 'Project cow not found.'):
+    with self.assertRaisesRegex(exceptions.NoSuchProjectException,
+                                'Project chicken not found.\n' +
+                                'Project cow not found.'):
       rnc.IngestIssueNames(
           self.cnxn, [
               'projects/chicken/issues/2', 'projects/cow/issues/3',
@@ -354,8 +355,8 @@
           self.cnxn, 'projects/doesnotexist/issues/1/comments/0', self.services)
 
   def testIngestCommentName_NoSuchIssue(self):
-    with self.assertRaisesRegexp(exceptions.NoSuchIssueException,
-                                 "['projects/proj/issues/404']"):
+    with self.assertRaisesRegex(exceptions.NoSuchIssueException,
+                                "['projects/proj/issues/404']"):
       rnc.IngestCommentName(
           self.cnxn, 'projects/proj/issues/404/comments/0', self.services)
 
@@ -641,8 +642,8 @@
 
     names = ['projects/xyz/componentDefs/1', 'projects/zyz/componentDefs/1']
     expected_error = 'Project not found: xyz.\nProject not found: zyz.'
-    with self.assertRaisesRegexp(exceptions.NoSuchProjectException,
-                                 expected_error):
+    with self.assertRaisesRegex(exceptions.NoSuchProjectException,
+                                expected_error):
       rnc.IngestComponentDefNames(self.cnxn, names, self.services)
 
   def testIngestComponentDefNames_NoSuchComponentException(self):
@@ -654,8 +655,8 @@
         'projects/proj/componentDefs/999', 'projects/proj/componentDefs/cow'
     ]
     expected_error = 'Component not found: 999.\nComponent not found: \'cow\'.'
-    with self.assertRaisesRegexp(exceptions.NoSuchComponentException,
-                                 expected_error):
+    with self.assertRaisesRegex(exceptions.NoSuchComponentException,
+                                expected_error):
       rnc.IngestComponentDefNames(self.cnxn, names, self.services)
 
   def testIngestComponentDefNames_InvalidNames(self):
diff --git a/api/test/sitewide_servicer_test.py b/api/test/sitewide_servicer_test.py
index a15fc75..4a76033 100644
--- a/api/test/sitewide_servicer_test.py
+++ b/api/test/sitewide_servicer_test.py
@@ -1,7 +1,6 @@
-# Copyright 2018 The Chromium Authors. All rights reserved.
-# Use of this source code is governed by a BSD-style
-# license that can be found in the LICENSE file or at
-# https://developers.google.com/open-source/licenses/bsd
+# Copyright 2018 The Chromium Authors
+# Use of this source code is governed by a BSD-style license that can be
+# found in the LICENSE file.
 
 """Tests for the sitewide servicer."""
 from __future__ import print_function
@@ -41,7 +40,7 @@
   @mock.patch('time.time')
   def testRefreshToken(self, mockTime, mockGetXSRFKey):
     """We can refresh an expired token."""
-    mockGetXSRFKey.side_effect = lambda: 'fakeXSRFKey'
+    mockGetXSRFKey.side_effect = lambda: b'fakeXSRFKey'
     # The token is at the brink of being too old
     mockTime.side_effect = lambda: 1 + xsrf.REFRESH_TOKEN_TIMEOUT_SEC
 
@@ -64,7 +63,7 @@
   @mock.patch('time.time')
   def testRefreshToken_InvalidToken(self, mockTime, mockGetXSRFKey):
     """We reject attempts to refresh an invalid token."""
-    mockGetXSRFKey.side_effect = ['fakeXSRFKey']
+    mockGetXSRFKey.side_effect = [b'fakeXSRFKey']
     mockTime.side_effect = [123]
 
     token_path = 'token_path'
@@ -82,7 +81,7 @@
   @mock.patch('time.time')
   def testRefreshToken_TokenTooOld(self, mockTime, mockGetXSRFKey):
     """We reject attempts to refresh a token that's too old."""
-    mockGetXSRFKey.side_effect = lambda: 'fakeXSRFKey'
+    mockGetXSRFKey.side_effect = lambda: b'fakeXSRFKey'
     mockTime.side_effect = lambda: 2 + xsrf.REFRESH_TOKEN_TIMEOUT_SEC
 
     token_path = 'token_path'
diff --git a/api/test/users_servicer_test.py b/api/test/users_servicer_test.py
index f9b6480..892e2ce 100644
--- a/api/test/users_servicer_test.py
+++ b/api/test/users_servicer_test.py
@@ -1,13 +1,14 @@
-# Copyright 2018 The Chromium Authors. All rights reserved.
-# Use of this source code is governed by a BSD-style
-# license that can be found in the LICENSE file or at
-# https://developers.google.com/open-source/licenses/bsd
+# Copyright 2018 The Chromium Authors
+# Use of this source code is governed by a BSD-style license that can be
+# found in the LICENSE file.
 
 """Tests for the users servicer."""
 from __future__ import print_function
 from __future__ import division
 from __future__ import absolute_import
 
+import pytest
+import six
 import unittest
 
 try:
@@ -25,9 +26,9 @@
 from framework import exceptions
 from framework import monorailcontext
 from framework import permissions
-from proto import project_pb2
-from proto import tracker_pb2
-from proto import user_pb2
+from mrproto import project_pb2
+from mrproto import tracker_pb2
+from mrproto import user_pb2
 from testing import fake
 from services import service_manager
 
@@ -84,7 +85,7 @@
             display_name='group2@test.com', user_id=self.group2_id)
     ]
 
-    self.assertItemsEqual(expected_group_refs, response.group_refs)
+    six.assertCountEqual(self, expected_group_refs, response.group_refs)
 
   def testGetMemberships_NonExistentUser(self):
     request = users_pb2.GetMembershipsRequest(
@@ -556,16 +557,19 @@
       self.services.project.TestAddProject(
           'owner-%s-%s' % (name, user_id), state=state, owner_ids=[user_id])
       self.services.project.TestAddProject(
-          'committer-%s-%s' % (name, user_id), state=state,\
+          'committer-%s-%s' % (name, user_id),
+          state=state,
           committer_ids=[user_id])
-      contributor = self.services.project.TestAddProject(
-          'contributor-%s-%s' % (name, user_id), state=state)
-      contributor.contributor_ids = [user_id]
+      self.services.project.TestAddProject(
+          'contributor-%s-%s' % (name, user_id),
+          state=state,
+          contrib_ids=[user_id])
 
     members_only = self.services.project.TestAddProject(
         'members-only-' + str(user_id), owner_ids=[user_id])
     members_only.access = project_pb2.ProjectAccess.MEMBERS_ONLY
 
+  @pytest.mark.skip(reason='Test is flaky (https://crbug.com/monorail/12052)')
   def testGetUsersProjects(self):
     self.user = self.services.user.TestAddUser('test3@example.com', 333)
     self.services.project_star.SetStar(