From ac2f9b0b5949d0e5275e4f75937b2bc652930f13 Mon Sep 17 00:00:00 2001 From: Takashi Matsuo Date: Mon, 6 Apr 2020 15:38:27 -0700 Subject: [PATCH 1/6] fix: mark tests with `@eventually_consistent.mark` fixes #2971 fixes #2972 fixes #2973 fixes #3085 It will likely fix those issues, not guaranteed, but it's worth a try. --- monitoring/api/v3/alerts-client/requirements-test.txt | 2 ++ monitoring/api/v3/alerts-client/snippets_test.py | 6 ++++++ 2 files changed, 8 insertions(+) diff --git a/monitoring/api/v3/alerts-client/requirements-test.txt b/monitoring/api/v3/alerts-client/requirements-test.txt index 781d4326c947..758bc040f3a8 100644 --- a/monitoring/api/v3/alerts-client/requirements-test.txt +++ b/monitoring/api/v3/alerts-client/requirements-test.txt @@ -1 +1,3 @@ pytest==5.3.2 +gcp-devrel-py-tools==0.0.15 +google-cloud-core diff --git a/monitoring/api/v3/alerts-client/snippets_test.py b/monitoring/api/v3/alerts-client/snippets_test.py index bd0cf4011924..7ff26335a830 100644 --- a/monitoring/api/v3/alerts-client/snippets_test.py +++ b/monitoring/api/v3/alerts-client/snippets_test.py @@ -17,6 +17,7 @@ import random import string +from gcp_devrel.testing import eventually_consistent from google.cloud import monitoring_v3 import google.protobuf.json_format import pytest @@ -74,12 +75,14 @@ def pochan(): yield pochan +@eventually_consistent.mark def test_list_alert_policies(capsys, pochan): snippets.list_alert_policies(pochan.project_name) out, _ = capsys.readouterr() assert pochan.alert_policy.display_name in out +@eventually_consistent.mark def test_enable_alert_policies(capsys, pochan): snippets.enable_alert_policies(pochan.project_name, False) out, _ = capsys.readouterr() @@ -97,6 +100,7 @@ def test_enable_alert_policies(capsys, pochan): assert "already enabled" in out +@eventually_consistent.mark def test_replace_channels(capsys, pochan): alert_policy_id = pochan.alert_policy.name.split('/')[-1] notification_channel_id = pochan.notification_channel.name.split('/')[-1] @@ -106,6 +110,7 @@ def test_replace_channels(capsys, pochan): assert "Updated {0}".format(pochan.alert_policy.name) in out +@eventually_consistent.mark def test_backup_and_restore(capsys, pochan): snippets.backup(pochan.project_name, 'backup.json') out, _ = capsys.readouterr() @@ -117,6 +122,7 @@ def test_backup_and_restore(capsys, pochan): pochan.notification_channel.display_name) in out +@eventually_consistent.mark def test_delete_channels(capsys, pochan): notification_channel_id = pochan.notification_channel.name.split('/')[-1] snippets.delete_notification_channels( From cdb488f62557311aba6a0171837eaef0c0973e19 Mon Sep 17 00:00:00 2001 From: Takashi Matsuo Date: Mon, 6 Apr 2020 16:48:57 -0700 Subject: [PATCH 2/6] use flaky instead --- .../api/v3/alerts-client/requirements-test.txt | 3 +-- monitoring/api/v3/alerts-client/snippets_test.py | 12 ++++++------ 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/monitoring/api/v3/alerts-client/requirements-test.txt b/monitoring/api/v3/alerts-client/requirements-test.txt index 758bc040f3a8..1b569cb4f2c7 100644 --- a/monitoring/api/v3/alerts-client/requirements-test.txt +++ b/monitoring/api/v3/alerts-client/requirements-test.txt @@ -1,3 +1,2 @@ pytest==5.3.2 -gcp-devrel-py-tools==0.0.15 -google-cloud-core +flaky==3.6.1 diff --git a/monitoring/api/v3/alerts-client/snippets_test.py b/monitoring/api/v3/alerts-client/snippets_test.py index 7ff26335a830..23764c13440f 100644 --- a/monitoring/api/v3/alerts-client/snippets_test.py +++ b/monitoring/api/v3/alerts-client/snippets_test.py @@ -17,9 +17,9 @@ import random import string -from gcp_devrel.testing import eventually_consistent from google.cloud import monitoring_v3 import google.protobuf.json_format +from flaky import flaky import pytest import snippets @@ -75,14 +75,14 @@ def pochan(): yield pochan -@eventually_consistent.mark +@flaky(max_runs=5) def test_list_alert_policies(capsys, pochan): snippets.list_alert_policies(pochan.project_name) out, _ = capsys.readouterr() assert pochan.alert_policy.display_name in out -@eventually_consistent.mark +@flaky(max_runs=5) def test_enable_alert_policies(capsys, pochan): snippets.enable_alert_policies(pochan.project_name, False) out, _ = capsys.readouterr() @@ -100,7 +100,7 @@ def test_enable_alert_policies(capsys, pochan): assert "already enabled" in out -@eventually_consistent.mark +@flaky(max_runs=5) def test_replace_channels(capsys, pochan): alert_policy_id = pochan.alert_policy.name.split('/')[-1] notification_channel_id = pochan.notification_channel.name.split('/')[-1] @@ -110,7 +110,7 @@ def test_replace_channels(capsys, pochan): assert "Updated {0}".format(pochan.alert_policy.name) in out -@eventually_consistent.mark +@flaky(max_runs=5) def test_backup_and_restore(capsys, pochan): snippets.backup(pochan.project_name, 'backup.json') out, _ = capsys.readouterr() @@ -122,7 +122,7 @@ def test_backup_and_restore(capsys, pochan): pochan.notification_channel.display_name) in out -@eventually_consistent.mark +@flaky(max_runs=5) def test_delete_channels(capsys, pochan): notification_channel_id = pochan.notification_channel.name.split('/')[-1] snippets.delete_notification_channels( From c3edce4a246a0480799abd48fd30ab50dc6b4521 Mon Sep 17 00:00:00 2001 From: Takashi Matsuo Date: Mon, 6 Apr 2020 17:05:44 -0700 Subject: [PATCH 3/6] use pytest marker --- monitoring/api/v3/alerts-client/snippets_test.py | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/monitoring/api/v3/alerts-client/snippets_test.py b/monitoring/api/v3/alerts-client/snippets_test.py index 23764c13440f..01c3f12bd3e2 100644 --- a/monitoring/api/v3/alerts-client/snippets_test.py +++ b/monitoring/api/v3/alerts-client/snippets_test.py @@ -19,7 +19,6 @@ from google.cloud import monitoring_v3 import google.protobuf.json_format -from flaky import flaky import pytest import snippets @@ -75,14 +74,14 @@ def pochan(): yield pochan -@flaky(max_runs=5) +@pytest.mark.flaky(max_runs=5) def test_list_alert_policies(capsys, pochan): snippets.list_alert_policies(pochan.project_name) out, _ = capsys.readouterr() assert pochan.alert_policy.display_name in out -@flaky(max_runs=5) +@pytest.mark.flaky(max_runs=5) def test_enable_alert_policies(capsys, pochan): snippets.enable_alert_policies(pochan.project_name, False) out, _ = capsys.readouterr() @@ -100,7 +99,7 @@ def test_enable_alert_policies(capsys, pochan): assert "already enabled" in out -@flaky(max_runs=5) +@pytest.mark.flaky(max_runs=5) def test_replace_channels(capsys, pochan): alert_policy_id = pochan.alert_policy.name.split('/')[-1] notification_channel_id = pochan.notification_channel.name.split('/')[-1] @@ -110,7 +109,7 @@ def test_replace_channels(capsys, pochan): assert "Updated {0}".format(pochan.alert_policy.name) in out -@flaky(max_runs=5) +@pytest.mark.flaky(max_runs=5) def test_backup_and_restore(capsys, pochan): snippets.backup(pochan.project_name, 'backup.json') out, _ = capsys.readouterr() @@ -122,7 +121,7 @@ def test_backup_and_restore(capsys, pochan): pochan.notification_channel.display_name) in out -@flaky(max_runs=5) +@pytest.mark.flaky(max_runs=5) def test_delete_channels(capsys, pochan): notification_channel_id = pochan.notification_channel.name.split('/')[-1] snippets.delete_notification_channels( From a78c2e68fdf60c9b950bd8c489cdc5d599cb0c9b Mon Sep 17 00:00:00 2001 From: Takashi Matsuo Date: Mon, 6 Apr 2020 17:24:43 -0700 Subject: [PATCH 4/6] use retrying module in the fixture class --- .../v3/alerts-client/requirements-test.txt | 2 +- .../api/v3/alerts-client/snippets_test.py | 44 ++++++++++--------- 2 files changed, 24 insertions(+), 22 deletions(-) diff --git a/monitoring/api/v3/alerts-client/requirements-test.txt b/monitoring/api/v3/alerts-client/requirements-test.txt index 1b569cb4f2c7..826c2b7161c4 100644 --- a/monitoring/api/v3/alerts-client/requirements-test.txt +++ b/monitoring/api/v3/alerts-client/requirements-test.txt @@ -1,2 +1,2 @@ pytest==5.3.2 -flaky==3.6.1 +retrying==1.3.3 diff --git a/monitoring/api/v3/alerts-client/snippets_test.py b/monitoring/api/v3/alerts-client/snippets_test.py index 01c3f12bd3e2..b9cc7d341efd 100644 --- a/monitoring/api/v3/alerts-client/snippets_test.py +++ b/monitoring/api/v3/alerts-client/snippets_test.py @@ -17,9 +17,11 @@ import random import string +from google.api_core.exceptions import Aborted from google.cloud import monitoring_v3 import google.protobuf.json_format import pytest +from retrying import retry import snippets @@ -42,22 +44,27 @@ def __init__(self): monitoring_v3.NotificationChannelServiceClient()) def __enter__(self): - # Create a policy. - policy = monitoring_v3.types.alert_pb2.AlertPolicy() - json = open('test_alert_policy.json').read() - google.protobuf.json_format.Parse(json, policy) - policy.display_name = 'snippets-test-' + random_name(10) - self.alert_policy = self.alert_policy_client.create_alert_policy( - self.project_name, policy) - # Create a notification channel. - notification_channel = ( - monitoring_v3.types.notification_pb2.NotificationChannel()) - json = open('test_notification_channel.json').read() - google.protobuf.json_format.Parse(json, notification_channel) - notification_channel.display_name = 'snippets-test-' + random_name(10) - self.notification_channel = ( - self.notification_channel_client.create_notification_channel( - self.project_name, notification_channel)) + @retry(wait_exponential_multiplier=1000, wait_exponential_max=10000, + stop_max_attempt_number=5, retry_on_exception=(Aborted,)) + def setup(): + # Create a policy. + policy = monitoring_v3.types.alert_pb2.AlertPolicy() + json = open('test_alert_policy.json').read() + google.protobuf.json_format.Parse(json, policy) + policy.display_name = 'snippets-test-' + random_name(10) + self.alert_policy = self.alert_policy_client.create_alert_policy( + self.project_name, policy) + # Create a notification channel. + notification_channel = ( + monitoring_v3.types.notification_pb2.NotificationChannel()) + json = open('test_notification_channel.json').read() + google.protobuf.json_format.Parse(json, notification_channel) + notification_channel.display_name = ( + 'snippets-test-' + random_name(10)) + self.notification_channel = ( + self.notification_channel_client.create_notification_channel( + self.project_name, notification_channel)) + setup() return self def __exit__(self, type, value, traceback): @@ -74,14 +81,12 @@ def pochan(): yield pochan -@pytest.mark.flaky(max_runs=5) def test_list_alert_policies(capsys, pochan): snippets.list_alert_policies(pochan.project_name) out, _ = capsys.readouterr() assert pochan.alert_policy.display_name in out -@pytest.mark.flaky(max_runs=5) def test_enable_alert_policies(capsys, pochan): snippets.enable_alert_policies(pochan.project_name, False) out, _ = capsys.readouterr() @@ -99,7 +104,6 @@ def test_enable_alert_policies(capsys, pochan): assert "already enabled" in out -@pytest.mark.flaky(max_runs=5) def test_replace_channels(capsys, pochan): alert_policy_id = pochan.alert_policy.name.split('/')[-1] notification_channel_id = pochan.notification_channel.name.split('/')[-1] @@ -109,7 +113,6 @@ def test_replace_channels(capsys, pochan): assert "Updated {0}".format(pochan.alert_policy.name) in out -@pytest.mark.flaky(max_runs=5) def test_backup_and_restore(capsys, pochan): snippets.backup(pochan.project_name, 'backup.json') out, _ = capsys.readouterr() @@ -121,7 +124,6 @@ def test_backup_and_restore(capsys, pochan): pochan.notification_channel.display_name) in out -@pytest.mark.flaky(max_runs=5) def test_delete_channels(capsys, pochan): notification_channel_id = pochan.notification_channel.name.split('/')[-1] snippets.delete_notification_channels( From 5532261eab682833b3f6ec8c1c9b1b121b86c86c Mon Sep 17 00:00:00 2001 From: Takashi Matsuo Date: Mon, 6 Apr 2020 17:33:25 -0700 Subject: [PATCH 5/6] pass callback for retry_on_exception --- monitoring/api/v3/alerts-client/snippets_test.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/monitoring/api/v3/alerts-client/snippets_test.py b/monitoring/api/v3/alerts-client/snippets_test.py index b9cc7d341efd..6679a8cfab10 100644 --- a/monitoring/api/v3/alerts-client/snippets_test.py +++ b/monitoring/api/v3/alerts-client/snippets_test.py @@ -31,6 +31,10 @@ def random_name(length): [random.choice(string.ascii_lowercase) for i in range(length)]) +def retry_if_aborted(exception): + return isinstance(exception, Aborted) + + class PochanFixture: """A test fixture that creates an alert POlicy and a notification CHANnel, hence the name, pochan. @@ -45,7 +49,7 @@ def __init__(self): def __enter__(self): @retry(wait_exponential_multiplier=1000, wait_exponential_max=10000, - stop_max_attempt_number=5, retry_on_exception=(Aborted,)) + stop_max_attempt_number=5, retry_on_exception=retry_if_aborted) def setup(): # Create a policy. policy = monitoring_v3.types.alert_pb2.AlertPolicy() From 5f4fcc17d5da8398e1a098145c72146e9ee7159a Mon Sep 17 00:00:00 2001 From: Takashi Matsuo Date: Mon, 6 Apr 2020 19:28:08 -0700 Subject: [PATCH 6/6] retry all the write operations --- .../api/v3/alerts-client/snippets_test.py | 64 ++++++++++++++----- 1 file changed, 47 insertions(+), 17 deletions(-) diff --git a/monitoring/api/v3/alerts-client/snippets_test.py b/monitoring/api/v3/alerts-client/snippets_test.py index 6679a8cfab10..1980601cdfa3 100644 --- a/monitoring/api/v3/alerts-client/snippets_test.py +++ b/monitoring/api/v3/alerts-client/snippets_test.py @@ -73,10 +73,15 @@ def setup(): def __exit__(self, type, value, traceback): # Delete the policy and channel we created. - self.alert_policy_client.delete_alert_policy(self.alert_policy.name) - if self.notification_channel.name: - self.notification_channel_client.delete_notification_channel( - self.notification_channel.name) + @retry(wait_exponential_multiplier=1000, wait_exponential_max=10000, + stop_max_attempt_number=5, retry_on_exception=retry_if_aborted) + def teardown(): + self.alert_policy_client.delete_alert_policy( + self.alert_policy.name) + if self.notification_channel.name: + self.notification_channel_client.delete_notification_channel( + self.notification_channel.name) + teardown() @pytest.fixture(scope='session') @@ -92,36 +97,55 @@ def test_list_alert_policies(capsys, pochan): def test_enable_alert_policies(capsys, pochan): - snippets.enable_alert_policies(pochan.project_name, False) - out, _ = capsys.readouterr() + @retry(wait_exponential_multiplier=1000, wait_exponential_max=10000, + stop_max_attempt_number=5, retry_on_exception=retry_if_aborted) + def invoke_sample(val): + snippets.enable_alert_policies(pochan.project_name, val) - snippets.enable_alert_policies(pochan.project_name, False) + invoke_sample(False) + invoke_sample(False) out, _ = capsys.readouterr() assert "already disabled" in out - snippets.enable_alert_policies(pochan.project_name, True) + invoke_sample(True) out, _ = capsys.readouterr() assert "Enabled {0}".format(pochan.project_name) in out - snippets.enable_alert_policies(pochan.project_name, True) + invoke_sample(True) out, _ = capsys.readouterr() assert "already enabled" in out def test_replace_channels(capsys, pochan): - alert_policy_id = pochan.alert_policy.name.split('/')[-1] - notification_channel_id = pochan.notification_channel.name.split('/')[-1] - snippets.replace_notification_channels( - pochan.project_name, alert_policy_id, [notification_channel_id]) + @retry(wait_exponential_multiplier=1000, wait_exponential_max=10000, + stop_max_attempt_number=5, retry_on_exception=retry_if_aborted) + def invoke_sample(): + alert_policy_id = pochan.alert_policy.name.split('/')[-1] + notification_channel_id = pochan.notification_channel.name.split( + '/')[-1] + snippets.replace_notification_channels( + pochan.project_name, alert_policy_id, [notification_channel_id]) + + invoke_sample() out, _ = capsys.readouterr() assert "Updated {0}".format(pochan.alert_policy.name) in out def test_backup_and_restore(capsys, pochan): - snippets.backup(pochan.project_name, 'backup.json') + @retry(wait_exponential_multiplier=1000, wait_exponential_max=10000, + stop_max_attempt_number=5, retry_on_exception=retry_if_aborted) + def invoke_backup(): + snippets.backup(pochan.project_name, 'backup.json') + + invoke_backup() out, _ = capsys.readouterr() - snippets.restore(pochan.project_name, 'backup.json') + @retry(wait_exponential_multiplier=1000, wait_exponential_max=10000, + stop_max_attempt_number=5, retry_on_exception=retry_if_aborted) + def invoke_restore(): + snippets.restore(pochan.project_name, 'backup.json') + + invoke_restore() out, _ = capsys.readouterr() assert "Updated {0}".format(pochan.alert_policy.name) in out assert "Updating channel {0}".format( @@ -130,8 +154,14 @@ def test_backup_and_restore(capsys, pochan): def test_delete_channels(capsys, pochan): notification_channel_id = pochan.notification_channel.name.split('/')[-1] - snippets.delete_notification_channels( - pochan.project_name, [notification_channel_id], force=True) + + @retry(wait_exponential_multiplier=1000, wait_exponential_max=10000, + stop_max_attempt_number=5, retry_on_exception=retry_if_aborted) + def invoke_delete(): + snippets.delete_notification_channels( + pochan.project_name, [notification_channel_id], force=True) + + invoke_delete() out, _ = capsys.readouterr() assert "{0} deleted".format(notification_channel_id) in out pochan.notification_channel.name = '' # So teardown is not tried