From 790fe2e898fab6a147d2edc27e8909d03fb8bbfe Mon Sep 17 00:00:00 2001 From: exaby73 Date: Tue, 23 Jul 2024 13:56:55 +0530 Subject: [PATCH 1/7] fix: Off by one assertion for invoker property --- src/firebase_functions/options.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/firebase_functions/options.py b/src/firebase_functions/options.py index 937a088..cf335a0 100644 --- a/src/firebase_functions/options.py +++ b/src/firebase_functions/options.py @@ -321,7 +321,8 @@ def _asdict_with_global_options(self) -> dict: merged_options: dict = {**global_options, **provider_options} if self.labels is not None and _GLOBAL_OPTIONS.labels is not None: - merged_options["labels"] = {**_GLOBAL_OPTIONS.labels, **self.labels} + merged_options["labels"] = { + **_GLOBAL_OPTIONS.labels, **self.labels} if "labels" not in merged_options: merged_options["labels"] = {} preserve_external_changes: bool = merged_options.get( @@ -464,7 +465,7 @@ def _endpoint( _manifest.TaskQueueTrigger( rateLimits=rate_limits, retryConfig=retry_config, - ), + ), } return _manifest.ManifestEndpoint( **_typing.cast(_typing.Dict, kwargs_merged)) @@ -853,7 +854,7 @@ def _endpoint( schedule=self.schedule, timeZone=time_zone, retryConfig=retry_config, - ), + ), } return _manifest.ManifestEndpoint( **_typing.cast(_typing.Dict, kwargs_merged)) @@ -1134,7 +1135,7 @@ def _endpoint( invoker = [invoker] assert len( invoker - ) > 1, "HttpsOptions: Invalid option for invoker - must be a non-empty list." + ) >= 1, "HttpsOptions: Invalid option for invoker - must be a non-empty list." assert "" not in invoker, ( "HttpsOptions: Invalid option for invoker - must be a non-empty string." ) From 2c278a40706765b2701a3fca2be2ad5ae77c7ba5 Mon Sep 17 00:00:00 2001 From: exaby73 Date: Tue, 23 Jul 2024 15:01:43 +0530 Subject: [PATCH 2/7] fix: Formatting --- src/firebase_functions/options.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/firebase_functions/options.py b/src/firebase_functions/options.py index cf335a0..2f7db7d 100644 --- a/src/firebase_functions/options.py +++ b/src/firebase_functions/options.py @@ -321,8 +321,7 @@ def _asdict_with_global_options(self) -> dict: merged_options: dict = {**global_options, **provider_options} if self.labels is not None and _GLOBAL_OPTIONS.labels is not None: - merged_options["labels"] = { - **_GLOBAL_OPTIONS.labels, **self.labels} + merged_options["labels"] = {**_GLOBAL_OPTIONS.labels, **self.labels} if "labels" not in merged_options: merged_options["labels"] = {} preserve_external_changes: bool = merged_options.get( @@ -465,7 +464,7 @@ def _endpoint( _manifest.TaskQueueTrigger( rateLimits=rate_limits, retryConfig=retry_config, - ), + ), } return _manifest.ManifestEndpoint( **_typing.cast(_typing.Dict, kwargs_merged)) @@ -854,7 +853,7 @@ def _endpoint( schedule=self.schedule, timeZone=time_zone, retryConfig=retry_config, - ), + ), } return _manifest.ManifestEndpoint( **_typing.cast(_typing.Dict, kwargs_merged)) From 6e844b916570a0ee500da0d65248b74417f098b7 Mon Sep 17 00:00:00 2001 From: exaby73 Date: Mon, 29 Jul 2024 02:14:58 +0530 Subject: [PATCH 3/7] test: add test --- tests/test_options.py | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/tests/test_options.py b/tests/test_options.py index 4c2913a..4fe152a 100644 --- a/tests/test_options.py +++ b/tests/test_options.py @@ -44,13 +44,14 @@ def test_global_options_merged_with_provider_options(): Testing a global option is used when no provider option is set. """ options.set_global_options(max_instances=66) - pubsub_options = options.PubSubOptions(topic="foo") #pylint: disable=unexpected-keyword-arg + pubsub_options = options.PubSubOptions( + topic="foo") # pylint: disable=unexpected-keyword-arg pubsub_options_dict = pubsub_options._asdict_with_global_options() assert (pubsub_options_dict["topic"] == "foo" - ), "'topic' property missing from dict" + ), "'topic' property missing from dict" assert "options" not in pubsub_options_dict, "'options' key should not exist in dict" assert (pubsub_options_dict["max_instances"] == 66 - ), "provider option did not update using the global option" + ), "provider option did not update using the global option" def test_https_options_removes_cors(): @@ -119,7 +120,8 @@ def test_merge_apis_empty_input(): expected_output = [] merged_apis = merge_required_apis(required_apis) - assert merged_apis == expected_output, f"Expected {expected_output}, but got {merged_apis}" + assert merged_apis == expected_output, f"Expected { + expected_output}, but got {merged_apis}" def test_merge_apis_no_duplicate_apis(): @@ -162,7 +164,8 @@ def test_merge_apis_no_duplicate_apis(): merged_apis = merge_required_apis(required_apis) - assert merged_apis == expected_output, f"Expected {expected_output}, but got {merged_apis}" + assert merged_apis == expected_output, f"Expected { + expected_output}, but got {merged_apis}" def test_merge_apis_duplicate_apis(): @@ -212,8 +215,12 @@ def test_merge_apis_duplicate_apis(): for expected_item in expected_output: assert (expected_item in merged_apis - ), f"Expected item {expected_item} missing from the merged list" + ), f"Expected item {expected_item} missing from the merged list" for actual_item in merged_apis: assert (actual_item in expected_output - ), f"Unexpected item {actual_item} found in the merged list" + ), f"Unexpected item {actual_item} found in the merged list" + + +def test_invoker_with_one_element_doesnt_throw(): + options.HttpsOptions(invoker=["public"])._endpoint(func_name="test") From ee1f3160236dba468caaacd2b80ba4b351c6687c Mon Sep 17 00:00:00 2001 From: exaby73 Date: Thu, 1 Aug 2024 19:07:22 +0530 Subject: [PATCH 4/7] fix: Unterminated format string --- tests/test_options.py | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/tests/test_options.py b/tests/test_options.py index 4fe152a..541280d 100644 --- a/tests/test_options.py +++ b/tests/test_options.py @@ -44,14 +44,13 @@ def test_global_options_merged_with_provider_options(): Testing a global option is used when no provider option is set. """ options.set_global_options(max_instances=66) - pubsub_options = options.PubSubOptions( - topic="foo") # pylint: disable=unexpected-keyword-arg + pubsub_options = options.PubSubOptions(topic="foo") # pylint: disable=unexpected-keyword-arg pubsub_options_dict = pubsub_options._asdict_with_global_options() assert (pubsub_options_dict["topic"] == "foo" - ), "'topic' property missing from dict" + ), "'topic' property missing from dict" assert "options" not in pubsub_options_dict, "'options' key should not exist in dict" assert (pubsub_options_dict["max_instances"] == 66 - ), "provider option did not update using the global option" + ), "provider option did not update using the global option" def test_https_options_removes_cors(): @@ -120,8 +119,7 @@ def test_merge_apis_empty_input(): expected_output = [] merged_apis = merge_required_apis(required_apis) - assert merged_apis == expected_output, f"Expected { - expected_output}, but got {merged_apis}" + assert merged_apis == expected_output, f"Expected {expected_output}, but got {merged_apis}" def test_merge_apis_no_duplicate_apis(): @@ -164,8 +162,7 @@ def test_merge_apis_no_duplicate_apis(): merged_apis = merge_required_apis(required_apis) - assert merged_apis == expected_output, f"Expected { - expected_output}, but got {merged_apis}" + assert merged_apis == expected_output, f"Expected {expected_output}, but got {merged_apis}" def test_merge_apis_duplicate_apis(): @@ -173,7 +170,7 @@ def test_merge_apis_duplicate_apis(): This test evaluates the merge_required_apis function when the input list contains duplicate APIs with different reasons. The desired outcome for this test is a list where the duplicate - APIs are merged properly and reasons are combined. + APIs are merged properly and reasons are combined. This test ensures that the function correctly merges the duplicate APIs and combines the reasons associated with them. """ @@ -215,11 +212,11 @@ def test_merge_apis_duplicate_apis(): for expected_item in expected_output: assert (expected_item in merged_apis - ), f"Expected item {expected_item} missing from the merged list" + ), f"Expected item {expected_item} missing from the merged list" for actual_item in merged_apis: assert (actual_item in expected_output - ), f"Unexpected item {actual_item} found in the merged list" + ), f"Unexpected item {actual_item} found in the merged list" def test_invoker_with_one_element_doesnt_throw(): From b666dbdf421c7b26b3f3e81116b7de4385672c51 Mon Sep 17 00:00:00 2001 From: exaby73 Date: Thu, 1 Aug 2024 19:32:34 +0530 Subject: [PATCH 5/7] test: Add test for checking if invoker with no elements throws --- tests/test_options.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/tests/test_options.py b/tests/test_options.py index 541280d..a5f5828 100644 --- a/tests/test_options.py +++ b/tests/test_options.py @@ -221,3 +221,10 @@ def test_merge_apis_duplicate_apis(): def test_invoker_with_one_element_doesnt_throw(): options.HttpsOptions(invoker=["public"])._endpoint(func_name="test") + + +def test_invoker_with_no_element_throws(): + try: + options.HttpsOptions(invoker=[])._endpoint(func_name="test") + except AssertionError: + return From c60e8890928a3cc607e1bcc71f1914a3b0ef889c Mon Sep 17 00:00:00 2001 From: exaby73 Date: Thu, 1 Aug 2024 19:44:05 +0530 Subject: [PATCH 6/7] test: Improve test for checking if invoker with no elements throws --- tests/test_options.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/tests/test_options.py b/tests/test_options.py index a5f5828..fa6fbcc 100644 --- a/tests/test_options.py +++ b/tests/test_options.py @@ -17,6 +17,7 @@ from firebase_functions import options, https_fn from firebase_functions import params from firebase_functions.private.serving import functions_as_yaml, merge_required_apis +from pytest import raises # pylint: disable=protected-access @@ -224,7 +225,5 @@ def test_invoker_with_one_element_doesnt_throw(): def test_invoker_with_no_element_throws(): - try: + with raises(AssertionError, match="HttpsOptions: Invalid option for invoker - must be a non-empty list."): options.HttpsOptions(invoker=[])._endpoint(func_name="test") - except AssertionError: - return From 12d3282f1777bcf7348a014fd06b1a66486e28cb Mon Sep 17 00:00:00 2001 From: exaby73 Date: Sun, 4 Aug 2024 12:11:48 +0530 Subject: [PATCH 7/7] chore: Linting --- tests/test_options.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/tests/test_options.py b/tests/test_options.py index fa6fbcc..baaf64a 100644 --- a/tests/test_options.py +++ b/tests/test_options.py @@ -225,5 +225,9 @@ def test_invoker_with_one_element_doesnt_throw(): def test_invoker_with_no_element_throws(): - with raises(AssertionError, match="HttpsOptions: Invalid option for invoker - must be a non-empty list."): + with raises( + AssertionError, + match= + "HttpsOptions: Invalid option for invoker - must be a non-empty list." + ): options.HttpsOptions(invoker=[])._endpoint(func_name="test")