From 27a56f522b2fa17580bb4fb1fd0e0af4a2f207ce Mon Sep 17 00:00:00 2001 From: Charles Lowell Date: Thu, 21 Nov 2019 06:45:22 -0800 Subject: [PATCH 1/2] ensure offline tests pass during live runs --- .../azure-identity/tests/test_default.py | 20 +++++++++++++++---- .../tests/test_default_async.py | 18 ++++++++++++++--- 2 files changed, 31 insertions(+), 7 deletions(-) diff --git a/sdk/identity/azure-identity/tests/test_default.py b/sdk/identity/azure-identity/tests/test_default.py index 1ea22f0beeef..9be8b4cc3f62 100644 --- a/sdk/identity/azure-identity/tests/test_default.py +++ b/sdk/identity/azure-identity/tests/test_default.py @@ -2,7 +2,8 @@ # Copyright (c) Microsoft Corporation. # Licensed under the MIT License. # ------------------------------------ -from azure.core.credentials import AccessToken +import os + from azure.identity import ( DefaultAzureCredential, InteractiveBrowserCredential, @@ -64,7 +65,9 @@ def send(request, **_): assert access_token == expected_access_token # shared cache credential should respect authority - account = get_account_event(username="spam@eggs", uid="guid", utid="tenant", authority=authority_kwarg) + upn = os.environ.get(EnvironmentVariables.AZURE_USERNAME, "spam@eggs") # preferring environment values to + tenant = os.environ.get(EnvironmentVariables.AZURE_TENANT_ID, "tenant") # prevent failure during live runs + account = get_account_event(username=upn, uid="guid", utid=tenant, authority=authority_kwarg) cache = populated_cache(account) with patch.object(SharedTokenCacheCredential, "supported"): credential = DefaultAzureCredential(_cache=cache, authority=authority_kwarg, transport=Mock(send=send)) @@ -115,7 +118,12 @@ def test_shared_cache_tenant_id(): expected_access_token = "expected-access-token" refresh_token_a = "refresh-token-a" refresh_token_b = "refresh-token-b" - upn = "spam@eggs" + + # The value of the UPN is arbitrary because this test verifies the credential's behavior given a specified + # tenant ID. During a complete live test run, $AZURE_USERNAME will have a value which DefaultAzureCredential + # should pass to SharedTokenCacheCredential. We prefer the environment value to prevent that breaking this test. + upn = os.environ.get(EnvironmentVariables.AZURE_USERNAME, "spam@eggs") + tenant_a = "tenant-a" tenant_b = "tenant-b" @@ -158,7 +166,11 @@ def test_shared_cache_username(): refresh_token_b = "refresh-token-b" upn_a = "spam@eggs" upn_b = "eggs@spam" - tenant_id = "the-tenant" + + # The value of the tenant ID is arbitrary because this test verifies the credential's behavior given a specified + # username. During a complete live test run, $AZURE_TENANT_ID will have a value which DefaultAzureCredential should + # pass to SharedTokenCacheCredential. We prefer the environment value here to prevent that breaking this test. + tenant_id = os.environ.get(EnvironmentVariables.AZURE_TENANT_ID, "the-tenant") # two cached accounts, same tenant, different usernames -> shared_cache_username should prevail account_a = get_account_event(username=upn_a, uid="another-guid", utid=tenant_id, refresh_token=refresh_token_a) diff --git a/sdk/identity/azure-identity/tests/test_default_async.py b/sdk/identity/azure-identity/tests/test_default_async.py index b445fcc7a791..51ccc95267ef 100644 --- a/sdk/identity/azure-identity/tests/test_default_async.py +++ b/sdk/identity/azure-identity/tests/test_default_async.py @@ -3,6 +3,7 @@ # Licensed under the MIT License. # ------------------------------------ import asyncio +import os from unittest.mock import Mock, patch from urllib.parse import urlparse @@ -69,7 +70,9 @@ async def send(request, **_): assert access_token == expected_access_token # shared cache credential should respect authority - account = get_account_event(username="spam@eggs", uid="guid", utid="tenant", authority=authority_kwarg) + upn = os.environ.get(EnvironmentVariables.AZURE_USERNAME, "spam@eggs") # preferring environment values to + tenant = os.environ.get(EnvironmentVariables.AZURE_TENANT_ID, "tenant") # prevent failure during live runs + account = get_account_event(username=upn, uid="guid", utid=tenant, authority=authority_kwarg) cache = populated_cache(account) with patch.object(SharedTokenCacheCredential, "supported"): credential = DefaultAzureCredential(_cache=cache, authority=authority_kwarg, transport=Mock(send=send)) @@ -115,7 +118,12 @@ async def test_shared_cache_tenant_id(): expected_access_token = "expected-access-token" refresh_token_a = "refresh-token-a" refresh_token_b = "refresh-token-b" - upn = "spam@eggs" + + # The value of the UPN is arbitrary because this test verifies the credential's behavior given a specified + # tenant ID. During a complete live test run, $AZURE_USERNAME will have a value which DefaultAzureCredential + # should pass to SharedTokenCacheCredential. We prefer the environment value to prevent that breaking this test. + upn = os.environ.get(EnvironmentVariables.AZURE_USERNAME, "spam@eggs") + tenant_a = "tenant-a" tenant_b = "tenant-b" @@ -159,7 +167,11 @@ async def test_shared_cache_username(): refresh_token_b = "refresh-token-b" upn_a = "spam@eggs" upn_b = "eggs@spam" - tenant_id = "the-tenant" + + # The value of the tenant ID is arbitrary because this test verifies the credential's behavior given a specified + # username. During a complete live test run, $AZURE_TENANT_ID will have a value which DefaultAzureCredential should + # pass to SharedTokenCacheCredential. We prefer the environment value here to prevent that breaking this test. + tenant_id = os.environ.get(EnvironmentVariables.AZURE_TENANT_ID, "the-tenant") # two cached accounts, same tenant, different usernames -> shared_cache_username should prevail account_a = get_account_event(username=upn_a, uid="another-guid", utid=tenant_id, refresh_token=refresh_token_a) From 422ae84689d876b13cd5713d9c2260d282d5dc9f Mon Sep 17 00:00:00 2001 From: Charles Lowell Date: Thu, 21 Nov 2019 10:41:27 -0800 Subject: [PATCH 2/2] reword comments for the sake of future generations --- sdk/identity/azure-identity/tests/test_default.py | 4 ++-- .../azure-identity/tests/test_default_async.py | 12 ++++++------ 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/sdk/identity/azure-identity/tests/test_default.py b/sdk/identity/azure-identity/tests/test_default.py index 9be8b4cc3f62..39282d7df85d 100644 --- a/sdk/identity/azure-identity/tests/test_default.py +++ b/sdk/identity/azure-identity/tests/test_default.py @@ -121,7 +121,7 @@ def test_shared_cache_tenant_id(): # The value of the UPN is arbitrary because this test verifies the credential's behavior given a specified # tenant ID. During a complete live test run, $AZURE_USERNAME will have a value which DefaultAzureCredential - # should pass to SharedTokenCacheCredential. We prefer the environment value to prevent that breaking this test. + # should pass to SharedTokenCacheCredential. This test will fail if the mock accounts don't match that value. upn = os.environ.get(EnvironmentVariables.AZURE_USERNAME, "spam@eggs") tenant_a = "tenant-a" @@ -169,7 +169,7 @@ def test_shared_cache_username(): # The value of the tenant ID is arbitrary because this test verifies the credential's behavior given a specified # username. During a complete live test run, $AZURE_TENANT_ID will have a value which DefaultAzureCredential should - # pass to SharedTokenCacheCredential. We prefer the environment value here to prevent that breaking this test. + # pass to SharedTokenCacheCredential. This test will fail if the mock accounts don't match that value. tenant_id = os.environ.get(EnvironmentVariables.AZURE_TENANT_ID, "the-tenant") # two cached accounts, same tenant, different usernames -> shared_cache_username should prevail diff --git a/sdk/identity/azure-identity/tests/test_default_async.py b/sdk/identity/azure-identity/tests/test_default_async.py index 51ccc95267ef..0efab8607e31 100644 --- a/sdk/identity/azure-identity/tests/test_default_async.py +++ b/sdk/identity/azure-identity/tests/test_default_async.py @@ -119,9 +119,9 @@ async def test_shared_cache_tenant_id(): refresh_token_a = "refresh-token-a" refresh_token_b = "refresh-token-b" - # The value of the UPN is arbitrary because this test verifies the credential's behavior given a specified - # tenant ID. During a complete live test run, $AZURE_USERNAME will have a value which DefaultAzureCredential - # should pass to SharedTokenCacheCredential. We prefer the environment value to prevent that breaking this test. + # The value of upn is arbitrary because this test verifies the credential's behavior given a specified + # tenant. During a complete live test run, $AZURE_USERNAME will have a value which DefaultAzureCredential + # should pass to SharedTokenCacheCredential. This test will fail if the mock accounts don't match that value. upn = os.environ.get(EnvironmentVariables.AZURE_USERNAME, "spam@eggs") tenant_a = "tenant-a" @@ -168,9 +168,9 @@ async def test_shared_cache_username(): upn_a = "spam@eggs" upn_b = "eggs@spam" - # The value of the tenant ID is arbitrary because this test verifies the credential's behavior given a specified - # username. During a complete live test run, $AZURE_TENANT_ID will have a value which DefaultAzureCredential should - # pass to SharedTokenCacheCredential. We prefer the environment value here to prevent that breaking this test. + # The value of tenant_id is arbitrary because this test verifies the credential's behavior given a specified + # username. During a complete live test run, $AZURE_TENANT_ID will have a value which DefaultAzureCredential + # should pass to SharedTokenCacheCredential. This test will fail if the mock accounts don't match that value. tenant_id = os.environ.get(EnvironmentVariables.AZURE_TENANT_ID, "the-tenant") # two cached accounts, same tenant, different usernames -> shared_cache_username should prevail