From 9c411bff1f7b6aacf9d0e78bdd73ecfbdbc92b24 Mon Sep 17 00:00:00 2001 From: Thibaut Le Page Date: Sat, 16 Feb 2019 10:47:04 +0100 Subject: [PATCH 1/9] update plugin sanitize_headers: be OnTask instead of OnTaskSequence This gets us closer to replacing Task2 with Task globally, and simplifies the plugin's code. Signed-off-by: Thibaut Le Page --- transformer/plugins/sanitize_headers.md | 20 +++-- transformer/plugins/sanitize_headers.py | 44 ++++----- transformer/plugins/test_sanitize_headers.py | 94 ++++++++------------ 3 files changed, 63 insertions(+), 95 deletions(-) diff --git a/transformer/plugins/sanitize_headers.md b/transformer/plugins/sanitize_headers.md index 10b8959..a976e89 100644 --- a/transformer/plugins/sanitize_headers.md +++ b/transformer/plugins/sanitize_headers.md @@ -1,11 +1,12 @@ # Sanitizing headers -The [`sanitize_headers` plugin](sanitize_headers.py) should be used for processing scenarios that were generated -in the Chrome browser, but is advised to be used whenever cookies handling is important. - -The plugin removes Chrome-specific, RFC non-compliant headers starting with ":". +The [`sanitize_headers` plugin](sanitize_headers.py) should be used for +processing scenarios generated in the Chrome browser, but is also advised to +use it whenever cookies handling is important. -Example of such headers: +The plugin removes Chrome-specific, RFC-non-compliant headers starting with `:`. + +Examples of such headers: ``` :authority: chrome.google.com :method: POST @@ -13,8 +14,9 @@ Example of such headers: :scheme: https ``` -Additionally the plugin: -- maps header keys to lowercase, which makes further overriding of headers deterministic, -- ignores the `cookie` header, as cookies are handled by [Locust's `HttpSession`][http-session]. +Additionally, the plugin: + - converts header names to lowercase, which simplifies further header overriding, + - ignores the `cookie` header, as cookies are handled by + [Locust's _HttpSession_][http-session]. -[http-session]: https://docs.locust.io/en/stable/api.html#httpsession-class \ No newline at end of file +[http-session]: https://docs.locust.io/en/stable/api.html#httpsession-class diff --git a/transformer/plugins/sanitize_headers.py b/transformer/plugins/sanitize_headers.py index 48b4562..3990720 100644 --- a/transformer/plugins/sanitize_headers.py +++ b/transformer/plugins/sanitize_headers.py @@ -1,38 +1,24 @@ -from typing import Sequence - -from transformer.task import Task, LocustRequest from transformer.helpers import zip_kv_pairs +from transformer.task import Task2 -def plugin(tasks: Sequence[Task]) -> Sequence[Task]: +def plugin(task: Task2) -> Task2: """ - Removes Chrome-specific, RFC non-compliant headers starting with ":". - Maps header keys to lowercase to make overriding deterministic. - Removes cookie header as it is handled by Locust's HttpSession. + Removes Chrome-specific, RFC-non-compliant headers starting with `:`. + Converts header names to lowercase to simplify further overriding. + Removes the cookie header as it is handled by Locust's HttpSession. """ - modified_tasks = [] - for task in tasks: - - if task.locust_request is None: - task = task._replace( - locust_request=LocustRequest.from_request(task.request) - ) - - headers = task.locust_request.headers - - if not isinstance(headers, dict): - headers = zip_kv_pairs(headers) + headers = task.request.headers - sanitized_headers = { - k.lower(): v - for (k, v) in headers.items() - if not k.startswith(":") and k.lower() != "cookie" - } + if not isinstance(headers, dict): + headers = zip_kv_pairs(headers) - task = task._replace( - locust_request=task.locust_request._replace(headers=sanitized_headers) - ) + sanitized_headers = { + k.lower(): v + for (k, v) in headers.items() + if not k.startswith(":") and k.lower() != "cookie" + } - modified_tasks.append(task) + task.request = task.request._replace(headers=sanitized_headers) - return modified_tasks + return task diff --git a/transformer/plugins/test_sanitize_headers.py b/transformer/plugins/test_sanitize_headers.py index b69a656..74d3641 100644 --- a/transformer/plugins/test_sanitize_headers.py +++ b/transformer/plugins/test_sanitize_headers.py @@ -1,74 +1,54 @@ # pylint: skip-file -from unittest.mock import MagicMock -from transformer.task import Task, LocustRequest -from transformer.request import HttpMethod, Header +from datetime import datetime +from urllib.parse import urlparse + +from transformer.request import HttpMethod, Header, Request +from transformer.task import Task2 from .sanitize_headers import plugin +def test_its_name_is_resolvable(): + from transformer.plugins import resolve + + assert list(resolve("transformer.plugins.sanitize_headers")) == [plugin] + + +TS = datetime(1970, 1, 1) + + +def task_with_header(name: str, value: str) -> Task2: + return Task2( + name="some task", + request=Request( + timestamp=TS, + method=HttpMethod.GET, + url=urlparse("https://example.com"), + headers=[Header(name=name, value=value)], + post_data={}, + query=[], + ), + ) + + def test_it_removes_headers_beginning_with_a_colon(): - tasks = [ - Task( - name="some task", - request=None, - locust_request=LocustRequest( - method=HttpMethod.GET, - url="", - headers=[Header(name=":non-rfc-header", value="some value")], - ), - ) - ] - sanitized_headers = plugin(tasks)[0].locust_request.headers + task = task_with_header(":non-rfc-header", "some value") + sanitized_headers = plugin(task).request.headers assert len(sanitized_headers) == 0 def test_it_downcases_header_names(): - tasks = [ - Task( - name="some task", - request=None, - locust_request=LocustRequest( - method=HttpMethod.GET, - url="", - headers=[Header(name="Some Name", value="some value")], - ), - ) - ] - sanitized_headers = plugin(tasks)[0].locust_request.headers + task = task_with_header("Some Name", "some value") + sanitized_headers = plugin(task).request.headers assert "some name" in sanitized_headers def test_it_removes_cookies(): - tasks = [ - Task( - name="someTask", - request=None, - locust_request=LocustRequest( - method=HttpMethod.GET, - url="", - headers=[Header(name="cookie", value="some value")], - ), - ) - ] - sanitized_headers = plugin(tasks)[0].locust_request.headers + task = task_with_header("Cookie", "some value") + sanitized_headers = plugin(task).request.headers assert len(sanitized_headers) == 0 -def test_it_does_not_remove_other_headers(): - tasks = [ - Task( - name="someTask", - request=None, - locust_request=LocustRequest( - method=HttpMethod.GET, - url="", - headers=[Header(name="some other header", value="some value")], - ), - ) - ] - sanitized_headers = plugin(tasks)[0].locust_request.headers +def test_it_does_not_change_nor_remove_other_headers(): + task = task_with_header("some other header", "some value") + sanitized_headers = plugin(task).request.headers assert len(sanitized_headers) == 1 - - -def test_it_creates_a_locust_request_if_none_exist(): - tasks = [Task(name="some task", request=MagicMock())] - assert plugin(tasks)[0].locust_request From 8d2f00882bc221192d6045d6bbc622c0bbaf2751 Mon Sep 17 00:00:00 2001 From: Thibaut Le Page Date: Tue, 19 Feb 2019 22:19:46 +0100 Subject: [PATCH 2/9] pyproject.toml: fix script entrypoint specification Signed-off-by: Thibaut Le Page --- pyproject.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyproject.toml b/pyproject.toml index 13cf055..b33de7d 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -23,7 +23,7 @@ classifiers=[ packages = [{ include = "transformer" }] [tool.poetry.scripts] -transformer = "transformer:cli.script_entrypoint" +transformer = "transformer.cli:script_entrypoint" [tool.poetry.dependencies] python = "^3.6" From 7ae15c9bd9f3f5dad37e39cac6c8efe5605111d2 Mon Sep 17 00:00:00 2001 From: Thibaut Le Page Date: Tue, 19 Feb 2019 22:21:16 +0100 Subject: [PATCH 3/9] cli: use dump, better error reporting Signed-off-by: Thibaut Le Page --- transformer/cli.py | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/transformer/cli.py b/transformer/cli.py index 759d5a2..f1a4f20 100644 --- a/transformer/cli.py +++ b/transformer/cli.py @@ -11,6 +11,8 @@ --help Print this help message and exit. -p, --plugin= Use the specified plugin. Repeatable. --version Print version information and exit. + +Documentation & code: https://github.com/zalando-incubator/transformer """ import logging import sys @@ -20,10 +22,7 @@ import ecological from docopt import docopt -from transformer import __version__ -from transformer import plugins as plug -from transformer.locust import locustfile -from transformer.scenario import Scenario +from transformer import __version__, dump class Config(ecological.AutoConfig, prefix="transformer"): @@ -86,6 +85,12 @@ def script_entrypoint() -> None: logging.info("Otherwise, here is the command-line manual:") print(__doc__, file=sys.stderr) exit(1) - plugins = tuple(p for name in config.plugins for p in plug.resolve(name)) - scenarios = [Scenario.from_path(path, plugins) for path in config.input_paths] - print(str(locustfile(scenarios))) + try: + dump(file=sys.stdout, scenario_paths=config.input_paths, plugins=config.plugins) + except ImportError as err: + logging.error(f"Failed loading plugins: {err}") + exit(2) + except Exception: + url = "https://github.com/zalando-incubator/Transformer/issues" + logging.exception(f"Please help us fix this error by reporting it! {url}") + exit(3) From 4941fe35d1ecc6a1ff7bdd34945ce91db55eff47 Mon Sep 17 00:00:00 2001 From: Thibaut Le Page Date: Wed, 20 Feb 2019 11:24:49 +0100 Subject: [PATCH 4/9] implement hooks for new plugin contracts; change contract implementation Signed-off-by: Thibaut Le Page --- transformer/locust.py | 16 +- transformer/plugins/__init__.py | 3 +- transformer/plugins/contracts.py | 171 ++++++++++--------- transformer/plugins/dummy.py | 19 ++- transformer/plugins/resolve.py | 73 +++++--- transformer/plugins/sanitize_headers.py | 2 + transformer/plugins/test_contracts.py | 141 +++++++++++---- transformer/plugins/test_dummy.py | 10 +- transformer/plugins/test_resolve.py | 51 +++--- transformer/plugins/test_sanitize_headers.py | 1 - transformer/scenario.py | 57 +++++-- transformer/test_locust.py | 36 ---- transformer/test_transform.py | 12 +- transformer/transform.py | 22 ++- 14 files changed, 380 insertions(+), 234 deletions(-) diff --git a/transformer/locust.py b/transformer/locust.py index 9c2fa57..92d2fd3 100644 --- a/transformer/locust.py +++ b/transformer/locust.py @@ -2,7 +2,9 @@ import warnings from typing import Sequence, List, Union, Iterator +import transformer.plugins as plug import transformer.python as py +from transformer.plugins.contracts import Plugin from transformer.scenario import Scenario from transformer.task import Task, Task2 @@ -50,7 +52,7 @@ def locust_taskset(scenario: Scenario) -> py.Class: fields: List[py.Statement] = [] for i, child in enumerate(scenario.children, start=1): seq_decorator = f"seq_task({i})" - if isinstance(child, Task): + if isinstance(child, (Task2, Task)): fields.append(py.Decoration(seq_decorator, _locust_task(child))) elif isinstance(child, Scenario): field = py.Decoration(f"task({child.weight})", locust_taskset(child)) @@ -119,12 +121,15 @@ def locust_program(scenarios: Sequence[Scenario]) -> py.Program: ] -def locustfile_lines(scenarios: Sequence[Scenario]) -> Iterator[str]: +def locustfile_lines( + scenarios: Sequence[Scenario], program_plugins: Sequence[Plugin] +) -> Iterator[str]: """ Converts the provided scenarios into a stream of Python statements and iterate on the resulting lines. """ - for stmt in locust_program(scenarios): + program = plug.apply(program_plugins, locust_program(scenarios)) + for stmt in program: for line in stmt.lines(): yield str(line) @@ -136,8 +141,9 @@ def locustfile(scenarios: Sequence[Scenario]) -> str: This function is deprecated and will be removed in a future version. Do not rely on it. Reason: It does not provide significant value over locustfile_lines and has - a less clear name and a less flexible API. + a less clear name and a less flexible API. It does not support new + generation plugins contracts like OnPythonProgram. Deprecated since: v1.0.2. """ warnings.warn(DeprecationWarning("locustfile: use locustfile_lines instead")) - return "\n".join(locustfile_lines(scenarios)) + return "\n".join(locustfile_lines(scenarios, ())) diff --git a/transformer/plugins/__init__.py b/transformer/plugins/__init__.py index 6a4ecaf..16aa8c2 100644 --- a/transformer/plugins/__init__.py +++ b/transformer/plugins/__init__.py @@ -1,3 +1,4 @@ from .resolve import resolve +from .contracts import plugin, Contract, apply, group_by_contract -__all__ = ["resolve"] +__all__ = ["resolve", "plugin", "Contract", "apply", "group_by_contract"] diff --git a/transformer/plugins/contracts.py b/transformer/plugins/contracts.py index 98d0143..2056b6d 100644 --- a/transformer/plugins/contracts.py +++ b/transformer/plugins/contracts.py @@ -10,13 +10,8 @@ Not all types of plugins can be applied at the same point in Transformer's pipeline (e.g. python.Program objects are built much later than Task objects), hence the multiplicity of contracts. -These input and output types are formalized here using Python's annotation -syntax and the typing module. -In addition to the plugin contracts, this module provides an "isvalid" method -for checking whether arbitrary Python objects conform to a given plugin contract. - -# Plugin kinds +# Plugin contracts ## OnTask @@ -52,109 +47,131 @@ Example: a plugin that injects some code in the global scope. -# Other types +# Other contracts This module also defines: ## Plugin -Any supported kind of Transformer plugin. +Any supported contract of Transformer plugin. """ -import inspect -from typing import Sequence, Callable, Union, Dict, Any +import enum +from collections import defaultdict +from typing import Callable, NewType, Iterable, TypeVar, List, DefaultDict -from transformer import python -from transformer.decision import Decision -from transformer.task import Task, Task2 -PluginValidator = Callable[[inspect.Signature], Decision] -_PluginValidatorDecorator = Callable[[PluginValidator], PluginValidator] +class Contract(enum.Flag): + """ + Enumeration of all supported plugin contracts. Each contract defines a way + for plugins to be used in Transformer. -_PLUGIN_VALIDATORS: Dict[type, PluginValidator] = {} + Any function may become a Transformer plugin by announcing that + it implements at least one contract, using the @plugin decorator. + """ + OnTask = enum.auto() + OnScenario = enum.auto() + OnPythonProgram = enum.auto() -def _register_contract(plugin_type: type) -> None: - if plugin_type in _PLUGIN_VALIDATORS: - raise ValueError(f"{plugin_type} already registered; cannot register it again") + # Historically Transformer has only one plugin contract, which transformed a + # sequence of Task objects into another such sequence. Operating on a full list + # of tasks (instead of task by task) offered more leeway: a plugin could e.g. + # add a new task, or change only the first task. + # However this OnTaskSequence model is too constraining for some use-cases, + # e.g. when a plugin needs to inject code in the global scope, and having to + # deal with a full, immutable list of tasks in plugins that independently + # operate on each task implies a lot of verbosity and redundancy. + # For these reasons, other plugin contracts were created to offer a more + # varied choice for plugin implementers. + # See https://github.com/zalando-incubator/Transformer/issues/10. + OnTaskSequence = enum.auto() - *expected_params, expected_return = plugin_type.__args__ - def _validator(sig: inspect.Signature) -> Decision: - if sig.return_annotation != expected_return: - return Decision.no( - f"expected {expected_return}, got {sig.return_annotation}" - ) +Plugin = NewType("Plugin", callable) - actual_params = [p.annotation for p in sig.parameters.values()] - if actual_params != expected_params: - return Decision.no( - f"expected parameters {expected_params}, got {actual_params}" - ) - return Decision.yes() +class InvalidContractError(ValueError): + """ + Raised for plugin functions associated with invalid contracts. + + What an "invalid contract" represents is not strictly specified, + but this includes at least objects that are not members of the Contract + enumeration. + """ - _PLUGIN_VALIDATORS[plugin_type] = _validator +class InvalidPluginError(ValueError): + """ + Raised when trying to use as plugin a function that has not been marked + as such. + """ -OnTask = Callable[[Task2], Task2] +def plugin(c: Contract) -> Callable[[callable], callable]: + """ + Function decorator. Use it to associate a function to a Contract, making it + a Transformer plugin that will be detected by resolve(). -OnScenario = Callable[["scenario.Scenario"], "scenario.Scenario"] + :param c: the contract to associate to the decorated function. + :raise InvalidContractError: if c is not a valid contract. + """ + if not isinstance(c, Contract): + raise InvalidContractError( + f"{c!r} is not a {Contract.__qualname__}. " + "Did you mean e.g. @plugin(Contract.OnTask)?" + ) + def _decorate(f: callable) -> callable: + f._transformer_plugin_contract = c + return f -OnPythonProgram = Callable[[python.Program], python.Program] + return _decorate -# Historically Transformer has only one kind of plugin, which transformed a -# sequence of Task objects into another such sequence. Operating on a full list -# of tasks (instead of task by task) offered more leeway: a plugin could e.g. -# add a new task, or change only the first task. -# However this OnTaskSequence model is too constraining for some use-cases, -# e.g. when a plugin needs to inject code in the global scope, and having to -# deal with a full, immutable list of tasks in plugins that independently -# operate on each task implies a lot of verbosity and redundancy. -# For these reasons, other plugin kinds were created to offer a more varied -# choice for plugin implementers. -# See https://github.com/zalando-incubator/Transformer/issues/10. -OnTaskSequence = Callable[[Sequence[Task]], Sequence[Task]] +def contract(f: Plugin) -> Contract: + """ + Returns the contract associated to a plugin function. + :raise InvalidPluginError: if f is not a plugin. + """ + try: + return getattr(f, "_transformer_plugin_contract") + except AttributeError: + raise InvalidPluginError(f) from None -Plugin = Union[OnTask, OnScenario, OnPythonProgram, OnTaskSequence] -for plugin_type in Plugin.__args__: - _register_contract(plugin_type) +_T = TypeVar("_T") -def isvalid(plugin_type: type, obj: Any) -> Decision: +def apply(plugins: Iterable[Plugin], init: _T) -> _T: """ - Checks whether obj is an implementation of the plugin contract plugin_type. - The return value is basically a boolean, with an additional string - describing the reason for this decision. - - :param plugin_type: plugin contract to verify obj against - :param obj: any Python object - :return: whether obj is conform to the plugin_type contract - :raise TypeError: if plugin_type is not a plugin contract + Applies each plugin to init in order, and returns the result. + + This just wraps a very simple but common operation. """ - if plugin_type is Plugin: - return Decision.any( - (isvalid(t, obj) for t in Plugin.__args__), - "should be valid for a Plugin subtype", - ) + for p in plugins: + init = p(init) + return init - if not callable(obj): - return Decision.no(f"{obj!r} is not a function") - try: - validator = _PLUGIN_VALIDATORS[plugin_type] - except KeyError: - raise TypeError(f"no Plugin contract registered for {plugin_type}") +_BASE_CONTRACTS = ( + Contract.OnTask, + Contract.OnTaskSequence, + Contract.OnScenario, + Contract.OnPythonProgram, +) - try: - actual_signature = inspect.signature(obj) - except (ValueError, TypeError) as err: - return Decision.no(f"could not extract signature from {obj!r}: {err}") - return Decision.whether( - validator(actual_signature), f"{obj.__name__!r} should implement {plugin_type}" - ) +def group_by_contract(plugins: Iterable[Plugin]) -> DefaultDict[Contract, List[Plugin]]: + """ + Groups plugins in lists according to their contracts. + Each plugin is found in as many lists as it implements base contracts. + Lists keep the order of the original plugins iterable. + """ + res = defaultdict(list) + for p in plugins: + c = contract(p) + for bc in _BASE_CONTRACTS: + if c & bc: + res[bc].append(p) + return res diff --git a/transformer/plugins/dummy.py b/transformer/plugins/dummy.py index dca7a71..3af44eb 100644 --- a/transformer/plugins/dummy.py +++ b/transformer/plugins/dummy.py @@ -1,9 +1,20 @@ import logging -from typing import Sequence +from typing import cast +from transformer.plugins import plugin, Contract +from transformer.request import Request +from transformer.scenario import Scenario from transformer.task import Task -def plugin(tasks: Sequence[Task]) -> Sequence[Task]: - logging.info(f"The first request was {tasks[0].request.url.geturl()}") - return tasks +@plugin(Contract.OnScenario) +def f(s: Scenario) -> Scenario: + first_req = first(s) + logging.info(f"The first request was {first_req.url.geturl()}") + return s + + +def first(s: Scenario) -> Request: + while isinstance(s, Scenario): + s = s.children[0] + return cast(Task, s).request diff --git a/transformer/plugins/resolve.py b/transformer/plugins/resolve.py index 413ea90..e771a75 100644 --- a/transformer/plugins/resolve.py +++ b/transformer/plugins/resolve.py @@ -4,8 +4,13 @@ from types import ModuleType from typing import Iterator -from transformer.plugins import contracts -from transformer.plugins.contracts import Plugin +from transformer.plugins.contracts import ( + Plugin, + Contract, + InvalidContractError, + contract, + InvalidPluginError, +) def resolve(name: str) -> Iterator[Plugin]: @@ -13,37 +18,55 @@ def resolve(name: str) -> Iterator[Plugin]: Transform a plugin name into the corresponding, actual plugins. The name of a plugin is the name of a Python module containing (at least) - one function which name begins with "plugin" and which is annotated - according to one of the "plugin contracts" (defined in the contracts module). + one function decorated with @plugin (from the contracts module). The "resolve" function loads that module and returns these plugin functions found inside the module. + + :raise ImportError: if name does not match an accessible module. + :raise TypeError: from load_load_plugins_from_module. + :raise InvalidContractError: from load_load_plugins_from_module. + :raise NoPluginError: from load_load_plugins_from_module. """ - try: - module = importlib.import_module(name) - except ImportError as err: - logging.error(f"failed to import plugin module {name!r}: {err}") - return iter(()) + module = importlib.import_module(name) - return load_plugins_from_module(module) + yield from load_plugins_from_module(module) -PLUGIN_PREFIX = "plugin" +class NoPluginError(ValueError): + """ + Raised for Python modules that should but don't contain any plugin function. + """ def load_plugins_from_module(module: ModuleType) -> Iterator[Plugin]: + """ + :param module: Python module from which to load plugin functions. + :raise TypeError: if module is not a Python module. + :raise InvalidContractError: if a function is associated to an invalid contract. + :raise NoPluginError: if module doesn't contain at least one plugin function. + """ if not inspect.ismodule(module): raise TypeError(f"expected a module, got {module!r}") - at_least_once = False - for obj_name, obj in inspect.getmembers(module, inspect.isfunction): - if obj_name.startswith(PLUGIN_PREFIX): - valid = contracts.isvalid(Plugin, obj) - if not valid: - logging.warning(f"ignoring {obj_name}: {valid.reason}") - else: - at_least_once = True - yield obj - else: - logging.debug(f"ignoring {obj_name}: doesn't start with {PLUGIN_PREFIX!r}") - - if not at_least_once: - logging.error(f"module {module} doesn't contain plugin functions") + + nb_plugins = 0 + + for _, obj in inspect.getmembers(module, inspect.isfunction): + try: + c = contract(obj) + except InvalidPluginError: + logging.debug(f"ignoring {_n(obj)}: not decorated with @plugin") + continue + + if not isinstance(c, Contract): + msg = f"{_n(obj)} associated to an invalid contract {c!r}" + raise InvalidContractError(msg) + + nb_plugins += 1 + yield obj + + if nb_plugins < 1: + raise NoPluginError(module) + + +def _n(x) -> str: + return getattr(x, "__qualname__", None) or repr(x) diff --git a/transformer/plugins/sanitize_headers.py b/transformer/plugins/sanitize_headers.py index 3990720..588d895 100644 --- a/transformer/plugins/sanitize_headers.py +++ b/transformer/plugins/sanitize_headers.py @@ -1,7 +1,9 @@ from transformer.helpers import zip_kv_pairs +from transformer.plugins import plugin, Contract from transformer.task import Task2 +@plugin(Contract.OnTask) def plugin(task: Task2) -> Task2: """ Removes Chrome-specific, RFC-non-compliant headers starting with `:`. diff --git a/transformer/plugins/test_contracts.py b/transformer/plugins/test_contracts.py index 78a547b..15c3a73 100644 --- a/transformer/plugins/test_contracts.py +++ b/transformer/plugins/test_contracts.py @@ -1,55 +1,122 @@ -from typing import Callable -from unittest.mock import MagicMock - import pytest +from hypothesis import given +from hypothesis.strategies import from_type -from transformer.task import Task2 -from .contracts import isvalid, Plugin, OnTask +from transformer.plugins import apply, group_by_contract +from .contracts import ( + Contract, + plugin, + contract, + InvalidContractError, + InvalidPluginError, +) -class TestIsvalid: - def test_no_if_obj_is_not_a_function_regardless_of_plugin(self): - class A: - pass +@given(from_type(Contract)) +def test_contract_returns_contract_associated_with_plugin_decorator(c: Contract): + @plugin(c) + def foo(): + ... + + assert contract(foo) is c + - assert not isvalid(MagicMock(), A()) - assert not isvalid(MagicMock(), 2) - assert not isvalid(MagicMock(), "x") +def test_plugin_decorator_raises_with_invalid_contract(): + with pytest.raises(InvalidContractError): - def test_raises_error_for_unknown_plugin(self): - def f(_: int) -> int: + @plugin(2) + def foo(): ... - IntPlugin = Callable[[int], int] - with pytest.raises(TypeError): - isvalid(IntPlugin, f) - def test_no_if_obj_has_no_signature(self): - def f(task): - return task +def test_plugin_decorator_raises_without_contract(): + with pytest.raises(InvalidContractError): + + @plugin + def foo(): + ... + + +def test_contract_raises_with_invalid_plugin(): + def foo(): + ... + + with pytest.raises(InvalidPluginError): + contract(foo) + + +def test_plugin_is_exported_by_the_transformer_plugins_module(): + try: + from transformer.plugins import plugin + except ImportError: + pytest.fail("plugin should be exported by transformer.plugins") + + +def test_Contract_is_exported_by_the_transformer_plugins_module(): + try: + from transformer.plugins import Contract + except ImportError: + pytest.fail("Contract should be exported by transformer.plugins") - assert not isvalid(OnTask, f) - def test_no_if_obj_has_wrong_signature(self): - def f(b: bool) -> bool: - return b +class TestApply: + def test_return_init_unchanged_without_plugins(self): + x = object() + assert apply([], x) is x - assert not isvalid(OnTask, f) + def test_return_plugin_result(self): + @plugin(Contract.OnTask) + def plugin_a(x: str) -> str: + return x + "a" - def test_yes_if_obj_has_right_signature(self): - def f(t: Task2) -> Task2: - return t + assert apply([plugin_a], "z") == "za" - assert isvalid(OnTask, f) + def test_runs_plugins_in_succession_on_input(self): + @plugin(Contract.OnTask) + def plugin_a(x: str) -> str: + return x + "a" - def test_isvalid_plugin_false_if_false_for_all_plugin_subtypes(self): - def f(t: bool) -> bool: - return t + @plugin(Contract.OnTask) + def plugin_b(x: str) -> str: + return x + "b" - assert not isvalid(Plugin, f) + assert apply((plugin_a, plugin_b), "") == "ab" + assert apply((plugin_b, plugin_a, plugin_b), "") == "bab" - def test_isvalid_plugin_true_if_true_for_a_plugin_subtype(self): - def f(t: Task2) -> Task2: - return t - assert isvalid(Plugin, f) +class TestGroupByContract: + def test_return_empty_dict_when_no_plugins(self): + assert group_by_contract([]) == {} + + def test_index_plugins_with_simple_contracts_by_their_contract(self): + @plugin(Contract.OnTask) + def plugin_a(): + pass + + @plugin(Contract.OnTask) + def plugin_b(): + pass + + @plugin(Contract.OnScenario) + def plugin_z(): + pass + + assert group_by_contract((plugin_a, plugin_b, plugin_z)) == { + Contract.OnScenario: [plugin_z], + Contract.OnTask: [plugin_a, plugin_b], + } + + def test_index_plugins_with_complex_contracts_by_their_basic_contracts(self): + @plugin(Contract.OnTask) + def plugin_task(): + pass + + @plugin(Contract.OnTask | Contract.OnScenario | Contract.OnPythonProgram) + def plugin_multi(): + pass + + assert group_by_contract((plugin_task, plugin_multi)) == { + Contract.OnTask: [plugin_task, plugin_multi], + Contract.OnScenario: [plugin_multi], + Contract.OnPythonProgram: [plugin_multi], + } diff --git a/transformer/plugins/test_dummy.py b/transformer/plugins/test_dummy.py index 0b8e37e..ce22701 100644 --- a/transformer/plugins/test_dummy.py +++ b/transformer/plugins/test_dummy.py @@ -1,18 +1,18 @@ import logging +import os from pathlib import Path +import transformer from transformer.helpers import DUMMY_HAR_STRING -from transformer.scenario import Scenario def test_dummy_plugin_works(tmp_path: Path, caplog): - from transformer.plugins import dummy - har_path = tmp_path / "test.har" har_path.write_text(DUMMY_HAR_STRING) caplog.set_level(logging.INFO) - Scenario.from_path(har_path, plugins=[dummy.plugin]) + with open(os.path.devnull, "w") as f: + transformer.dump(f, [har_path], plugins=["transformer.plugins.dummy"]) - assert "https://www.zalando.de" in caplog.text + assert "The first request was https://www.zalando.de" in caplog.text diff --git a/transformer/plugins/test_resolve.py b/transformer/plugins/test_resolve.py index 52705a8..0d38a38 100644 --- a/transformer/plugins/test_resolve.py +++ b/transformer/plugins/test_resolve.py @@ -6,9 +6,11 @@ from types import ModuleType import pytest +from hypothesis import given +from hypothesis._strategies import permutations -from transformer.task import Task2 -from .resolve import load_plugins_from_module, resolve +from transformer.plugins.contracts import plugin, Contract +from .resolve import load_plugins_from_module, resolve, NoPluginError @pytest.fixture() @@ -18,23 +20,28 @@ def module_root(tmp_path: Path, monkeypatch) -> Path: class TestResolve: - def test_returns_empty_and_logs_for_module_not_found(self, caplog): + def test_raises_for_module_not_found(self): modname = f"that_module_does_not_exist.{uuid.uuid4().hex}" - assert list(resolve(modname)) == [] - assert f"failed to import plugin module {modname!r}" in caplog.text + with pytest.raises(ImportError): + list(resolve(modname)) # must force evaluation of the generator def test_calls_load_plugins_from_module_with_module(self, module_root: Path): modname = "ab.cd.ef" modpath = Path(*modname.split(".")).with_suffix(".py") Path(module_root, modpath.parent).mkdir(parents=True) with Path(module_root, modpath).open("w") as f: - f.write("from transformer.plugins.contracts import Task2\n") - f.write("def plugin_f(t: Task2) -> Task2:\n") + f.write("from transformer.plugins.contracts import plugin, Contract\n") + f.write("@plugin(Contract.OnTask)\n") + f.write("def f(t):\n") f.write(" ...\n") + f.write("def helper(t):\n") + f.write(" ...\n") + plugins = list(resolve(modname)) assert len(plugins) == 1 f = plugins[0] - assert f.__name__ == "plugin_f" + assert callable(f) + assert f.__name__ == "f" def test_resolve_is_exported_by_the_transformer_plugins_module(self): try: @@ -58,18 +65,18 @@ class A: # Iterators are lazy, we need list() list(load_plugins_from_module(A)) - def test_ignores_non_plugin_stuff_in_module(self, module, caplog): - def signature_not_a_plugin(_: Task2) -> Task2: - ... + def not_a_plugin(_): + ... - def plugin_prefixed_but_no(): - ... + def plugin_not_a_plugin_either(_): + ... - def plugin_valid(_: Task2) -> Task2: - ... + @plugin(Contract.OnTask) + def plugin_valid(_): + ... - non_plugin_functions = (signature_not_a_plugin, plugin_prefixed_but_no) - functions = (*non_plugin_functions, plugin_valid) + @given(permutations((not_a_plugin, plugin_not_a_plugin_either, plugin_valid))) + def test_ignores_non_plugin_stuff_in_module(self, module, caplog, functions): for f in functions: module.__dict__[f.__name__] = f @@ -77,15 +84,17 @@ def plugin_valid(_: Task2) -> Task2: caplog.set_level(logging.DEBUG) plugins = list(load_plugins_from_module(module)) + plugin_valid = next(f for f in functions if f.__name__ == "plugin_valid") assert plugins == [plugin_valid] + non_plugin_functions = {f for f in functions if f is not plugin_valid} print(f">>> log messages: {caplog.messages}") for f in non_plugin_functions: assert any( f.__name__ in msg for msg in caplog.messages ), "ignored function names should be logged" - def test_empty_iterator_for_modules_without_any_plugin(self, module, caplog): - plugins = list(load_plugins_from_module(module)) - assert plugins == [] - assert module.__name__ in caplog.text + def test_raises_for_modules_without_any_plugin(self, module): + with pytest.raises(NoPluginError, match=module.__name__): + # must force evaluation of the generator + list(load_plugins_from_module(module)) diff --git a/transformer/plugins/test_sanitize_headers.py b/transformer/plugins/test_sanitize_headers.py index 74d3641..f76e475 100644 --- a/transformer/plugins/test_sanitize_headers.py +++ b/transformer/plugins/test_sanitize_headers.py @@ -1,4 +1,3 @@ -# pylint: skip-file from datetime import datetime from urllib.parse import urlparse diff --git a/transformer/scenario.py b/transformer/scenario.py index 920f8cd..08da472 100644 --- a/transformer/scenario.py +++ b/transformer/scenario.py @@ -3,7 +3,6 @@ from collections import defaultdict from pathlib import Path from typing import ( - NamedTuple, Sequence, Mapping, Union, @@ -12,10 +11,13 @@ Optional, Dict, Tuple, + NamedTuple, ) + +import transformer.plugins as plug from transformer.naming import to_identifier -from transformer.plugins.contracts import OnTaskSequence +from transformer.plugins.contracts import Plugin from transformer.request import Request from transformer.task import Task, Task2 @@ -83,7 +85,8 @@ class Scenario(NamedTuple): def from_path( cls, path: Path, - plugins: Sequence[OnTaskSequence] = (), + plugins: Sequence[Plugin] = (), + ts_plugins: Sequence[Plugin] = (), short_name: bool = False, ) -> "Scenario": """ @@ -102,13 +105,21 @@ def from_path( names are "scoped" by the parent directory). """ if path.is_dir(): - return cls.from_dir(path, plugins, short_name=short_name) + return cls.from_dir( + path, plugins, ts_plugins=ts_plugins, short_name=short_name + ) else: - return cls.from_har_file(path, plugins, short_name=short_name) + return cls.from_har_file( + path, plugins, ts_plugins=ts_plugins, short_name=short_name + ) @classmethod def from_dir( - cls, path: Path, plugins: Sequence[OnTaskSequence], short_name: bool + cls, + path: Path, + plugins: Sequence[Plugin], + ts_plugins: Sequence[Plugin], + short_name: bool, ) -> "Scenario": """ Makes a Scenario out of the provided directory path. @@ -151,7 +162,9 @@ def from_dir( if child in weight_files: continue try: - scenario = cls.from_path(child, plugins, short_name=True) + scenario = cls.from_path( + child, plugins, ts_plugins=ts_plugins, short_name=True + ) except SkippableScenarioError as err: logging.warning( "while searching for HAR files, skipping %s: %s", child, err.reason @@ -211,7 +224,11 @@ def _check_dangling_weights(cls, path, scenarios, weight_files): @classmethod def from_har_file( - cls, path: Path, plugins: Sequence[OnTaskSequence], short_name: bool + cls, + path: Path, + plugins: Sequence[Plugin], + ts_plugins: Sequence[Plugin], + short_name: bool, ) -> "Scenario": """ Creates a Scenario given a HAR file. @@ -222,10 +239,13 @@ def from_har_file( with path.open() as file: har = json.load(file) requests = Request.all_from_har(har) - tasks = list(Task.from_requests(requests)) + tasks = Task.from_requests(requests) - for plugin in plugins: - tasks = plugin(tasks) + # TODO: Remove this when Contract.OnTaskSequence is removed. + tasks = plug.apply(ts_plugins, tasks) + + # TODO: Remove Task-to-Task2 conversion once both are merged. + tasks = tuple(plug.apply(plugins, Task2.from_task(t)) for t in tasks) return Scenario( name=to_identifier( @@ -278,3 +298,18 @@ def global_code_blocks(self) -> Mapping[str, Sequence[str]]: for child in self.children for block_name, block_lines in child.global_code_blocks.items() } + + def apply_plugins(self, plugins: Sequence[Plugin]) -> "Scenario": + """ + Recursively builds a new scenario tree from the leaves by applying all + plugins to each cloned scenario subtree. + Does not do anything if plugins is empty. + """ + if not plugins: + return self + + children = [ + c.apply_plugins(plugins) if isinstance(c, Scenario) else c + for c in self.children + ] + return plug.apply(plugins, self._replace(children=children)) diff --git a/transformer/test_locust.py b/transformer/test_locust.py index 4090d4f..c63b56e 100644 --- a/transformer/test_locust.py +++ b/transformer/test_locust.py @@ -52,42 +52,6 @@ class LocustForScenarioGroup(HttpLocust): assert expected.strip() == script.strip() -def test_generates_passed_global_code_blocks(): - sg1 = Scenario( - "sg1", - children=[ - MagicMock( - spec_set=Scenario, children=[], global_code_blocks={"b1": ["ab"]} - ), - MagicMock( - spec_set=Scenario, children=[], global_code_blocks={"b2": ["cd"]} - ), - ], - origin=None, - ) - sg2 = Scenario( - "sg2", - children=[MagicMock(spec_set=Scenario, children=[], global_code_blocks={})], - origin=None, - ) - sg3 = Scenario( - "sg3", - children=[ - MagicMock( - spec_set=Scenario, - children=[], - global_code_blocks={"b3": ["yz"], "b2": ["yyy", "zzz"]}, - ) - ], - origin=None, - ) - - code = locustfile([sg1, sg2, sg3]) - assert code.endswith( - "\n# b1\nab\n# b2\nyyy\nzzz\n# b3\nyz" - ), "the latter b2 block should override the former" - - def test_locust_taskset_raises_on_malformed_scenario(): bad_child = cast(Scenario, 7) bad_scenario = Scenario(name="x", children=[bad_child], origin=None) diff --git a/transformer/test_transform.py b/transformer/test_transform.py index b287eca..8a00f93 100644 --- a/transformer/test_transform.py +++ b/transformer/test_transform.py @@ -8,6 +8,7 @@ import transformer.transform as tt from transformer.helpers import DUMMY_HAR_STRING from transformer.locust import locustfile_lines +from transformer.plugins import plugin, Contract class TestTransform: @@ -55,7 +56,9 @@ def dump_as_str(*args, **kwargs): class TestDumpAndDumps: @pytest.mark.parametrize("f", (transformer.dumps, dump_as_str)) def test_with_no_paths_it_returns_empty_locustfile(self, f): - expected_empty_locustfile = "\n".join(locustfile_lines(scenarios=[])) + expected_empty_locustfile = "\n".join( + locustfile_lines(scenarios=[], program_plugins=()) + ) assert f([]) == expected_empty_locustfile def test_dump_and_dumps_have_same_output_for_simple_har(self, tmp_path): @@ -80,12 +83,11 @@ def test_it_uses_default_plugins( times_plugin_called = 0 - # We don't need to specify a plugin signature here because signatures - # are only checked at plugin name resolution. - def fake_plugin(tasks): + @plugin(Contract.OnScenario) + def fake_plugin(t): nonlocal times_plugin_called times_plugin_called += 1 - return tasks + return t monkeypatch.setattr(tt, "DEFAULT_PLUGINS", [fake_plugin]) diff --git a/transformer/transform.py b/transformer/transform.py index 378552a..4df396e 100644 --- a/transformer/transform.py +++ b/transformer/transform.py @@ -5,9 +5,10 @@ from pathlib import Path from typing import Sequence, Union, Iterable, TextIO, Iterator, TypeVar +import transformer.plugins as plug from transformer.locust import locustfile, locustfile_lines -from transformer.plugins import sanitize_headers -from transformer.plugins.contracts import OnTaskSequence +from transformer.plugins import sanitize_headers, Contract +from transformer.plugins.contracts import Plugin from transformer.scenario import Scenario DEFAULT_PLUGINS = (sanitize_headers.plugin,) @@ -15,7 +16,7 @@ def transform( scenarios_path: Union[str, Path], - plugins: Sequence[OnTaskSequence] = (), + plugins: Sequence[Plugin] = (), with_default_plugins: bool = True, ) -> str: """ @@ -86,11 +87,20 @@ def _dump_as_lines( plugins: Sequence[PluginName], with_default_plugins: bool, ) -> Iterator[str]: + plugins = [p for name in plugins for p in plug.resolve(name)] if with_default_plugins: plugins = (*DEFAULT_PLUGINS, *plugins) - yield from locustfile_lines( - [Scenario.from_path(path, plugins) for path in scenario_paths] - ) + + plugins_for = plug.group_by_contract(plugins) + + scenarios = [ + Scenario.from_path( + path, plugins_for[Contract.OnTask], plugins_for[Contract.OnTaskSequence] + ).apply_plugins(plugins_for[Contract.OnScenario]) + for path in scenario_paths + ] + + yield from locustfile_lines(scenarios, plugins_for[Contract.OnPythonProgram]) T = TypeVar("T") From 007dda9e834dba28b96998d997fe3c532aed8ebf Mon Sep 17 00:00:00 2001 From: Thibaut Le Page Date: Wed, 20 Feb 2019 14:06:06 +0100 Subject: [PATCH 5/9] add diagrams for wiki documentation of plugin architecture Signed-off-by: Thibaut Le Page --- .../transformer-design-internal-objects.svg | 192 +++++++++++++++++ images/transformer-design-plugins.svg | 202 ++++++++++++++++++ images/transformer-design-simple.svg | 102 +++++++++ 3 files changed, 496 insertions(+) create mode 100644 images/transformer-design-internal-objects.svg create mode 100644 images/transformer-design-plugins.svg create mode 100644 images/transformer-design-simple.svg diff --git a/images/transformer-design-internal-objects.svg b/images/transformer-design-internal-objects.svg new file mode 100644 index 0000000..e5fb9dd --- /dev/null +++ b/images/transformer-design-internal-objects.svg @@ -0,0 +1,192 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + image/svg+xml + + + + + + + HAR + locustfile + + Transformer + internalobjects + + + + + + diff --git a/images/transformer-design-plugins.svg b/images/transformer-design-plugins.svg new file mode 100644 index 0000000..2156946 --- /dev/null +++ b/images/transformer-design-plugins.svg @@ -0,0 +1,202 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + image/svg+xml + + + + + + + HAR + locustfile + + Transformer + internalobjects + + + + + plugins + + diff --git a/images/transformer-design-simple.svg b/images/transformer-design-simple.svg new file mode 100644 index 0000000..51a7da5 --- /dev/null +++ b/images/transformer-design-simple.svg @@ -0,0 +1,102 @@ + + + + + + + + + + + + + + + + + image/svg+xml + + + + + + + HAR + locustfile + + Transformer + + From 419a16e39819a05c39384ff4f2294c2e399f5d26 Mon Sep 17 00:00:00 2001 From: Thibaut Le Page Date: Wed, 20 Feb 2019 15:21:36 +0100 Subject: [PATCH 6/9] update CHANGELOG.md Signed-off-by: Thibaut Le Page --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2165238..711b4f5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -27,6 +27,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 an iterator over lines (as strings) instead of a string containing the full locustfile. This design allows for more flexibility in `dump`/`dumps` and should result in smaller memory usage for huge locustfiles. (#14) + - Preliminary support for new-generation plugins. (#25) ### Deprecated From 6be4406ba57de715e82708ad332babc17c5a64a9 Mon Sep 17 00:00:00 2001 From: Thibaut Le Page Date: Wed, 20 Feb 2019 15:56:21 +0100 Subject: [PATCH 7/9] test_locust: re-add unit test removed by accident Signed-off-by: Thibaut Le Page --- transformer/test_locust.py | 36 ++++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/transformer/test_locust.py b/transformer/test_locust.py index c63b56e..4090d4f 100644 --- a/transformer/test_locust.py +++ b/transformer/test_locust.py @@ -52,6 +52,42 @@ class LocustForScenarioGroup(HttpLocust): assert expected.strip() == script.strip() +def test_generates_passed_global_code_blocks(): + sg1 = Scenario( + "sg1", + children=[ + MagicMock( + spec_set=Scenario, children=[], global_code_blocks={"b1": ["ab"]} + ), + MagicMock( + spec_set=Scenario, children=[], global_code_blocks={"b2": ["cd"]} + ), + ], + origin=None, + ) + sg2 = Scenario( + "sg2", + children=[MagicMock(spec_set=Scenario, children=[], global_code_blocks={})], + origin=None, + ) + sg3 = Scenario( + "sg3", + children=[ + MagicMock( + spec_set=Scenario, + children=[], + global_code_blocks={"b3": ["yz"], "b2": ["yyy", "zzz"]}, + ) + ], + origin=None, + ) + + code = locustfile([sg1, sg2, sg3]) + assert code.endswith( + "\n# b1\nab\n# b2\nyyy\nzzz\n# b3\nyz" + ), "the latter b2 block should override the former" + + def test_locust_taskset_raises_on_malformed_scenario(): bad_child = cast(Scenario, 7) bad_scenario = Scenario(name="x", children=[bad_child], origin=None) From 3b0306e6b391a2e27adb5feab4d46319631d776c Mon Sep 17 00:00:00 2001 From: Thibaut Le Page Date: Wed, 20 Feb 2019 16:28:56 +0100 Subject: [PATCH 8/9] contracts: @plugin: clearer error message with suggestions Signed-off-by: Thibaut Le Page --- transformer/plugins/contracts.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/transformer/plugins/contracts.py b/transformer/plugins/contracts.py index 2056b6d..4b891c9 100644 --- a/transformer/plugins/contracts.py +++ b/transformer/plugins/contracts.py @@ -116,9 +116,10 @@ def plugin(c: Contract) -> Callable[[callable], callable]: :raise InvalidContractError: if c is not a valid contract. """ if not isinstance(c, Contract): + suggestions = (f"@plugin(Contract.{x.name})" for x in Contract) raise InvalidContractError( f"{c!r} is not a {Contract.__qualname__}. " - "Did you mean e.g. @plugin(Contract.OnTask)?" + f"Did you mean {', '.join(suggestions)}?" ) def _decorate(f: callable) -> callable: From 0b682f6dbce53fde2194070fe53935d503237b61 Mon Sep 17 00:00:00 2001 From: Thibaut Le Page Date: Wed, 20 Feb 2019 17:01:02 +0100 Subject: [PATCH 9/9] contracts: add comment explaining & in the context of enum.Flag Signed-off-by: Thibaut Le Page --- transformer/plugins/contracts.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/transformer/plugins/contracts.py b/transformer/plugins/contracts.py index 4b891c9..7b02ab9 100644 --- a/transformer/plugins/contracts.py +++ b/transformer/plugins/contracts.py @@ -173,6 +173,6 @@ def group_by_contract(plugins: Iterable[Plugin]) -> DefaultDict[Contract, List[P for p in plugins: c = contract(p) for bc in _BASE_CONTRACTS: - if c & bc: + if c & bc: # Contract is an enum.Flag: & computes the intersection. res[bc].append(p) return res