Merge branch 'main' into avm99963-monorail
Merged commit cd4b3b336f1f14afa02990fdc2eec5d9467a827e
GitOrigin-RevId: e67bbf185d5538e1472bb42e0abb2a141f88bac1
diff --git a/framework/test/deleteusers_test.py b/framework/test/deleteusers_test.py
index 4cadbbd..2609867 100644
--- a/framework/test/deleteusers_test.py
+++ b/framework/test/deleteusers_test.py
@@ -11,7 +11,7 @@
import logging
import mock
import unittest
-import urllib
+from six.moves import urllib
from framework import cloud_tasks_helpers
from framework import deleteusers
@@ -175,7 +175,7 @@
self.assertEqual(get_client_mock().create_task.call_count, 2)
- query = urllib.urlencode(
+ query = urllib.parse.urlencode(
{'emails': 'user1@gmail.com,user2@gmail.com,user3@gmail.com'})
expected_task = self.generate_simple_task(
urls.DELETE_USERS_TASK + '.do', query)
@@ -185,7 +185,7 @@
expected_task,
retry=cloud_tasks_helpers._DEFAULT_RETRY)
- query = urllib.urlencode({'emails': 'user4@gmail.com'})
+ query = urllib.parse.urlencode({'emails': 'user4@gmail.com'})
expected_task = self.generate_simple_task(
urls.DELETE_USERS_TASK + '.do', query)
@@ -204,7 +204,7 @@
self.assertEqual(get_client_mock().create_task.call_count, 1)
emails = 'user1@gmail.com,user2@gmail.com,user3@gmail.com,user4@gmail.com'
- query = urllib.urlencode({'emails': emails})
+ query = urllib.parse.urlencode({'emails': emails})
expected_task = self.generate_simple_task(
urls.DELETE_USERS_TASK + '.do', query)
diff --git a/framework/test/flask_servlet_test.py b/framework/test/flask_servlet_test.py
index 443f919..4c47209 100644
--- a/framework/test/flask_servlet_test.py
+++ b/framework/test/flask_servlet_test.py
@@ -71,13 +71,15 @@
self.page_class._CheckForMovedProject(mr, request)
mock_abort.assert_not_called()
- @mock.patch('flask.abort')
- def testCheckForMovedProject_Redirect(self, mock_abort):
+ @mock.patch('flask.redirect')
+ def testCheckForMovedProject_Redirect(self, mock_redirect):
project = fake.Project(project_name='proj', moved_to='http://example.com')
request, mr = testing_helpers.GetRequestObjects(
path='/p/proj', project=project)
+ self.page_class.request_path = '/p/test'
self.page_class._CheckForMovedProject(mr, request)
- mock_abort.assert_called_once_with(302)
+ mock_redirect.assert_called_once_with(
+ 'http://127.0.0.1/hosting/moved?project=proj', code=302)
def testGatherBaseData(self):
project = self.page_class.services.project.TestAddProject(
diff --git a/framework/test/gcs_helpers_test.py b/framework/test/gcs_helpers_test.py
index 3500e40..a7c01d0 100644
--- a/framework/test/gcs_helpers_test.py
+++ b/framework/test/gcs_helpers_test.py
@@ -10,143 +10,123 @@
import mock
import unittest
-import uuid
-import mox
-
-from google.appengine.api import app_identity
-from google.appengine.api import images
from google.appengine.api import urlfetch
from google.appengine.ext import testbed
-from third_party import cloudstorage
+from google.cloud import storage
-from framework import filecontent
from framework import gcs_helpers
-from testing import fake
from testing import testing_helpers
class GcsHelpersTest(unittest.TestCase):
def setUp(self):
- self.mox = mox.Mox()
self.testbed = testbed.Testbed()
self.testbed.activate()
self.testbed.init_memcache_stub()
+ self.testbed.init_app_identity_stub()
+
+ self.test_storage_client = mock.create_autospec(
+ storage.Client, instance=True)
+ mock.patch.object(
+ storage, 'Client', return_value=self.test_storage_client).start()
def tearDown(self):
- self.mox.UnsetStubs()
- self.mox.ResetAll()
self.testbed.deactivate()
+ self.test_storage_client = None
+ mock.patch.stopall()
def testDeleteObjectFromGCS(self):
object_id = 'aaaaa'
- bucket_name = 'test_bucket'
- object_path = '/' + bucket_name + object_id
-
- self.mox.StubOutWithMock(app_identity, 'get_default_gcs_bucket_name')
- app_identity.get_default_gcs_bucket_name().AndReturn(bucket_name)
-
- self.mox.StubOutWithMock(cloudstorage, 'delete')
- cloudstorage.delete(object_path)
-
- self.mox.ReplayAll()
-
gcs_helpers.DeleteObjectFromGCS(object_id)
- self.mox.VerifyAll()
+ # Verify order of client calls.
+ self.test_storage_client.assert_has_calls(
+ [
+ mock.call.bucket().get_blob(object_id),
+ mock.call.bucket().get_blob().delete()
+ ])
- def testStoreObjectInGCS_ResizableMimeType(self):
+ def testDeleteLegacyObjectFromGCS(self):
+ # A previous python module expected object ids with leading '/'
+ object_id = '/aaaaa'
+ object_id_without_leading_slash = 'aaaaa'
+ gcs_helpers.DeleteObjectFromGCS(object_id)
+ # Verify order of client calls.
+ self.test_storage_client.assert_has_calls(
+ [
+ mock.call.bucket().get_blob(object_id_without_leading_slash),
+ mock.call.bucket().get_blob().delete()
+ ])
+
+ @mock.patch(
+ 'google.appengine.api.images.resize', return_value=mock.MagicMock())
+ @mock.patch('uuid.uuid4')
+ def testStoreObjectInGCS_ResizableMimeType(self, mock_uuid4, mock_resize):
guid = 'aaaaa'
+ mock_uuid4.return_value = guid
project_id = 100
- object_id = '/%s/attachments/%s' % (project_id, guid)
- bucket_name = 'test_bucket'
- object_path = '/' + bucket_name + object_id
+ blob_name = '%s/attachments/%s' % (project_id, guid)
+ thumb_blob_name = '%s/attachments/%s-thumbnail' % (project_id, guid)
mime_type = 'image/png'
content = 'content'
- thumb_content = 'thumb_content'
-
- self.mox.StubOutWithMock(app_identity, 'get_default_gcs_bucket_name')
- app_identity.get_default_gcs_bucket_name().AndReturn(bucket_name)
-
- self.mox.StubOutWithMock(uuid, 'uuid4')
- uuid.uuid4().AndReturn(guid)
-
- self.mox.StubOutWithMock(cloudstorage, 'open')
- cloudstorage.open(
- object_path, 'w', mime_type, options={}
- ).AndReturn(fake.FakeFile())
- cloudstorage.open(object_path + '-thumbnail', 'w', mime_type).AndReturn(
- fake.FakeFile())
-
- self.mox.StubOutWithMock(images, 'resize')
- images.resize(content, gcs_helpers.DEFAULT_THUMB_WIDTH,
- gcs_helpers.DEFAULT_THUMB_HEIGHT).AndReturn(thumb_content)
-
- self.mox.ReplayAll()
ret_id = gcs_helpers.StoreObjectInGCS(
content, mime_type, project_id, gcs_helpers.DEFAULT_THUMB_WIDTH,
gcs_helpers.DEFAULT_THUMB_HEIGHT)
- self.mox.VerifyAll()
- self.assertEqual(object_id, ret_id)
+ self.assertEqual('/%s' % blob_name, ret_id)
+ self.test_storage_client.assert_has_calls(
+ [
+ mock.call.bucket().blob(blob_name),
+ mock.call.bucket().blob().upload_from_string(
+ content, content_type=mime_type),
+ mock.call.bucket().blob(thumb_blob_name),
+ ])
+ mock_resize.assert_called()
- def testStoreObjectInGCS_NotResizableMimeType(self):
+ @mock.patch(
+ 'google.appengine.api.images.resize', return_value=mock.MagicMock())
+ @mock.patch('uuid.uuid4')
+ def testStoreObjectInGCS_NotResizableMimeType(self, mock_uuid4, mock_resize):
guid = 'aaaaa'
+ mock_uuid4.return_value = guid
project_id = 100
- object_id = '/%s/attachments/%s' % (project_id, guid)
- bucket_name = 'test_bucket'
- object_path = '/' + bucket_name + object_id
+ blob_name = '%s/attachments/%s' % (project_id, guid)
mime_type = 'not_resizable_mime_type'
content = 'content'
- self.mox.StubOutWithMock(app_identity, 'get_default_gcs_bucket_name')
- app_identity.get_default_gcs_bucket_name().AndReturn(bucket_name)
-
- self.mox.StubOutWithMock(uuid, 'uuid4')
- uuid.uuid4().AndReturn(guid)
-
- self.mox.StubOutWithMock(cloudstorage, 'open')
- options = {'Content-Disposition': 'inline; filename="file.ext"'}
- cloudstorage.open(
- object_path, 'w', mime_type, options=options
- ).AndReturn(fake.FakeFile())
-
- self.mox.ReplayAll()
-
ret_id = gcs_helpers.StoreObjectInGCS(
content, mime_type, project_id, gcs_helpers.DEFAULT_THUMB_WIDTH,
- gcs_helpers.DEFAULT_THUMB_HEIGHT, filename='file.ext')
- self.mox.VerifyAll()
- self.assertEqual(object_id, ret_id)
+ gcs_helpers.DEFAULT_THUMB_HEIGHT)
+ self.assertEqual('/%s' % blob_name, ret_id)
+ self.test_storage_client.assert_has_calls(
+ [
+ mock.call.bucket().blob(blob_name),
+ mock.call.bucket().blob().upload_from_string(
+ content, content_type=mime_type),
+ ])
+ mock_resize.assert_not_called()
- def testCheckMemeTypeResizable(self):
+ def testCheckMimeTypeResizable(self):
for resizable_mime_type in gcs_helpers.RESIZABLE_MIME_TYPES:
gcs_helpers.CheckMimeTypeResizable(resizable_mime_type)
with self.assertRaises(gcs_helpers.UnsupportedMimeType):
gcs_helpers.CheckMimeTypeResizable('not_resizable_mime_type')
- def testStoreLogoInGCS(self):
- file_name = 'test_file.png'
+ @mock.patch('framework.filecontent.GuessContentTypeFromFilename')
+ @mock.patch('framework.gcs_helpers.StoreObjectInGCS')
+ def testStoreLogoInGCS(self, mock_store_object, mock_guess_content):
+ blob_name = 123
+ mock_store_object.return_value = blob_name
mime_type = 'image/png'
+ mock_guess_content.return_value = mime_type
+ file_name = 'test_file.png'
content = 'test content'
project_id = 100
- object_id = 123
-
- self.mox.StubOutWithMock(filecontent, 'GuessContentTypeFromFilename')
- filecontent.GuessContentTypeFromFilename(file_name).AndReturn(mime_type)
-
- self.mox.StubOutWithMock(gcs_helpers, 'StoreObjectInGCS')
- gcs_helpers.StoreObjectInGCS(
- content, mime_type, project_id,
- thumb_width=gcs_helpers.LOGO_THUMB_WIDTH,
- thumb_height=gcs_helpers.LOGO_THUMB_HEIGHT).AndReturn(object_id)
-
- self.mox.ReplayAll()
ret_id = gcs_helpers.StoreLogoInGCS(file_name, content, project_id)
- self.mox.VerifyAll()
- self.assertEqual(object_id, ret_id)
+ self.assertEqual(blob_name, ret_id)
@mock.patch('google.appengine.api.urlfetch.fetch')
def testFetchSignedURL_Success(self, mock_fetch):
diff --git a/framework/test/jsonfeed_test.py b/framework/test/jsonfeed_test.py
index 0a569e2..4ca83fa 100644
--- a/framework/test/jsonfeed_test.py
+++ b/framework/test/jsonfeed_test.py
@@ -8,7 +8,7 @@
from __future__ import division
from __future__ import absolute_import
-import httplib
+from six.moves import http_client
import logging
import unittest
@@ -102,10 +102,10 @@
# Note that request has no X-Appengine-Inbound-Appid set.
self.assertIsNone(feed.get())
self.assertFalse(feed.handle_request_called)
- self.assertEqual(httplib.FORBIDDEN, feed.response.status)
+ self.assertEqual(http_client.FORBIDDEN, feed.response.status)
self.assertIsNone(feed.post())
self.assertFalse(feed.handle_request_called)
- self.assertEqual(httplib.FORBIDDEN, feed.response.status)
+ self.assertEqual(http_client.FORBIDDEN, feed.response.status)
def testSameAppOnly_InternalOnlyCalledFromWrongApp(self):
feed = TestableJsonFeed()
@@ -114,10 +114,10 @@
feed.mr.request.headers['X-Appengine-Inbound-Appid'] = 'wrong'
self.assertIsNone(feed.get())
self.assertFalse(feed.handle_request_called)
- self.assertEqual(httplib.FORBIDDEN, feed.response.status)
+ self.assertEqual(http_client.FORBIDDEN, feed.response.status)
self.assertIsNone(feed.post())
self.assertFalse(feed.handle_request_called)
- self.assertEqual(httplib.FORBIDDEN, feed.response.status)
+ self.assertEqual(http_client.FORBIDDEN, feed.response.status)
class TestableJsonFeed(jsonfeed.JsonFeed):
diff --git a/framework/test/redis_utils_test.py b/framework/test/redis_utils_test.py
deleted file mode 100644
index a4128ce..0000000
--- a/framework/test/redis_utils_test.py
+++ /dev/null
@@ -1,64 +0,0 @@
-# Copyright 2020 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.
-"""Tests for the Redis utility module."""
-from __future__ import print_function
-from __future__ import division
-from __future__ import absolute_import
-
-import fakeredis
-import unittest
-
-from framework import redis_utils
-from proto import features_pb2
-
-
-class RedisHelperTest(unittest.TestCase):
-
- def testFormatRedisKey(self):
- redis_key = redis_utils.FormatRedisKey(111)
- self.assertEqual('111', redis_key)
- redis_key = redis_utils.FormatRedisKey(222, prefix='foo:')
- self.assertEqual('foo:222', redis_key)
- redis_key = redis_utils.FormatRedisKey(333, prefix='bar')
- self.assertEqual('bar:333', redis_key)
-
- def testCreateRedisClient(self):
- self.assertIsNone(redis_utils.connection_pool)
- redis_client_1 = redis_utils.CreateRedisClient()
- self.assertIsNotNone(redis_client_1)
- self.assertIsNotNone(redis_utils.connection_pool)
- redis_client_2 = redis_utils.CreateRedisClient()
- self.assertIsNotNone(redis_client_2)
- self.assertIsNot(redis_client_1, redis_client_2)
-
- def testConnectionVerification(self):
- server = fakeredis.FakeServer()
- client = None
- self.assertFalse(redis_utils.VerifyRedisConnection(client))
- server.connected = True
- client = fakeredis.FakeRedis(server=server)
- self.assertTrue(redis_utils.VerifyRedisConnection(client))
- server.connected = False
- self.assertFalse(redis_utils.VerifyRedisConnection(client))
-
- def testSerializeDeserializeInt(self):
- serialized_int = redis_utils.SerializeValue(123)
- self.assertEqual('123', serialized_int)
- self.assertEquals(123, redis_utils.DeserializeValue(serialized_int))
-
- def testSerializeDeserializeStr(self):
- serialized = redis_utils.SerializeValue('123')
- self.assertEqual('"123"', serialized)
- self.assertEquals('123', redis_utils.DeserializeValue(serialized))
-
- def testSerializeDeserializePB(self):
- features = features_pb2.Hotlist.HotlistItem(
- issue_id=7949, rank=0, adder_id=333, date_added=1525)
- serialized = redis_utils.SerializeValue(
- features, pb_class=features_pb2.Hotlist.HotlistItem)
- self.assertIsInstance(serialized, str)
- deserialized = redis_utils.DeserializeValue(
- serialized, pb_class=features_pb2.Hotlist.HotlistItem)
- self.assertIsInstance(deserialized, features_pb2.Hotlist.HotlistItem)
- self.assertEquals(deserialized, features)
diff --git a/framework/test/servlet_helpers_test.py b/framework/test/servlet_helpers_test.py
index 19f4ea4..870de40 100644
--- a/framework/test/servlet_helpers_test.py
+++ b/framework/test/servlet_helpers_test.py
@@ -110,17 +110,14 @@
path='/p/proj/issues/detail?id=123&q=term',
project=self.project)
- url = servlet_helpers.ComputeIssueEntryURL(mr, self.config)
+ url = servlet_helpers.ComputeIssueEntryURL(mr)
self.assertEqual('/p/proj/issues/entry', url)
- def testComputeIssueEntryURL_Customized(self):
+ def testComputeIssueEntryURL_Chromium(self):
_request, mr = testing_helpers.GetRequestObjects(
- path='/p/proj/issues/detail?id=123&q=term',
- project=self.project)
- mr.auth.user_id = 111
- self.config.custom_issue_entry_url = FORM_URL
+ path='/p/chromium/issues/detail?id=123&q=term', project=self.project)
- url = servlet_helpers.ComputeIssueEntryURL(mr, self.config)
+ url = servlet_helpers.ComputeIssueEntryURL(mr)
self.assertIn('/issues/wizard', url)
class IssueListURLTest(unittest.TestCase):
@@ -223,8 +220,27 @@
def testCreateLoginUrl(self):
_, mr = testing_helpers.GetRequestObjects(
path='/p/proj/issues/detail?id=123&q=term', project=self.project)
- url = servlet_helpers.SafeCreateLoginURL(mr, '/continue')
- self.assertIn('/continue', url)
+ url = servlet_helpers.SafeCreateLoginURL(mr, 'current.url.to.return.to')
+ # Ensure that users can pick their account to use with Monorail.
+ self.assertIn('/AccountChooser', url)
+ self.assertIn('current.url.to.return.to', url)
+
+ def testCreateLoginUrl(self):
+ _, mr = testing_helpers.GetRequestObjects(
+ path='/p/proj/issues/detail?id=123&q=term', project=self.project)
+ url = servlet_helpers.SafeCreateLoginURL(mr, 'current.url.to.return.to')
+ # Ensure that users can pick their account to use with Monorail.
+ self.assertIn('/AccountChooser', url)
+ self.assertIn('current.url.to.return.to', url)
+
+ def testCreateEscapedLoginUrlFromMR(self):
+ _, mr = testing_helpers.GetRequestObjects(
+ path='/p/proj/issues/detail?id=123&q=term', project=self.project)
+ mr.current_page_url_encoded = (
+ 'https%3A%2F%2Fbugs.chromium.org'
+ '%2Fp%2Fchromium%2Fissues%2Fentry')
+ url = servlet_helpers.SafeCreateLoginURL(mr)
+ self.assertIn('https%3A%2F%2Fbugs.chromium.org%2Fp', url)
def testCreateLogoutUrl(self):
_, mr = testing_helpers.GetRequestObjects(
diff --git a/framework/test/ts_mon_js_test.py b/framework/test/ts_mon_js_test.py
index bcd4060..4231b76 100644
--- a/framework/test/ts_mon_js_test.py
+++ b/framework/test/ts_mon_js_test.py
@@ -12,9 +12,11 @@
import unittest
from mock import patch
+import flask
import webapp2
from google.appengine.ext import testbed
+from framework.ts_mon_js import FlaskMonorailTSMonJSHandler
from framework.ts_mon_js import MonorailTSMonJSHandler
from services import service_manager
@@ -71,3 +73,69 @@
self.assertEqual(res.status_int, 201)
self.assertEqual(res.body, 'Ok.')
+
+
+class FlaskMonorailTSMonJSHandlerTest(unittest.TestCase):
+
+ def setUp(self):
+ self.testbed = testbed.Testbed()
+ self.testbed.activate()
+ self.testbed.init_user_stub()
+ self.services = service_manager.Services()
+ self.app = flask.Flask('test_app')
+ self.app.config['TESTING'] = True
+ self.app.add_url_rule(
+ '/_/ts_mon_js.do',
+ view_func=FlaskMonorailTSMonJSHandler(
+ services=self.services).PostMonorailTSMonJSHandler,
+ methods=['POST'])
+
+ def tearDown(self):
+ self.testbed.deactivate()
+
+ @patch('framework.xsrf.ValidateToken')
+ @patch('time.time')
+ def testFlaskSubmitMetrics(self, _mockTime, _mockValidateToken):
+ """Test normal case POSTing metrics."""
+ _mockTime.return_value = 1537821859
+ res = self.app.test_client().post(
+ '/_/ts_mon_js.do',
+ data = json.dumps(
+ {
+ 'metrics':
+ [
+ {
+ 'MetricInfo':
+ {
+ 'Name':
+ 'monorail/frontend/issue_update_latency',
+ 'ValueType':
+ 2,
+ },
+ 'Cells':
+ [
+ {
+ 'value':
+ {
+ 'sum': 1234,
+ 'count': 4321,
+ 'buckets':
+ {
+ 0: 123,
+ 1: 321,
+ 2: 213,
+ },
+ },
+ 'fields':
+ {
+ 'client_id': '789',
+ 'host_name': 'rutabaga',
+ 'document_visible': True,
+ },
+ 'start_time': 1537821799,
+ }
+ ],
+ }
+ ],
+ }))
+ self.assertEqual(res.status_code, 201)
diff --git a/framework/test/warmup_test.py b/framework/test/warmup_test.py
index d8ddb65..8140fc7 100644
--- a/framework/test/warmup_test.py
+++ b/framework/test/warmup_test.py
@@ -24,8 +24,7 @@
#self.services = service_manager.Services(
# cache_manager=self.cache_manager)
self.services = service_manager.Services()
- self.servlet = warmup.Warmup(
- 'req', 'res', services=self.services)
+ self.servlet = warmup.Warmup('req', 'res', services=self.services)
def testHandleRequest_NothingToDo(self):