Skip to content

Commit c8662ac

Browse files
committed
Fix e2e test skip logic for undefined ADO pipeline variables
When a pipeline variable referenced in a step env: block is not defined, ADO injects the literal string '' into the process environment instead of an empty value. That literal is truthy, so plain os.getenv() guards fail to skip and tests error trying to open '' as a path. Fix: add _clean_env() helper in both lab_config.py and test_e2e.py that returns None for unset values AND for ADO-literal '' strings. - lab_config.py: _get_credential() and get_client_certificate() use _clean_env() - test_e2e.py: get_lab_app() uses _clean_env(); PublicCloudScenariosTestCase setUpClass() guard uses _clean_env() This makes all LabBasedTestCase and PublicCloudScenariosTestCase tests skip cleanly (rather than error) when LAB_APP_CLIENT_ID is not configured as a pipeline variable.
1 parent 66a42f1 commit c8662ac

File tree

2 files changed

+31
-7
lines changed

2 files changed

+31
-7
lines changed

tests/lab_config.py

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -164,6 +164,21 @@ class AppConfig:
164164
_msal_team_client: Optional[SecretClient] = None
165165

166166

167+
def _clean_env(name: str) -> Optional[str]:
168+
"""Return the env var value, or None if unset or it contains an unexpanded
169+
ADO pipeline variable literal such as ``$(VAR_NAME)``.
170+
171+
Azure DevOps injects the literal string ``$(VAR_NAME)`` when a ``$(...)``
172+
reference in a step ``env:`` block refers to a variable that has not been
173+
defined at runtime. That literal is truthy, so a plain ``os.getenv()``
174+
check would incorrectly proceed as if the variable were set.
175+
"""
176+
value = os.getenv(name)
177+
if value and value.startswith("$("):
178+
return None
179+
return value or None
180+
181+
167182
def _get_credential():
168183
"""
169184
Create an Azure credential for Key Vault access.
@@ -177,8 +192,8 @@ def _get_credential():
177192
Raises:
178193
EnvironmentError: If required environment variables are not set.
179194
"""
180-
client_id = os.getenv("LAB_APP_CLIENT_ID")
181-
cert_path = os.getenv("LAB_APP_CLIENT_CERT_PFX_PATH")
195+
client_id = _clean_env("LAB_APP_CLIENT_ID")
196+
cert_path = _clean_env("LAB_APP_CLIENT_CERT_PFX_PATH")
182197
tenant_id = "72f988bf-86f1-41af-91ab-2d7cd011db47" # Microsoft tenant
183198

184199
if not client_id:
@@ -396,7 +411,7 @@ def get_client_certificate() -> Dict[str, object]:
396411
Raises:
397412
EnvironmentError: If LAB_APP_CLIENT_CERT_PFX_PATH is not set.
398413
"""
399-
cert_path = os.getenv("LAB_APP_CLIENT_CERT_PFX_PATH")
414+
cert_path = _clean_env("LAB_APP_CLIENT_CERT_PFX_PATH")
400415
if not cert_path:
401416
raise EnvironmentError(
402417
"LAB_APP_CLIENT_CERT_PFX_PATH environment variable is required "

tests/test_e2e.py

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,14 @@
5454
os.getenv("TRAVIS") or os.getenv("TF_BUILD") or not os.getenv("CI")
5555
)
5656

57+
58+
def _clean_env(name):
59+
"""Return the env var value, or None if unset or it contains an unexpanded
60+
ADO pipeline variable literal such as ``$(VAR_NAME)``."""
61+
value = os.getenv(name)
62+
return None if (not value or value.startswith("$(")) else value
63+
64+
5765
def _get_app_and_auth_code(
5866
client_id,
5967
client_secret=None,
@@ -345,7 +353,7 @@ class PublicCloudScenariosTestCase(E2eTestCase):
345353

346354
@classmethod
347355
def setUpClass(cls):
348-
if not os.getenv("LAB_APP_CLIENT_ID"):
356+
if not _clean_env("LAB_APP_CLIENT_ID"):
349357
raise unittest.SkipTest(
350358
"LAB_APP_CLIENT_ID not set; skipping PublicCloud e2e tests")
351359
pca_app = get_app_config(AppSecrets.PCA_CLIENT)
@@ -473,13 +481,14 @@ def get_lab_app(
473481
"Reading ENV variables %s and %s for lab app defined at "
474482
"https://docs.msidlab.com/accounts/confidentialclient.html",
475483
env_client_id, env_client_cert_path)
476-
if os.getenv(env_client_id) and os.getenv(env_client_cert_path):
484+
client_id = _clean_env(env_client_id)
485+
cert_path = _clean_env(env_client_cert_path)
486+
if client_id and cert_path:
477487
# id came from https://docs.msidlab.com/accounts/confidentialclient.html
478-
client_id = os.getenv(env_client_id)
479488
client_credential = {
480489
"private_key_pfx_path":
481490
# Cert came from https://ms.portal.azure.com/#@microsoft.onmicrosoft.com/asset/Microsoft_Azure_KeyVault/Certificate/https://msidlabs.vault.azure.net/certificates/LabAuth
482-
os.getenv(env_client_cert_path),
491+
cert_path,
483492
"public_certificate": True, # Opt in for SNI
484493
}
485494
else:

0 commit comments

Comments
 (0)