From 9b3a1adf3e5b72f6e762c3fd0440521a3a128637 Mon Sep 17 00:00:00 2001 From: Takashi Matsuo Date: Tue, 7 Apr 2020 12:41:49 -0700 Subject: [PATCH 1/4] [iam] fix: reuse the same custom role for read/update tests fixes #2885 fixes #2886 This PR will reduce the number of temporary custom roles to half. Not a fundamental fix, but it will definitely mitigate the above issues. --- iam/api-client/custom_roles_test.py | 30 ++++++++++++++--------------- 1 file changed, 14 insertions(+), 16 deletions(-) diff --git a/iam/api-client/custom_roles_test.py b/iam/api-client/custom_roles_test.py index f5bd42a81b4..9f003f29311 100644 --- a/iam/api-client/custom_roles_test.py +++ b/iam/api-client/custom_roles_test.py @@ -21,7 +21,7 @@ GCLOUD_PROJECT = os.environ["GCLOUD_PROJECT"] -CUSTOM_ROLE_NAME = "pythonTestCustomRole" + str(uuid.uuid1().int) +CUSTOM_ROLE_NAME = "pythonTestCustomRole" CUSTOM_ROLE_TITLE = "Python Test Custom Role" CUSTOM_ROLE_DESCRIPTION = "This is a python test custom role" CUSTOM_ROLE_PERMISSIONS = ["iam.roles.get"] @@ -33,23 +33,21 @@ @pytest.fixture(scope="module") def test_custom_role(): - # section to create custom role to test policy updates. - custom_roles.create_role( - CUSTOM_ROLE_NAME, - GCLOUD_PROJECT, - CUSTOM_ROLE_TITLE, - CUSTOM_ROLE_DESCRIPTION, - CUSTOM_ROLE_PERMISSIONS, - CUSTOM_ROLE_STAGE, - ) - yield CUSTOM_ROLE_NAME - - # Delete the custom role + # This custom role is reused in read/update tests. try: - custom_roles.delete_role(CUSTOM_ROLE_NAME, GCLOUD_PROJECT) + custom_roles.create_role( + CUSTOM_ROLE_NAME, + GCLOUD_PROJECT, + CUSTOM_ROLE_TITLE, + CUSTOM_ROLE_DESCRIPTION, + CUSTOM_ROLE_PERMISSIONS, + CUSTOM_ROLE_STAGE, + ) except googleapiclient.errors.HttpError: - print("Custom role already deleted.") - + # Ignore error since we just reuse the same custom role. + pass + yield CUSTOM_ROLE_NAME + # we don't delete this custom role for future tests. @pytest.fixture(scope="function") def unique_custom_role_name(): From f048312ac1f231110313e7b36f8ffb3526871b1d Mon Sep 17 00:00:00 2001 From: Takashi Matsuo Date: Tue, 7 Apr 2020 12:51:09 -0700 Subject: [PATCH 2/4] lint --- iam/api-client/custom_roles_test.py | 1 + 1 file changed, 1 insertion(+) diff --git a/iam/api-client/custom_roles_test.py b/iam/api-client/custom_roles_test.py index 9f003f29311..2e8b7ce0d0e 100644 --- a/iam/api-client/custom_roles_test.py +++ b/iam/api-client/custom_roles_test.py @@ -49,6 +49,7 @@ def test_custom_role(): yield CUSTOM_ROLE_NAME # we don't delete this custom role for future tests. + @pytest.fixture(scope="function") def unique_custom_role_name(): UNIQUE_CUSTOM_ROLE_NAME = "pythonTestCustomRole" + str(uuid.uuid1().int) From 0eecff32bdc577d9dd0c5fe4972f587e7cecd7e3 Mon Sep 17 00:00:00 2001 From: Takashi Matsuo Date: Tue, 7 Apr 2020 13:01:37 -0700 Subject: [PATCH 3/4] only ignore HttpError 409 --- iam/api-client/custom_roles_test.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/iam/api-client/custom_roles_test.py b/iam/api-client/custom_roles_test.py index 2e8b7ce0d0e..f9486f4ebf3 100644 --- a/iam/api-client/custom_roles_test.py +++ b/iam/api-client/custom_roles_test.py @@ -43,7 +43,9 @@ def test_custom_role(): CUSTOM_ROLE_PERMISSIONS, CUSTOM_ROLE_STAGE, ) - except googleapiclient.errors.HttpError: + except googleapiclient.errors.HttpError as e: + if "HttpError 409" not in str(e): + raise e # Ignore error since we just reuse the same custom role. pass yield CUSTOM_ROLE_NAME From c4c169d632b3945868abda9a16c831f16efab0c3 Mon Sep 17 00:00:00 2001 From: Takashi Matsuo Date: Tue, 7 Apr 2020 13:51:02 -0700 Subject: [PATCH 4/4] address review comments --- iam/api-client/custom_roles_test.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/iam/api-client/custom_roles_test.py b/iam/api-client/custom_roles_test.py index f9486f4ebf3..19214110476 100644 --- a/iam/api-client/custom_roles_test.py +++ b/iam/api-client/custom_roles_test.py @@ -47,7 +47,7 @@ def test_custom_role(): if "HttpError 409" not in str(e): raise e # Ignore error since we just reuse the same custom role. - pass + print('Re-using the custom role "{}".'.format(CUSTOM_ROLE_NAME)) yield CUSTOM_ROLE_NAME # we don't delete this custom role for future tests.