From 0c5b98541a09b4733bc1cd3b03e8a01c80ab318c Mon Sep 17 00:00:00 2001 From: acocuzzo Date: Sun, 2 Apr 2023 20:10:52 -0400 Subject: [PATCH 01/11] CI: Add typed_flaky to streaming_pull system tests --- tests/system.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/tests/system.py b/tests/system.py index d7f7c5bea..335b40cd5 100644 --- a/tests/system.py +++ b/tests/system.py @@ -23,6 +23,7 @@ import sys import threading import time +from typing import Any, Callable, cast, TypeVar # special case python < 3.8 if sys.version_info.major == 3 and sys.version_info.minor < 8: @@ -30,6 +31,7 @@ else: from unittest import mock +from flaky import flaky import pytest import google.auth @@ -43,6 +45,9 @@ from test_utils.system import unique_resource_id +C = TypeVar("C", bound=Callable[..., Any]) +typed_flaky = cast(Callable[[C], C], flaky(max_runs=3, min_passes=1)) + @pytest.fixture(scope="module") def project(): @@ -512,6 +517,7 @@ class CallbackError(Exception): with pytest.raises(CallbackError): future.result(timeout=30) + @typed_flaky def test_streaming_pull_ack_deadline( self, publisher, subscriber, project, topic_path, subscription_path, cleanup ): @@ -563,6 +569,7 @@ def test_streaming_pull_ack_deadline( finally: subscription_future.cancel() + @typed_flaky def test_streaming_pull_max_messages( self, publisher, topic_path, subscriber, subscription_path, cleanup ): @@ -619,6 +626,7 @@ def test_streaming_pull_max_messages( finally: subscription_future.cancel() # trigger clean shutdown + @typed_flaky def test_streaming_pull_blocking_shutdown( self, publisher, topic_path, subscriber, subscription_path, cleanup ): From 84c9a43950c3b84e72344bce4eb3b0956dd05011 Mon Sep 17 00:00:00 2001 From: acocuzzo Date: Sun, 2 Apr 2023 20:16:25 -0400 Subject: [PATCH 02/11] Revert "CI: Add typed_flaky to streaming_pull system tests" This reverts commit 0c5b98541a09b4733bc1cd3b03e8a01c80ab318c. --- tests/system.py | 8 -------- 1 file changed, 8 deletions(-) diff --git a/tests/system.py b/tests/system.py index 335b40cd5..d7f7c5bea 100644 --- a/tests/system.py +++ b/tests/system.py @@ -23,7 +23,6 @@ import sys import threading import time -from typing import Any, Callable, cast, TypeVar # special case python < 3.8 if sys.version_info.major == 3 and sys.version_info.minor < 8: @@ -31,7 +30,6 @@ else: from unittest import mock -from flaky import flaky import pytest import google.auth @@ -45,9 +43,6 @@ from test_utils.system import unique_resource_id -C = TypeVar("C", bound=Callable[..., Any]) -typed_flaky = cast(Callable[[C], C], flaky(max_runs=3, min_passes=1)) - @pytest.fixture(scope="module") def project(): @@ -517,7 +512,6 @@ class CallbackError(Exception): with pytest.raises(CallbackError): future.result(timeout=30) - @typed_flaky def test_streaming_pull_ack_deadline( self, publisher, subscriber, project, topic_path, subscription_path, cleanup ): @@ -569,7 +563,6 @@ def test_streaming_pull_ack_deadline( finally: subscription_future.cancel() - @typed_flaky def test_streaming_pull_max_messages( self, publisher, topic_path, subscriber, subscription_path, cleanup ): @@ -626,7 +619,6 @@ def test_streaming_pull_max_messages( finally: subscription_future.cancel() # trigger clean shutdown - @typed_flaky def test_streaming_pull_blocking_shutdown( self, publisher, topic_path, subscriber, subscription_path, cleanup ): From 8929f28934d81d24187b2af9b3e96a3c9a5df3e6 Mon Sep 17 00:00:00 2001 From: acocuzzo Date: Sun, 2 Apr 2023 21:34:43 -0400 Subject: [PATCH 03/11] separate shared resources --- tests/system.py | 112 ++++++++++++++++++++++++++++++++++++++---------- 1 file changed, 89 insertions(+), 23 deletions(-) diff --git a/tests/system.py b/tests/system.py index d7f7c5bea..af65542b6 100644 --- a/tests/system.py +++ b/tests/system.py @@ -61,13 +61,13 @@ def subscriber(request): @pytest.fixture -def topic_path(project, publisher): +def topic_path_base(project, publisher): topic_name = "t" + unique_resource_id("-") yield publisher.topic_path(project, topic_name) @pytest.fixture -def subscription_path(project, subscriber): +def subscription_path_base(project, subscriber): sub_name = "s" + unique_resource_id("-") yield subscriber.subscription_path(project, sub_name) @@ -82,7 +82,9 @@ def cleanup(): to_call(*args, **kwargs) -def test_publish_messages(publisher, topic_path, cleanup): +def test_publish_messages(publisher, topic_path_base, cleanup): + # Customize topic path to test. + topic_path = topic_path_base + "-publish-messages" # Make sure the topic gets deleted. cleanup.append((publisher.delete_topic, (), {"topic": topic_path})) @@ -100,7 +102,9 @@ def test_publish_messages(publisher, topic_path, cleanup): assert isinstance(result, str) -def test_publish_large_messages(publisher, topic_path, cleanup): +def test_publish_large_messages(publisher, topic_path_base, cleanup): + # Customize topic path to test. + topic_path = topic_path_base + "-publish-large-messages" # Make sure the topic gets deleted. cleanup.append((publisher.delete_topic, (), {"topic": topic_path})) @@ -130,8 +134,11 @@ def test_publish_large_messages(publisher, topic_path, cleanup): def test_subscribe_to_messages( - publisher, topic_path, subscriber, subscription_path, cleanup + publisher, topic_path_base, subscriber, subscription_path_base, cleanup ): + # Customize topic path to test. + topic_path = topic_path_base + "-subscribe-to-messages" + subscription_path = subscription_path_base + "-subscribe-to-messages" # Make sure the topic and subscription get deleted. cleanup.append((publisher.delete_topic, (), {"topic": topic_path})) cleanup.append( @@ -175,8 +182,12 @@ def test_subscribe_to_messages( def test_subscribe_to_messages_async_callbacks( - publisher, topic_path, subscriber, subscription_path, cleanup + publisher, topic_path_base, subscriber, subscription_path_base, cleanup ): + # Customize topic path to test. + custom_str = "-subscribe-to-messages-async-callback" + topic_path = topic_path_base + custom_str + subscription_path = subscription_path_base + custom_str # Make sure the topic and subscription get deleted. cleanup.append((publisher.delete_topic, (), {"topic": topic_path})) cleanup.append( @@ -227,8 +238,12 @@ def test_subscribe_to_messages_async_callbacks( def test_creating_subscriptions_with_non_default_settings( - publisher, subscriber, project, topic_path, subscription_path, cleanup + publisher, subscriber, project, topic_path_base, subscription_path_base, cleanup ): + # Customize topic path to test. + custom_str = "-creating-subscriptions-with-non-default-settings" + topic_path = topic_path_base + custom_str + subscription_path = subscription_path_base + custom_str # Make sure the topic and subscription get deleted. cleanup.append((publisher.delete_topic, (), {"topic": topic_path})) cleanup.append( @@ -346,7 +361,8 @@ def test_listing_topic_subscriptions(publisher, subscriber, project, cleanup): assert subscriptions == {subscription_paths[0], subscription_paths[2]} -def test_managing_topic_iam_policy(publisher, topic_path, cleanup): +def test_managing_topic_iam_policy(publisher, topic_path_base, cleanup): + topic_path = topic_path_base + "-managing-topic-iam-policy" cleanup.append((publisher.delete_topic, (), {"topic": topic_path})) # create a topic and customize its policy @@ -375,8 +391,11 @@ def test_managing_topic_iam_policy(publisher, topic_path, cleanup): def test_managing_subscription_iam_policy( - publisher, subscriber, topic_path, subscription_path, cleanup + publisher, subscriber, topic_path_base, subscription_path_base, cleanup ): + custom_str = "-managing-subscription-iam-policy" + topic_path = topic_path_base + custom_str + subscription_path = subscription_path_base + custom_str # Make sure the topic and subscription get deleted. cleanup.append((publisher.delete_topic, (), {"topic": topic_path})) cleanup.append( @@ -410,7 +429,7 @@ def test_managing_subscription_iam_policy( def test_subscriber_not_leaking_open_sockets( - publisher, topic_path, subscription_path, cleanup + publisher, topic_path_base, subscription_path_base, cleanup ): # Make sure the topic and the supscription get deleted. # NOTE: Since subscriber client will be closed in the test, we should not @@ -419,8 +438,12 @@ def test_subscriber_not_leaking_open_sockets( # Also, since the client will get closed, we need another subscriber client # to clean up the subscription. We also need to make sure that auxiliary # subscriber releases the sockets, too. + custom_str = "-not-leaking-open-sockets" + subscription_path = subscription_path_base + custom_str + topic_path = topic_path_base + custom_str subscriber = pubsub_v1.SubscriberClient(transport="grpc") subscriber_2 = pubsub_v1.SubscriberClient(transport="grpc") + cleanup.append( (subscriber_2.delete_subscription, (), {"subscription": subscription_path}) ) @@ -460,8 +483,11 @@ def test_subscriber_not_leaking_open_sockets( def test_synchronous_pull_no_deadline_error_if_no_messages( - publisher, topic_path, subscriber, subscription_path, cleanup + publisher, topic_path_base, subscriber, subscription_path_base, cleanup ): + custom_str = "-synchronous-pull-deadline-error-if-no-messages" + topic_path = topic_path_base + custom_str + subscription_path = subscription_path_base + custom_str # Make sure the topic and subscription get deleted. cleanup.append((publisher.delete_topic, (), {"topic": topic_path})) cleanup.append( @@ -485,8 +511,11 @@ def test_synchronous_pull_no_deadline_error_if_no_messages( class TestStreamingPull(object): def test_streaming_pull_callback_error_propagation( - self, publisher, topic_path, subscriber, subscription_path, cleanup + self, publisher, topic_path_base, subscriber, subscription_path_base, cleanup ): + custom_str = "-streaming-pull-callback-error-propagation" + topic_path = topic_path_base + custom_str + subscription_path = subscription_path_base + custom_str # Make sure the topic and subscription get deleted. cleanup.append((publisher.delete_topic, (), {"topic": topic_path})) cleanup.append( @@ -513,8 +542,17 @@ class CallbackError(Exception): future.result(timeout=30) def test_streaming_pull_ack_deadline( - self, publisher, subscriber, project, topic_path, subscription_path, cleanup + self, + publisher, + subscriber, + project, + topic_path_base, + subscription_path_base, + cleanup, ): + custom_str = "-streaming-pull-ack-deadline" + topic_path = topic_path_base + custom_str + subscription_path = subscription_path_base + custom_str # Make sure the topic and subscription get deleted. cleanup.append((publisher.delete_topic, (), {"topic": topic_path})) cleanup.append( @@ -564,8 +602,11 @@ def test_streaming_pull_ack_deadline( subscription_future.cancel() def test_streaming_pull_max_messages( - self, publisher, topic_path, subscriber, subscription_path, cleanup + self, publisher, topic_path_base, subscriber, subscription_path_base, cleanup ): + custom_str = "-streaming-pull-max-messages" + topic_path = topic_path_base + custom_str + subscription_path = subscription_path_base + custom_str # Make sure the topic and subscription get deleted. cleanup.append((publisher.delete_topic, (), {"topic": topic_path})) cleanup.append( @@ -620,8 +661,11 @@ def test_streaming_pull_max_messages( subscription_future.cancel() # trigger clean shutdown def test_streaming_pull_blocking_shutdown( - self, publisher, topic_path, subscriber, subscription_path, cleanup + self, publisher, topic_path_base, subscriber, subscription_path_base, cleanup ): + custom_str = "-streaming-pull-blocking-shutdown" + topic_path = topic_path_base + custom_str + subscription_path = subscription_path_base + custom_str # Make sure the topic and subscription get deleted. cleanup.append((publisher.delete_topic, (), {"topic": topic_path})) cleanup.append( @@ -702,9 +746,11 @@ def callback2(message): ) class TestBasicRBAC(object): def test_streaming_pull_subscriber_permissions_sufficient( - self, publisher, topic_path, subscriber, subscription_path, cleanup + self, publisher, topic_path_base, subscriber, subscription_path_base, cleanup ): - + custom_str = "-streaming-pull-subscriber-permissions-sufficient" + topic_path = topic_path_base + custom_str + subscription_path = subscription_path_base + custom_str # Make sure the topic and subscription get deleted. cleanup.append((publisher.delete_topic, (), {"topic": topic_path})) cleanup.append( @@ -739,9 +785,11 @@ def test_streaming_pull_subscriber_permissions_sufficient( future.cancel() def test_publisher_role_can_publish_messages( - self, publisher, topic_path, subscriber, subscription_path, cleanup + self, publisher, topic_path_base, subscriber, subscription_path_base, cleanup ): - + custom_str = "-publisher-role-can-publish-messages" + topic_path = topic_path_base + custom_str + subscription_path = subscription_path_base + custom_str # Make sure the topic and subscription get deleted. cleanup.append((publisher.delete_topic, (), {"topic": topic_path})) cleanup.append( @@ -767,8 +815,17 @@ def test_publisher_role_can_publish_messages( "Snapshot creation is not instant on the backend, causing test falkiness." ) def test_snapshot_seek_subscriber_permissions_sufficient( - self, project, publisher, topic_path, subscriber, subscription_path, cleanup + self, + project, + publisher, + topic_path_base, + subscriber, + subscription_path_base, + cleanup, ): + custom_str = "-snapshot-seek-subscriber-permissions-sufficient" + topic_path = topic_path_base + custom_str + subscription_path = subscription_path_base + custom_str snapshot_name = "snap" + unique_resource_id("-") snapshot_path = "projects/{}/snapshots/{}".format(project, snapshot_name) @@ -813,10 +870,10 @@ def test_snapshot_seek_subscriber_permissions_sufficient( assert len(response.received_messages) == 1 def test_viewer_role_can_list_resources( - self, project, publisher, topic_path, subscriber, cleanup + self, project, publisher, topic_path_base, subscriber, cleanup ): project_path = "projects/" + project - + topic_path = topic_path_base + "-viewer-role-can-list-resources" # Make sure the created topic gets deleted. cleanup.append((publisher.delete_topic, (), {"topic": topic_path})) @@ -844,8 +901,17 @@ def test_viewer_role_can_list_resources( next(iter(viewer_only_subscriber.list_snapshots(project=project_path)), None) def test_editor_role_can_create_resources( - self, project, publisher, topic_path, subscriber, subscription_path, cleanup + self, + project, + publisher, + topic_path_base, + subscriber, + subscription_path_base, + cleanup, ): + custom_str = "-editor-role-can-create-resources" + topic_path = topic_path_base + custom_str + subscription_path = subscription_path_base + custom_str snapshot_name = "snap" + unique_resource_id("-") snapshot_path = "projects/{}/snapshots/{}".format(project, snapshot_name) From 4868f4af163710e98f655b96b57bf62b2cd393ca Mon Sep 17 00:00:00 2001 From: acocuzzo Date: Sun, 2 Apr 2023 21:37:56 -0400 Subject: [PATCH 04/11] Revert "Revert "CI: Add typed_flaky to streaming_pull system tests"" This reverts commit 84c9a43950c3b84e72344bce4eb3b0956dd05011. --- tests/system.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/tests/system.py b/tests/system.py index af65542b6..8a1273bd3 100644 --- a/tests/system.py +++ b/tests/system.py @@ -23,6 +23,7 @@ import sys import threading import time +from typing import Any, Callable, cast, TypeVar # special case python < 3.8 if sys.version_info.major == 3 and sys.version_info.minor < 8: @@ -30,6 +31,7 @@ else: from unittest import mock +from flaky import flaky import pytest import google.auth @@ -43,6 +45,9 @@ from test_utils.system import unique_resource_id +C = TypeVar("C", bound=Callable[..., Any]) +typed_flaky = cast(Callable[[C], C], flaky(max_runs=3, min_passes=1)) + @pytest.fixture(scope="module") def project(): @@ -541,6 +546,7 @@ class CallbackError(Exception): with pytest.raises(CallbackError): future.result(timeout=30) + @typed_flaky def test_streaming_pull_ack_deadline( self, publisher, @@ -601,6 +607,7 @@ def test_streaming_pull_ack_deadline( finally: subscription_future.cancel() + @typed_flaky def test_streaming_pull_max_messages( self, publisher, topic_path_base, subscriber, subscription_path_base, cleanup ): @@ -660,6 +667,7 @@ def test_streaming_pull_max_messages( finally: subscription_future.cancel() # trigger clean shutdown + @typed_flaky def test_streaming_pull_blocking_shutdown( self, publisher, topic_path_base, subscriber, subscription_path_base, cleanup ): From a28b2ab9badbfe91da329d7486414b01786e0e5c Mon Sep 17 00:00:00 2001 From: acocuzzo Date: Tue, 4 Apr 2023 16:41:55 -0400 Subject: [PATCH 05/11] remove typed_flaky from max_messages --- tests/system.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/system.py b/tests/system.py index 8a1273bd3..bb1265453 100644 --- a/tests/system.py +++ b/tests/system.py @@ -607,7 +607,6 @@ def test_streaming_pull_ack_deadline( finally: subscription_future.cancel() - @typed_flaky def test_streaming_pull_max_messages( self, publisher, topic_path_base, subscriber, subscription_path_base, cleanup ): From 2c0f26596281396c3cc5ec3a6591bc9080007f14 Mon Sep 17 00:00:00 2001 From: acocuzzo Date: Tue, 4 Apr 2023 20:47:07 -0400 Subject: [PATCH 06/11] Add flaky to system test deps --- noxfile.py | 1 + 1 file changed, 1 insertion(+) diff --git a/noxfile.py b/noxfile.py index 574fbd644..734e8d35b 100644 --- a/noxfile.py +++ b/noxfile.py @@ -54,6 +54,7 @@ "google-cloud-testutils", ] SYSTEM_TEST_EXTERNAL_DEPENDENCIES = [ + "flaky", "psutil", ] SYSTEM_TEST_LOCAL_DEPENDENCIES = [] From 064fef10d7e1208fb1a7e835b1266e107c3842f3 Mon Sep 17 00:00:00 2001 From: Owl Bot Date: Wed, 5 Apr 2023 00:50:37 +0000 Subject: [PATCH 07/11] =?UTF-8?q?=F0=9F=A6=89=20Updates=20from=20OwlBot=20?= =?UTF-8?q?post-processor?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md --- noxfile.py | 1 - 1 file changed, 1 deletion(-) diff --git a/noxfile.py b/noxfile.py index 734e8d35b..574fbd644 100644 --- a/noxfile.py +++ b/noxfile.py @@ -54,7 +54,6 @@ "google-cloud-testutils", ] SYSTEM_TEST_EXTERNAL_DEPENDENCIES = [ - "flaky", "psutil", ] SYSTEM_TEST_LOCAL_DEPENDENCIES = [] From 3b6822164c84ea8351233f991fac64a102b2cc32 Mon Sep 17 00:00:00 2001 From: acocuzzo Date: Tue, 4 Apr 2023 20:59:07 -0400 Subject: [PATCH 08/11] add flaky dep --- noxfile.py | 1 + 1 file changed, 1 insertion(+) diff --git a/noxfile.py b/noxfile.py index 574fbd644..734e8d35b 100644 --- a/noxfile.py +++ b/noxfile.py @@ -54,6 +54,7 @@ "google-cloud-testutils", ] SYSTEM_TEST_EXTERNAL_DEPENDENCIES = [ + "flaky", "psutil", ] SYSTEM_TEST_LOCAL_DEPENDENCIES = [] From 98188a782fb90b69cf5363855fe8fca8828ce782 Mon Sep 17 00:00:00 2001 From: Owl Bot Date: Wed, 5 Apr 2023 01:22:52 +0000 Subject: [PATCH 09/11] =?UTF-8?q?=F0=9F=A6=89=20Updates=20from=20OwlBot=20?= =?UTF-8?q?post-processor?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md --- noxfile.py | 1 - 1 file changed, 1 deletion(-) diff --git a/noxfile.py b/noxfile.py index 734e8d35b..574fbd644 100644 --- a/noxfile.py +++ b/noxfile.py @@ -54,7 +54,6 @@ "google-cloud-testutils", ] SYSTEM_TEST_EXTERNAL_DEPENDENCIES = [ - "flaky", "psutil", ] SYSTEM_TEST_LOCAL_DEPENDENCIES = [] From b75cfb6223e12811f11896e6e1ea0ffe155a86d2 Mon Sep 17 00:00:00 2001 From: acocuzzo Date: Tue, 4 Apr 2023 22:12:16 -0400 Subject: [PATCH 10/11] add flaky to owlbot.py --- owlbot.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/owlbot.py b/owlbot.py index 8787d90c6..7539adfdb 100644 --- a/owlbot.py +++ b/owlbot.py @@ -338,7 +338,7 @@ versions=gcp.common.detect_versions(path="./google", default_first=True), unit_test_python_versions=["3.7", "3.8", "3.9", "3.10", "3.11"], system_test_python_versions=["3.10"], - system_test_external_dependencies=["psutil"], + system_test_external_dependencies=["psutil","flaky"], ) s.move(templated_files, excludes=[".coveragerc", ".github/release-please.yml", "README.rst", "docs/index.rst"]) From 0fb80e3f1b12c49f4f1a077da83ebeaf72187103 Mon Sep 17 00:00:00 2001 From: Owl Bot Date: Wed, 5 Apr 2023 02:16:05 +0000 Subject: [PATCH 11/11] =?UTF-8?q?=F0=9F=A6=89=20Updates=20from=20OwlBot=20?= =?UTF-8?q?post-processor?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md --- noxfile.py | 1 + 1 file changed, 1 insertion(+) diff --git a/noxfile.py b/noxfile.py index 574fbd644..35e5916a9 100644 --- a/noxfile.py +++ b/noxfile.py @@ -55,6 +55,7 @@ ] SYSTEM_TEST_EXTERNAL_DEPENDENCIES = [ "psutil", + "flaky", ] SYSTEM_TEST_LOCAL_DEPENDENCIES = [] SYSTEM_TEST_DEPENDENCIES = []