From d76096ff3f8cf69540d9cadb533736153a6af965 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?McCoy=20Pati=C3=B1o?= Date: Wed, 6 Jul 2022 20:23:59 -0700 Subject: [PATCH] wip; Repo-wide recording and variables fixtures --- sdk/conftest.py | 138 +++++++++++++++++- .../azure-data-tables/tests/preparers.py | 19 +++ .../tests/test_table_batch.py | 27 ++-- .../devtools_testutils/__init__.py | 3 +- .../devtools_testutils/proxy_startup.py | 25 ++-- .../devtools_testutils/proxy_testcase.py | 48 +----- 6 files changed, 184 insertions(+), 76 deletions(-) diff --git a/sdk/conftest.py b/sdk/conftest.py index f4cdaaade7ce..9f91a4ec35da 100644 --- a/sdk/conftest.py +++ b/sdk/conftest.py @@ -23,8 +23,23 @@ # IN THE SOFTWARE. # # -------------------------------------------------------------------------- +import logging import os import pytest +import sys +from typing import TYPE_CHECKING +import urllib.parse as url_parse + +from azure.core.exceptions import ResourceNotFoundError +from azure.core.pipeline.policies import ContentDecodePolicy +from azure.core.pipeline.transport import RequestsTransport +from devtools_testutils import test_proxy +from devtools_testutils.helpers import get_test_id, is_live, is_live_and_not_recording +from devtools_testutils.proxy_testcase import start_record_or_playback, stop_record_or_playback, transform_request + +if TYPE_CHECKING: + from typing import Any, Optional + def pytest_configure(config): # register an additional marker @@ -55,4 +70,125 @@ 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) + + +@pytest.fixture +def start_proxy_session() -> "Optional[tuple[str, str, dict[str, Any]]]": + """Begins a playback or recording session and returns the current test ID, recording ID, and recorded variables. + + This 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 values. If no variable dictionary was stored when the test was recorded, c is an empty + dictionary. + """ + 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") + + 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) + + +@pytest.fixture +def recorded_test(test_proxy, start_proxy_session, request) -> "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. + """ + test_id, recording_id, variables = start_proxy_session + 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 + + # store info pertinent to the test in a dictionary that other fixtures can access + variable_recorder = VariableRecorder(variables) + test_info = {"test_id": test_id, "variables": variable_recorder} + yield test_info # yield and allow test to run + + 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}") + + stop_record_or_playback(test_id, recording_id, variables) + + +@pytest.fixture +def variable_recorder(recorded_test) -> "dict[str, Any]": + """Fixture that invokes the `recorded_test` fixture and returns a dictionary of recorded test variables. + + The dictionary returned by this fixture maps test variables to values. If no variable dictionary was stored when the + test was recorded, this returns an empty dictionary. + """ + yield recorded_test["variables"] + + +class VariableRecorder(): + """Interface for fetching recorded test variables and recording new variables.""" + + def __init__(self, variables: "dict[str, Any]") -> None: + self.variables = variables + + def get(self, name: str) -> "Any": + """Returns the value of the recorded variable with the provided name. + + :param str name: The name of the recorded variable. For example, "vault_name". + """ + return self.variables.get(name) + + def record(self, variables: "dict[str, Any]") -> None: + """Records the provided variables in the test recording, making them available for future playback. + + :param variables: A dictionary mapping variable names to their values. + :type variables: dict[str, Any] + """ + self.variables = variables diff --git a/sdk/tables/azure-data-tables/tests/preparers.py b/sdk/tables/azure-data-tables/tests/preparers.py index d4da5573ccfb..0ffab549799f 100644 --- a/sdk/tables/azure-data-tables/tests/preparers.py +++ b/sdk/tables/azure-data-tables/tests/preparers.py @@ -53,6 +53,25 @@ def wrapper(*args, **kwargs): return wrapper +def tables_decorator_with_wraps(func, **kwargs): + @TablesPreparer() + @functools.wraps(func) + def wrapper(*args, **kwargs): + key = kwargs.pop("tables_primary_storage_account_key") + name = kwargs.pop("tables_storage_account_name") + key = AzureNamedKeyCredential(key=key, name=name) + + kwargs["tables_primary_storage_account_key"] = key + kwargs["tables_storage_account_name"] = name + + trimmed_kwargs = {k: v for k, v in kwargs.items()} + trim_kwargs_from_test_function(func, trimmed_kwargs) + + func(*args, **trimmed_kwargs) + + return wrapper + + def cosmos_decorator(func, **kwargs): @CosmosPreparer() def wrapper(*args, **kwargs): diff --git a/sdk/tables/azure-data-tables/tests/test_table_batch.py b/sdk/tables/azure-data-tables/tests/test_table_batch.py index 768f446c1a8a..27e04dfccd44 100644 --- a/sdk/tables/azure-data-tables/tests/test_table_batch.py +++ b/sdk/tables/azure-data-tables/tests/test_table_batch.py @@ -6,6 +6,7 @@ # license information. # -------------------------------------------------------------------------- +from multiprocessing.sharedctypes import Value import pytest from datetime import datetime, timedelta @@ -37,7 +38,7 @@ ) from _shared.testcase import TableTestCase -from preparers import tables_decorator +from preparers import tables_decorator, tables_decorator_with_wraps #------------------------------------------------------------------------------ TEST_TABLE_PREFIX = 'table' @@ -80,6 +81,7 @@ def test_batch_single_insert(self, tables_storage_account_name, tables_primary_s assert e['test4'] == entity['test4'].value finally: self._tear_down() + recorded_test.variables = {"test": False} @pytest.mark.skipif(sys.version_info < (3, 0), reason="requires Python3") @tables_decorator @@ -249,9 +251,9 @@ def test_batch_update_if_match(self, tables_storage_account_name, tables_primary @recorded_by_proxy def test_batch_update_if_doesnt_match(self, tables_storage_account_name, tables_primary_storage_account_key): # this can be reverted to set_bodiless_matcher() after tests are re-recorded and don't contain these headers - set_custom_default_matcher( - compare_bodies=False, excluded_headers="Authorization,Content-Length,x-ms-client-request-id,x-ms-request-id" - ) + # set_custom_default_matcher( + # compare_bodies=False, excluded_headers="Authorization,Content-Length,x-ms-client-request-id,x-ms-request-id" + # ) # Arrange self._set_up(tables_storage_account_name, tables_primary_storage_account_key) @@ -279,13 +281,18 @@ def test_batch_update_if_doesnt_match(self, tables_storage_account_name, tables_ self._tear_down() @pytest.mark.skipif(sys.version_info < (3, 0), reason="requires Python3") - @tables_decorator - @recorded_by_proxy - def test_batch_single_op_if_doesnt_match(self, tables_storage_account_name, tables_primary_storage_account_key): + @tables_decorator_with_wraps + def test_batch_single_op_if_doesnt_match(self, variable_recorder, tables_storage_account_name=None, tables_primary_storage_account_key=None): # this can be reverted to set_bodiless_matcher() after tests are re-recorded and don't contain these headers - set_custom_default_matcher( - compare_bodies=False, excluded_headers="Authorization,Content-Length,x-ms-client-request-id,x-ms-request-id" - ) + # set_custom_default_matcher( + # compare_bodies=False, excluded_headers="Authorization,Content-Length,x-ms-client-request-id,x-ms-request-id" + # ) + + # Above section is intentionally commented to trigger a playback error + variables = variable_recorder.variables if self.is_live else {"variable_name": "value"} + particular_variable = variable_recorder.get("variable_name") + ... + variable_recorder.record(variables) # Arrange self._set_up(tables_storage_account_name, tables_primary_storage_account_key) diff --git a/tools/azure-sdk-tools/devtools_testutils/__init__.py b/tools/azure-sdk-tools/devtools_testutils/__init__.py index 44a865a73d92..02d2e7791178 100644 --- a/tools/azure-sdk-tools/devtools_testutils/__init__.py +++ b/tools/azure-sdk-tools/devtools_testutils/__init__.py @@ -19,7 +19,7 @@ from .envvariable_loader import EnvironmentVariableLoader PowerShellPreparer = EnvironmentVariableLoader # Backward compat from .proxy_startup import start_test_proxy, stop_test_proxy, test_proxy -from .proxy_testcase import recorded_by_proxy, recorded_test +from .proxy_testcase import recorded_by_proxy from .sanitizers import ( add_body_key_sanitizer, add_body_regex_sanitizer, @@ -66,7 +66,6 @@ "PowerShellPreparer", "EnvironmentVariableLoader", "recorded_by_proxy", - "recorded_test", "test_proxy", "set_bodiless_matcher", "set_custom_default_matcher", diff --git a/tools/azure-sdk-tools/devtools_testutils/proxy_startup.py b/tools/azure-sdk-tools/devtools_testutils/proxy_startup.py index e7a62876062a..525d784e99df 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 25c4af56580d..57ebc316bf37 100644 --- a/tools/azure-sdk-tools/devtools_testutils/proxy_testcase.py +++ b/tools/azure-sdk-tools/devtools_testutils/proxy_testcase.py @@ -7,7 +7,6 @@ import requests import six import sys -from typing import TYPE_CHECKING try: # py3 @@ -29,9 +28,6 @@ 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 - # 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,8 +39,7 @@ 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]: """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 @@ -197,44 +192,3 @@ def combined_call(*args, **kwargs): return test_output return record_wrap - - -@pytest.fixture -def recorded_test(request): - 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] - - transform_request(request, recording_id) - - return tuple(copied_positional_args), kwargs - - if is_live_and_not_recording(): - return - - test_id = get_test_id() - recording_id, variables = start_record_or_playback(test_id) - original_transport_func = RequestsTransport.send - - 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 - yield # test gets run here - RequestsTransport.send = original_transport_func # test finished running -- tear down - stop_record_or_playback(test_id, recording_id, None) # TODO: how do we provide variables to record?