From 418e89f4232a1de64a300b57068bf725c157ac11 Mon Sep 17 00:00:00 2001 From: Tres Seaver Date: Wed, 11 Nov 2020 15:22:46 -0500 Subject: [PATCH 1/7] chore: avoid importing module-under-test at testcase moclue scope --- tests/unit/test_retry.py | 47 +++++++++++++++++++++++++--------------- 1 file changed, 29 insertions(+), 18 deletions(-) diff --git a/tests/unit/test_retry.py b/tests/unit/test_retry.py index 7c5a6ba1e..9b73539e5 100644 --- a/tests/unit/test_retry.py +++ b/tests/unit/test_retry.py @@ -14,64 +14,75 @@ import unittest -from google.cloud.storage.retry import DEFAULT_RETRY -from google.cloud.storage.retry import DEFAULT_RETRY_IF_GENERATION_SPECIFIED -from google.cloud.storage.retry import DEFAULT_RETRY_IF_METAGENERATION_SPECIFIED -from google.cloud.storage.retry import DEFAULT_RETRY_IF_ETAG_IN_JSON - -class TestConditionalRetryPolicy(unittest.TestCase): +class Test_default_conditional_retry_policies(unittest.TestCase): def test_is_generation_specified_match_metageneration(self): - conditional_policy = DEFAULT_RETRY_IF_GENERATION_SPECIFIED + from google.cloud.storage import retry + + conditional_policy = retry.DEFAULT_RETRY_IF_GENERATION_SPECIFIED policy = conditional_policy.get_retry_policy_if_conditions_met( query_params={"if_generation_match": 1} ) - self.assertEqual(policy, DEFAULT_RETRY) + self.assertEqual(policy, retry.DEFAULT_RETRY) def test_is_generation_specified_match_generation(self): - conditional_policy = DEFAULT_RETRY_IF_GENERATION_SPECIFIED + from google.cloud.storage import retry + + conditional_policy = retry.DEFAULT_RETRY_IF_GENERATION_SPECIFIED policy = conditional_policy.get_retry_policy_if_conditions_met( query_params={"generation": 1} ) - self.assertEqual(policy, DEFAULT_RETRY) + self.assertEqual(policy, retry.DEFAULT_RETRY) def test_is_generation_specified_mismatch(self): - conditional_policy = DEFAULT_RETRY_IF_GENERATION_SPECIFIED + from google.cloud.storage import retry + + conditional_policy = retry.DEFAULT_RETRY_IF_GENERATION_SPECIFIED policy = conditional_policy.get_retry_policy_if_conditions_met( query_params={"if_metageneration_match": 1} ) self.assertEqual(policy, None) def test_is_metageneration_specified_match(self): - conditional_policy = DEFAULT_RETRY_IF_METAGENERATION_SPECIFIED + from google.cloud.storage import retry + + conditional_policy = retry.DEFAULT_RETRY_IF_METAGENERATION_SPECIFIED policy = conditional_policy.get_retry_policy_if_conditions_met( query_params={"if_metageneration_match": 1} ) - self.assertEqual(policy, DEFAULT_RETRY) + self.assertEqual(policy, retry.DEFAULT_RETRY) def test_is_metageneration_specified_mismatch(self): - conditional_policy = DEFAULT_RETRY_IF_METAGENERATION_SPECIFIED + from google.cloud.storage import retry + + conditional_policy = retry.DEFAULT_RETRY_IF_METAGENERATION_SPECIFIED policy = conditional_policy.get_retry_policy_if_conditions_met( query_params={"if_generation_match": 1} ) self.assertEqual(policy, None) def test_is_etag_in_json_etag_match(self): - conditional_policy = DEFAULT_RETRY_IF_ETAG_IN_JSON + from google.cloud.storage import retry + + conditional_policy = retry.DEFAULT_RETRY_IF_ETAG_IN_JSON policy = conditional_policy.get_retry_policy_if_conditions_met( query_params={"if_generation_match": 1}, data='{"etag": "12345678"}' ) - self.assertEqual(policy, DEFAULT_RETRY) + self.assertEqual(policy, retry.DEFAULT_RETRY) def test_is_etag_in_json_mismatch(self): - conditional_policy = DEFAULT_RETRY_IF_ETAG_IN_JSON + from google.cloud.storage import retry + + conditional_policy = retry.DEFAULT_RETRY_IF_ETAG_IN_JSON policy = conditional_policy.get_retry_policy_if_conditions_met( query_params={"if_generation_match": 1}, data="{}" ) self.assertEqual(policy, None) def test_is_meta_or_etag_in_json_invalid(self): - conditional_policy = DEFAULT_RETRY_IF_ETAG_IN_JSON + from google.cloud.storage import retry + + conditional_policy = retry.DEFAULT_RETRY_IF_ETAG_IN_JSON policy = conditional_policy.get_retry_policy_if_conditions_met( query_params={"if_generation_match": 1}, data="I am invalid JSON!" ) From 626d9e0a18bc74ae501d34c645c125a822f6ce5c Mon Sep 17 00:00:00 2001 From: Tres Seaver Date: Wed, 11 Nov 2020 15:38:49 -0500 Subject: [PATCH 2/7] tests: add explicit tests for '_should_retry' --- tests/unit/test_retry.py | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/tests/unit/test_retry.py b/tests/unit/test_retry.py index 9b73539e5..9a96fb1e6 100644 --- a/tests/unit/test_retry.py +++ b/tests/unit/test_retry.py @@ -15,6 +15,38 @@ import unittest +class Test_should_retry(unittest.TestCase): + def _call_fut(self, exc): + from google.cloud.storage import retry + + return retry._should_retry(exc) + + def test_w_retryable_types(self): + from google.cloud.storage import retry + + for exc_type in retry._RETRYABLE_TYPES: + exc = exc_type("testing") + self.assertTrue(self._call_fut(exc)) + + def test_w_google_api_call_error_hit(self): + from google.api_core import exceptions + + exc = exceptions.GoogleAPICallError("testing") + exc.code = 408 + self.assertTrue(self._call_fut(exc)) + + def test_w_google_api_call_error_miss(self): + from google.api_core import exceptions + + exc = exceptions.GoogleAPICallError("testing") + exc.code = 999 + self.assertFalse(self._call_fut(exc)) + + def test_w_requests_connection_error(self): + exc = ValueError("testing") + self.assertFalse(self._call_fut(exc)) + + class Test_default_conditional_retry_policies(unittest.TestCase): def test_is_generation_specified_match_metageneration(self): from google.cloud.storage import retry From fae0d2ffa63a484a5e4215caa01a6d011ae556f9 Mon Sep 17 00:00:00 2001 From: Tres Seaver Date: Wed, 11 Nov 2020 15:49:31 -0500 Subject: [PATCH 3/7] tests: add explicit tests for ConditionalRetryPolicy --- tests/unit/test_retry.py | 48 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 48 insertions(+) diff --git a/tests/unit/test_retry.py b/tests/unit/test_retry.py index 9a96fb1e6..28953905c 100644 --- a/tests/unit/test_retry.py +++ b/tests/unit/test_retry.py @@ -14,6 +14,8 @@ import unittest +import mock + class Test_should_retry(unittest.TestCase): def _call_fut(self, exc): @@ -47,6 +49,52 @@ def test_w_requests_connection_error(self): self.assertFalse(self._call_fut(exc)) +class TestConditionalRetryPolicy(unittest.TestCase): + def _make_one(self, retry_policy, conditional_predicate, required_kwargs): + from google.cloud.storage import retry + + return retry.ConditionalRetryPolicy( + retry_policy, conditional_predicate, required_kwargs + ) + + def test_ctor(self): + retry_policy = mock.Mock() + conditional_predicate = mock.Mock() + required_kwargs = ("kwarg",) + + policy = self._make_one(retry_policy, conditional_predicate, required_kwargs) + + self.assertIs(policy.retry_policy, retry_policy) + self.assertIs(policy.conditional_predicate, conditional_predicate) + self.assertEqual(policy.required_kwargs, required_kwargs) + + def test_get_retry_policy_if_conditions_met_single_kwarg_hit(self): + retry_policy = mock.Mock() + conditional_predicate = mock.Mock(return_value=True) + required_kwargs = ("foo",) + policy = self._make_one(retry_policy, conditional_predicate, required_kwargs) + + kwargs = {"foo": 1, "bar": 2, "baz": 3} + result = policy.get_retry_policy_if_conditions_met(**kwargs) + + self.assertIs(result, retry_policy) + + conditional_predicate.assert_called_once_with(1) + + def test_get_retry_policy_if_conditions_met_multiple_kwargs_miss(self): + retry_policy = mock.Mock() + conditional_predicate = mock.Mock(return_value=False) + required_kwargs = ("foo", "bar") + policy = self._make_one(retry_policy, conditional_predicate, required_kwargs) + + kwargs = {"foo": 1, "bar": 2, "baz": 3} + result = policy.get_retry_policy_if_conditions_met(**kwargs) + + self.assertIsNone(result) + + conditional_predicate.assert_called_once_with(1, 2) + + class Test_default_conditional_retry_policies(unittest.TestCase): def test_is_generation_specified_match_metageneration(self): from google.cloud.storage import retry From d3540db8fafafa6536fb5f5bb5ff3de42f285840 Mon Sep 17 00:00:00 2001 From: Tres Seaver Date: Wed, 11 Nov 2020 15:53:57 -0500 Subject: [PATCH 4/7] tests: add explicit tests for is_generation_specified --- tests/unit/test_retry.py | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/tests/unit/test_retry.py b/tests/unit/test_retry.py index 28953905c..ab7660eb1 100644 --- a/tests/unit/test_retry.py +++ b/tests/unit/test_retry.py @@ -95,6 +95,28 @@ def test_get_retry_policy_if_conditions_met_multiple_kwargs_miss(self): conditional_predicate.assert_called_once_with(1, 2) +class Test_is_generation_specified(unittest.TestCase): + def _call_fut(self, query_params): + from google.cloud.storage import retry + + return retry.is_generation_specified(query_params) + + def test_w_empty(self): + query_params = {} + + self.assertFalse(self._call_fut(query_params)) + + def test_w_generation(self): + query_params = {"generation": 123} + + self.assertTrue(self._call_fut(query_params)) + + def test_wo_generation_w_if_generation_match(self): + query_params = {"if_generation_match": 123} + + self.assertTrue(self._call_fut(query_params)) + + class Test_default_conditional_retry_policies(unittest.TestCase): def test_is_generation_specified_match_metageneration(self): from google.cloud.storage import retry From 0763dd0922d8882952713cc09f958eaf40c71513 Mon Sep 17 00:00:00 2001 From: Tres Seaver Date: Wed, 11 Nov 2020 15:55:58 -0500 Subject: [PATCH 5/7] tests: add explicit tests for is_metageneration_specified --- tests/unit/test_retry.py | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/tests/unit/test_retry.py b/tests/unit/test_retry.py index ab7660eb1..4ee2f4e84 100644 --- a/tests/unit/test_retry.py +++ b/tests/unit/test_retry.py @@ -117,6 +117,23 @@ def test_wo_generation_w_if_generation_match(self): self.assertTrue(self._call_fut(query_params)) +class Test_is_metageneration_specified(unittest.TestCase): + def _call_fut(self, query_params): + from google.cloud.storage import retry + + return retry.is_metageneration_specified(query_params) + + def test_w_empty(self): + query_params = {} + + self.assertFalse(self._call_fut(query_params)) + + def test_w_if_metageneration_match(self): + query_params = {"if_metageneration_match": 123} + + self.assertTrue(self._call_fut(query_params)) + + class Test_default_conditional_retry_policies(unittest.TestCase): def test_is_generation_specified_match_metageneration(self): from google.cloud.storage import retry From a98b5b7a10fe6d7a09288a0b745654488f1ae53e Mon Sep 17 00:00:00 2001 From: Tres Seaver Date: Wed, 11 Nov 2020 16:04:12 -0500 Subject: [PATCH 6/7] tests: add explicit tests for is_etag_in_json --- tests/unit/test_retry.py | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/tests/unit/test_retry.py b/tests/unit/test_retry.py index 4ee2f4e84..06780362e 100644 --- a/tests/unit/test_retry.py +++ b/tests/unit/test_retry.py @@ -134,6 +134,34 @@ def test_w_if_metageneration_match(self): self.assertTrue(self._call_fut(query_params)) +class Test_is_etag_in_json(unittest.TestCase): + def _call_fut(self, data): + from google.cloud.storage import retry + + return retry.is_etag_in_json(data) + + @staticmethod + def _make_json_data(**kw): + import json + + return json.dumps(kw) + + def test_w_empty(self): + data = self._make_json_data() + + self.assertFalse(self._call_fut(data)) + + def test_w_etag_in_data(self): + data = self._make_json_data(etag="123") + + self.assertTrue(self._call_fut(data)) + + def test_w_empty_data(self): + data = "" + + self.assertFalse(self._call_fut(data)) + + class Test_default_conditional_retry_policies(unittest.TestCase): def test_is_generation_specified_match_metageneration(self): from google.cloud.storage import retry From 591d8e6b90727ef999b0ee441fe30e6b11eaf636 Mon Sep 17 00:00:00 2001 From: Tres Seaver Date: Thu, 12 Nov 2020 17:08:02 -0500 Subject: [PATCH 7/7] tests: require 100% unit test coverage --- noxfile.py | 6 ++++-- synth.py | 2 +- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/noxfile.py b/noxfile.py index 7bab302f2..7f50fcf66 100644 --- a/noxfile.py +++ b/noxfile.py @@ -70,7 +70,9 @@ def lint_setup_py(session): def default(session): # Install all test dependencies, then install this package in-place. - session.install("mock", "pytest", "pytest-cov") + session.install( + "mock", "pytest", "pytest-cov", + ) session.install("-e", ".") # Run py.test against the unit tests. @@ -144,7 +146,7 @@ def cover(session): test runs (not system test runs), and then erases coverage data. """ session.install("coverage", "pytest-cov") - session.run("coverage", "report", "--show-missing", "--fail-under=99") + session.run("coverage", "report", "--show-missing", "--fail-under=100") session.run("coverage", "erase") diff --git a/synth.py b/synth.py index 7d616d44d..9774b1a01 100644 --- a/synth.py +++ b/synth.py @@ -25,7 +25,7 @@ # Add templated files # ---------------------------------------------------------------------------- templated_files = common.py_library( - cov_level=99, + cov_level=100, system_test_external_dependencies=[ "google-cloud-iam", "google-cloud-pubsub < 2.0.0",