Skip to content

Commit e1c57d1

Browse files
authoredAug 4, 2024··
fix: Off by one assertion for invoker property (#216)
1 parent db1eb6c commit e1c57d1

File tree

2 files changed

+17
-3
lines changed

2 files changed

+17
-3
lines changed
 

Diff for: ‎src/firebase_functions/options.py

+1-1
Original file line numberDiff line numberDiff line change
@@ -1134,7 +1134,7 @@ def _endpoint(
11341134
invoker = [invoker]
11351135
assert len(
11361136
invoker
1137-
) > 1, "HttpsOptions: Invalid option for invoker - must be a non-empty list."
1137+
) >= 1, "HttpsOptions: Invalid option for invoker - must be a non-empty list."
11381138
assert "" not in invoker, (
11391139
"HttpsOptions: Invalid option for invoker - must be a non-empty string."
11401140
)

Diff for: ‎tests/test_options.py

+16-2
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
from firebase_functions import options, https_fn
1818
from firebase_functions import params
1919
from firebase_functions.private.serving import functions_as_yaml, merge_required_apis
20+
from pytest import raises
2021
# pylint: disable=protected-access
2122

2223

@@ -44,7 +45,7 @@ def test_global_options_merged_with_provider_options():
4445
Testing a global option is used when no provider option is set.
4546
"""
4647
options.set_global_options(max_instances=66)
47-
pubsub_options = options.PubSubOptions(topic="foo") #pylint: disable=unexpected-keyword-arg
48+
pubsub_options = options.PubSubOptions(topic="foo") # pylint: disable=unexpected-keyword-arg
4849
pubsub_options_dict = pubsub_options._asdict_with_global_options()
4950
assert (pubsub_options_dict["topic"] == "foo"
5051
), "'topic' property missing from dict"
@@ -170,7 +171,7 @@ def test_merge_apis_duplicate_apis():
170171
This test evaluates the merge_required_apis function when the
171172
input list contains duplicate APIs with different reasons.
172173
The desired outcome for this test is a list where the duplicate
173-
APIs are merged properly and reasons are combined.
174+
APIs are merged properly and reasons are combined.
174175
This test ensures that the function correctly merges the duplicate
175176
APIs and combines the reasons associated with them.
176177
"""
@@ -217,3 +218,16 @@ def test_merge_apis_duplicate_apis():
217218
for actual_item in merged_apis:
218219
assert (actual_item in expected_output
219220
), f"Unexpected item {actual_item} found in the merged list"
221+
222+
223+
def test_invoker_with_one_element_doesnt_throw():
224+
options.HttpsOptions(invoker=["public"])._endpoint(func_name="test")
225+
226+
227+
def test_invoker_with_no_element_throws():
228+
with raises(
229+
AssertionError,
230+
match=
231+
"HttpsOptions: Invalid option for invoker - must be a non-empty list."
232+
):
233+
options.HttpsOptions(invoker=[])._endpoint(func_name="test")

1 commit comments

Comments
 (1)

sjudd commented on Sep 19, 2024

@sjudd

I'd love to see this released! :)

Please sign in to comment.