From 2bf6d9fbf81fe64e1f646012303d60f8d5184312 Mon Sep 17 00:00:00 2001 From: Tres Seaver Date: Mon, 11 Nov 2019 14:48:24 -0500 Subject: [PATCH 1/5] refactor: normalize testcase names for 'create_bucket' tests --- storage/tests/unit/test_client.py | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/storage/tests/unit/test_client.py b/storage/tests/unit/test_client.py index 5141ba6e83d0..81581bcf7597 100644 --- a/storage/tests/unit/test_client.py +++ b/storage/tests/unit/test_client.py @@ -517,7 +517,7 @@ def test_lookup_bucket_hit(self): method="GET", url=URI, data=mock.ANY, headers=mock.ANY ) - def test_create_bucket_with_string_conflict(self): + def test_create_bucket_w_string_conflict(self): from google.cloud.exceptions import Conflict project = "PROJECT" @@ -553,7 +553,7 @@ def test_create_bucket_with_string_conflict(self): json_sent = http.request.call_args_list[0][1]["data"] self.assertEqual(json_expected, json.loads(json_sent)) - def test_create_bucket_with_object_conflict(self): + def test_create_bucket_w_object_conflict(self): from google.cloud.exceptions import Conflict from google.cloud.storage.bucket import Bucket @@ -588,13 +588,13 @@ def test_create_bucket_with_object_conflict(self): json_sent = http.request.call_args_list[0][1]["data"] self.assertEqual(json_expected, json.loads(json_sent)) - def test_create_w_missing_client_project(self): + def test_create_bucket_w_missing_client_project(self): client = self._make_one(project=None) with self.assertRaises(ValueError): client.create_bucket("bucket") - def test_create_w_predefined_acl_invalid(self): + def test_create_bucket_w_predefined_acl_invalid(self): PROJECT = "PROJECT" BUCKET_NAME = "bucket-name" credentials = _make_credentials() @@ -603,7 +603,7 @@ def test_create_w_predefined_acl_invalid(self): with self.assertRaises(ValueError): client.create_bucket(BUCKET_NAME, predefined_acl="bogus") - def test_create_w_predefined_acl_valid(self): + def test_create_bucket_w_predefined_acl_valid(self): from google.cloud.storage.client import Client PROJECT = "PROJECT" @@ -623,7 +623,7 @@ def test_create_w_predefined_acl_valid(self): _target_object=bucket, ) - def test_create_w_predefined_default_object_acl_invalid(self): + def test_create_bucket_w_predefined_default_object_acl_invalid(self): PROJECT = "PROJECT" BUCKET_NAME = "bucket-name" @@ -633,7 +633,7 @@ def test_create_w_predefined_default_object_acl_invalid(self): with self.assertRaises(ValueError): client.create_bucket(BUCKET_NAME, predefined_default_object_acl="bogus") - def test_create_w_predefined_default_object_acl_valid(self): + def test_create_bucket_w_predefined_default_object_acl_valid(self): from google.cloud.storage.client import Client PROJECT = "PROJECT" @@ -658,7 +658,7 @@ def test_create_w_predefined_default_object_acl_valid(self): _target_object=bucket, ) - def test_create_w_explicit_location(self): + def test_create_bucket_w_explicit_location(self): from google.cloud.storage.client import Client PROJECT = "PROJECT" @@ -684,7 +684,7 @@ def test_create_w_explicit_location(self): ) self.assertEqual(bucket.location, LOCATION) - def test_create_bucket_with_string_success(self): + def test_create_bucket_w_string_success(self): from google.cloud.storage.bucket import Bucket project = "PROJECT" @@ -716,7 +716,7 @@ def test_create_bucket_with_string_success(self): json_sent = http.request.call_args_list[0][1]["data"] self.assertEqual(json_expected, json.loads(json_sent)) - def test_create_bucket_with_object_success(self): + def test_create_bucket_w_object_success(self): from google.cloud.storage.bucket import Bucket project = "PROJECT" From 524ee1158f02d80204db25ff255da8bf007e0dbe Mon Sep 17 00:00:00 2001 From: Tres Seaver Date: Mon, 11 Nov 2019 14:54:25 -0500 Subject: [PATCH 2/5] refactor: normalize variable names, client creation for 'create_bucket' tests --- storage/tests/unit/test_client.py | 80 +++++++++++++++---------------- 1 file changed, 39 insertions(+), 41 deletions(-) diff --git a/storage/tests/unit/test_client.py b/storage/tests/unit/test_client.py index 81581bcf7597..135638fd587e 100644 --- a/storage/tests/unit/test_client.py +++ b/storage/tests/unit/test_client.py @@ -589,100 +589,98 @@ def test_create_bucket_w_object_conflict(self): self.assertEqual(json_expected, json.loads(json_sent)) def test_create_bucket_w_missing_client_project(self): - client = self._make_one(project=None) + credentials = _make_credentials() + client = self._make_one(project=None, credentials=credentials) with self.assertRaises(ValueError): client.create_bucket("bucket") def test_create_bucket_w_predefined_acl_invalid(self): - PROJECT = "PROJECT" - BUCKET_NAME = "bucket-name" + project = "PROJECT" + bucket_name = "bucket-name" credentials = _make_credentials() - client = self._make_one(project=PROJECT, credentials=credentials) + client = self._make_one(project=project, credentials=credentials) with self.assertRaises(ValueError): - client.create_bucket(BUCKET_NAME, predefined_acl="bogus") + client.create_bucket(bucket_name, predefined_acl="bogus") def test_create_bucket_w_predefined_acl_valid(self): - from google.cloud.storage.client import Client - - PROJECT = "PROJECT" - BUCKET_NAME = "bucket-name" - DATA = {"name": BUCKET_NAME} + project = "PROJECT" + bucket_name = "bucket-name" + data = {"name": bucket_name} - client = Client(project=PROJECT) - connection = _make_connection(DATA) + credentials = _make_credentials() + client = self._make_one(project=project, credentials=credentials) + connection = _make_connection(data) client._base_connection = connection - bucket = client.create_bucket(BUCKET_NAME, predefined_acl="publicRead") + bucket = client.create_bucket(bucket_name, predefined_acl="publicRead") connection.api_request.assert_called_once_with( method="POST", path="/b", - query_params={"project": PROJECT, "predefinedAcl": "publicRead"}, - data=DATA, + query_params={"project": project, "predefinedAcl": "publicRead"}, + data=data, _target_object=bucket, ) def test_create_bucket_w_predefined_default_object_acl_invalid(self): - PROJECT = "PROJECT" - BUCKET_NAME = "bucket-name" + project = "PROJECT" + bucket_name = "bucket-name" credentials = _make_credentials() - client = self._make_one(project=PROJECT, credentials=credentials) + client = self._make_one(project=project, credentials=credentials) with self.assertRaises(ValueError): - client.create_bucket(BUCKET_NAME, predefined_default_object_acl="bogus") + client.create_bucket(bucket_name, predefined_default_object_acl="bogus") def test_create_bucket_w_predefined_default_object_acl_valid(self): - from google.cloud.storage.client import Client - - PROJECT = "PROJECT" - BUCKET_NAME = "bucket-name" - DATA = {"name": BUCKET_NAME} + project = "PROJECT" + bucket_name = "bucket-name" + data = {"name": bucket_name} - client = Client(project=PROJECT) - connection = _make_connection(DATA) + credentials = _make_credentials() + client = self._make_one(project=project, credentials=credentials) + connection = _make_connection(data) client._base_connection = connection bucket = client.create_bucket( - BUCKET_NAME, predefined_default_object_acl="publicRead" + bucket_name, predefined_default_object_acl="publicRead" ) connection.api_request.assert_called_once_with( method="POST", path="/b", query_params={ - "project": PROJECT, + "project": project, "predefinedDefaultObjectAcl": "publicRead", }, - data=DATA, + data=data, _target_object=bucket, ) def test_create_bucket_w_explicit_location(self): - from google.cloud.storage.client import Client - - PROJECT = "PROJECT" - BUCKET_NAME = "bucket-name" - LOCATION = "us-central1" - DATA = {"location": LOCATION, "name": BUCKET_NAME} + project = "PROJECT" + bucket_name = "bucket-name" + location = "us-central1" + data = {"location": location, "name": bucket_name} connection = _make_connection( - DATA, "{'location': 'us-central1', 'name': 'bucket-name'}" + data, "{'location': 'us-central1', 'name': 'bucket-name'}" ) - client = Client(project=PROJECT) + credentials = _make_credentials() + client = self._make_one(project=project, credentials=credentials) client._base_connection = connection - bucket = client.create_bucket(BUCKET_NAME, location=LOCATION) + bucket = client.create_bucket(bucket_name, location=location) connection.api_request.assert_called_once_with( method="POST", path="/b", - data=DATA, + data=data, _target_object=bucket, - query_params={"project": "PROJECT"}, + query_params={"project": project}, ) - self.assertEqual(bucket.location, LOCATION) + self.assertEqual(bucket.location, location) def test_create_bucket_w_string_success(self): from google.cloud.storage.bucket import Bucket From 06327bfe929c870b9f8e98411d2fa91733031c4e Mon Sep 17 00:00:00 2001 From: Tres Seaver Date: Mon, 11 Nov 2019 14:56:43 -0500 Subject: [PATCH 3/5] refactor: remove now-redundant unit test --- storage/tests/unit/test_client.py | 35 ------------------------------- 1 file changed, 35 deletions(-) diff --git a/storage/tests/unit/test_client.py b/storage/tests/unit/test_client.py index 135638fd587e..9d9c9d585004 100644 --- a/storage/tests/unit/test_client.py +++ b/storage/tests/unit/test_client.py @@ -553,41 +553,6 @@ def test_create_bucket_w_string_conflict(self): json_sent = http.request.call_args_list[0][1]["data"] self.assertEqual(json_expected, json.loads(json_sent)) - def test_create_bucket_w_object_conflict(self): - from google.cloud.exceptions import Conflict - from google.cloud.storage.bucket import Bucket - - project = "PROJECT" - other_project = "OTHER_PROJECT" - credentials = _make_credentials() - client = self._make_one(project=project, credentials=credentials) - - bucket_name = "bucket-name" - bucket_obj = Bucket(client, bucket_name) - URI = "/".join( - [ - client._connection.API_BASE_URL, - "storage", - client._connection.API_VERSION, - "b?project=%s" % (other_project,), - ] - ) - data = {"error": {"message": "Conflict"}} - http = _make_requests_session( - [_make_json_response(data, status=http_client.CONFLICT)] - ) - client._http_internal = http - - with self.assertRaises(Conflict): - client.create_bucket(bucket_obj, project=other_project) - - http.request.assert_called_once_with( - method="POST", url=URI, data=mock.ANY, headers=mock.ANY - ) - json_expected = {"name": bucket_name} - json_sent = http.request.call_args_list[0][1]["data"] - self.assertEqual(json_expected, json.loads(json_sent)) - def test_create_bucket_w_missing_client_project(self): credentials = _make_credentials() client = self._make_one(project=None, credentials=credentials) From 0f526c9f9893b42a68266f781f40d74079c54f66 Mon Sep 17 00:00:00 2001 From: Tres Seaver Date: Mon, 11 Nov 2019 14:59:54 -0500 Subject: [PATCH 4/5] refactor: reorder 'create_bucket' unit tests to match FUT --- storage/tests/unit/test_client.py | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/storage/tests/unit/test_client.py b/storage/tests/unit/test_client.py index 9d9c9d585004..5f13f746cb7b 100644 --- a/storage/tests/unit/test_client.py +++ b/storage/tests/unit/test_client.py @@ -517,7 +517,14 @@ def test_lookup_bucket_hit(self): method="GET", url=URI, data=mock.ANY, headers=mock.ANY ) - def test_create_bucket_w_string_conflict(self): + def test_create_bucket_w_missing_client_project(self): + credentials = _make_credentials() + client = self._make_one(project=None, credentials=credentials) + + with self.assertRaises(ValueError): + client.create_bucket("bucket") + + def test_create_bucket_w_conflict(self): from google.cloud.exceptions import Conflict project = "PROJECT" @@ -553,13 +560,6 @@ def test_create_bucket_w_string_conflict(self): json_sent = http.request.call_args_list[0][1]["data"] self.assertEqual(json_expected, json.loads(json_sent)) - def test_create_bucket_w_missing_client_project(self): - credentials = _make_credentials() - client = self._make_one(project=None, credentials=credentials) - - with self.assertRaises(ValueError): - client.create_bucket("bucket") - def test_create_bucket_w_predefined_acl_invalid(self): project = "PROJECT" bucket_name = "bucket-name" From e17d0358fc7462da0953e987a9c263b6f4753caf Mon Sep 17 00:00:00 2001 From: Tres Seaver Date: Mon, 11 Nov 2019 15:05:11 -0500 Subject: [PATCH 5/5] fix: avoid depending on qs order in 'create_bucket' unit test. --- storage/tests/unit/test_client.py | 28 ++++++++++------------------ 1 file changed, 10 insertions(+), 18 deletions(-) diff --git a/storage/tests/unit/test_client.py b/storage/tests/unit/test_client.py index 5f13f746cb7b..36bf3d97cb65 100644 --- a/storage/tests/unit/test_client.py +++ b/storage/tests/unit/test_client.py @@ -532,33 +532,25 @@ def test_create_bucket_w_conflict(self): other_project = "OTHER_PROJECT" credentials = _make_credentials() client = self._make_one(project=project, credentials=credentials) + connection = _make_connection() + client._base_connection = connection + connection.api_request.side_effect = Conflict("testing") bucket_name = "bucket-name" - URI = "/".join( - [ - client._connection.API_BASE_URL, - "storage", - client._connection.API_VERSION, - "b?project=%s&userProject=%s" % (other_project, user_project), - ] - ) - data = {"error": {"message": "Conflict"}} - json_expected = {"name": bucket_name} - http = _make_requests_session( - [_make_json_response(data, status=http_client.CONFLICT)] - ) - client._http_internal = http + data = {"name": bucket_name} with self.assertRaises(Conflict): client.create_bucket( bucket_name, project=other_project, user_project=user_project ) - http.request.assert_called_once_with( - method="POST", url=URI, data=mock.ANY, headers=mock.ANY + connection.api_request.assert_called_once_with( + method="POST", + path="/b", + query_params={"project": other_project, "userProject": user_project}, + data=data, + _target_object=mock.ANY, ) - json_sent = http.request.call_args_list[0][1]["data"] - self.assertEqual(json_expected, json.loads(json_sent)) def test_create_bucket_w_predefined_acl_invalid(self): project = "PROJECT"