From 0c156c9d139326703d1a50bad19d42ea6561cc15 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?McCoy=20Pati=C3=B1o?= <39780829+mccoyp@users.noreply.github.com> Date: Thu, 21 Jul 2022 11:13:53 -0700 Subject: [PATCH] [Test Proxy] Add test recording fixture (#24848) --- .vscode/cspell.json | 1 + .../dev_requirements.txt | 3 +- sdk/conftest.py | 36 ++- .../azure-keyvault/dev_requirements.txt | 3 +- .../dev_requirements.txt | 3 +- .../devtools_testutils/__init__.py | 3 + .../devtools_testutils/proxy_fixtures.py | 233 ++++++++++++++++++ .../devtools_testutils/proxy_startup.py | 25 +- .../devtools_testutils/proxy_testcase.py | 60 ++--- 9 files changed, 306 insertions(+), 61 deletions(-) create mode 100644 tools/azure-sdk-tools/devtools_testutils/proxy_fixtures.py diff --git a/.vscode/cspell.json b/.vscode/cspell.json index b31e3365ac74..6c3127f7eae3 100644 --- a/.vscode/cspell.json +++ b/.vscode/cspell.json @@ -254,6 +254,7 @@ "prebuilts", "pschema", "PSECRET", + "pyfuncitem", "pygobject", "parameterizing", "pytyped", diff --git a/sdk/baremetalinfrastructure/azure-mgmt-baremetalinfrastructure/dev_requirements.txt b/sdk/baremetalinfrastructure/azure-mgmt-baremetalinfrastructure/dev_requirements.txt index fc236b060c0c..92ee26f9fc75 100644 --- a/sdk/baremetalinfrastructure/azure-mgmt-baremetalinfrastructure/dev_requirements.txt +++ b/sdk/baremetalinfrastructure/azure-mgmt-baremetalinfrastructure/dev_requirements.txt @@ -1,2 +1,3 @@ aiohttp>=3.0; python_version >= '3.5' --e ../../../tools/azure-devtools \ No newline at end of file +-e ../../../tools/azure-devtools +-e ../../../tools/azure-sdk-tools \ No newline at end of file diff --git a/sdk/conftest.py b/sdk/conftest.py index f4cdaaade7ce..f98006995cb3 100644 --- a/sdk/conftest.py +++ b/sdk/conftest.py @@ -26,33 +26,51 @@ import os import pytest +from devtools_testutils import recorded_test, test_proxy, variable_recorder + + def pytest_configure(config): # register an additional marker - config.addinivalue_line( - "markers", "live_test_only: mark test to be a live test only" - ) - config.addinivalue_line( - "markers", "playback_test_only: mark test to be a playback test only" - ) + config.addinivalue_line("markers", "live_test_only: mark test to be a live test only") + config.addinivalue_line("markers", "playback_test_only: mark test to be a playback test only") + def pytest_runtest_setup(item): is_live_only_test_marked = bool([mark for mark in item.iter_markers(name="live_test_only")]) if is_live_only_test_marked: from devtools_testutils import is_live + if not is_live(): pytest.skip("live test only") is_playback_test_marked = bool([mark for mark in item.iter_markers(name="playback_test_only")]) if is_playback_test_marked: from devtools_testutils import is_live - if is_live() and os.environ.get('AZURE_SKIP_LIVE_RECORDING', '').lower() == 'true': + + if is_live() and os.environ.get("AZURE_SKIP_LIVE_RECORDING", "").lower() == "true": pytest.skip("playback test only") + try: from azure_devtools.scenario_tests import AbstractPreparer - @pytest.fixture(scope='session', autouse=True) + + @pytest.fixture(scope="session", autouse=True) def clean_cached_resources(): yield AbstractPreparer._perform_pending_deletes() + + except ImportError: - pass \ No newline at end of file + pass + + +@pytest.hookimpl(tryfirst=True, hookwrapper=True) +def pytest_runtest_makereport(item, call) -> None: + """Captures test exception info and makes it available to other fixtures.""" + # execute all other hooks to obtain the report object + outcome = yield + result = outcome.get_result() + if result.outcome == "failed": + error = call.excinfo.value + # set a test_error attribute on the item (available to other fixtures from request.node) + setattr(item, "test_error", error) diff --git a/sdk/keyvault/azure-keyvault/dev_requirements.txt b/sdk/keyvault/azure-keyvault/dev_requirements.txt index 51436184c10f..a6fc97da776d 100644 --- a/sdk/keyvault/azure-keyvault/dev_requirements.txt +++ b/sdk/keyvault/azure-keyvault/dev_requirements.txt @@ -1 +1,2 @@ --e ../../../tools/azure-devtools \ No newline at end of file +-e ../../../tools/azure-devtools +-e ../../../tools/azure-sdk-tools \ No newline at end of file diff --git a/sdk/streamanalytics/azure-mgmt-streamanalytics/dev_requirements.txt b/sdk/streamanalytics/azure-mgmt-streamanalytics/dev_requirements.txt index fc236b060c0c..92ee26f9fc75 100644 --- a/sdk/streamanalytics/azure-mgmt-streamanalytics/dev_requirements.txt +++ b/sdk/streamanalytics/azure-mgmt-streamanalytics/dev_requirements.txt @@ -1,2 +1,3 @@ aiohttp>=3.0; python_version >= '3.5' --e ../../../tools/azure-devtools \ No newline at end of file +-e ../../../tools/azure-devtools +-e ../../../tools/azure-sdk-tools \ No newline at end of file diff --git a/tools/azure-sdk-tools/devtools_testutils/__init__.py b/tools/azure-sdk-tools/devtools_testutils/__init__.py index 02d2e7791178..632e60c2d2d2 100644 --- a/tools/azure-sdk-tools/devtools_testutils/__init__.py +++ b/tools/azure-sdk-tools/devtools_testutils/__init__.py @@ -18,6 +18,7 @@ # cSpell:disable from .envvariable_loader import EnvironmentVariableLoader PowerShellPreparer = EnvironmentVariableLoader # Backward compat +from .proxy_fixtures import recorded_test, variable_recorder from .proxy_startup import start_test_proxy, stop_test_proxy, test_proxy from .proxy_testcase import recorded_by_proxy from .sanitizers import ( @@ -66,12 +67,14 @@ "PowerShellPreparer", "EnvironmentVariableLoader", "recorded_by_proxy", + "recorded_test", "test_proxy", "set_bodiless_matcher", "set_custom_default_matcher", "set_default_settings", "start_test_proxy", "stop_test_proxy", + "variable_recorder", "ResponseCallback", "RetryCounter", "FakeTokenCredential", diff --git a/tools/azure-sdk-tools/devtools_testutils/proxy_fixtures.py b/tools/azure-sdk-tools/devtools_testutils/proxy_fixtures.py new file mode 100644 index 000000000000..acdddc30fa17 --- /dev/null +++ b/tools/azure-sdk-tools/devtools_testutils/proxy_fixtures.py @@ -0,0 +1,233 @@ +# ------------------------------------------------------------------------- +# Copyright (c) Microsoft Corporation. All rights reserved. +# Licensed under the MIT License. See License.txt in the project root for +# license information. +# -------------------------------------------------------------------------- +from inspect import iscoroutinefunction +import logging +from typing import TYPE_CHECKING +import urllib.parse as url_parse + +import pytest + +from azure.core.exceptions import ResourceNotFoundError +from azure.core.pipeline.policies import ContentDecodePolicy + +# the functions we patch +from azure.core.pipeline.transport import RequestsTransport + +from .helpers import get_test_id, is_live_and_not_recording +from .proxy_testcase import start_record_or_playback, stop_record_or_playback, transform_request +from .proxy_startup import test_proxy + +if TYPE_CHECKING: + from typing import Any, Callable, Dict, Optional, Tuple + from pytest import FixtureRequest + + +@pytest.fixture +async def recorded_test(test_proxy: None, request: "FixtureRequest") -> "Dict[str, Any]": + """Fixture that redirects network requests to target the azure-sdk-tools test proxy. + + Use with recorded tests. For more details and usage examples, refer to + https://github.com/Azure/azure-sdk-for-python/blob/main/doc/dev/test_proxy_migration_guide.md. + + :param test_proxy: The fixture responsible for starting up the test proxy server. + :type test_proxy: None + :param request: The built-in `request` fixture. + :type request: ~pytest.FixtureRequest + + :yields: A dictionary containing information relevant to the currently executing test. + """ + + test_id, recording_id, variables = start_proxy_session() + + # True if the function requesting the fixture is an async test + if iscoroutinefunction(request._pyfuncitem.function): + original_transport_func = await redirect_async_traffic(recording_id) + yield {"variables": variables} # yield relevant test info and allow tests to run + restore_async_traffic(original_transport_func, request) + else: + original_transport_func = redirect_traffic(recording_id) + yield {"variables": variables} # yield relevant test info and allow tests to run + restore_traffic(original_transport_func, request) + + stop_record_or_playback(test_id, recording_id, variables) + + +@pytest.fixture +def variable_recorder(recorded_test: "Dict[str, Any]") -> "Dict[str, str]": + """Fixture that invokes the `recorded_test` fixture and returns a dictionary of recorded test variables. + + :param recorded_test: The fixture responsible for redirecting network traffic to target the test proxy. + This should return a dictionary containing information about the current test -- in particular, the variables + that were recorded with the test. + :type recorded_test: Dict[str, Any] + + :returns: A dictionary that maps test variables to string values. If no variable dictionary was stored when the test + was recorded, this returns an empty dictionary. + """ + return VariableRecorder(recorded_test["variables"]) + + +# ----------HELPERS---------- + + +class VariableRecorder(): + def __init__(self, variables: "Dict[str, str]") -> None: + self.variables = variables + + def get_or_record(self, variable: str, default: str) -> str: + """Returns the recorded value of `variable`, or records and returns `default` as the value for `variable`. + + In recording mode, `get_or_record("a", "b")` will record "b" for the value of the variable `a` and return "b". + In playback, it will return the recorded value of `a`. This is an analogue of a Python dictionary's `setdefault` + method: https://docs.python.org/library/stdtypes.html#dict.setdefault. + + :param str variable: The name of the variable to search the value of, or record a value for. + :param str default: The variable value to record. + + :returns: str + """ + if not isinstance(default, str): + raise ValueError('"default" must be a string. The test proxy cannot record non-string variable values.') + return self.variables.setdefault(variable, default) + + +def start_proxy_session() -> "Optional[Tuple[str, str, Dict[str, str]]]": + """Begins a playback or recording session and returns the current test ID, recording ID, and recorded variables. + + :returns: A tuple, (a, b, c), where a is the test ID, b is the recording ID, and c is the `variables` dictionary + that maps test variables to string values. If no variable dictionary was stored when the test was recorded, c is + an empty dictionary. If the current test session is live but recording is disabled, this returns None. + """ + if is_live_and_not_recording(): + return + + test_id = get_test_id() + recording_id, variables = start_record_or_playback(test_id) + return (test_id, recording_id, variables) + + +async def redirect_async_traffic(recording_id: str) -> "Callable": + """Redirects asynchronous network requests to target the test proxy. + + :param str recording_id: Recording ID of the currently executing test. + + :returns: The original transport function used by the currently executing test. + """ + from azure.core.pipeline.transport import AioHttpTransport + + original_transport_func = AioHttpTransport.send + + def transform_args(*args, **kwargs): + copied_positional_args = list(args) + request = copied_positional_args[1] + + transform_request(request, recording_id) + + return tuple(copied_positional_args), kwargs + + async def combined_call(*args, **kwargs): + adjusted_args, adjusted_kwargs = transform_args(*args, **kwargs) + result = await original_transport_func(*adjusted_args, **adjusted_kwargs) + + # make the x-recording-upstream-base-uri the URL of the request + # this makes the request look like it was made to the original endpoint instead of to the proxy + # without this, things like LROPollers can get broken by polling the wrong endpoint + parsed_result = url_parse.urlparse(result.request.url) + upstream_uri = url_parse.urlparse(result.request.headers["x-recording-upstream-base-uri"]) + upstream_uri_dict = {"scheme": upstream_uri.scheme, "netloc": upstream_uri.netloc} + original_target = parsed_result._replace(**upstream_uri_dict).geturl() + + result.request.url = original_target + return result + + AioHttpTransport.send = combined_call + return original_transport_func + + +def redirect_traffic(recording_id: str) -> "Callable": + """Redirects network requests to target the test proxy. + + :param str recording_id: Recording ID of the currently executing test. + + :returns: The original transport function used by the currently executing test. + """ + original_transport_func = RequestsTransport.send + + def transform_args(*args, **kwargs): + copied_positional_args = list(args) + http_request = copied_positional_args[1] + + transform_request(http_request, recording_id) + + return tuple(copied_positional_args), kwargs + + def combined_call(*args, **kwargs): + adjusted_args, adjusted_kwargs = transform_args(*args, **kwargs) + result = original_transport_func(*adjusted_args, **adjusted_kwargs) + + # make the x-recording-upstream-base-uri the URL of the request + # this makes the request look like it was made to the original endpoint instead of to the proxy + # without this, things like LROPollers can get broken by polling the wrong endpoint + parsed_result = url_parse.urlparse(result.request.url) + upstream_uri = url_parse.urlparse(result.request.headers["x-recording-upstream-base-uri"]) + upstream_uri_dict = {"scheme": upstream_uri.scheme, "netloc": upstream_uri.netloc} + original_target = parsed_result._replace(**upstream_uri_dict).geturl() + + result.request.url = original_target + return result + + RequestsTransport.send = combined_call + return original_transport_func + + +def restore_async_traffic(original_transport_func: "Callable", request: "FixtureRequest") -> None: + """Resets asynchronous network traffic to no longer target the test proxy. + + :param original_transport_func: The original transport function used by the currently executing test. + :type original_transport_func: Callable + :param request: The built-in `request` pytest fixture. + :type request: ~pytest.FixtureRequest + """ + from azure.core.pipeline.transport import AioHttpTransport + + AioHttpTransport.send = original_transport_func # test finished running -- tear down + + if hasattr(request.node, "test_error"): + # Exceptions are logged here instead of being raised because of how pytest handles error raising from inside + # fixtures and hooks. Raising from a fixture raises an error in addition to the test failure report, and the + # test proxy error is logged before the test failure output (making it difficult to find in pytest output). + # Raising from a hook isn't allowed, and produces an internal error that disrupts test execution. + # ResourceNotFoundErrors during playback indicate a recording mismatch + error = request.node.test_error + if isinstance(error, ResourceNotFoundError): + error_body = ContentDecodePolicy.deserialize_from_http_generics(error.response) + message = error_body.get("message") or error_body.get("Message") + logger = logging.getLogger() + logger.error(f"\n\n-----Test proxy playback error:-----\n\n{message}") + + +def restore_traffic(original_transport_func: "Callable", request: "FixtureRequest") -> None: + """Resets network traffic to no longer target the test proxy. + + :param original_transport_func: The original transport function used by the currently executing test. + :type original_transport_func: Callable + :param request: The built-in `request` pytest fixture. + :type request: ~pytest.FixtureRequest + """ + RequestsTransport.send = original_transport_func # test finished running -- tear down + + if hasattr(request.node, "test_error"): + # Exceptions are logged here instead of being raised because of how pytest handles error raising from inside + # fixtures and hooks. Raising from a fixture raises an error in addition to the test failure report, and the + # test proxy error is logged before the test failure output (making it difficult to find in pytest output). + # Raising from a hook isn't allowed, and produces an internal error that disrupts test execution. + # ResourceNotFoundErrors during playback indicate a recording mismatch + error = request.node.test_error + if isinstance(error, ResourceNotFoundError): + error_body = ContentDecodePolicy.deserialize_from_http_generics(error.response) + message = error_body.get("message") or error_body.get("Message") + logger = logging.getLogger() + logger.error(f"\n\n-----Test proxy playback error:-----\n\n{message}") diff --git a/tools/azure-sdk-tools/devtools_testutils/proxy_startup.py b/tools/azure-sdk-tools/devtools_testutils/proxy_startup.py index 47e1329c0985..a98c3a5881c9 100644 --- a/tools/azure-sdk-tools/devtools_testutils/proxy_startup.py +++ b/tools/azure-sdk-tools/devtools_testutils/proxy_startup.py @@ -37,8 +37,7 @@ TOOL_ENV_VAR = "PROXY_PID" -def get_image_tag(): - # type: () -> str +def get_image_tag() -> str: """Gets the test proxy Docker image tag from the target_version.txt file in /eng/common/testproxy""" version_file_location = os.path.relpath("eng/common/testproxy/target_version.txt") version_file_location_from_root = os.path.abspath(os.path.join(REPO_ROOT, version_file_location)) @@ -62,8 +61,7 @@ def get_image_tag(): return image_tag -def get_container_info(): - # type: () -> Optional[dict] +def get_container_info() -> "Optional[dict]": """Returns a dictionary containing the test proxy container's information, or None if the container isn't present""" proc = subprocess.Popen( shlex.split("docker container ls -a --format '{{json .}}' --filter name=" + CONTAINER_NAME), @@ -82,23 +80,21 @@ def get_container_info(): return None -def check_availability(): - # type: () -> None +def check_availability() -> None: """Attempts request to /Info/Available. If a test-proxy instance is responding, we should get a response.""" try: response = requests.get(PROXY_CHECK_URL, timeout=60) return response.status_code # We get an SSLError if the container is started but the endpoint isn't available yet except requests.exceptions.SSLError as sslError: - _LOGGER.error(sslError) + _LOGGER.debug(sslError) return 404 except Exception as e: _LOGGER.error(e) return 404 -def check_proxy_availability(): - # type: () -> None +def check_proxy_availability() -> None: """Waits for the availability of the test-proxy.""" start = time.time() now = time.time() @@ -108,8 +104,7 @@ def check_proxy_availability(): now = time.time() -def create_container(): - # type: () -> None +def create_container() -> None: """Creates the test proxy Docker container""" # Most of the time, running this script on a Windows machine will work just fine, as Docker defaults to Linux # containers. However, in CI, Windows images default to _Windows_ containers. We cannot swap them. We can tell @@ -134,8 +129,7 @@ def create_container(): proc.communicate() -def start_test_proxy(): - # type: () -> None +def start_test_proxy() -> None: """Starts the test proxy and returns when the proxy server is ready to receive requests. In regular use cases, this will auto-start the test-proxy docker container. In CI, or when environment variable TF_BUILD is set, this function will start the test-proxy .NET tool.""" @@ -186,8 +180,7 @@ def start_test_proxy(): set_custom_default_matcher(excluded_headers=headers_to_ignore) -def stop_test_proxy(): - # type: () -> None +def stop_test_proxy() -> None: """Stops any running instance of the test proxy""" if not PROXY_MANUALLY_STARTED: @@ -213,7 +206,7 @@ def stop_test_proxy(): @pytest.fixture(scope="session") -def test_proxy(): +def test_proxy() -> None: """Pytest fixture to be used before running any tests that are recorded with the test proxy""" if is_live_and_not_recording(): yield diff --git a/tools/azure-sdk-tools/devtools_testutils/proxy_testcase.py b/tools/azure-sdk-tools/devtools_testutils/proxy_testcase.py index 88b2a1f311f4..a0b49718b4d2 100644 --- a/tools/azure-sdk-tools/devtools_testutils/proxy_testcase.py +++ b/tools/azure-sdk-tools/devtools_testutils/proxy_testcase.py @@ -6,20 +6,12 @@ import logging import requests import six -import sys from typing import TYPE_CHECKING - -try: - # py3 - import urllib.parse as url_parse -except: - # py2 - import urlparse as url_parse - -import pytest +import urllib.parse as url_parse from azure.core.exceptions import HttpResponseError, ResourceNotFoundError from azure.core.pipeline.policies import ContentDecodePolicy + # the functions we patch from azure.core.pipeline.transport import RequestsTransport @@ -27,10 +19,10 @@ from azure_devtools.scenario_tests.utilities import trim_kwargs_from_test_function from .config import PROXY_URL from .helpers import get_test_id, is_live, is_live_and_not_recording, set_recording_id -from .sanitizers import add_remove_header_sanitizer, set_custom_default_matcher if TYPE_CHECKING: - from typing import Tuple + from typing import Callable, Dict, Tuple + from azure.core.pipeline.transport import HttpRequest # To learn about how to migrate SDK tests to the test proxy, please refer to the migration guide at # https://github.com/Azure/azure-sdk-for-python/blob/main/doc/dev/test_proxy_migration_guide.md @@ -43,10 +35,9 @@ PLAYBACK_STOP_URL = "{}/playback/stop".format(PROXY_URL) -def start_record_or_playback(test_id): - # type: (str) -> Tuple(str, dict) +def start_record_or_playback(test_id: str) -> "Tuple[str, Dict[str, str]]": """Sends a request to begin recording or playing back the provided test. - + This returns a tuple, (a, b), where a is the recording ID of the test and b is the `variables` dictionary that maps test variables to values. If no variable dictionary was stored when the test was recorded, b is an empty dictionary. """ @@ -88,32 +79,38 @@ def start_record_or_playback(test_id): return (recording_id, variables) -def stop_record_or_playback(test_id, recording_id, test_output): - # type: (str, str, dict) -> None +def stop_record_or_playback(test_id: str, recording_id: str, test_variables: "Dict[str, str]") -> None: if is_live(): - requests.post( + response = requests.post( RECORDING_STOP_URL, headers={ "x-recording-file": test_id, "x-recording-id": recording_id, "x-recording-save": "true", - "Content-Type": "application/json" + "Content-Type": "application/json", }, - json=test_output or {} # tests don't record successfully unless test_output is a dictionary + json=test_variables or {}, # tests don't record successfully unless test_variables is a dictionary ) else: - requests.post( + response = requests.post( PLAYBACK_STOP_URL, headers={"x-recording-id": recording_id}, ) + try: + response.raise_for_status() + except requests.HTTPError as e: + raise HttpResponseError( + "The test proxy ran into an error while ending the session. Make sure any test variables you record have " + "string values." + ) from e -def get_proxy_netloc(): +def get_proxy_netloc() -> "Dict[str, str]": parsed_result = url_parse.urlparse(PROXY_URL) return {"scheme": parsed_result.scheme, "netloc": parsed_result.netloc} -def transform_request(request, recording_id): +def transform_request(request: "HttpRequest", recording_id: str) -> None: """Redirect the request to the test proxy, and store the original request URI in a header""" headers = request.headers @@ -126,7 +123,7 @@ def transform_request(request, recording_id): request.url = updated_target -def recorded_by_proxy(test_func): +def recorded_by_proxy(test_func: "Callable") -> None: """Decorator that redirects network requests to target the azure-sdk-tools test proxy. Use with recorded tests. For more details and usage examples, refer to @@ -134,9 +131,6 @@ def recorded_by_proxy(test_func): """ def record_wrap(*args, **kwargs): - if sys.version_info.major == 2 and not is_live(): - pytest.skip("Playback testing is incompatible with the azure-sdk-tools test proxy on Python 2") - def transform_args(*args, **kwargs): copied_positional_args = list(args) request = copied_positional_args[1] @@ -173,18 +167,18 @@ def combined_call(*args, **kwargs): RequestsTransport.send = combined_call # call the modified function - # we define test_output before invoking the test so the variable is defined in case of an exception - test_output = None + # we define test_variables before invoking the test so the variable is defined in case of an exception + test_variables = None try: try: - test_output = test_func(*args, variables=variables, **trimmed_kwargs) + test_variables = test_func(*args, variables=variables, **trimmed_kwargs) except TypeError: logger = logging.getLogger() logger.info( "This test can't accept variables as input. The test method should accept `**kwargs` and/or a " "`variables` parameter to make use of recorded test variables." ) - test_output = test_func(*args, **trimmed_kwargs) + test_variables = test_func(*args, **trimmed_kwargs) except ResourceNotFoundError as error: error_body = ContentDecodePolicy.deserialize_from_http_generics(error.response) message = error_body.get("message") or error_body.get("Message") @@ -192,8 +186,8 @@ def combined_call(*args, **kwargs): six.raise_from(error_with_message, error) finally: RequestsTransport.send = original_transport_func - stop_record_or_playback(test_id, recording_id, test_output) + stop_record_or_playback(test_id, recording_id, test_variables) - return test_output + return test_variables return record_wrap