From ab1c307f56e0110d605e45568223c906a7ef64a6 Mon Sep 17 00:00:00 2001 From: Michael Boulton Date: Thu, 30 Jul 2026 22:00:53 +0100 Subject: [PATCH 01/12] feat(starlark): Add per-stage 'if' and 'retry_until' expressions Writing a whole 'control_flow' script is a lot of ceremony when all you want is "only run this stage if the last one returned something". Add two per-stage keys which are single Starlark expressions evaluated by the same embedded interpreter, with all test variables bound directly as globals: - name: second stage if: var_x > 2 - name: poll until ready max_retries: 20 delay_after: 1 retry_until: response.body["status"] == "ready" No script, no load(), and stages don't need an 'id'. Both are gated behind the existing --tavern-experimental-starlark-pipeline flag. 'if' is the Starlark counterpart of the simpleeval 'skip' key (inverted); using both on one stage is a schema error. It applies to normal stages only, not 'finally' stages. 'retry_until' is an extra success predicate inside the existing retry() loop, so it reuses max_retries/delay_after as-is. It additionally gets a 'response' struct with the same properties as the one returned by run_stage(), for which _create_response_struct is extracted into its own module. max_retries is required alongside it, enforced by the schema. Note these expressions are deliberately not format-string interpolated - variables are real Starlark values, so 'var_x > 2' works and '{var_x} > 2' does not. Also fixes the 'pip install tavern[starlark]' hint, which should be tavern[scriptable]. --- docs/source/core_concepts/flow.md | 3 + docs/source/core_concepts/marks.md | 3 + docs/source/scripting.md | 102 +++++++- tavern/_core/run.py | 34 ++- tavern/_core/schema/tests.jsonschema.yaml | 23 +- tavern/_core/starlark/expressions.py | 241 ++++++++++++++++++ tavern/_core/starlark/response_struct.py | 69 +++++ tavern/_core/starlark/starlark_env.py | 39 +-- tavern/_core/testhelpers.py | 68 ++++- tests/integration/starlark/README.md | 1 + .../test_stage_conditions.tavern.yaml | 155 +++++++++++ tests/unit/starlark/test_expressions.py | 146 +++++++++++ tests/unit/test_schema.py | 33 +++ tests/unit/test_stage_conditions.py | 198 ++++++++++++++ 14 files changed, 1077 insertions(+), 38 deletions(-) create mode 100644 tavern/_core/starlark/expressions.py create mode 100644 tavern/_core/starlark/response_struct.py create mode 100644 tests/integration/starlark/test_stage_conditions.tavern.yaml create mode 100644 tests/unit/starlark/test_expressions.py create mode 100644 tests/unit/test_stage_conditions.py diff --git a/docs/source/core_concepts/flow.md b/docs/source/core_concepts/flow.md index e9b2f46a9..384d6b233 100644 --- a/docs/source/core_concepts/flow.md +++ b/docs/source/core_concepts/flow.md @@ -86,6 +86,9 @@ MQTT tests can be retried as well, but you should think whether this is what you want - you could also try increasing the timeout on an expected MQTT response to achieve something similar. +If the request itself succeeds but you want to keep retrying until something about the response is true, see the +experimental [`retry_until` key](../scripting.md#polling-with-retry_until). + ## Finalising stages If you need a stage to run after a test runs, whether it passes or fails (for example, to log out of a service or diff --git a/docs/source/core_concepts/marks.md b/docs/source/core_concepts/marks.md index fc2db1f09..3b815e4ce 100644 --- a/docs/source/core_concepts/marks.md +++ b/docs/source/core_concepts/marks.md @@ -140,6 +140,9 @@ stages: In this example, the stage will be skipped if `v_int` is greater than 50. Any valid simpleeval expression can be used. +There is also an experimental [`if` key](../scripting.md#running-a-stage-conditionally-with-if) which does the same +thing but inverted, using Starlark instead of simpleeval. `skip` and `if` cannot both be used on the same stage. + #### skipif Sometimes you just want to skip some tests, perhaps based on which server you're diff --git a/docs/source/scripting.md b/docs/source/scripting.md index b222d2d04..28b2000bb 100644 --- a/docs/source/scripting.md +++ b/docs/source/scripting.md @@ -47,6 +47,14 @@ use. To try and combine all of these into one unified test execution model, we need a way to express complex logic declaratively, in a format that is more readable than interpolated strings in YAML. +There are two levels to this: + +- [Per-stage expressions](#per-stage-expressions) (`if` and `retry_until`) - keep the normal sequential stage list and + just annotate individual stages. This is the closest thing to the GitHub Actions example above and is where you + should start. +- A full [`control_flow` script](#basic-usage) - replaces sequential execution entirely, for things which can't be + expressed as a per-stage condition (loops over entities, fallback paths, extracting values with regexes, etc). + ## Starlark Overview Starlark is a Python-like language designed for configuration and build systems. It provides: @@ -66,6 +74,92 @@ Starlark control flow is an experimental feature. Enable it with the pytest flag pytest --tavern-experimental-starlark-pipeline ``` +## Per-stage expressions + +Rewriting a whole test as a `control_flow` script is a lot of ceremony if all you want is "only run this stage if the +last one returned something". For that, stages support two keys which are single Starlark _expressions_, evaluated by +the same embedded interpreter. There is no script, no `load()`, and stages do not need an `id`. These need the same +`--tavern-experimental-starlark-pipeline` flag as `control_flow`. + +All test variables - anything from `save`, `includes`, global config, fixtures, parametrisation, and the `tavern` box - +are bound directly as Starlark globals. + +> **Important:** unlike almost everywhere else in Tavern, these expressions are **not** format-string interpolated. +> Write `if: var_x > 2`, not `if: "{var_x} > 2"` - the latter compares the literal string `"{var_x}"` and will silently +> do the wrong thing. Any variable whose name is not a valid Starlark identifier (for example one containing a dash) is +> not bound. + +### Running a stage conditionally with `if` + +The stage only runs if the expression evaluates to `True`. This is the Starlark counterpart of +the ['skip' key](./core_concepts/marks.md#skipping-stages-with-simpleeval-expressions), inverted - a stage cannot use +both. + +```yaml +stages: + - name: Create a user + request: + url: "{global_host}/users" + method: POST + response: + status_code: 201 + save: + json: + n_existing: existing_count + + - name: Only tidy up if there was something there already + if: n_existing > 0 + request: + url: "{global_host}/users/cleanup" + method: POST + response: + status_code: 200 +``` + +The expression must evaluate to a boolean, and referring to a variable which has not been saved yet is an error rather +than being treated as false. + +`if` is only evaluated for normal stages - stages in a [`finally` block](./core_concepts/flow.md#finalising-stages) +always run. + +### Polling with `retry_until` + +`retry_until` is evaluated after each successful attempt at a stage. If it is `False`, the stage is run again, up to +`max_retries` times, sleeping for `delay_after` in between. If it is never `True`, the test fails. + +```yaml +stages: + - name: Poll until the job is ready + request: + url: "{global_host}/poll" + method: GET + response: + status_code: 200 + save: + json: + job_id: id + max_retries: 20 + delay_after: 1 + retry_until: response.body["status"] == "ready" +``` + +As well as the test variables, the expression has a `response` struct in scope with the same properties as the one +returned by [`run_stage()`](#run_stage): + +```starlark +response.status_code == 200 and response.body["status"] == expected_status +``` + +Note that: + +- `max_retries` is required - `retry_until` without it is a schema error. +- The `response` block still has to verify successfully. An attempt which fails verification is retried as normal (this + is the existing `max_retries` behaviour) and `retry_until` is not evaluated for it, so `retry_until` is an _extra_ + condition on top of the response block rather than a replacement for it. +- Variables from the `save` block of the attempt that finally succeeded are kept and are available to later stages. +- `retry_until` does not apply inside a `control_flow` script, which bypasses the retry machinery - use + `run_stage(..., continue_on_fail=True)` in a `for` loop instead, as shown in [Retry and Polling](#retry-and-polling). + ## Basic Usage ### Inline Control Flow @@ -388,7 +482,8 @@ control_flow: | **Important:** Starlark control flow currently only works with HTTP/REST tests. Other protocol backends (MQTT, gRPC, GraphQL) are not yet supported. -Attempting to use `run_stage()` with non-HTTP stages will raise a `NotImplementedError`. +Attempting to use `run_stage()` - or `retry_until` - with non-HTTP stages will raise a `NotImplementedError`. The `if` +key works with any backend, as it only sees test variables. ### Error Messages @@ -433,7 +528,7 @@ Key differences from Python: ## Examples See the integration test files in `tests/integration/starlark/` for complete working examples of basic control flow, -includes, regex extraction, retry patterns +includes, regex extraction, retry patterns, and the per-stage `if`/`retry_until` keys. ## Possible future improvements @@ -446,3 +541,6 @@ includes, regex extraction, retry patterns - Make this auto-export functions into either this document with mkdocstrings into - Let users import their own functions into starlark? - Add a new CLI/ini flag to say "run 'finally' stages when using starlark script" +- Allow `if` on `finally` stages, and give it access to the previous stage's response. +- Make `re`/`time` and any other helper modules available in per-stage `if`/`retry_until` expressions - currently only + the Starlark builtins and `struct` are in scope. diff --git a/tavern/_core/run.py b/tavern/_core/run.py index 4fe57db8c..06aa09b1b 100644 --- a/tavern/_core/run.py +++ b/tavern/_core/run.py @@ -62,7 +62,7 @@ def _run_with_starlark_control_flow( import starlark except ImportError as e: raise exceptions.DependencyMissingError( - "starlark", "pip install tavern[starlark]" + "starlark", "pip install tavern[scriptable]" ) from e from tavern._core.starlark.starlark_env import StarlarkPipelineRunner @@ -326,6 +326,14 @@ def getonly(stage): if eval_skip(content, test_block_config): continue + if (condition := stage.get("if")) is not None: + if not _eval_stage_condition(condition, stage, test_block_config): + logger.info( + "Skipping stage '%s' as 'if' condition was false", + stage["name"], + ) + continue + if has_only and not getonly(stage): continue @@ -352,6 +360,30 @@ def getonly(stage): logger.debug("no 'finally' stages to run") +def _eval_stage_condition( + condition: str, stage: Mapping, test_block_config: TestConfig +) -> bool: + """Evaluate the 'if' key on a stage to see whether it should be run + + Args: + condition: Starlark expression from the 'if' key + stage: the stage it came from + test_block_config: current test config + + Returns: + Whether the stage should be run + """ + # Local import to avoid a circular dependency, and to keep starlark optional + from tavern._core.starlark.expressions import eval_stage_expression + + if not isinstance(condition, str): + raise exceptions.BadSchemaError( + f"Unexpected '{type(condition)}' in if key - should be a string" + ) + + return eval_stage_expression("if", condition, stage, test_block_config) + + def _calculate_stage_strictness( stage: dict, test_block_config: TestConfig, test_spec: Mapping ) -> StrictLevel: diff --git a/tavern/_core/schema/tests.jsonschema.yaml b/tavern/_core/schema/tests.jsonschema.yaml index 3534ff7e2..e9d76c997 100644 --- a/tavern/_core/schema/tests.jsonschema.yaml +++ b/tavern/_core/schema/tests.jsonschema.yaml @@ -94,6 +94,17 @@ definitions: required: - name + # 'retry_until' is meaningless without something to bound the number of retries + dependencies: + retry_until: + - max_retries + + # 'if' is the starlark equivalent of the older simpleeval 'skip' key + not: + required: + - skip + - if + properties: tinctures: type: array @@ -117,7 +128,17 @@ definitions: default: false - type: string - description: CEL expression saying whether to skip this stage + description: simpleeval expression saying whether to skip this stage + + if: + type: string + description: Starlark expression - this stage is only run if it evaluates to True + + retry_until: + type: string + description: + Starlark expression evaluated after each attempt at this stage - the stage is + retried until it evaluates to True, up to max_retries times only: type: boolean diff --git a/tavern/_core/starlark/expressions.py b/tavern/_core/starlark/expressions.py new file mode 100644 index 000000000..3a8a443db --- /dev/null +++ b/tavern/_core/starlark/expressions.py @@ -0,0 +1,241 @@ +"""Evaluation of single Starlark expressions embedded in a stage. + +This is used for the per-stage ``if`` and ``retry_until`` keys, which are a much +lighter-weight alternative to writing a whole ``control_flow`` script. Unlike the +simpleeval based ``skip`` key, expressions here are _not_ format-string interpolated - +variables are bound directly as Starlark globals, so ``if: var_x > 2`` works but +``if: "{var_x} > 2"`` does not. + +This module must not import anything from tavern._core.run, and must not import +starlark at the top level, so that it can be imported (lazily) from the normal +non-starlark test path. +""" + +import logging +from collections.abc import Mapping +from typing import TYPE_CHECKING, Any + +from tavern._core import exceptions + +if TYPE_CHECKING: + import starlark + + from tavern._core.pytest.config import TestConfig + +logger: logging.Logger = logging.getLogger(__name__) + +# Reserved words in Starlark which can't be used as variable names. Anything in the +# available variables which clashes with one of these is just not bound. +_STARLARK_RESERVED = frozenset( + { + "and", + "break", + "continue", + "def", + "elif", + "else", + "for", + "if", + "in", + "lambda", + "load", + "not", + "or", + "pass", + "return", + "while", + # Reserved for future use by the spec + "as", + "assert", + "class", + "del", + "except", + "finally", + "from", + "global", + "import", + "is", + "nonlocal", + "raise", + "try", + "with", + "yield", + } +) + +# Name the response dict is bound to before being turned into a struct +_RESPONSE_DICT_NAME = "__tavern_response" + +_RESPONSE_PRELUDE = f"response = struct(**{_RESPONSE_DICT_NAME})" + + +def _import_starlark(): + """Import the starlark module, raising a useful error if it isn't installed""" + try: + import starlark + except ImportError as e: + raise exceptions.DependencyMissingError( + "starlark", "pip install tavern[scriptable]" + ) from e + + return starlark + + +def _get_dialect() -> "starlark.Dialect": + starlark = _import_starlark() + dialect = starlark.Dialect.extended() + dialect.enable_keyword_only_arguments = True + return dialect + + +def _get_globals() -> "starlark.Globals": + starlark = _import_starlark() + return starlark.Globals.standard().extended_by( + [ + starlark.LibraryExtension.StructType, + ] + ) + + +def parse_expression(expr: str, description: str) -> None: + """Check that an expression parses, without running it + + Args: + expr: the Starlark expression + description: what this expression is, used in error messages + + Raises: + exceptions.BadSchemaError: if it could not be parsed + """ + starlark = _import_starlark() + + try: + starlark.parse(description, expr, dialect=_get_dialect()) + except starlark.StarlarkError as e: + raise exceptions.BadSchemaError( + f"Failed to parse Starlark expression for {description}: {expr}" + ) from e + + +def eval_stage_expression( + key: str, + expr: str, + stage: Mapping[str, Any], + test_block_config: "TestConfig", + *, + response: Mapping[str, Any] | None = None, +) -> bool: + """Evaluate the Starlark expression from a per-stage 'if' or 'retry_until' key + + Args: + key: name of the stage key the expression came from + expr: the Starlark expression + stage: the stage it came from, used for error messages + test_block_config: current test config, the variables from which are bound as + globals in the expression + response: response values to bind as a 'response' struct, if any + + Returns: + the result of the expression + + Raises: + exceptions.UnexpectedKeysError: if the experimental starlark pipeline was not enabled + exceptions.EvalError: if the expression could not be run, or did not evaluate to + a boolean + """ + if not test_block_config.experimental_starlark_pipeline: + raise exceptions.UnexpectedKeysError( + f"'{key}' requires the experimental starlark pipeline to be enabled - pass " + "--tavern-experimental-starlark-pipeline or set " + "tavern-experimental-starlark-pipeline in your pytest ini file" + ) + + return eval_expression( + expr, + test_block_config.variables, + response=response, + description=f"'{key}' in stage '{stage.get('name', 'unnamed-stage')}'", + ) + + +def eval_expression( + expr: str, + variables: Mapping[str, Any], + *, + response: Mapping[str, Any] | None = None, + description: str, +) -> bool: + """Evaluate a Starlark expression with the given variables bound as globals + + Args: + expr: the Starlark expression to evaluate + variables: test variables to bind as globals. Any key which is not a valid + Starlark identifier is skipped. + response: if given, a dict of response values (see + :func:`tavern._core.starlark.response_struct.create_response_struct`) which + is bound as a struct called 'response' + description: what this expression is, used in error messages + + Returns: + the result of the expression + + Raises: + exceptions.EvalError: if the expression could not be run, or if it did not + evaluate to a boolean + """ + starlark = _import_starlark() + + from .types import from_starlark, to_starlark + + dialect = _get_dialect() + module = starlark.Module() + module_globals = _get_globals() + + for name, value in variables.items(): + if not isinstance(name, str) or not name.isidentifier(): + logger.debug( + "Not binding variable '%s' in %s - not a valid identifier", + name, + description, + ) + continue + if name in _STARLARK_RESERVED: + logger.debug( + "Not binding variable '%s' in %s - reserved word in Starlark", + name, + description, + ) + continue + + module[name] = to_starlark(value) + + if response is not None: + module[_RESPONSE_DICT_NAME] = to_starlark(dict(response)) + prelude = starlark.parse(description, _RESPONSE_PRELUDE, dialect=dialect) + starlark.eval(module, prelude, module_globals) + + try: + ast = starlark.parse(description, expr, dialect=dialect) + except starlark.StarlarkError as e: + raise exceptions.EvalError( + f"Error parsing Starlark expression for {description}: {expr}" + ) from e + + logger.debug("Evaluating Starlark expression for %s: %s", description, expr) + + try: + result = starlark.eval(module, ast, module_globals) + except starlark.StarlarkError as e: + raise exceptions.EvalError( + f"Error evaluating Starlark expression for {description}: {expr} ({e})" + ) from e + + result = from_starlark(result) + + if not isinstance(result, bool): + raise exceptions.EvalError( + f"Starlark expression for {description} did not evaluate to True/False " + f"(got {result} of type {type(result)}): {expr}" + ) + + return result diff --git a/tavern/_core/starlark/response_struct.py b/tavern/_core/starlark/response_struct.py new file mode 100644 index 000000000..ff3ae6e26 --- /dev/null +++ b/tavern/_core/starlark/response_struct.py @@ -0,0 +1,69 @@ +"""Conversion of a plugin response object into a dict for use in Starlark. + +This is used both by the full ``control_flow`` pipeline (where it becomes the struct +returned by ``run_stage()``) and by per-stage ``retry_until`` expressions. +""" + +from typing import Any + +import requests + + +def create_response_struct( + response: Any | None, + *, + success: bool, + request_vars: dict[str, Any], + stage_name: str, +) -> dict[str, Any]: + """Convert a response from running a stage into a dict of Starlark-safe values. + + The returned dict is intended to be splatted into a Starlark ``struct()`` so that + users can write ``response.status_code`` etc. + + Args: + response: the response from the plugin that ran the stage, if any + success: whether the stage passed all of its verifications + request_vars: any variables captured during the request + stage_name: name of the stage that was run + + Returns: + dict of response values + + Raises: + NotImplementedError: if the response is from a plugin which is not supported yet + """ + base_dict: dict[str, Any] = { + # Add "failed" so people don't have to do "if not resp.success" when people will almost certainly + # want to do "if resp.failed" most of the time + "failed": not success, + "success": success, + "request_vars": request_vars, + "stage_name": stage_name, + } + + if response is None: + return base_dict + + if isinstance(response, requests.Response): + content_type = response.headers.get("Content-Type", "") + + # Try to parse JSON body, fall back to raw content + if "application/json" in content_type: + body = response.json() + else: + body = response.content + + base_dict.update( + { + "status_code": response.status_code, + "body": body, + "headers": response.headers, + "cookies": response.cookies, + } + ) + return base_dict + + raise NotImplementedError( + f"gRPC, MQTT, etc. are not supported yet. Got {type(response)}" + ) diff --git a/tavern/_core/starlark/starlark_env.py b/tavern/_core/starlark/starlark_env.py index 6e4e052f2..3eda323b4 100644 --- a/tavern/_core/starlark/starlark_env.py +++ b/tavern/_core/starlark/starlark_env.py @@ -9,7 +9,6 @@ import time from typing import Any, TypedDict -import requests import starlark from tavern._core import exceptions @@ -19,6 +18,7 @@ from tavern._core.strict_util import StrictLevel from tavern._core.tincture import get_stage_tinctures +from .response_struct import create_response_struct from .stage_registry import StageRegistry from .types import from_starlark, to_starlark @@ -254,38 +254,11 @@ def _run_stage( def _create_response_struct(self, stage_response: StageResponse) -> dict[str, Any]: """Convert StageResponse to dict that starlark converts to struct.""" - base_dict: dict[str, Any] = { - # Add "failed" so people don't have to do "if not resp.success" when people will almost certainly - # want to do "if resp.failed" most of the time - "failed": not stage_response.success, - "success": stage_response.success, - "request_vars": stage_response.request_vars, - "stage_name": stage_response.stage_name, - } - if stage_response.response is None: - return base_dict - elif isinstance(stage_response.response, requests.Response): - rest_response = stage_response.response - content_type = rest_response.headers.get("Content-Type", "") - - # Try to parse JSON body, fall back to raw content - if "application/json" in content_type: - body = rest_response.json() - else: - body = rest_response.content - - base_dict.update( - { - "status_code": rest_response.status_code, - "body": body, - "headers": rest_response.headers, - "cookies": rest_response.cookies, - } - ) - return base_dict - - raise NotImplementedError( - f"gRPC, MQTT, etc. are not supported yet. Got {type(stage_response.response)}" + return create_response_struct( + stage_response.response, + success=stage_response.success, + request_vars=stage_response.request_vars, + stage_name=stage_response.stage_name, ) def _setup_builtins(self, module: "starlark.Module") -> None: diff --git a/tavern/_core/testhelpers.py b/tavern/_core/testhelpers.py index 93cfa4a7a..9a93d6874 100644 --- a/tavern/_core/testhelpers.py +++ b/tavern/_core/testhelpers.py @@ -2,6 +2,7 @@ import time from collections.abc import Callable, Mapping from functools import wraps +from typing import Any from tavern._core import exceptions from tavern._core.dict_util import format_keys @@ -28,6 +29,43 @@ def delay(stage: Mapping, when: str, variables: Mapping) -> None: time.sleep(length) +def _check_retry_until( + retry_until: str, + stage: Mapping, + test_block_config: TestConfig, + response: Any, +) -> bool: + """Evaluate the 'retry_until' expression against the response from a stage + + Args: + retry_until: Starlark expression from the 'retry_until' key + stage: test stage + test_block_config: Configuration for current test + response: the response returned from running the stage + + Returns: + Whether the stage should be considered finished + """ + # Local import to avoid a circular dependency, and to keep starlark optional + from tavern._core.starlark.expressions import eval_stage_expression + from tavern._core.starlark.response_struct import create_response_struct + + response_values = create_response_struct( + response, + success=True, + request_vars=dict(test_block_config.variables), + stage_name=stage["name"], + ) + + return eval_stage_expression( + "retry_until", + retry_until, + stage, + test_block_config, + response=response_values, + ) + + def retry(stage: Mapping, test_block_config: TestConfig) -> Callable: """Look for retry and try to repeat the stage `retry` times. @@ -41,6 +79,14 @@ def retry(stage: Mapping, test_block_config: TestConfig) -> Callable: else: max_retries = 0 + retry_until = stage.get("retry_until", None) + + if retry_until and max_retries == 0: + raise exceptions.InvalidRetryException( + f"Stage '{stage['name']}' used 'retry_until' but max_retries was 0 - " + "'retry_until' requires a nonzero 'max_retries'" + ) + if max_retries == 0: def catch_wrapper(fn): @@ -89,7 +135,27 @@ def wrapped(*args, **kwargs): ) ) from e else: - break + if not retry_until: + break + + if _check_retry_until( + retry_until, stage, test_block_config, res + ): + break + + if i < max_retries: + logger.info( + "Stage '%s' ran successfully but 'retry_until' was false for %i time. Retrying.", + stage["name"], + i + 1, + ) + delay(stage, "after", test_block_config.variables) + else: + raise exceptions.TestFailError( + "Test '{}' failed: 'retry_until' expression was never true in {} retries: {}".format( + stage["name"], max_retries, retry_until + ) + ) logger.debug("Stage '%s' succeed after %i retries.", stage["name"], i) return res diff --git a/tests/integration/starlark/README.md b/tests/integration/starlark/README.md index b58119479..42ebce1da 100644 --- a/tests/integration/starlark/README.md +++ b/tests/integration/starlark/README.md @@ -35,3 +35,4 @@ docker-compose -f tests/integration/docker-compose.yml down ## Test Files - `test_control_flow_inline.tavern.yaml` - Basic pipeline test +- `test_stage_conditions.tavern.yaml` - Per-stage `if`/`retry_until` expressions (no `control_flow` script) diff --git a/tests/integration/starlark/test_stage_conditions.tavern.yaml b/tests/integration/starlark/test_stage_conditions.tavern.yaml new file mode 100644 index 000000000..0ad652f4e --- /dev/null +++ b/tests/integration/starlark/test_stage_conditions.tavern.yaml @@ -0,0 +1,155 @@ +is_defaults: True +marks: + - starlark_control_flow + +--- +# Tests for the per-stage 'if' and 'retry_until' starlark expressions. These do not +# need a 'control_flow' script, but do need the same experimental flag. + +test_name: Test per-stage 'if' with a saved variable + +stages: + - name: Echo a number to save + request: + url: "{global_host}/echo" + method: POST + json: + value: 5 + response: + status_code: 200 + save: + json: + var_x: value + + - name: This stage should run + if: var_x > 2 + request: + url: "{global_host}/echo" + method: POST + json: + value: "ran" + response: + status_code: 200 + json: + value: "ran" + + - name: This stage should not run + if: var_x > 100 + request: + url: "{global_host}/echo" + method: POST + json: + value: "should not have run" + response: + # If this stage is ever actually run it will fail here + status_code: 999 + +--- +test_name: Test per-stage 'if' using a nested value + +stages: + - name: Echo a dict to save + request: + url: "{global_host}/echo" + method: POST + json: + value: + status: ready + response: + status_code: 200 + save: + json: + echoed: value + + - name: This stage should run + if: echoed["status"] == "ready" + request: + url: "{global_host}/echo" + method: POST + json: + value: "ran" + response: + status_code: 200 + json: + value: "ran" + +--- +test_name: Test per-stage 'if' referring to a variable that was never saved + +_xfail: run + +stages: + - name: This stage errors because the variable is not defined + if: never_saved == 1 + request: + url: "{global_host}/echo" + method: POST + json: + value: "hello" + response: + status_code: 200 + +--- +test_name: Test 'retry_until' polling until the response body is ready + +stages: + - name: Poll until ready + request: + url: "{global_host}/poll" + method: GET + response: + status_code: 200 + save: + json: + polled_status: status + max_retries: 5 + delay_after: 0.1 + retry_until: response.body["status"] == "ready" + + - name: Check the value saved from the successful attempt was kept + if: polled_status == "ready" + request: + url: "{global_host}/echo" + method: POST + json: + value: "{polled_status}" + response: + status_code: 200 + json: + value: "ready" + +--- +test_name: Test 'retry_until' which is never true fails the test + +_xfail: run + +stages: + - name: Poll for something that never happens + request: + url: "{global_host}/poll" + method: GET + response: + status_code: 200 + max_retries: 2 + delay_after: 0.1 + retry_until: response.body["status"] == "never-going-to-happen" + +--- +test_name: Test 'retry_until' can use the status code and test variables + +includes: + - name: retry_until_vars + description: variables used in the retry_until expression + variables: + expected_status: ready + +stages: + - name: Poll until ready + request: + url: "{global_host}/poll" + method: GET + response: + status_code: 200 + max_retries: 5 + delay_after: 0.1 + retry_until: response.status_code == 200 and response.body["status"] == expected_status diff --git a/tests/unit/starlark/test_expressions.py b/tests/unit/starlark/test_expressions.py new file mode 100644 index 000000000..daf309058 --- /dev/null +++ b/tests/unit/starlark/test_expressions.py @@ -0,0 +1,146 @@ +import dataclasses + +import pytest + +from tavern._core import exceptions +from tavern._core.starlark.expressions import eval_expression, eval_stage_expression + + +class TestEvalExpression: + def test_simple_true(self): + assert eval_expression("1 < 2", {}, description="test") is True + + def test_simple_false(self): + assert eval_expression("1 > 2", {}, description="test") is False + + def test_variable_bound_directly(self): + """Variables are bound as real values, not format-string interpolated""" + assert eval_expression("var_x > 2", {"var_x": 3}, description="test") is True + assert eval_expression("var_x > 2", {"var_x": 1}, description="test") is False + + def test_string_variable(self): + assert ( + eval_expression( + "some_var == 'value'", {"some_var": "value"}, description="test" + ) + is True + ) + + def test_nested_variable(self): + assert ( + eval_expression( + "thing['a']['b'] == 1", + {"thing": {"a": {"b": 1}}}, + description="test", + ) + is True + ) + + def test_format_syntax_is_not_supported(self): + """'{var}' style formatting is deliberately not done - the string is just a + literal string, so this quietly compares '{some_var}' to 'value'""" + assert ( + eval_expression( + "'{some_var}' == 'value'", {"some_var": "value"}, description="test" + ) + is False + ) + + def test_undefined_variable(self): + with pytest.raises(exceptions.EvalError) as exc_info: + eval_expression("not_a_variable", {}, description="test") + + assert "not_a_variable" in str(exc_info.value) + + def test_invalid_syntax(self): + with pytest.raises(exceptions.EvalError): + eval_expression("hello i am a test <<<", {}, description="test") + + def test_non_bool_result(self): + with pytest.raises(exceptions.EvalError) as exc_info: + eval_expression("'a string'", {}, description="test") + + assert "did not evaluate to True/False" in str(exc_info.value) + + def test_non_identifier_variables_are_ignored(self): + """Variables which can't be used as starlark names shouldn't break everything""" + variables = {"with-a-dash": 1, "1_starts_with_number": 2, "fine": 3} + + assert eval_expression("fine == 3", variables, description="test") is True + + def test_reserved_word_variables_are_ignored(self): + assert ( + eval_expression("fine == 3", {"load": 1, "fine": 3}, description="test") + is True + ) + + def test_opaque_variables_can_be_bound(self): + """Objects which can't be represented in starlark shouldn't break binding""" + + class Something: + pass + + variables = {"opaque": Something(), "fine": 3} + + assert eval_expression("fine == 3", variables, description="test") is True + + def test_response_struct_attribute_access(self): + response = {"status_code": 200, "body": {"status": "ready"}, "failed": False} + + assert ( + eval_expression( + "response.status_code == 200 and response.body['status'] == 'ready'", + {}, + response=response, + description="test", + ) + is True + ) + + def test_response_and_variables_together(self): + response = {"status_code": 500} + + assert ( + eval_expression( + "response.status_code == expected_code", + {"expected_code": 500}, + response=response, + description="test", + ) + is True + ) + + def test_response_missing_key(self): + with pytest.raises(exceptions.EvalError): + eval_expression( + "response.nonexistent == 1", + {}, + response={"status_code": 200}, + description="test", + ) + + +class TestStageExpressionGuard: + def test_requires_experimental_flag(self, fix_test_config): + config = dataclasses.replace( + fix_test_config, experimental_starlark_pipeline=False + ) + + with pytest.raises(exceptions.UnexpectedKeysError) as exc_info: + eval_stage_expression("if", "1 < 2", {"name": "a stage"}, config) + + assert "--tavern-experimental-starlark-pipeline" in str(exc_info.value) + + def test_works_with_experimental_flag(self, fix_test_config): + assert ( + eval_stage_expression("if", "1 < 2", {"name": "a stage"}, fix_test_config) + is True + ) + + def test_stage_name_in_error(self, fix_test_config): + with pytest.raises(exceptions.EvalError) as exc_info: + eval_stage_expression( + "if", "not_a_variable", {"name": "a stage"}, fix_test_config + ) + + assert "a stage" in str(exc_info.value) diff --git a/tests/unit/test_schema.py b/tests/unit/test_schema.py index 3cedd9e57..ea3dbcb57 100644 --- a/tests/unit/test_schema.py +++ b/tests/unit/test_schema.py @@ -133,6 +133,39 @@ def test_verify_with_incorrect_value(self, test_dict, incorrect_value): verify_tests(test_dict) +class TestStageConditions: + """The per-stage starlark 'if'/'retry_until' keys""" + + def test_if_alone(self, test_dict): + test_dict["stages"][0]["if"] = "var_x > 2" + verify_tests(test_dict) + + def test_if_and_skip_together(self, test_dict): + """'if' is the starlark equivalent of 'skip' so using both is nonsense""" + test_dict["stages"][0]["if"] = "var_x > 2" + test_dict["stages"][0]["skip"] = "True" + + with pytest.raises(BadSchemaError): + verify_tests(test_dict) + + def test_retry_until_with_max_retries(self, test_dict): + test_dict["stages"][0]["retry_until"] = "response.status_code == 200" + test_dict["stages"][0]["max_retries"] = 3 + verify_tests(test_dict) + + def test_retry_until_without_max_retries(self, test_dict): + test_dict["stages"][0]["retry_until"] = "response.status_code == 200" + + with pytest.raises(BadSchemaError): + verify_tests(test_dict) + + def test_if_must_be_a_string(self, test_dict): + test_dict["stages"][0]["if"] = True + + with pytest.raises(BadSchemaError): + verify_tests(test_dict) + + class TestBadSchemaAtCollect: """Some errors happen at collection time - harder to test""" diff --git a/tests/unit/test_stage_conditions.py b/tests/unit/test_stage_conditions.py new file mode 100644 index 000000000..058ce5ce0 --- /dev/null +++ b/tests/unit/test_stage_conditions.py @@ -0,0 +1,198 @@ +"""Tests for the per-stage 'if' and 'retry_until' starlark expressions""" + +import dataclasses +import unittest.mock +from collections.abc import Mapping +from unittest.mock import Mock, patch + +import pytest +import requests + +from tavern._core import exceptions +from tavern._core.pytest.config import TestConfig +from tavern._core.run import run_test +from tavern._core.testhelpers import retry + + +def _run_test( + stage: Mapping, test_block_config: TestConfig, run_mock: unittest.mock.Mock +) -> bool: + """runs the test and returns whether the stage was run or not""" + + full_test = { + "test_name": "A test with a single stage", + "stages": [stage], + } + + run_test("test_file_name", full_test, test_block_config) + + return run_mock.called + + +class TestIfStage: + @pytest.fixture(autouse=True) + def run_mock(self): + with patch("tavern._core.run._TestRunner.run_stage") as run_mock: + yield run_mock + + @pytest.fixture(scope="function") + def stage(self): + return { + "name": "test stage", + "request": {"url": "https://example.com", "method": "GET"}, + "response": {"status_code": 200}, + } + + @pytest.fixture + def test_block_config(self, includes): + return dataclasses.replace( + includes, + variables={"env_vars": {}}, + experimental_starlark_pipeline=True, + ) + + def test_if_true_runs_stage(self, stage, test_block_config, run_mock): + stage["if"] = "1 < 2" + assert _run_test(stage, test_block_config, run_mock) is True + + def test_if_false_skips_stage(self, stage, test_block_config, run_mock): + stage["if"] = "1 > 2" + assert _run_test(stage, test_block_config, run_mock) is False + + def test_if_uses_saved_variable(self, stage, test_block_config, run_mock): + stage["if"] = "var_x > 2" + test_block_config.variables.update({"var_x": 3}) + + assert _run_test(stage, test_block_config, run_mock) is True + + def test_if_uses_saved_variable_false(self, stage, test_block_config, run_mock): + stage["if"] = "var_x > 2" + test_block_config.variables.update({"var_x": 1}) + + assert _run_test(stage, test_block_config, run_mock) is False + + def test_if_undefined_variable(self, stage, test_block_config, run_mock): + stage["if"] = "not_saved_yet == 1" + + with pytest.raises(exceptions.EvalError): + _run_test(stage, test_block_config, run_mock) + + def test_if_non_bool_result(self, stage, test_block_config, run_mock): + stage["if"] = "'a string'" + + with pytest.raises(exceptions.EvalError): + _run_test(stage, test_block_config, run_mock) + + def test_if_requires_experimental_flag(self, stage, test_block_config, run_mock): + stage["if"] = "1 < 2" + test_block_config = dataclasses.replace( + test_block_config, experimental_starlark_pipeline=False + ) + + with pytest.raises(exceptions.UnexpectedKeysError): + _run_test(stage, test_block_config, run_mock) + + +def _mock_response(body): + response = Mock(spec=requests.Response) + response.status_code = 200 + response.headers = {"Content-Type": "application/json"} + response.json.return_value = body + response.cookies = {} + response.content = b"{}" + return response + + +class TestRetryUntil: + @pytest.fixture + def test_block_config(self, includes): + return dataclasses.replace( + includes, + variables={"env_vars": {}}, + experimental_starlark_pipeline=True, + ) + + @pytest.fixture + def stage(self): + return { + "name": "test stage", + "max_retries": 3, + "retry_until": "response.body['status'] == 'ready'", + } + + def test_succeeds_on_later_attempt(self, stage, test_block_config): + responses = [ + _mock_response({"status": "pending"}), + _mock_response({"status": "pending"}), + _mock_response({"status": "ready"}), + ] + inner = Mock(side_effect=responses) + + wrapped = retry(stage, test_block_config)(inner) + assert wrapped() is responses[-1] + assert inner.call_count == 3 + + def test_succeeds_immediately(self, stage, test_block_config): + inner = Mock(return_value=_mock_response({"status": "ready"})) + + retry(stage, test_block_config)(inner)() + assert inner.call_count == 1 + + def test_fails_after_exhausting_retries(self, stage, test_block_config): + inner = Mock(return_value=_mock_response({"status": "pending"})) + + with pytest.raises(exceptions.TestFailError) as exc_info: + retry(stage, test_block_config)(inner)() + + # max_retries = 3 means 4 attempts in total + assert inner.call_count == 4 + assert "retry_until" in str(exc_info.value) + + def test_not_evaluated_when_stage_raised(self, stage, test_block_config): + """If the response block didn't verify, retry as normal without evaluating""" + inner = Mock( + side_effect=[ + exceptions.TestFailError("nope"), + _mock_response({"status": "ready"}), + ] + ) + + retry(stage, test_block_config)(inner)() + assert inner.call_count == 2 + + def test_delay_after_between_attempts(self, stage, test_block_config): + stage["delay_after"] = 0.01 + inner = Mock( + side_effect=[ + _mock_response({"status": "pending"}), + _mock_response({"status": "ready"}), + ] + ) + + with patch("tavern._core.testhelpers.time.sleep") as sleep_mock: + retry(stage, test_block_config)(inner)() + + sleep_mock.assert_called_once_with(0.01) + + def test_uses_test_variables(self, stage, test_block_config): + stage["retry_until"] = "response.body['status'] == expected_status" + test_block_config.variables["expected_status"] = "ready" + inner = Mock(return_value=_mock_response({"status": "ready"})) + + retry(stage, test_block_config)(inner)() + assert inner.call_count == 1 + + def test_without_max_retries_is_an_error(self, stage, test_block_config): + del stage["max_retries"] + + with pytest.raises(exceptions.InvalidRetryException): + retry(stage, test_block_config) + + def test_requires_experimental_flag(self, stage, test_block_config): + test_block_config = dataclasses.replace( + test_block_config, experimental_starlark_pipeline=False + ) + inner = Mock(return_value=_mock_response({"status": "ready"})) + + with pytest.raises(exceptions.UnexpectedKeysError): + retry(stage, test_block_config)(inner)() From 2fa0034d08a1b9860764c48c396e4c056c78e2b0 Mon Sep 17 00:00:00 2001 From: Michael Boulton Date: Thu, 30 Jul 2026 22:04:21 +0100 Subject: [PATCH 02/12] feat(starlark): Deprecate the stage level 'skip' key in favour of 'if' Using 'skip' on a stage now raises a DeprecationWarning pointing at the 'if' key, which does the same thing with the logic inverted. The 'skip' *marker*, which skips a whole test, is unaffected. The warning fires whenever the key is present, including 'skip: False', so that people migrating see it regardless of the value. The docs for both the boolean and simpleeval forms are marked deprecated and link to the 'if' documentation, with a side by side example of the equivalent expression. --- docs/source/core_concepts/marks.md | 21 +++++++++++++++++++-- docs/source/scripting.md | 6 +++--- tavern/_core/run.py | 10 ++++++++++ tavern/_core/schema/tests.jsonschema.yaml | 7 +++++-- tests/unit/test_skip.py | 19 +++++++++++++++++++ 5 files changed, 56 insertions(+), 7 deletions(-) diff --git a/docs/source/core_concepts/marks.md b/docs/source/core_concepts/marks.md index 3b815e4ce..1d8a1d508 100644 --- a/docs/source/core_concepts/marks.md +++ b/docs/source/core_concepts/marks.md @@ -121,8 +121,15 @@ stages: n_queries: 10000 ``` +**The stage level `skip` key is deprecated** and using it raises a `DeprecationWarning`. Use +the [`if` key](../scripting.md#running-a-stage-conditionally-with-if) instead - it does the same thing, but the logic +is inverted and it uses Starlark rather than simpleeval. The two cannot both be used on the same stage. Note that the +`skip` _marker_ (above), which skips a whole test, is not deprecated. + ##### Skipping stages with simpleeval expressions +**Deprecated** - use the [`if` key](../scripting.md#running-a-stage-conditionally-with-if) instead. + Stages can be skipped by using a `skip` key that contains a [simpleeval](https://pypi.org/project/simpleeval/) expression. This allows for more complex conditional logic to determine if a stage should be skipped. @@ -140,8 +147,18 @@ stages: In this example, the stage will be skipped if `v_int` is greater than 50. Any valid simpleeval expression can be used. -There is also an experimental [`if` key](../scripting.md#running-a-stage-conditionally-with-if) which does the same -thing but inverted, using Starlark instead of simpleeval. `skip` and `if` cannot both be used on the same stage. +The equivalent using the `if` key, where variables are bound directly rather than interpolated into the string: + +```yaml +stages: + - name: Run based on variable value + if: v_int <= 50 + request: + url: "{host}/fake_list" + method: GET + response: + status_code: 200 +``` #### skipif diff --git a/docs/source/scripting.md b/docs/source/scripting.md index 28b2000bb..616a7ded4 100644 --- a/docs/source/scripting.md +++ b/docs/source/scripting.md @@ -91,9 +91,9 @@ are bound directly as Starlark globals. ### Running a stage conditionally with `if` -The stage only runs if the expression evaluates to `True`. This is the Starlark counterpart of -the ['skip' key](./core_concepts/marks.md#skipping-stages-with-simpleeval-expressions), inverted - a stage cannot use -both. +The stage only runs if the expression evaluates to `True`. This replaces the now deprecated +['skip' key](./core_concepts/marks.md#skipping-stages-with-simpleeval-expressions) - `if` is the same thing with the +logic inverted, and a stage cannot use both. ```yaml stages: diff --git a/tavern/_core/run.py b/tavern/_core/run.py index 06aa09b1b..715533b1f 100644 --- a/tavern/_core/run.py +++ b/tavern/_core/run.py @@ -4,6 +4,7 @@ import logging import os import pathlib +import warnings from collections.abc import Mapping, MutableMapping from contextlib import ExitStack from copy import deepcopy @@ -303,6 +304,15 @@ def getonly(stage): try: # Run tests in a path in order for idx, stage in enumerate(test_spec["stages"]): + if "skip" in stage: + warnings.warn( # noqa + f"Stage '{stage['name']}' uses the 'skip' key, which is deprecated - " + "use the 'if' key instead (note that the logic is inverted, and it uses " + "Starlark rather than simpleeval). See 'Running a stage conditionally " + "with if' in the scripting documentation.", + DeprecationWarning, + ) + if content := stage.get("skip"): if content is True: # If it's a literal boolean true or false diff --git a/tavern/_core/schema/tests.jsonschema.yaml b/tavern/_core/schema/tests.jsonschema.yaml index e9d76c997..eac4f3d0c 100644 --- a/tavern/_core/schema/tests.jsonschema.yaml +++ b/tavern/_core/schema/tests.jsonschema.yaml @@ -122,13 +122,16 @@ definitions: default: 0 skip: + deprecated: true oneOf: - type: boolean - description: Whether to skip this stage + description: Deprecated, use 'if' - whether to skip this stage default: false - type: string - description: simpleeval expression saying whether to skip this stage + description: + Deprecated, use 'if' - simpleeval expression saying whether to skip this + stage if: type: string diff --git a/tests/unit/test_skip.py b/tests/unit/test_skip.py index a0e311a0e..fdad3741f 100644 --- a/tests/unit/test_skip.py +++ b/tests/unit/test_skip.py @@ -1,5 +1,6 @@ import dataclasses import unittest.mock +import warnings from collections.abc import Mapping from unittest.mock import patch @@ -122,3 +123,21 @@ def test_skip_empty_string(self, stage, test_block_config, run_mock): stage["skip"] = "" assert _run_test(stage, test_block_config, run_mock) is True + + @pytest.mark.parametrize("skip_value", [True, False, "False", ""]) + def test_skip_is_deprecated( + self, stage, test_block_config, run_mock, skip_value + ) -> None: + """Using the 'skip' key at all should tell people to use 'if' instead""" + + stage["skip"] = skip_value + + with pytest.warns(DeprecationWarning, match="use the 'if' key instead"): + _run_test(stage, test_block_config, run_mock) + + def test_no_deprecation_warning_without_skip( + self, stage, test_block_config, run_mock + ) -> None: + with warnings.catch_warnings(): + warnings.simplefilter("error", DeprecationWarning) + _run_test(stage, test_block_config, run_mock) From 0570eb0f130c236d6b2914a70e0785ce9915584c Mon Sep 17 00:00:00 2001 From: Michael Boulton Date: Thu, 30 Jul 2026 22:10:57 +0100 Subject: [PATCH 03/12] fix(starlark): Only evaluate 'retry_until' when a stage fails 'retry_until' was being evaluated after a stage passed, making it an extra condition on top of the response block. It should instead decide whether to keep retrying a stage that failed - a stage which passes is finished and is never retried, so the expression is not evaluated at all. On failure the expression is evaluated against the response from the failed attempt. If it is true the stage is treated as finished and the test carries on, which is the 'continue_on_fail' behaviour originally intended; if false the stage is retried as before. To make the response available on the failure path, TavernException grows a 'response' attribute which wrapped_run_stage sets when a verifier raises. If the request itself failed there is no response, so the stage is just retried. --- docs/source/core_concepts/flow.md | 2 +- docs/source/scripting.md | 30 +++++--- tavern/_core/exceptions.py | 4 +- tavern/_core/run.py | 16 ++-- tavern/_core/schema/tests.jsonschema.yaml | 5 +- tavern/_core/testhelpers.py | 56 +++++++------- .../test_stage_conditions.tavern.yaml | 32 ++++---- tests/unit/test_stage_conditions.py | 74 +++++++++++++------ 8 files changed, 141 insertions(+), 78 deletions(-) diff --git a/docs/source/core_concepts/flow.md b/docs/source/core_concepts/flow.md index 384d6b233..a71719de2 100644 --- a/docs/source/core_concepts/flow.md +++ b/docs/source/core_concepts/flow.md @@ -86,7 +86,7 @@ MQTT tests can be retried as well, but you should think whether this is what you want - you could also try increasing the timeout on an expected MQTT response to achieve something similar. -If the request itself succeeds but you want to keep retrying until something about the response is true, see the +To control _when_ to stop retrying a failing stage, rather than just retrying a fixed number of times, see the experimental [`retry_until` key](../scripting.md#polling-with-retry_until). ## Finalising stages diff --git a/docs/source/scripting.md b/docs/source/scripting.md index 616a7ded4..5aa28fb2a 100644 --- a/docs/source/scripting.md +++ b/docs/source/scripting.md @@ -124,8 +124,18 @@ always run. ### Polling with `retry_until` -`retry_until` is evaluated after each successful attempt at a stage. If it is `False`, the stage is run again, up to -`max_retries` times, sleeping for `delay_after` in between. If it is never `True`, the test fails. +`retry_until` is a second opinion on a stage that **failed**. It works like `max_retries`, except that instead of +blindly retrying it lets you say when to stop: + +- If the stage **passes**, it is finished. **`retry_until` is not evaluated at all** - a passing stage is never retried, + even if the expression would have been `False`. +- If the stage **fails**, `retry_until` is evaluated against the response that came back. If it is `True` the stage is + treated as finished and the test carries on to the next stage, even though the response block did not match. If it is + `False` the stage is retried, up to `max_retries` times, sleeping for `delay_after` in between. +- If the stage never passes and `retry_until` is never `True`, the test fails. + +In other words, adding `retry_until` gives the stage something like the `continue_on_fail` behaviour of +[`run_stage()`](#run_stage), with the expression deciding when to give up retrying and call it a success. ```yaml stages: @@ -135,9 +145,8 @@ stages: method: GET response: status_code: 200 - save: - json: - job_id: id + json: + status: ready max_retries: 20 delay_after: 1 retry_until: response.body["status"] == "ready" @@ -153,10 +162,13 @@ response.status_code == 200 and response.body["status"] == expected_status Note that: - `max_retries` is required - `retry_until` without it is a schema error. -- The `response` block still has to verify successfully. An attempt which fails verification is retried as normal (this - is the existing `max_retries` behaviour) and `retry_until` is not evaluated for it, so `retry_until` is an _extra_ - condition on top of the response block rather than a replacement for it. -- Variables from the `save` block of the attempt that finally succeeded are kept and are available to later stages. +- Because `retry_until` is only consulted on failure, an expression which is already implied by the `response` block + will never be evaluated. Write the `response` block for what you expect once the polling has finished, as in the + example above. +- If the request itself failed and no response was received at all (a connection error, say) there is nothing to + evaluate the expression against, so the stage is just retried. +- Values in the `save` block of an attempt which failed verification are **not** saved, so if a stage finishes because + `retry_until` was `True` rather than because it passed, later stages will not see them. - `retry_until` does not apply inside a `control_flow` script, which bypasses the retry machinery - use `run_stage(..., continue_on_fail=True)` in a `for` loop instead, as shown in [Retry and Polling](#retry-and-polling). diff --git a/tavern/_core/exceptions.py b/tavern/_core/exceptions.py index 2c99f6a72..fbb6d58c5 100644 --- a/tavern/_core/exceptions.py +++ b/tavern/_core/exceptions.py @@ -1,4 +1,4 @@ -from typing import TYPE_CHECKING, Optional +from typing import TYPE_CHECKING, Any, Optional if TYPE_CHECKING: from tavern._core.pytest.config import TestConfig @@ -13,11 +13,13 @@ class TavernException(Exception): is_final: whether this exception came from a 'finally' block stage: stage that caused this issue test_block_config: config for stage + response: the response from the stage, if one was received before the failure """ stage: Optional[dict] test_block_config: Optional["TestConfig"] is_final: bool = False + response: Optional[Any] = None class BadSchemaError(TavernException): diff --git a/tavern/_core/run.py b/tavern/_core/run.py index 715533b1f..12e5dfaa5 100644 --- a/tavern/_core/run.py +++ b/tavern/_core/run.py @@ -550,11 +550,17 @@ def wrapped_run_stage( tinctures.end_tinctures(expected, response) - for response_type, response_verifiers in verifiers.items(): - logger.debug("Running verifiers for %s", response_type) - for v in response_verifiers: - saved = v.verify(response) - stage_config.variables.update(saved) + try: + for response_type, response_verifiers in verifiers.items(): + logger.debug("Running verifiers for %s", response_type) + for v in response_verifiers: + saved = v.verify(response) + stage_config.variables.update(saved) + except exceptions.TavernException as e: + # Attach the response so that things like 'retry_until' can still inspect it + # even though the stage failed verification + e.response = response + raise tavern_box.pop("request_vars") delay(stage, "after", stage_config.variables) diff --git a/tavern/_core/schema/tests.jsonschema.yaml b/tavern/_core/schema/tests.jsonschema.yaml index eac4f3d0c..7cf543a1d 100644 --- a/tavern/_core/schema/tests.jsonschema.yaml +++ b/tavern/_core/schema/tests.jsonschema.yaml @@ -140,8 +140,9 @@ definitions: retry_until: type: string description: - Starlark expression evaluated after each attempt at this stage - the stage is - retried until it evaluates to True, up to max_retries times + Starlark expression evaluated after each *failed* attempt at this stage - the + stage is retried until it evaluates to True, up to max_retries times. Not + evaluated if the stage passes. only: type: boolean diff --git a/tavern/_core/testhelpers.py b/tavern/_core/testhelpers.py index 9a93d6874..b8fa208dd 100644 --- a/tavern/_core/testhelpers.py +++ b/tavern/_core/testhelpers.py @@ -35,16 +35,16 @@ def _check_retry_until( test_block_config: TestConfig, response: Any, ) -> bool: - """Evaluate the 'retry_until' expression against the response from a stage + """Evaluate the 'retry_until' expression against the response from a failed stage Args: retry_until: Starlark expression from the 'retry_until' key stage: test stage test_block_config: Configuration for current test - response: the response returned from running the stage + response: the response from the attempt that just failed Returns: - Whether the stage should be considered finished + Whether the stage should be considered finished anyway """ # Local import to avoid a circular dependency, and to keep starlark optional from tavern._core.starlark.expressions import eval_stage_expression @@ -52,7 +52,7 @@ def _check_retry_until( response_values = create_response_struct( response, - success=True, + success=False, request_vars=dict(test_block_config.variables), stage_name=stage["name"], ) @@ -112,6 +112,24 @@ def wrapped(*args, **kwargs): except exceptions.BadSchemaError: raise except exceptions.TavernException as e: + # The stage failed, so if there's a 'retry_until' expression see + # whether it considers the stage finished anyway + if retry_until and e.response is not None: + if _check_retry_until( + retry_until, stage, test_block_config, e.response + ): + logger.info( + "Stage '%s' failed but 'retry_until' was true, continuing.", + stage["name"], + ) + res = e.response + break + elif retry_until: + logger.debug( + "No response from stage '%s' so 'retry_until' could not be evaluated", + stage["name"], + ) + if i < max_retries: logger.info( "Stage '%s' failed for %i time. Retrying.", @@ -126,7 +144,13 @@ def wrapped(*args, **kwargs): max_retries, ) - if isinstance(e, exceptions.TestFailError): + if retry_until: + raise exceptions.TestFailError( + "Test '{}' failed: stage did not succeed and 'retry_until' was never true in {} retries: {}".format( + stage["name"], max_retries, retry_until + ) + ) from e + elif isinstance(e, exceptions.TestFailError): raise else: raise exceptions.TestFailError( @@ -135,27 +159,7 @@ def wrapped(*args, **kwargs): ) ) from e else: - if not retry_until: - break - - if _check_retry_until( - retry_until, stage, test_block_config, res - ): - break - - if i < max_retries: - logger.info( - "Stage '%s' ran successfully but 'retry_until' was false for %i time. Retrying.", - stage["name"], - i + 1, - ) - delay(stage, "after", test_block_config.variables) - else: - raise exceptions.TestFailError( - "Test '{}' failed: 'retry_until' expression was never true in {} retries: {}".format( - stage["name"], max_retries, retry_until - ) - ) + break logger.debug("Stage '%s' succeed after %i retries.", stage["name"], i) return res diff --git a/tests/integration/starlark/test_stage_conditions.tavern.yaml b/tests/integration/starlark/test_stage_conditions.tavern.yaml index 0ad652f4e..b858c2dfb 100644 --- a/tests/integration/starlark/test_stage_conditions.tavern.yaml +++ b/tests/integration/starlark/test_stage_conditions.tavern.yaml @@ -93,30 +93,35 @@ stages: test_name: Test 'retry_until' polling until the response body is ready stages: + # The response block only matches when the poll endpoint says 'ready'. Until then the + # stage fails, and 'retry_until' is consulted to decide whether to keep going. - name: Poll until ready request: url: "{global_host}/poll" method: GET response: status_code: 200 - save: - json: - polled_status: status + json: + status: ready max_retries: 5 delay_after: 0.1 retry_until: response.body["status"] == "ready" - - name: Check the value saved from the successful attempt was kept - if: polled_status == "ready" +--- +test_name: Test 'retry_until' is not evaluated when the stage passes + +stages: + # 'retry_until' can never be true, but the stage itself passes first time, so it is + # never evaluated and the stage is not retried + - name: Poll once request: - url: "{global_host}/echo" - method: POST - json: - value: "{polled_status}" + url: "{global_host}/poll" + method: GET response: status_code: 200 - json: - value: "ready" + max_retries: 2 + delay_after: 0.1 + retry_until: response.body["status"] == "never-going-to-happen" --- test_name: Test 'retry_until' which is never true fails the test @@ -129,7 +134,8 @@ stages: url: "{global_host}/poll" method: GET response: - status_code: 200 + # Never matches, so the stage always fails and retry_until is always consulted + status_code: 418 max_retries: 2 delay_after: 0.1 retry_until: response.body["status"] == "never-going-to-happen" @@ -149,7 +155,7 @@ stages: url: "{global_host}/poll" method: GET response: - status_code: 200 + status_code: 418 max_retries: 5 delay_after: 0.1 retry_until: response.status_code == 200 and response.body["status"] == expected_status diff --git a/tests/unit/test_stage_conditions.py b/tests/unit/test_stage_conditions.py index 058ce5ce0..37d2ae629 100644 --- a/tests/unit/test_stage_conditions.py +++ b/tests/unit/test_stage_conditions.py @@ -93,9 +93,9 @@ def test_if_requires_experimental_flag(self, stage, test_block_config, run_mock) _run_test(stage, test_block_config, run_mock) -def _mock_response(body): +def _mock_response(body, status_code=200): response = Mock(spec=requests.Response) - response.status_code = 200 + response.status_code = status_code response.headers = {"Content-Type": "application/json"} response.json.return_value = body response.cookies = {} @@ -103,6 +103,13 @@ def _mock_response(body): return response +def _stage_failure(body, status_code=200): + """A stage which failed verification, but which did get a response back""" + error = exceptions.TestFailError("stage did not verify") + error.response = _mock_response(body, status_code) + return error + + class TestRetryUntil: @pytest.fixture def test_block_config(self, includes): @@ -120,26 +127,39 @@ def stage(self): "retry_until": "response.body['status'] == 'ready'", } - def test_succeeds_on_later_attempt(self, stage, test_block_config): - responses = [ - _mock_response({"status": "pending"}), - _mock_response({"status": "pending"}), - _mock_response({"status": "ready"}), + def test_not_evaluated_when_stage_passes(self, stage, test_block_config): + """A stage which passes is finished - retry_until is not consulted at all, + even though it would have been false""" + response = _mock_response({"status": "pending"}) + inner = Mock(return_value=response) + + assert retry(stage, test_block_config)(inner)() is response + assert inner.call_count == 1 + + def test_stops_when_retry_until_is_true(self, stage, test_block_config): + """The stage keeps failing, but retry_until eventually becomes true""" + failures = [ + _stage_failure({"status": "pending"}), + _stage_failure({"status": "pending"}), + _stage_failure({"status": "ready"}), ] - inner = Mock(side_effect=responses) + inner = Mock(side_effect=failures) - wrapped = retry(stage, test_block_config)(inner) - assert wrapped() is responses[-1] + assert retry(stage, test_block_config)(inner)() is failures[-1].response assert inner.call_count == 3 - def test_succeeds_immediately(self, stage, test_block_config): - inner = Mock(return_value=_mock_response({"status": "ready"})) + def test_stops_immediately_when_retry_until_is_true(self, stage, test_block_config): + inner = Mock(side_effect=_stage_failure({"status": "ready"})) retry(stage, test_block_config)(inner)() assert inner.call_count == 1 def test_fails_after_exhausting_retries(self, stage, test_block_config): - inner = Mock(return_value=_mock_response({"status": "pending"})) + inner = Mock( + side_effect=lambda: (_ for _ in ()).throw( + _stage_failure({"status": "pending"}) + ) + ) with pytest.raises(exceptions.TestFailError) as exc_info: retry(stage, test_block_config)(inner)() @@ -148,12 +168,12 @@ def test_fails_after_exhausting_retries(self, stage, test_block_config): assert inner.call_count == 4 assert "retry_until" in str(exc_info.value) - def test_not_evaluated_when_stage_raised(self, stage, test_block_config): - """If the response block didn't verify, retry as normal without evaluating""" + def test_not_evaluated_without_a_response(self, stage, test_block_config): + """If the request itself failed there is no response to inspect, so just retry""" inner = Mock( side_effect=[ - exceptions.TestFailError("nope"), - _mock_response({"status": "ready"}), + exceptions.TestFailError("no response at all"), + _mock_response({"status": "pending"}), ] ) @@ -164,8 +184,8 @@ def test_delay_after_between_attempts(self, stage, test_block_config): stage["delay_after"] = 0.01 inner = Mock( side_effect=[ - _mock_response({"status": "pending"}), - _mock_response({"status": "ready"}), + _stage_failure({"status": "pending"}), + _stage_failure({"status": "ready"}), ] ) @@ -177,11 +197,23 @@ def test_delay_after_between_attempts(self, stage, test_block_config): def test_uses_test_variables(self, stage, test_block_config): stage["retry_until"] = "response.body['status'] == expected_status" test_block_config.variables["expected_status"] = "ready" - inner = Mock(return_value=_mock_response({"status": "ready"})) + inner = Mock(side_effect=_stage_failure({"status": "ready"})) retry(stage, test_block_config)(inner)() assert inner.call_count == 1 + def test_uses_status_code(self, stage, test_block_config): + stage["retry_until"] = "response.status_code == 201" + inner = Mock( + side_effect=[ + _stage_failure({}, status_code=503), + _stage_failure({}, status_code=201), + ] + ) + + retry(stage, test_block_config)(inner)() + assert inner.call_count == 2 + def test_without_max_retries_is_an_error(self, stage, test_block_config): del stage["max_retries"] @@ -192,7 +224,7 @@ def test_requires_experimental_flag(self, stage, test_block_config): test_block_config = dataclasses.replace( test_block_config, experimental_starlark_pipeline=False ) - inner = Mock(return_value=_mock_response({"status": "ready"})) + inner = Mock(side_effect=_stage_failure({"status": "ready"})) with pytest.raises(exceptions.UnexpectedKeysError): retry(stage, test_block_config)(inner)() From ae3fa8fa33f851d351260279920f63981d243d35 Mon Sep 17 00:00:00 2001 From: Michael Boulton Date: Thu, 30 Jul 2026 22:24:27 +0100 Subject: [PATCH 04/12] test(starlark): Cover polling until any terminal state with 'retry_until' Adds coverage for the use case in taverntesting/tavern#751 - polling a long running job until it reaches any terminal state, rather than guessing how many retries it needs. The integration server gets a '/job/' endpoint which is in progress for the first couple of polls and then settles on SUCCESS or FAILED. --- docs/source/scripting.md | 23 +++++++++ tests/integration/server.py | 22 +++++++++ .../test_stage_conditions.tavern.yaml | 47 +++++++++++++++++++ tests/unit/test_stage_conditions.py | 40 ++++++++++++++++ 4 files changed, 132 insertions(+) diff --git a/docs/source/scripting.md b/docs/source/scripting.md index 5aa28fb2a..2b5576516 100644 --- a/docs/source/scripting.md +++ b/docs/source/scripting.md @@ -159,6 +159,29 @@ returned by [`run_stage()`](#run_stage): response.status_code == 200 and response.body["status"] == expected_status ``` +Because it is an arbitrary expression it can also stop on more than one outcome, which is the usual shape for polling a +long running job that might end up in any one of several terminal states: + +```yaml +stages: + - name: Poll until the job finishes + request: + url: "{global_host}/job/{job_id}" + method: GET + response: + status_code: 200 + json: + status: SUCCESS + max_retries: 20 + delay_after: 1 + retry_until: response.body["status"] == "SUCCESS" or response.body["status"] == "FAILED" +``` + +Here the `response` block says what the happy path looks like, and `retry_until` says when there is no point polling any +more. A job which ends up as `FAILED` stops the retries immediately rather than waiting out all 20 of them - but note +that, as above, a stage which finished because `retry_until` was `True` does not fail the test even though its response +block did not match. If you need to assert on how the job actually ended, do it in a following stage. + Note that: - `max_retries` is required - `retry_until` without it is a schema error. diff --git a/tests/integration/server.py b/tests/integration/server.py index 315876098..6eced58fd 100644 --- a/tests/integration/server.py +++ b/tests/integration/server.py @@ -344,6 +344,28 @@ def poll(): return jsonify(response) +job_polls: dict = {} + + +@app.route("/job/", methods=["GET"]) +def job_status(job_name): + """A long running job which is in progress for the first couple of polls and then + reaches a terminal state, which is 'FAILED' if the job name ends with 'fail'. + + Once a job has finished it stays finished, so it can be polled again afterwards. + """ + polls = job_polls[job_name] = job_polls.get(job_name, 0) + 1 + + if polls < 3: + status = "IN_PROGRESS" + elif job_name.endswith("fail"): + status = "FAILED" + else: + status = "SUCCESS" + + return jsonify({"status": status}) + + def _maybe_get_cookie_name(): return (request.get_json(silent=True) or {}).get("cookie_name", "tavern-cookie") diff --git a/tests/integration/starlark/test_stage_conditions.tavern.yaml b/tests/integration/starlark/test_stage_conditions.tavern.yaml index b858c2dfb..93393780d 100644 --- a/tests/integration/starlark/test_stage_conditions.tavern.yaml +++ b/tests/integration/starlark/test_stage_conditions.tavern.yaml @@ -140,6 +140,53 @@ stages: delay_after: 0.1 retry_until: response.body["status"] == "never-going-to-happen" +--- +# https://github.com/taverntesting/tavern/issues/751 - poll a long running job until it +# reaches _any_ terminal state, rather than guessing how many retries it will need +test_name: Test 'retry_until' stopping on any terminal state + +stages: + - name: Poll until the job finishes + request: + url: "{global_host}/job/works" + method: GET + response: + status_code: 200 + json: + status: SUCCESS + max_retries: 5 + delay_after: 0.1 + retry_until: response.body["status"] == "SUCCESS" or response.body["status"] == "FAILED" + +--- +test_name: Test 'retry_until' stopping on a terminal state which is a failure + +stages: + # The job reaches 'FAILED', which is still finished as far as the polling is + # concerned, so 'retry_until' stops the retries even though this stage fails + - name: Poll until the job finishes + request: + url: "{global_host}/job/doomed-to-fail" + method: GET + response: + status_code: 200 + json: + status: SUCCESS + max_retries: 5 + delay_after: 0.1 + retry_until: response.body["status"] == "SUCCESS" or response.body["status"] == "FAILED" + + # Because a stage which finished via 'retry_until' does not save anything, check what + # the job actually ended up as in a separate stage + - name: Check the job failed + request: + url: "{global_host}/job/doomed-to-fail" + method: GET + response: + status_code: 200 + json: + status: FAILED + --- test_name: Test 'retry_until' can use the status code and test variables diff --git a/tests/unit/test_stage_conditions.py b/tests/unit/test_stage_conditions.py index 37d2ae629..6cfd598c0 100644 --- a/tests/unit/test_stage_conditions.py +++ b/tests/unit/test_stage_conditions.py @@ -168,6 +168,46 @@ def test_fails_after_exhausting_retries(self, stage, test_block_config): assert inner.call_count == 4 assert "retry_until" in str(exc_info.value) + @pytest.mark.parametrize("terminal_status", ["SUCCESS", "FAILED"]) + def test_stops_on_any_terminal_state( + self, stage, test_block_config, terminal_status + ): + """Poll a long running job until it finishes, whether it succeeded or not + + https://github.com/taverntesting/tavern/issues/751 + """ + stage["retry_until"] = ( + "response.body['status'] == 'SUCCESS'" + " or response.body['status'] == 'FAILED'" + ) + stage["max_retries"] = 5 + failures = [ + _stage_failure({"status": "IN_PROGRESS"}), + _stage_failure({"status": "IN_PROGRESS"}), + _stage_failure({"status": terminal_status}), + ] + inner = Mock(side_effect=failures) + + assert retry(stage, test_block_config)(inner)() is failures[-1].response + assert inner.call_count == 3 + + def test_never_reaching_a_terminal_state_fails(self, stage, test_block_config): + stage["retry_until"] = ( + "response.body['status'] == 'SUCCESS'" + " or response.body['status'] == 'FAILED'" + ) + stage["max_retries"] = 2 + inner = Mock( + side_effect=lambda: (_ for _ in ()).throw( + _stage_failure({"status": "IN_PROGRESS"}) + ) + ) + + with pytest.raises(exceptions.TestFailError): + retry(stage, test_block_config)(inner)() + + assert inner.call_count == 3 + def test_not_evaluated_without_a_response(self, stage, test_block_config): """If the request itself failed there is no response to inspect, so just retry""" inner = Mock( From fefcef043f6efbda5b490aedacbf50864590de0d Mon Sep 17 00:00:00 2001 From: Michael Boulton Date: Fri, 31 Jul 2026 19:25:34 +0100 Subject: [PATCH 05/12] feat(starlark): Add per-stage 'fail_if' expression 'retry_until' can only say when to stop polling, which counts as a pass. 'fail_if' is the negative counterpart - it is evaluated after every attempt at a stage, and if it is true the test fails immediately without any further retries. This means polling a long running job can stop as soon as it reaches a state it will never recover from. https://github.com/taverntesting/tavern/issues/751 --- docs/source/core_concepts/flow.md | 3 +- docs/source/scripting.md | 62 +++++- tavern/_core/exceptions.py | 7 + tavern/_core/run.py | 53 +++++ tavern/_core/schema/tests.jsonschema.yaml | 6 + tavern/_core/starlark/expressions.py | 53 +++++ tavern/_core/testhelpers.py | 23 ++- .../test_stage_conditions.tavern.yaml | 71 +++++++ tests/unit/test_schema.py | 17 ++ tests/unit/test_stage_conditions.py | 191 +++++++++++++++++- 10 files changed, 468 insertions(+), 18 deletions(-) diff --git a/docs/source/core_concepts/flow.md b/docs/source/core_concepts/flow.md index a71719de2..5aa49cbb0 100644 --- a/docs/source/core_concepts/flow.md +++ b/docs/source/core_concepts/flow.md @@ -87,7 +87,8 @@ is what you want - you could also try increasing the timeout on an expected MQTT response to achieve something similar. To control _when_ to stop retrying a failing stage, rather than just retrying a fixed number of times, see the -experimental [`retry_until` key](../scripting.md#polling-with-retry_until). +experimental [`retry_until`](../scripting.md#polling-with-retry_until) and +[`fail_if`](../scripting.md#failing-fast-with-fail_if) keys. ## Finalising stages diff --git a/docs/source/scripting.md b/docs/source/scripting.md index 2b5576516..ddb1b9400 100644 --- a/docs/source/scripting.md +++ b/docs/source/scripting.md @@ -49,7 +49,7 @@ declaratively, in a format that is more readable than interpolated strings in YA There are two levels to this: -- [Per-stage expressions](#per-stage-expressions) (`if` and `retry_until`) - keep the normal sequential stage list and +- [Per-stage expressions](#per-stage-expressions) (`if`, `retry_until` and `fail_if`) - keep the normal sequential stage list and just annotate individual stages. This is the closest thing to the GitHub Actions example above and is where you should start. - A full [`control_flow` script](#basic-usage) - replaces sequential execution entirely, for things which can't be @@ -195,6 +195,62 @@ Note that: - `retry_until` does not apply inside a `control_flow` script, which bypasses the retry machinery - use `run_stage(..., continue_on_fail=True)` in a `for` loop instead, as shown in [Retry and Polling](#retry-and-polling). +### Failing fast with `fail_if` + +`fail_if` is the mirror image of `retry_until` - a negative assertion which fails the stage as soon as it is `True`: + +- It is evaluated after **every** attempt at the stage, whether that attempt passed or failed. +- If it is `True` the test fails immediately. The stage is **not** retried, no matter what `max_retries` or + `retry_until` say. +- If it is `False` nothing changes - a stage which passed carries on to the next stage, and a stage which failed is + retried as normal. + +It has the same `response` struct in scope as `retry_until`, which includes `response.failed` if you want to +distinguish an attempt which passed its response block from one which did not. + +The main use for this is polling something which can end up in a state it will never recover from. `retry_until` alone +can only say "stop polling", which counts as a pass; `fail_if` says "stop polling, and this is a failure": + +```yaml +stages: + - name: Poll until the job succeeds + request: + url: "{global_host}/job/{job_id}" + method: GET + response: + status_code: 200 + json: + status: SUCCESS + max_retries: 60 + delay_after: 10 + retry_until: response.body["status"] == "SUCCESS" + fail_if: response.body["status"] == "FAILED" +``` + +A job which goes to `FAILED` fails the test on the next poll instead of spending ten minutes retrying something which +was never going to succeed. + +It is also useful on its own, with no retries at all, as an assertion which is easier to express as an expression than +as a `response` block: + +```yaml +- name: Check the response does not leak internal errors + request: + url: "{global_host}/search" + method: GET + response: + status_code: 200 + fail_if: 'response.body["message"] != None and "traceback" in response.body["message"]' +``` + +Note that: + +- Unlike `retry_until`, `fail_if` does not need `max_retries`. +- If the request itself failed and no response was received at all, `fail_if` is not evaluated and the stage fails or + retries as it normally would. +- Like `retry_until`, it does not apply inside a `control_flow` script - check the struct returned by `run_stage()` + instead. + ## Basic Usage ### Inline Control Flow @@ -517,8 +573,8 @@ control_flow: | **Important:** Starlark control flow currently only works with HTTP/REST tests. Other protocol backends (MQTT, gRPC, GraphQL) are not yet supported. -Attempting to use `run_stage()` - or `retry_until` - with non-HTTP stages will raise a `NotImplementedError`. The `if` -key works with any backend, as it only sees test variables. +Attempting to use `run_stage()` - or `retry_until`/`fail_if` - with non-HTTP stages will raise a `NotImplementedError`. +The `if` key works with any backend, as it only sees test variables. ### Error Messages diff --git a/tavern/_core/exceptions.py b/tavern/_core/exceptions.py index fbb6d58c5..e7e1ab811 100644 --- a/tavern/_core/exceptions.py +++ b/tavern/_core/exceptions.py @@ -38,6 +38,13 @@ def __init__(self, msg, failures=None) -> None: self.failures = failures or [] +class FailIfError(TestFailError): + """A stage's 'fail_if' expression was true + + This is separate from a normal test failure because it should never be retried + """ + + class KeyMismatchError(TavernException): """Mismatch found while validating keys in response""" diff --git a/tavern/_core/run.py b/tavern/_core/run.py index 12e5dfaa5..80401c5bc 100644 --- a/tavern/_core/run.py +++ b/tavern/_core/run.py @@ -394,6 +394,49 @@ def _eval_stage_condition( return eval_stage_expression("if", condition, stage, test_block_config) +def _check_fail_if( + fail_if: str, + stage: Mapping, + test_block_config: TestConfig, + response: Any, + *, + success: bool, +) -> None: + """Evaluate the 'fail_if' key on a stage against the response it got back + + Args: + fail_if: Starlark expression from the 'fail_if' key + stage: the stage it came from + test_block_config: current test config + response: the response from running the stage + success: whether the stage passed all of its verifications + + Raises: + exceptions.FailIfError: if the expression was true + """ + # Local import to avoid a circular dependency, and to keep starlark optional + from tavern._core.starlark.expressions import eval_response_expression + + if not eval_response_expression( + "fail_if", + fail_if, + stage, + test_block_config, + response=response, + success=success, + request_vars=test_block_config.variables, + ): + return + + error = exceptions.FailIfError( + "Test '{}' failed: 'fail_if' expression was true: {}".format( + stage["name"], fail_if + ) + ) + error.response = response + raise error + + def _calculate_stage_strictness( stage: dict, test_block_config: TestConfig, test_spec: Mapping ) -> StrictLevel: @@ -550,6 +593,8 @@ def wrapped_run_stage( tinctures.end_tinctures(expected, response) + fail_if = stage.get("fail_if", None) + try: for response_type, response_verifiers in verifiers.items(): logger.debug("Running verifiers for %s", response_type) @@ -560,8 +605,16 @@ def wrapped_run_stage( # Attach the response so that things like 'retry_until' can still inspect it # even though the stage failed verification e.response = response + + # A stage which failed can still be in a state which should not be retried + if fail_if is not None: + _check_fail_if(fail_if, stage, stage_config, response, success=False) + raise + if fail_if is not None: + _check_fail_if(fail_if, stage, stage_config, response, success=True) + tavern_box.pop("request_vars") delay(stage, "after", stage_config.variables) diff --git a/tavern/_core/schema/tests.jsonschema.yaml b/tavern/_core/schema/tests.jsonschema.yaml index 7cf543a1d..159031754 100644 --- a/tavern/_core/schema/tests.jsonschema.yaml +++ b/tavern/_core/schema/tests.jsonschema.yaml @@ -144,6 +144,12 @@ definitions: stage is retried until it evaluates to True, up to max_retries times. Not evaluated if the stage passes. + fail_if: + type: string + description: + Starlark expression evaluated after every attempt at this stage - if it + evaluates to True the test fails immediately, without any further retries. + only: type: boolean description: Only run this stage diff --git a/tavern/_core/starlark/expressions.py b/tavern/_core/starlark/expressions.py index 3a8a443db..55d47e3bb 100644 --- a/tavern/_core/starlark/expressions.py +++ b/tavern/_core/starlark/expressions.py @@ -158,6 +158,59 @@ def eval_stage_expression( ) +def eval_response_expression( + key: str, + expr: str, + stage: Mapping[str, Any], + test_block_config: "TestConfig", + *, + response: Any, + success: bool, + request_vars: Mapping[str, Any], +) -> bool: + """Evaluate a stage expression which can also inspect the response from the stage + + This is used for the 'retry_until' and 'fail_if' keys, which both get a 'response' + struct bound in the expression. + + Args: + key: name of the stage key the expression came from + expr: the Starlark expression + stage: the stage it came from + test_block_config: current test config + response: the response from running the stage, if any + success: whether the stage passed all of its verifications + request_vars: any variables captured during the request + + Returns: + the result of the expression + + Raises: + exceptions.BadSchemaError: if the expression was not a string + """ + from .response_struct import create_response_struct + + if not isinstance(expr, str): + raise exceptions.BadSchemaError( + f"Unexpected '{type(expr)}' in {key} key - should be a string" + ) + + response_values = create_response_struct( + response, + success=success, + request_vars=dict(request_vars), + stage_name=stage.get("name", "unnamed-stage"), + ) + + return eval_stage_expression( + key, + expr, + stage, + test_block_config, + response=response_values, + ) + + def eval_expression( expr: str, variables: Mapping[str, Any], diff --git a/tavern/_core/testhelpers.py b/tavern/_core/testhelpers.py index b8fa208dd..3db50f5b1 100644 --- a/tavern/_core/testhelpers.py +++ b/tavern/_core/testhelpers.py @@ -47,22 +47,16 @@ def _check_retry_until( Whether the stage should be considered finished anyway """ # Local import to avoid a circular dependency, and to keep starlark optional - from tavern._core.starlark.expressions import eval_stage_expression - from tavern._core.starlark.response_struct import create_response_struct + from tavern._core.starlark.expressions import eval_response_expression - response_values = create_response_struct( - response, - success=False, - request_vars=dict(test_block_config.variables), - stage_name=stage["name"], - ) - - return eval_stage_expression( + return eval_response_expression( "retry_until", retry_until, stage, test_block_config, - response=response_values, + response=response, + success=False, + request_vars=test_block_config.variables, ) @@ -111,6 +105,13 @@ def wrapped(*args, **kwargs): res = fn(*args, **kwargs) except exceptions.BadSchemaError: raise + except exceptions.FailIfError: + # 'fail_if' is a terminal state, there is no point retrying + logger.error( + "Stage '%s' matched its 'fail_if' expression, not retrying.", + stage["name"], + ) + raise except exceptions.TavernException as e: # The stage failed, so if there's a 'retry_until' expression see # whether it considers the stage finished anyway diff --git a/tests/integration/starlark/test_stage_conditions.tavern.yaml b/tests/integration/starlark/test_stage_conditions.tavern.yaml index 93393780d..df39a507f 100644 --- a/tests/integration/starlark/test_stage_conditions.tavern.yaml +++ b/tests/integration/starlark/test_stage_conditions.tavern.yaml @@ -206,3 +206,74 @@ stages: max_retries: 5 delay_after: 0.1 retry_until: response.status_code == 200 and response.body["status"] == expected_status + +--- +test_name: Test 'fail_if' as a negative assertion on a stage which otherwise passes + +_xfail: run + +stages: + # The response block matches, but 'fail_if' says this is a failure anyway + - name: Echo something which should never come back + request: + url: "{global_host}/echo" + method: POST + json: + value: "a bad value" + response: + status_code: 200 + fail_if: response.body["value"] == "a bad value" + +--- +test_name: Test 'fail_if' which is false does not affect the stage + +stages: + - name: Echo something fine + request: + url: "{global_host}/echo" + method: POST + json: + value: "a good value" + response: + status_code: 200 + json: + value: "a good value" + fail_if: response.body["value"] == "a bad value" + +--- +# https://github.com/taverntesting/tavern/issues/751 - stop polling as soon as the job +# is in a state it can never recover from, rather than waiting out all the retries +test_name: Test 'fail_if' stops polling at a terminal failure + +_xfail: run + +stages: + - name: Poll until the job succeeds + request: + url: "{global_host}/job/fail-if-doomed-to-fail" + method: GET + response: + status_code: 200 + json: + status: SUCCESS + max_retries: 20 + delay_after: 0.1 + retry_until: response.body["status"] == "SUCCESS" + fail_if: response.body["status"] == "FAILED" + +--- +test_name: Test 'fail_if' does not trigger for a job which succeeds + +stages: + - name: Poll until the job succeeds + request: + url: "{global_host}/job/fail-if-works" + method: GET + response: + status_code: 200 + json: + status: SUCCESS + max_retries: 20 + delay_after: 0.1 + retry_until: response.body["status"] == "SUCCESS" + fail_if: response.body["status"] == "FAILED" diff --git a/tests/unit/test_schema.py b/tests/unit/test_schema.py index ea3dbcb57..ed3710cbc 100644 --- a/tests/unit/test_schema.py +++ b/tests/unit/test_schema.py @@ -165,6 +165,23 @@ def test_if_must_be_a_string(self, test_dict): with pytest.raises(BadSchemaError): verify_tests(test_dict) + def test_fail_if_alone(self, test_dict): + """Unlike 'retry_until', 'fail_if' does not need any retries to be useful""" + test_dict["stages"][0]["fail_if"] = "response.status_code == 500" + verify_tests(test_dict) + + def test_fail_if_with_retry_until(self, test_dict): + test_dict["stages"][0]["fail_if"] = "response.body['status'] == 'FAILED'" + test_dict["stages"][0]["retry_until"] = "response.body['status'] == 'SUCCESS'" + test_dict["stages"][0]["max_retries"] = 3 + verify_tests(test_dict) + + def test_fail_if_must_be_a_string(self, test_dict): + test_dict["stages"][0]["fail_if"] = True + + with pytest.raises(BadSchemaError): + verify_tests(test_dict) + class TestBadSchemaAtCollect: """Some errors happen at collection time - harder to test""" diff --git a/tests/unit/test_stage_conditions.py b/tests/unit/test_stage_conditions.py index 6cfd598c0..534b3c548 100644 --- a/tests/unit/test_stage_conditions.py +++ b/tests/unit/test_stage_conditions.py @@ -1,17 +1,21 @@ -"""Tests for the per-stage 'if' and 'retry_until' starlark expressions""" +"""Tests for the per-stage 'if', 'retry_until' and 'fail_if' starlark expressions""" import dataclasses import unittest.mock from collections.abc import Mapping -from unittest.mock import Mock, patch +from unittest.mock import Mock, create_autospec, patch import pytest import requests from tavern._core import exceptions from tavern._core.pytest.config import TestConfig -from tavern._core.run import run_test +from tavern._core.run import _TestRunner, run_test +from tavern._core.strict_util import StrictLevel from tavern._core.testhelpers import retry +from tavern._core.tincture import Tinctures +from tavern.request import BaseRequest +from tavern.response import BaseResponse def _run_test( @@ -268,3 +272,184 @@ def test_requires_experimental_flag(self, stage, test_block_config): with pytest.raises(exceptions.UnexpectedKeysError): retry(stage, test_block_config)(inner)() + + +def _stage_callable(): + """The signature of the function that 'retry' wraps, used as a Mock spec""" + + +def _mock_stage_callable(**kwargs) -> Mock: + """A mock of the function that 'retry' wraps + + This is autospecced rather than a plain Mock because the retry wrapper calls + functools.wraps on it, which needs the real function attributes. + """ + return create_autospec(_stage_callable, **kwargs) + + +def _run_stage(stage, test_block_config, response, *, verify_error=None): + """Run a single stage, mocking out everything to do with actually making a request + + Args: + stage: the stage to run + test_block_config: config for the test + response: what the 'request' should return + verify_error: if given, an exception raised when verifying the response + """ + + verifier = Mock(spec=BaseResponse) + if verify_error is not None: + verifier.verify.side_effect = verify_error + else: + verifier.verify.return_value = {} + + request = Mock(spec=BaseRequest) + request.request_vars = {} + request.run.return_value = response + + runner = _TestRunner( + default_global_strictness=StrictLevel.all_on(), + sessions={}, + test_block_config=test_block_config, + test_spec={"test_name": "a test", "stages": [stage]}, + ) + + with ( + patch("tavern._core.run.attach_stage_content"), + patch("tavern._core.run.call_hook"), + patch("tavern._core.run.get_request_type", return_value=request), + patch("tavern._core.run.get_expected", return_value={}), + patch("tavern._core.run.get_verifiers", return_value={"response": [verifier]}), + ): + return runner.wrapped_run_stage(stage, test_block_config, Mock(spec=Tinctures)) + + +class TestFailIf: + @pytest.fixture + def test_block_config(self, includes): + return dataclasses.replace( + includes, + variables={"env_vars": {}, "tavern": {}}, + experimental_starlark_pipeline=True, + ) + + @pytest.fixture + def stage(self): + return { + "name": "test stage", + "request": {"url": "https://example.com", "method": "GET"}, + "response": {"status_code": 200}, + "fail_if": "response.body['status'] == 'FAILED'", + } + + def test_passing_stage_with_false_expression(self, stage, test_block_config): + response = _mock_response({"status": "SUCCESS"}) + + assert _run_stage(stage, test_block_config, response) is response + + def test_passing_stage_with_true_expression(self, stage, test_block_config): + """The response block matched, but the stage is a failure anyway""" + response = _mock_response({"status": "FAILED"}) + + with pytest.raises(exceptions.FailIfError) as exc_info: + _run_stage(stage, test_block_config, response) + + assert "fail_if" in str(exc_info.value) + + def test_failing_stage_with_true_expression(self, stage, test_block_config): + response = _mock_response({"status": "FAILED"}) + + with pytest.raises(exceptions.FailIfError): + _run_stage( + stage, + test_block_config, + response, + verify_error=exceptions.TestFailError("stage did not verify"), + ) + + def test_failing_stage_with_false_expression(self, stage, test_block_config): + """The normal failure is unaffected by a 'fail_if' which was false""" + response = _mock_response({"status": "IN_PROGRESS"}) + + with pytest.raises(exceptions.TestFailError) as exc_info: + _run_stage( + stage, + test_block_config, + response, + verify_error=exceptions.TestFailError("stage did not verify"), + ) + + assert not isinstance(exc_info.value, exceptions.FailIfError) + + def test_uses_test_variables(self, stage, test_block_config): + stage["fail_if"] = "response.body['status'] == bad_status" + test_block_config.variables["bad_status"] = "FAILED" + + with pytest.raises(exceptions.FailIfError): + _run_stage(stage, test_block_config, _mock_response({"status": "FAILED"})) + + def test_knows_the_stage_failed(self, stage, test_block_config): + stage["fail_if"] = "response.failed" + + with pytest.raises(exceptions.FailIfError): + _run_stage( + stage, + test_block_config, + _mock_response({"status": "SUCCESS"}), + verify_error=exceptions.TestFailError("stage did not verify"), + ) + + def test_knows_the_stage_passed(self, stage, test_block_config): + stage["fail_if"] = "response.failed" + response = _mock_response({"status": "SUCCESS"}) + + assert _run_stage(stage, test_block_config, response) is response + + def test_non_bool_result(self, stage, test_block_config): + stage["fail_if"] = "response.body['status']" + + with pytest.raises(exceptions.EvalError): + _run_stage(stage, test_block_config, _mock_response({"status": "FAILED"})) + + def test_must_be_a_string(self, stage, test_block_config): + stage["fail_if"] = True + + with pytest.raises(exceptions.BadSchemaError): + _run_stage(stage, test_block_config, _mock_response({"status": "FAILED"})) + + def test_requires_experimental_flag(self, stage, test_block_config): + test_block_config = dataclasses.replace( + test_block_config, experimental_starlark_pipeline=False + ) + + with pytest.raises(exceptions.UnexpectedKeysError): + _run_stage(stage, test_block_config, _mock_response({"status": "FAILED"})) + + def test_is_not_retried(self, stage, test_block_config): + """A stage which hit its 'fail_if' is in a terminal state, so don't retry it + + https://github.com/taverntesting/tavern/issues/751 + """ + stage["max_retries"] = 5 + inner = _mock_stage_callable( + side_effect=exceptions.FailIfError("fail_if was true") + ) + + with pytest.raises(exceptions.FailIfError): + retry(stage, test_block_config)(inner)() + + assert inner.call_count == 1 + + def test_takes_priority_over_retry_until(self, stage, test_block_config): + """Both keys are evaluated, but 'fail_if' is checked while running the stage so + it never reaches the 'retry_until' handling in the retry wrapper""" + stage["max_retries"] = 5 + stage["retry_until"] = "True" + inner = _mock_stage_callable( + side_effect=exceptions.FailIfError("fail_if was true") + ) + + with pytest.raises(exceptions.FailIfError): + retry(stage, test_block_config)(inner)() + + assert inner.call_count == 1 From 45f6c259df8a43eaecff3a730429a6c0b13d7546 Mon Sep 17 00:00:00 2001 From: Michael Boulton Date: Sun, 2 Aug 2026 14:58:45 +0100 Subject: [PATCH 06/12] fix(testhelpers): Refactor retry logic for better clarity and structure - Updated the condition to first check for `retry_until` before evaluating `e.response` in `_core/testhelpers.py`. - Consolidated nested checks to improve readability. - Separated the logic handling cases where `e.response` is `None` for better debugging and maintainability. --- tavern/_core/testhelpers.py | 27 ++++++++++++++------------- 1 file changed, 14 insertions(+), 13 deletions(-) diff --git a/tavern/_core/testhelpers.py b/tavern/_core/testhelpers.py index 3db50f5b1..109880af3 100644 --- a/tavern/_core/testhelpers.py +++ b/tavern/_core/testhelpers.py @@ -115,21 +115,22 @@ def wrapped(*args, **kwargs): except exceptions.TavernException as e: # The stage failed, so if there's a 'retry_until' expression see # whether it considers the stage finished anyway - if retry_until and e.response is not None: - if _check_retry_until( - retry_until, stage, test_block_config, e.response - ): - logger.info( - "Stage '%s' failed but 'retry_until' was true, continuing.", + if retry_until: + if e.response is not None: + if _check_retry_until( + retry_until, stage, test_block_config, e.response + ): + logger.info( + "Stage '%s' failed but 'retry_until' was true, continuing.", + stage["name"], + ) + res = e.response + break + else: + logger.debug( + "No response from stage '%s' so 'retry_until' could not be evaluated", stage["name"], ) - res = e.response - break - elif retry_until: - logger.debug( - "No response from stage '%s' so 'retry_until' could not be evaluated", - stage["name"], - ) if i < max_retries: logger.info( From fed19fd05538cb877cf288dde45c74cc3eae645c Mon Sep 17 00:00:00 2001 From: Michael Boulton Date: Sun, 2 Aug 2026 15:09:47 +0100 Subject: [PATCH 07/12] refactor(starlark): Interpolate per-stage expressions and undeprecate 'skip' Per-stage 'if'/'retry_until'/'fail_if' expressions are now format-string interpolated like everywhere else in Tavern, rather than binding test variables as Starlark globals. This makes them consistent with the existing 'skip' key and lets them refer to variables whose names are not valid Starlark identifiers, such as ones containing a dash. Errors quote both the original expression and the interpolated one. 'skip' is also no longer deprecated - 'if' only works on HTTP stages, so it is not a full replacement yet. --- docs/source/core_concepts/marks.md | 12 +- docs/source/scripting.md | 28 ++-- tavern/_core/run.py | 10 -- tavern/_core/schema/tests.jsonschema.yaml | 7 +- tavern/_core/starlark/expressions.py | 123 +++++------------- .../test_stage_conditions.tavern.yaml | 34 ++++- tests/unit/starlark/test_expressions.py | 62 +++++---- tests/unit/test_skip.py | 19 --- tests/unit/test_stage_conditions.py | 10 +- 9 files changed, 127 insertions(+), 178 deletions(-) diff --git a/docs/source/core_concepts/marks.md b/docs/source/core_concepts/marks.md index 1d8a1d508..7f7828347 100644 --- a/docs/source/core_concepts/marks.md +++ b/docs/source/core_concepts/marks.md @@ -121,15 +121,11 @@ stages: n_queries: 10000 ``` -**The stage level `skip` key is deprecated** and using it raises a `DeprecationWarning`. Use -the [`if` key](../scripting.md#running-a-stage-conditionally-with-if) instead - it does the same thing, but the logic -is inverted and it uses Starlark rather than simpleeval. The two cannot both be used on the same stage. Note that the -`skip` _marker_ (above), which skips a whole test, is not deprecated. +The [`if` key](../scripting.md#running-a-stage-conditionally-with-if) does the same thing, but the logic is inverted +and it uses Starlark rather than simpleeval. The two cannot both be used on the same stage. ##### Skipping stages with simpleeval expressions -**Deprecated** - use the [`if` key](../scripting.md#running-a-stage-conditionally-with-if) instead. - Stages can be skipped by using a `skip` key that contains a [simpleeval](https://pypi.org/project/simpleeval/) expression. This allows for more complex conditional logic to determine if a stage should be skipped. @@ -147,12 +143,12 @@ stages: In this example, the stage will be skipped if `v_int` is greater than 50. Any valid simpleeval expression can be used. -The equivalent using the `if` key, where variables are bound directly rather than interpolated into the string: +The equivalent using the `if` key, which is a Starlark expression rather than a simpleeval one: ```yaml stages: - name: Run based on variable value - if: v_int <= 50 + if: "{v_int} <= 50" request: url: "{host}/fake_list" method: GET diff --git a/docs/source/scripting.md b/docs/source/scripting.md index ddb1b9400..672a89c51 100644 --- a/docs/source/scripting.md +++ b/docs/source/scripting.md @@ -81,17 +81,27 @@ last one returned something". For that, stages support two keys which are single the same embedded interpreter. There is no script, no `load()`, and stages do not need an `id`. These need the same `--tavern-experimental-starlark-pipeline` flag as `control_flow`. -All test variables - anything from `save`, `includes`, global config, fixtures, parametrisation, and the `tavern` box - -are bound directly as Starlark globals. +Test variables - anything from `save`, `includes`, global config, fixtures, parametrisation, and the `tavern` box - are +referred to with the same `{format_string}` syntax as everywhere else in Tavern. The expression is interpolated first +and the result is what gets evaluated, so `if: "{var_x} > 2"` with `var_x` saved as `3` evaluates `3 > 2`. Referring to +a variable which does not exist is an error, and error messages include both the original expression and the +interpolated one. -> **Important:** unlike almost everywhere else in Tavern, these expressions are **not** format-string interpolated. -> Write `if: var_x > 2`, not `if: "{var_x} > 2"` - the latter compares the literal string `"{var_x}"` and will silently -> do the wrong thing. Any variable whose name is not a valid Starlark identifier (for example one containing a dash) is -> not bound. +Two things follow from this which are worth being aware of: + +> **Quote your strings.** A string variable is interpolated in as-is, not as a Starlark string literal, so write +> `if: "'{name}' == 'bob'"` rather than `if: "{name} == 'bob'"` - the latter evaluates `bob == 'bob'` and fails with an +> undefined name. +> +> **Escape literal braces.** A Starlark dict or set literal in an expression needs doubled braces, as in +> `if: "{{'a': 1}}['a'] == 1"`. + +Because interpolation happens before evaluation, variable names which are not valid Starlark identifiers - anything +with a dash in it, or a Starlark reserved word - work fine. ### Running a stage conditionally with `if` -The stage only runs if the expression evaluates to `True`. This replaces the now deprecated +The stage only runs if the expression evaluates to `True`. This is an alternative to the ['skip' key](./core_concepts/marks.md#skipping-stages-with-simpleeval-expressions) - `if` is the same thing with the logic inverted, and a stage cannot use both. @@ -108,7 +118,7 @@ stages: n_existing: existing_count - name: Only tidy up if there was something there already - if: n_existing > 0 + if: "{n_existing} > 0" request: url: "{global_host}/users/cleanup" method: POST @@ -156,7 +166,7 @@ As well as the test variables, the expression has a `response` struct in scope w returned by [`run_stage()`](#run_stage): ```starlark -response.status_code == 200 and response.body["status"] == expected_status +response.status_code == 200 and response.body["status"] == "{expected_status}" ``` Because it is an arbitrary expression it can also stop on more than one outcome, which is the usual shape for polling a diff --git a/tavern/_core/run.py b/tavern/_core/run.py index 80401c5bc..5899e11a2 100644 --- a/tavern/_core/run.py +++ b/tavern/_core/run.py @@ -4,7 +4,6 @@ import logging import os import pathlib -import warnings from collections.abc import Mapping, MutableMapping from contextlib import ExitStack from copy import deepcopy @@ -304,15 +303,6 @@ def getonly(stage): try: # Run tests in a path in order for idx, stage in enumerate(test_spec["stages"]): - if "skip" in stage: - warnings.warn( # noqa - f"Stage '{stage['name']}' uses the 'skip' key, which is deprecated - " - "use the 'if' key instead (note that the logic is inverted, and it uses " - "Starlark rather than simpleeval). See 'Running a stage conditionally " - "with if' in the scripting documentation.", - DeprecationWarning, - ) - if content := stage.get("skip"): if content is True: # If it's a literal boolean true or false diff --git a/tavern/_core/schema/tests.jsonschema.yaml b/tavern/_core/schema/tests.jsonschema.yaml index 159031754..98f48787e 100644 --- a/tavern/_core/schema/tests.jsonschema.yaml +++ b/tavern/_core/schema/tests.jsonschema.yaml @@ -122,16 +122,13 @@ definitions: default: 0 skip: - deprecated: true oneOf: - type: boolean - description: Deprecated, use 'if' - whether to skip this stage + description: Whether to skip this stage default: false - type: string - description: - Deprecated, use 'if' - simpleeval expression saying whether to skip this - stage + description: simpleeval expression saying whether to skip this stage if: type: string diff --git a/tavern/_core/starlark/expressions.py b/tavern/_core/starlark/expressions.py index 55d47e3bb..092beb52c 100644 --- a/tavern/_core/starlark/expressions.py +++ b/tavern/_core/starlark/expressions.py @@ -1,10 +1,9 @@ """Evaluation of single Starlark expressions embedded in a stage. -This is used for the per-stage ``if`` and ``retry_until`` keys, which are a much -lighter-weight alternative to writing a whole ``control_flow`` script. Unlike the -simpleeval based ``skip`` key, expressions here are _not_ format-string interpolated - -variables are bound directly as Starlark globals, so ``if: var_x > 2`` works but -``if: "{var_x} > 2"`` does not. +This is used for the per-stage ``if``, ``retry_until`` and ``fail_if`` keys, which are a +much lighter-weight alternative to writing a whole ``control_flow`` script. Like the +simpleeval based ``skip`` key, expressions here are format-string interpolated before +being evaluated, so ``if: "{var_x} > 2"`` is the way to refer to a test variable. This module must not import anything from tavern._core.run, and must not import starlark at the top level, so that it can be imported (lazily) from the normal @@ -16,6 +15,7 @@ from typing import TYPE_CHECKING, Any from tavern._core import exceptions +from tavern._core.dict_util import format_keys if TYPE_CHECKING: import starlark @@ -24,45 +24,6 @@ logger: logging.Logger = logging.getLogger(__name__) -# Reserved words in Starlark which can't be used as variable names. Anything in the -# available variables which clashes with one of these is just not bound. -_STARLARK_RESERVED = frozenset( - { - "and", - "break", - "continue", - "def", - "elif", - "else", - "for", - "if", - "in", - "lambda", - "load", - "not", - "or", - "pass", - "return", - "while", - # Reserved for future use by the spec - "as", - "assert", - "class", - "del", - "except", - "finally", - "from", - "global", - "import", - "is", - "nonlocal", - "raise", - "try", - "with", - "yield", - } -) - # Name the response dict is bound to before being turned into a struct _RESPONSE_DICT_NAME = "__tavern_response" @@ -97,26 +58,6 @@ def _get_globals() -> "starlark.Globals": ) -def parse_expression(expr: str, description: str) -> None: - """Check that an expression parses, without running it - - Args: - expr: the Starlark expression - description: what this expression is, used in error messages - - Raises: - exceptions.BadSchemaError: if it could not be parsed - """ - starlark = _import_starlark() - - try: - starlark.parse(description, expr, dialect=_get_dialect()) - except starlark.StarlarkError as e: - raise exceptions.BadSchemaError( - f"Failed to parse Starlark expression for {description}: {expr}" - ) from e - - def eval_stage_expression( key: str, expr: str, @@ -218,12 +159,12 @@ def eval_expression( response: Mapping[str, Any] | None = None, description: str, ) -> bool: - """Evaluate a Starlark expression with the given variables bound as globals + """Evaluate a Starlark expression, interpolating the given variables into it first Args: - expr: the Starlark expression to evaluate - variables: test variables to bind as globals. Any key which is not a valid - Starlark identifier is skipped. + expr: the Starlark expression to evaluate, which may contain format strings + referring to test variables + variables: test variables to interpolate into the expression response: if given, a dict of response values (see :func:`tavern._core.starlark.response_struct.create_response_struct`) which is bound as a struct called 'response' @@ -233,54 +174,50 @@ def eval_expression( the result of the expression Raises: - exceptions.EvalError: if the expression could not be run, or if it did not - evaluate to a boolean + exceptions.EvalError: if the expression could not be formatted or run, or if it + did not evaluate to a boolean """ starlark = _import_starlark() from .types import from_starlark, to_starlark + try: + formatted = format_keys(expr, variables) + except exceptions.MissingFormatError as e: + raise exceptions.EvalError( + f"Undefined variable used in Starlark expression for {description}: {expr}" + ) from e + dialect = _get_dialect() module = starlark.Module() module_globals = _get_globals() - for name, value in variables.items(): - if not isinstance(name, str) or not name.isidentifier(): - logger.debug( - "Not binding variable '%s' in %s - not a valid identifier", - name, - description, - ) - continue - if name in _STARLARK_RESERVED: - logger.debug( - "Not binding variable '%s' in %s - reserved word in Starlark", - name, - description, - ) - continue - - module[name] = to_starlark(value) - if response is not None: module[_RESPONSE_DICT_NAME] = to_starlark(dict(response)) prelude = starlark.parse(description, _RESPONSE_PRELUDE, dialect=dialect) starlark.eval(module, prelude, module_globals) try: - ast = starlark.parse(description, expr, dialect=dialect) + ast = starlark.parse(description, formatted, dialect=dialect) except starlark.StarlarkError as e: raise exceptions.EvalError( - f"Error parsing Starlark expression for {description}: {expr}" + f"Error parsing Starlark expression for {description}: {formatted} " + f"(from {expr})" ) from e - logger.debug("Evaluating Starlark expression for %s: %s", description, expr) + logger.debug( + "Evaluating Starlark expression for %s: %s (from %s)", + description, + formatted, + expr, + ) try: result = starlark.eval(module, ast, module_globals) except starlark.StarlarkError as e: raise exceptions.EvalError( - f"Error evaluating Starlark expression for {description}: {expr} ({e})" + f"Error evaluating Starlark expression for {description}: {formatted} " + f"(from {expr}) ({e})" ) from e result = from_starlark(result) @@ -288,7 +225,7 @@ def eval_expression( if not isinstance(result, bool): raise exceptions.EvalError( f"Starlark expression for {description} did not evaluate to True/False " - f"(got {result} of type {type(result)}): {expr}" + f"(got {result} of type {type(result)}): {formatted} (from {expr})" ) return result diff --git a/tests/integration/starlark/test_stage_conditions.tavern.yaml b/tests/integration/starlark/test_stage_conditions.tavern.yaml index df39a507f..84086ac8c 100644 --- a/tests/integration/starlark/test_stage_conditions.tavern.yaml +++ b/tests/integration/starlark/test_stage_conditions.tavern.yaml @@ -22,7 +22,7 @@ stages: var_x: value - name: This stage should run - if: var_x > 2 + if: "{var_x} > 2" request: url: "{global_host}/echo" method: POST @@ -34,7 +34,7 @@ stages: value: "ran" - name: This stage should not run - if: var_x > 100 + if: "{var_x} > 100" request: url: "{global_host}/echo" method: POST @@ -62,7 +62,31 @@ stages: echoed: value - name: This stage should run - if: echoed["status"] == "ready" + if: "'{echoed.status}' == 'ready'" + request: + url: "{global_host}/echo" + method: POST + json: + value: "ran" + response: + status_code: 200 + json: + value: "ran" + +--- +# Because expressions are format-string interpolated rather than bound as starlark +# names, variables which aren't valid starlark identifiers work just as well +test_name: Test per-stage 'if' using a variable with a dash in the name + +includes: + - name: dashed_vars + description: a variable which is not a valid starlark identifier + variables: + my-thing: 5 + +stages: + - name: This stage should run + if: "{my-thing} > 4" request: url: "{global_host}/echo" method: POST @@ -80,7 +104,7 @@ _xfail: run stages: - name: This stage errors because the variable is not defined - if: never_saved == 1 + if: "{never_saved} == 1" request: url: "{global_host}/echo" method: POST @@ -205,7 +229,7 @@ stages: status_code: 418 max_retries: 5 delay_after: 0.1 - retry_until: response.status_code == 200 and response.body["status"] == expected_status + retry_until: response.status_code == 200 and response.body["status"] == "{expected_status}" --- test_name: Test 'fail_if' as a negative assertion on a stage which otherwise passes diff --git a/tests/unit/starlark/test_expressions.py b/tests/unit/starlark/test_expressions.py index daf309058..fa26777fc 100644 --- a/tests/unit/starlark/test_expressions.py +++ b/tests/unit/starlark/test_expressions.py @@ -13,15 +13,15 @@ def test_simple_true(self): def test_simple_false(self): assert eval_expression("1 > 2", {}, description="test") is False - def test_variable_bound_directly(self): - """Variables are bound as real values, not format-string interpolated""" - assert eval_expression("var_x > 2", {"var_x": 3}, description="test") is True - assert eval_expression("var_x > 2", {"var_x": 1}, description="test") is False + def test_variable_interpolated(self): + """Variables are interpolated into the expression before it is evaluated""" + assert eval_expression("{var_x} > 2", {"var_x": 3}, description="test") is True + assert eval_expression("{var_x} > 2", {"var_x": 1}, description="test") is False def test_string_variable(self): assert ( eval_expression( - "some_var == 'value'", {"some_var": "value"}, description="test" + "'{some_var}' == 'value'", {"some_var": "value"}, description="test" ) is True ) @@ -29,24 +29,36 @@ def test_string_variable(self): def test_nested_variable(self): assert ( eval_expression( - "thing['a']['b'] == 1", + "{thing.a.b} == 1", {"thing": {"a": {"b": 1}}}, description="test", ) is True ) - def test_format_syntax_is_not_supported(self): - """'{var}' style formatting is deliberately not done - the string is just a - literal string, so this quietly compares '{some_var}' to 'value'""" + def test_variable_with_a_dash(self): + """Names which aren't valid starlark identifiers work fine when interpolated""" + assert ( + eval_expression("{with-a-dash} > 4", {"with-a-dash": 5}, description="test") + is True + ) + + def test_format_spec(self): assert ( eval_expression( - "'{some_var}' == 'value'", {"some_var": "value"}, description="test" + "'{my_float:.2f}' == '1.50'", {"my_float": 1.5}, description="test" ) - is False + is True ) def test_undefined_variable(self): + with pytest.raises(exceptions.EvalError) as exc_info: + eval_expression("{not_a_variable} > 1", {}, description="test") + + assert "not_a_variable" in str(exc_info.value) + + def test_undefined_bare_name(self): + """A bare name which isn't interpolated is just undefined in starlark""" with pytest.raises(exceptions.EvalError) as exc_info: eval_expression("not_a_variable", {}, description="test") @@ -62,27 +74,29 @@ def test_non_bool_result(self): assert "did not evaluate to True/False" in str(exc_info.value) - def test_non_identifier_variables_are_ignored(self): - """Variables which can't be used as starlark names shouldn't break everything""" - variables = {"with-a-dash": 1, "1_starts_with_number": 2, "fine": 3} - - assert eval_expression("fine == 3", variables, description="test") is True - - def test_reserved_word_variables_are_ignored(self): + def test_reserved_word_variables(self): + """Names which are reserved words in starlark are fine when interpolated""" assert ( - eval_expression("fine == 3", {"load": 1, "fine": 3}, description="test") + eval_expression("{load} == 1", {"load": 1, "fine": 3}, description="test") is True ) - def test_opaque_variables_can_be_bound(self): - """Objects which can't be represented in starlark shouldn't break binding""" + def test_unreferenced_variables_are_ignored(self): + """Variables which aren't referenced shouldn't break anything, whatever they are""" class Something: pass - variables = {"opaque": Something(), "fine": 3} + variables = { + "opaque": Something(), + "1_starts_with_number": 2, + "fine": 3, + } + + assert eval_expression("{fine} == 3", variables, description="test") is True - assert eval_expression("fine == 3", variables, description="test") is True + def test_literal_braces_must_be_escaped(self): + assert eval_expression("{{'a': 1}}['a'] == 1", {}, description="test") is True def test_response_struct_attribute_access(self): response = {"status_code": 200, "body": {"status": "ready"}, "failed": False} @@ -102,7 +116,7 @@ def test_response_and_variables_together(self): assert ( eval_expression( - "response.status_code == expected_code", + "response.status_code == {expected_code}", {"expected_code": 500}, response=response, description="test", diff --git a/tests/unit/test_skip.py b/tests/unit/test_skip.py index fdad3741f..a0e311a0e 100644 --- a/tests/unit/test_skip.py +++ b/tests/unit/test_skip.py @@ -1,6 +1,5 @@ import dataclasses import unittest.mock -import warnings from collections.abc import Mapping from unittest.mock import patch @@ -123,21 +122,3 @@ def test_skip_empty_string(self, stage, test_block_config, run_mock): stage["skip"] = "" assert _run_test(stage, test_block_config, run_mock) is True - - @pytest.mark.parametrize("skip_value", [True, False, "False", ""]) - def test_skip_is_deprecated( - self, stage, test_block_config, run_mock, skip_value - ) -> None: - """Using the 'skip' key at all should tell people to use 'if' instead""" - - stage["skip"] = skip_value - - with pytest.warns(DeprecationWarning, match="use the 'if' key instead"): - _run_test(stage, test_block_config, run_mock) - - def test_no_deprecation_warning_without_skip( - self, stage, test_block_config, run_mock - ) -> None: - with warnings.catch_warnings(): - warnings.simplefilter("error", DeprecationWarning) - _run_test(stage, test_block_config, run_mock) diff --git a/tests/unit/test_stage_conditions.py b/tests/unit/test_stage_conditions.py index 534b3c548..bb660eb5f 100644 --- a/tests/unit/test_stage_conditions.py +++ b/tests/unit/test_stage_conditions.py @@ -64,19 +64,19 @@ def test_if_false_skips_stage(self, stage, test_block_config, run_mock): assert _run_test(stage, test_block_config, run_mock) is False def test_if_uses_saved_variable(self, stage, test_block_config, run_mock): - stage["if"] = "var_x > 2" + stage["if"] = "{var_x} > 2" test_block_config.variables.update({"var_x": 3}) assert _run_test(stage, test_block_config, run_mock) is True def test_if_uses_saved_variable_false(self, stage, test_block_config, run_mock): - stage["if"] = "var_x > 2" + stage["if"] = "{var_x} > 2" test_block_config.variables.update({"var_x": 1}) assert _run_test(stage, test_block_config, run_mock) is False def test_if_undefined_variable(self, stage, test_block_config, run_mock): - stage["if"] = "not_saved_yet == 1" + stage["if"] = "{not_saved_yet} == 1" with pytest.raises(exceptions.EvalError): _run_test(stage, test_block_config, run_mock) @@ -239,7 +239,7 @@ def test_delay_after_between_attempts(self, stage, test_block_config): sleep_mock.assert_called_once_with(0.01) def test_uses_test_variables(self, stage, test_block_config): - stage["retry_until"] = "response.body['status'] == expected_status" + stage["retry_until"] = "response.body['status'] == '{expected_status}'" test_block_config.variables["expected_status"] = "ready" inner = Mock(side_effect=_stage_failure({"status": "ready"})) @@ -382,7 +382,7 @@ def test_failing_stage_with_false_expression(self, stage, test_block_config): assert not isinstance(exc_info.value, exceptions.FailIfError) def test_uses_test_variables(self, stage, test_block_config): - stage["fail_if"] = "response.body['status'] == bad_status" + stage["fail_if"] = "response.body['status'] == '{bad_status}'" test_block_config.variables["bad_status"] = "FAILED" with pytest.raises(exceptions.FailIfError): From 61ae0edd294a918ff58aebaebd72fbd758e3752b Mon Sep 17 00:00:00 2001 From: Michael Boulton Date: Sun, 2 Aug 2026 15:11:03 +0100 Subject: [PATCH 08/12] docs(marks): Clarify potential removal of `skip` in favor of `if` - Added a note in `docs/source/core_concepts/marks.md` mentioning that `skip` may be removed in the future in favor of `if`. --- docs/source/core_concepts/marks.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/source/core_concepts/marks.md b/docs/source/core_concepts/marks.md index 7f7828347..c585f9bd6 100644 --- a/docs/source/core_concepts/marks.md +++ b/docs/source/core_concepts/marks.md @@ -124,6 +124,8 @@ stages: The [`if` key](../scripting.md#running-a-stage-conditionally-with-if) does the same thing, but the logic is inverted and it uses Starlark rather than simpleeval. The two cannot both be used on the same stage. +In future, `skip` may be removed in favour of `if`. + ##### Skipping stages with simpleeval expressions Stages can be skipped by using a `skip` key that contains a [simpleeval](https://pypi.org/project/simpleeval/) expression. From 458f9919c0e494c5c1a5f51448137d0927268eaa Mon Sep 17 00:00:00 2001 From: Michael Boulton Date: Sun, 2 Aug 2026 15:26:41 +0100 Subject: [PATCH 09/12] 2iwipwipwipw --- tavern/_core/starlark/builtins.py | 110 +++++++++++++++++++++++ tavern/_core/starlark/starlark_env.py | 104 +++------------------ tests/unit/starlark/test_expressions.py | 32 +++++++ tests/unit/starlark/test_starlark_env.py | 20 ++--- 4 files changed, 164 insertions(+), 102 deletions(-) create mode 100644 tavern/_core/starlark/builtins.py diff --git a/tavern/_core/starlark/builtins.py b/tavern/_core/starlark/builtins.py new file mode 100644 index 000000000..2fa687527 --- /dev/null +++ b/tavern/_core/starlark/builtins.py @@ -0,0 +1,110 @@ +"""Starlark library functions shared between control_flow scripts and per-stage expressions. + +These are the parts of the Starlark environment which do not need to run a stage, so +they can be used from the lightweight per-stage expressions as well as from a full +``control_flow`` script. + +This module must not import anything from tavern._core.run, and must not import starlark +at the top level, so that it can be imported (lazily) from the normal non-starlark test +path. +""" + +import functools +import importlib.resources +import logging +import re +import time +from typing import TYPE_CHECKING + +if TYPE_CHECKING: + import starlark + +logger: logging.Logger = logging.getLogger(__name__) + + +def wrap_callable(fn): + """Decorator that converts all arguments from starlark→Python before + calling *fn*, and converts the return value from Python→starlark.""" + + from .types import from_starlark, to_starlark + + @functools.wraps(fn) + def wrapper(*args, **kwargs): + converted_args = [from_starlark(a) for a in args] + converted_kwargs = {k: from_starlark(v) for k, v in kwargs.items()} + result = fn(*converted_args, **converted_kwargs) + return to_starlark(result) + + return wrapper + + +def get_helpers_source() -> str: + """Load the Starlark builtins from the tavern_helpers.star file. + + Returns: + The Starlark code for built-in helper functions + """ + return ( + importlib.resources.files(__package__) + .joinpath("tavern_helpers.star") + .read_text() + ) + + +def _match_dict(result: "re.Match | None") -> dict | None: + if result is None: + return None + return { + "group0": result.group(0), + "groups": list(result.groups()), + "start": result.start(), + "end": result.end(), + } + + +@wrap_callable +def re_match(pattern: str, string: str | bytes) -> dict | None: + if isinstance(string, bytes): + string = string.decode("utf-8") + return _match_dict(re.match(pattern, string)) + + +@wrap_callable +def re_search(pattern: str, string: str | bytes) -> dict | None: + if isinstance(string, bytes): + string = string.decode("utf-8") + return _match_dict(re.search(pattern, string)) + + +@wrap_callable +def re_sub(pattern: str, repl: str, string: str | bytes) -> str: + if isinstance(string, bytes): + return re.sub(pattern, repl, string.decode("utf-8")) + return re.sub(pattern, repl, string) + + +@wrap_callable +def time_sleep(seconds: float) -> None: + time.sleep(seconds) + + +@wrap_callable +def log(s: str) -> None: + """log a string to stdout.""" + logger.info(s) + + +def register_library_builtins(module: "starlark.Module") -> None: + """Add the functions which don't need to run a stage to a module + + Apart from 'log' these are the dunder names which tavern_helpers.star wraps up into + the 're' and 'time' structs. + + Args: + module: the starlark module to add them to + """ + module.add_callable("log", log) + module.add_callable("__re_match", re_match) + module.add_callable("__re_search", re_search) + module.add_callable("__re_sub", re_sub) + module.add_callable("__time_sleep", time_sleep) diff --git a/tavern/_core/starlark/starlark_env.py b/tavern/_core/starlark/starlark_env.py index 3eda323b4..ff882948f 100644 --- a/tavern/_core/starlark/starlark_env.py +++ b/tavern/_core/starlark/starlark_env.py @@ -2,11 +2,7 @@ import copy import dataclasses -import functools -import importlib.resources import logging -import re -import time from typing import Any, TypedDict import starlark @@ -18,6 +14,7 @@ from tavern._core.strict_util import StrictLevel from tavern._core.tincture import get_stage_tinctures +from .builtins import get_helpers_source, register_library_builtins, wrap_callable from .response_struct import create_response_struct from .stage_registry import StageRegistry from .types import from_starlark, to_starlark @@ -25,20 +22,6 @@ logger: logging.Logger = logging.getLogger(__name__) -def _wrap_callable(fn): - """Decorator that converts all arguments from starlark→Python before - calling *fn*, and converts the return value from Python→starlark.""" - - @functools.wraps(fn) - def wrapper(*args, **kwargs): - converted_args = [from_starlark(a) for a in args] - converted_kwargs = {k: from_starlark(v) for k, v in kwargs.items()} - result = fn(*converted_args, **converted_kwargs) - return to_starlark(result) - - return wrapper - - class PipelineContext(TypedDict): """Context object passed between stages in starlark pipelines. @@ -88,19 +71,6 @@ def from_starlark(cls, obj: dict) -> "StageResponse": ) -def _get_starlark_builtins() -> str: - """Load the Starlark builtins from the tavern_helpers.star file. - - Returns: - The Starlark code for built-in helper functions - """ - return ( - importlib.resources.files(__package__) - .joinpath("tavern_helpers.star") - .read_text() - ) - - class StarlarkPipelineRunner: """Runner for executing starlark pipeline scripts. @@ -163,9 +133,7 @@ def load_and_run(self, script: str) -> Any: def load(filename: str) -> starlark.FrozenModule: """Implements the 'load' function in starlark. Currently only supports loading tavern helpers.""" if filename == "@tavern_helpers.star": - ast = starlark.parse( - filename, _get_starlark_builtins(), dialect=dialect - ) + ast = starlark.parse(filename, get_helpers_source(), dialect=dialect) mod = starlark.Module() self._setup_builtins(mod) starlark.eval(mod, ast, self.globals) @@ -289,20 +257,23 @@ def _re_sub(pattern, repl, s): re = struct(match=_re_match, sub=_re_sub) - 2. Add a wrapper function into this function and add it with module.add_callable. - dunder names are used to 'hide' the original function from the user. + 2. Add a wrapper function into builtins.register_library_builtins and add it + with module.add_callable. dunder names are used to 'hide' the original + function from the user. - @_wrap_callable + @wrap_callable def re_match(pattern, s): return re.match(pattern, s) - @_wrap_callable + @wrap_callable def re_sub(pattern, repl, s): return re.sub(pattern, repl, s) module.add_callable("__re_match", re_match) module.add_callable("__re_sub", re_sub) + Anything registered there is also available in the per-stage expressions. + 3. Use from starlark by loading as before: load("@tavern_helpers.star", "re") @@ -315,7 +286,9 @@ def re_sub(pattern, repl, s): for stage_id, stage in self._stage_registry.get_all_stages().items(): module[stage_id] = to_starlark(stage) - @_wrap_callable + register_library_builtins(module) + + @wrap_callable def run_stage_binding( stage_id: str, continue_on_fail: bool, extra_vars: dict | None ) -> Any: @@ -337,56 +310,3 @@ def run_stage_binding( ) from e module.add_callable("__run_stage", run_stage_binding) - - @_wrap_callable - def log(s: str) -> None: - """log a string to stdout.""" - logger.info(s) - - module.add_callable("log", log) - - @_wrap_callable - def re_match(pattern: str, string: str | bytes) -> dict | None: - if isinstance(string, bytes): - string = string.decode("utf-8") - result = re.match(pattern, string) - if result is None: - return None - return { - "group0": result.group(0), - "groups": list(result.groups()), - "start": result.start(), - "end": result.end(), - } - - module.add_callable("__re_match", re_match) - - @_wrap_callable - def re_search(pattern: str, string: str | bytes) -> dict | None: - if isinstance(string, bytes): - string = string.decode("utf-8") - result = re.search(pattern, string) - if result is None: - return None - return { - "group0": result.group(0), - "groups": list(result.groups()), - "start": result.start(), - "end": result.end(), - } - - module.add_callable("__re_search", re_search) - - @_wrap_callable - def re_sub(pattern: str, repl: str, string: str | bytes) -> str: - if isinstance(string, bytes): - return re.sub(pattern, repl, string.decode("utf-8")) - return re.sub(pattern, repl, string) - - module.add_callable("__re_sub", re_sub) - - @_wrap_callable - def time_sleep(seconds: float) -> None: - time.sleep(seconds) - - module.add_callable("__time_sleep", time_sleep) diff --git a/tests/unit/starlark/test_expressions.py b/tests/unit/starlark/test_expressions.py index fa26777fc..92190d803 100644 --- a/tests/unit/starlark/test_expressions.py +++ b/tests/unit/starlark/test_expressions.py @@ -98,6 +98,38 @@ class Something: def test_literal_braces_must_be_escaped(self): assert eval_expression("{{'a': 1}}['a'] == 1", {}, description="test") is True + def test_multiline_script(self): + """An expression can be a whole script - the last statement is the result""" + expr = """ +n_big = len([i for i in {numbers} if i > 2]) +n_big == 2 +""" + + assert ( + eval_expression(expr, {"numbers": [1, 2, 3, 4]}, description="test") is True + ) + + def test_multiline_script_using_regex(self): + """The 're' helper module is available, as in a control_flow script""" + expr = """ +match = re.search("v(\\\\d+)\\\\.(\\\\d+)", "{version}") +match != None and all([int(g) > 0 for g in match.groups]) +""" + + assert eval_expression(expr, {"version": "v2.5"}, description="test") is True + assert eval_expression(expr, {"version": "v0.5"}, description="test") is False + + def test_regex_which_does_not_match(self): + expr = 're.search("v(\\\\d+)", "{version}") != None' + + assert eval_expression(expr, {"version": "banana"}, description="test") is False + + def test_run_stage_is_not_available(self): + with pytest.raises(exceptions.EvalError) as exc_info: + eval_expression('run_stage("a_stage").failed', {}, description="test") + + assert "only available in a 'control_flow' script" in str(exc_info.value) + def test_response_struct_attribute_access(self): response = {"status_code": 200, "body": {"status": "ready"}, "failed": False} diff --git a/tests/unit/starlark/test_starlark_env.py b/tests/unit/starlark/test_starlark_env.py index 1a57245e5..a47f3b965 100644 --- a/tests/unit/starlark/test_starlark_env.py +++ b/tests/unit/starlark/test_starlark_env.py @@ -12,11 +12,11 @@ from tavern._core import exceptions from tavern._core.run import _TestRunner +from tavern._core.starlark.builtins import wrap_callable from tavern._core.starlark.stage_registry import StageRegistry from tavern._core.starlark.starlark_env import ( StageResponse, StarlarkPipelineRunner, - _wrap_callable, ) from tavern._core.tincture import Tinctures @@ -57,12 +57,12 @@ def sample_stages(): class TestWrapCallable: - """Tests for the _wrap_callable decorator.""" + """Tests for the wrap_callable decorator.""" def test_wrap_callable_converts_args_to_starlark(self): - """Test that _wrap_callable converts Python args to starlark format.""" + """Test that wrap_callable converts Python args to starlark format.""" - @_wrap_callable + @wrap_callable def add(a, b): return a + b @@ -71,9 +71,9 @@ def add(a, b): assert result == 3 def test_wrap_callable_converts_kwargs_to_starlark(self): - """Test that _wrap_callable converts Python kwargs to starlark format.""" + """Test that wrap_callable converts Python kwargs to starlark format.""" - @_wrap_callable + @wrap_callable def format_url(base, path=""): return f"{base}{path}" @@ -81,9 +81,9 @@ def format_url(base, path=""): assert result == "http://example.com/api" def test_wrap_callable_converts_return_to_starlark(self): - """Test that _wrap_callable converts return value to starlark format.""" + """Test that wrap_callable converts return value to starlark format.""" - @_wrap_callable + @wrap_callable def get_dict(): return {"key": "value"} @@ -91,12 +91,12 @@ def get_dict(): assert result == {"key": "value"} def test_wrap_callable_converts_opaque_return_to_starlark(self): - """Test that _wrap_callable converts opaque return value to starlark format.""" + """Test that wrap_callable converts opaque return value to starlark format.""" class _boobllb: pass - @_wrap_callable + @wrap_callable def get_dict(): return _boobllb From a109545d53a78640b85a2de7da79d924a37d8ccf Mon Sep 17 00:00:00 2001 From: Michael Boulton Date: Sun, 2 Aug 2026 15:26:56 +0100 Subject: [PATCH 10/12] Revert "2iwipwipwipw" This reverts commit 914a02b5b574590bc5f42b6109ef3268a2fc7c86. --- tavern/_core/starlark/builtins.py | 110 ----------------------- tavern/_core/starlark/starlark_env.py | 104 ++++++++++++++++++--- tests/unit/starlark/test_expressions.py | 32 ------- tests/unit/starlark/test_starlark_env.py | 20 ++--- 4 files changed, 102 insertions(+), 164 deletions(-) delete mode 100644 tavern/_core/starlark/builtins.py diff --git a/tavern/_core/starlark/builtins.py b/tavern/_core/starlark/builtins.py deleted file mode 100644 index 2fa687527..000000000 --- a/tavern/_core/starlark/builtins.py +++ /dev/null @@ -1,110 +0,0 @@ -"""Starlark library functions shared between control_flow scripts and per-stage expressions. - -These are the parts of the Starlark environment which do not need to run a stage, so -they can be used from the lightweight per-stage expressions as well as from a full -``control_flow`` script. - -This module must not import anything from tavern._core.run, and must not import starlark -at the top level, so that it can be imported (lazily) from the normal non-starlark test -path. -""" - -import functools -import importlib.resources -import logging -import re -import time -from typing import TYPE_CHECKING - -if TYPE_CHECKING: - import starlark - -logger: logging.Logger = logging.getLogger(__name__) - - -def wrap_callable(fn): - """Decorator that converts all arguments from starlark→Python before - calling *fn*, and converts the return value from Python→starlark.""" - - from .types import from_starlark, to_starlark - - @functools.wraps(fn) - def wrapper(*args, **kwargs): - converted_args = [from_starlark(a) for a in args] - converted_kwargs = {k: from_starlark(v) for k, v in kwargs.items()} - result = fn(*converted_args, **converted_kwargs) - return to_starlark(result) - - return wrapper - - -def get_helpers_source() -> str: - """Load the Starlark builtins from the tavern_helpers.star file. - - Returns: - The Starlark code for built-in helper functions - """ - return ( - importlib.resources.files(__package__) - .joinpath("tavern_helpers.star") - .read_text() - ) - - -def _match_dict(result: "re.Match | None") -> dict | None: - if result is None: - return None - return { - "group0": result.group(0), - "groups": list(result.groups()), - "start": result.start(), - "end": result.end(), - } - - -@wrap_callable -def re_match(pattern: str, string: str | bytes) -> dict | None: - if isinstance(string, bytes): - string = string.decode("utf-8") - return _match_dict(re.match(pattern, string)) - - -@wrap_callable -def re_search(pattern: str, string: str | bytes) -> dict | None: - if isinstance(string, bytes): - string = string.decode("utf-8") - return _match_dict(re.search(pattern, string)) - - -@wrap_callable -def re_sub(pattern: str, repl: str, string: str | bytes) -> str: - if isinstance(string, bytes): - return re.sub(pattern, repl, string.decode("utf-8")) - return re.sub(pattern, repl, string) - - -@wrap_callable -def time_sleep(seconds: float) -> None: - time.sleep(seconds) - - -@wrap_callable -def log(s: str) -> None: - """log a string to stdout.""" - logger.info(s) - - -def register_library_builtins(module: "starlark.Module") -> None: - """Add the functions which don't need to run a stage to a module - - Apart from 'log' these are the dunder names which tavern_helpers.star wraps up into - the 're' and 'time' structs. - - Args: - module: the starlark module to add them to - """ - module.add_callable("log", log) - module.add_callable("__re_match", re_match) - module.add_callable("__re_search", re_search) - module.add_callable("__re_sub", re_sub) - module.add_callable("__time_sleep", time_sleep) diff --git a/tavern/_core/starlark/starlark_env.py b/tavern/_core/starlark/starlark_env.py index ff882948f..3eda323b4 100644 --- a/tavern/_core/starlark/starlark_env.py +++ b/tavern/_core/starlark/starlark_env.py @@ -2,7 +2,11 @@ import copy import dataclasses +import functools +import importlib.resources import logging +import re +import time from typing import Any, TypedDict import starlark @@ -14,7 +18,6 @@ from tavern._core.strict_util import StrictLevel from tavern._core.tincture import get_stage_tinctures -from .builtins import get_helpers_source, register_library_builtins, wrap_callable from .response_struct import create_response_struct from .stage_registry import StageRegistry from .types import from_starlark, to_starlark @@ -22,6 +25,20 @@ logger: logging.Logger = logging.getLogger(__name__) +def _wrap_callable(fn): + """Decorator that converts all arguments from starlark→Python before + calling *fn*, and converts the return value from Python→starlark.""" + + @functools.wraps(fn) + def wrapper(*args, **kwargs): + converted_args = [from_starlark(a) for a in args] + converted_kwargs = {k: from_starlark(v) for k, v in kwargs.items()} + result = fn(*converted_args, **converted_kwargs) + return to_starlark(result) + + return wrapper + + class PipelineContext(TypedDict): """Context object passed between stages in starlark pipelines. @@ -71,6 +88,19 @@ def from_starlark(cls, obj: dict) -> "StageResponse": ) +def _get_starlark_builtins() -> str: + """Load the Starlark builtins from the tavern_helpers.star file. + + Returns: + The Starlark code for built-in helper functions + """ + return ( + importlib.resources.files(__package__) + .joinpath("tavern_helpers.star") + .read_text() + ) + + class StarlarkPipelineRunner: """Runner for executing starlark pipeline scripts. @@ -133,7 +163,9 @@ def load_and_run(self, script: str) -> Any: def load(filename: str) -> starlark.FrozenModule: """Implements the 'load' function in starlark. Currently only supports loading tavern helpers.""" if filename == "@tavern_helpers.star": - ast = starlark.parse(filename, get_helpers_source(), dialect=dialect) + ast = starlark.parse( + filename, _get_starlark_builtins(), dialect=dialect + ) mod = starlark.Module() self._setup_builtins(mod) starlark.eval(mod, ast, self.globals) @@ -257,23 +289,20 @@ def _re_sub(pattern, repl, s): re = struct(match=_re_match, sub=_re_sub) - 2. Add a wrapper function into builtins.register_library_builtins and add it - with module.add_callable. dunder names are used to 'hide' the original - function from the user. + 2. Add a wrapper function into this function and add it with module.add_callable. + dunder names are used to 'hide' the original function from the user. - @wrap_callable + @_wrap_callable def re_match(pattern, s): return re.match(pattern, s) - @wrap_callable + @_wrap_callable def re_sub(pattern, repl, s): return re.sub(pattern, repl, s) module.add_callable("__re_match", re_match) module.add_callable("__re_sub", re_sub) - Anything registered there is also available in the per-stage expressions. - 3. Use from starlark by loading as before: load("@tavern_helpers.star", "re") @@ -286,9 +315,7 @@ def re_sub(pattern, repl, s): for stage_id, stage in self._stage_registry.get_all_stages().items(): module[stage_id] = to_starlark(stage) - register_library_builtins(module) - - @wrap_callable + @_wrap_callable def run_stage_binding( stage_id: str, continue_on_fail: bool, extra_vars: dict | None ) -> Any: @@ -310,3 +337,56 @@ def run_stage_binding( ) from e module.add_callable("__run_stage", run_stage_binding) + + @_wrap_callable + def log(s: str) -> None: + """log a string to stdout.""" + logger.info(s) + + module.add_callable("log", log) + + @_wrap_callable + def re_match(pattern: str, string: str | bytes) -> dict | None: + if isinstance(string, bytes): + string = string.decode("utf-8") + result = re.match(pattern, string) + if result is None: + return None + return { + "group0": result.group(0), + "groups": list(result.groups()), + "start": result.start(), + "end": result.end(), + } + + module.add_callable("__re_match", re_match) + + @_wrap_callable + def re_search(pattern: str, string: str | bytes) -> dict | None: + if isinstance(string, bytes): + string = string.decode("utf-8") + result = re.search(pattern, string) + if result is None: + return None + return { + "group0": result.group(0), + "groups": list(result.groups()), + "start": result.start(), + "end": result.end(), + } + + module.add_callable("__re_search", re_search) + + @_wrap_callable + def re_sub(pattern: str, repl: str, string: str | bytes) -> str: + if isinstance(string, bytes): + return re.sub(pattern, repl, string.decode("utf-8")) + return re.sub(pattern, repl, string) + + module.add_callable("__re_sub", re_sub) + + @_wrap_callable + def time_sleep(seconds: float) -> None: + time.sleep(seconds) + + module.add_callable("__time_sleep", time_sleep) diff --git a/tests/unit/starlark/test_expressions.py b/tests/unit/starlark/test_expressions.py index 92190d803..fa26777fc 100644 --- a/tests/unit/starlark/test_expressions.py +++ b/tests/unit/starlark/test_expressions.py @@ -98,38 +98,6 @@ class Something: def test_literal_braces_must_be_escaped(self): assert eval_expression("{{'a': 1}}['a'] == 1", {}, description="test") is True - def test_multiline_script(self): - """An expression can be a whole script - the last statement is the result""" - expr = """ -n_big = len([i for i in {numbers} if i > 2]) -n_big == 2 -""" - - assert ( - eval_expression(expr, {"numbers": [1, 2, 3, 4]}, description="test") is True - ) - - def test_multiline_script_using_regex(self): - """The 're' helper module is available, as in a control_flow script""" - expr = """ -match = re.search("v(\\\\d+)\\\\.(\\\\d+)", "{version}") -match != None and all([int(g) > 0 for g in match.groups]) -""" - - assert eval_expression(expr, {"version": "v2.5"}, description="test") is True - assert eval_expression(expr, {"version": "v0.5"}, description="test") is False - - def test_regex_which_does_not_match(self): - expr = 're.search("v(\\\\d+)", "{version}") != None' - - assert eval_expression(expr, {"version": "banana"}, description="test") is False - - def test_run_stage_is_not_available(self): - with pytest.raises(exceptions.EvalError) as exc_info: - eval_expression('run_stage("a_stage").failed', {}, description="test") - - assert "only available in a 'control_flow' script" in str(exc_info.value) - def test_response_struct_attribute_access(self): response = {"status_code": 200, "body": {"status": "ready"}, "failed": False} diff --git a/tests/unit/starlark/test_starlark_env.py b/tests/unit/starlark/test_starlark_env.py index a47f3b965..1a57245e5 100644 --- a/tests/unit/starlark/test_starlark_env.py +++ b/tests/unit/starlark/test_starlark_env.py @@ -12,11 +12,11 @@ from tavern._core import exceptions from tavern._core.run import _TestRunner -from tavern._core.starlark.builtins import wrap_callable from tavern._core.starlark.stage_registry import StageRegistry from tavern._core.starlark.starlark_env import ( StageResponse, StarlarkPipelineRunner, + _wrap_callable, ) from tavern._core.tincture import Tinctures @@ -57,12 +57,12 @@ def sample_stages(): class TestWrapCallable: - """Tests for the wrap_callable decorator.""" + """Tests for the _wrap_callable decorator.""" def test_wrap_callable_converts_args_to_starlark(self): - """Test that wrap_callable converts Python args to starlark format.""" + """Test that _wrap_callable converts Python args to starlark format.""" - @wrap_callable + @_wrap_callable def add(a, b): return a + b @@ -71,9 +71,9 @@ def add(a, b): assert result == 3 def test_wrap_callable_converts_kwargs_to_starlark(self): - """Test that wrap_callable converts Python kwargs to starlark format.""" + """Test that _wrap_callable converts Python kwargs to starlark format.""" - @wrap_callable + @_wrap_callable def format_url(base, path=""): return f"{base}{path}" @@ -81,9 +81,9 @@ def format_url(base, path=""): assert result == "http://example.com/api" def test_wrap_callable_converts_return_to_starlark(self): - """Test that wrap_callable converts return value to starlark format.""" + """Test that _wrap_callable converts return value to starlark format.""" - @wrap_callable + @_wrap_callable def get_dict(): return {"key": "value"} @@ -91,12 +91,12 @@ def get_dict(): assert result == {"key": "value"} def test_wrap_callable_converts_opaque_return_to_starlark(self): - """Test that wrap_callable converts opaque return value to starlark format.""" + """Test that _wrap_callable converts opaque return value to starlark format.""" class _boobllb: pass - @wrap_callable + @_wrap_callable def get_dict(): return _boobllb From 06a499c8e8a0ea6157feec9b4c80ecc141d5709e Mon Sep 17 00:00:00 2001 From: Michael Boulton Date: Sun, 2 Aug 2026 15:36:58 +0100 Subject: [PATCH 11/12] feat(starlark): Allow multiline scripts in per-stage expressions 'if', 'retry_until' and 'fail_if' can now be a YAML block scalar with several statements in it, where the value of the last statement decides the result. The helper modules can be load()ed as in a 'control_flow' script, so a condition can use 're' to pull a group out of a response. 'run_stage' is not available - the stage the expression is attached to is already being run - and loading anything other than @tavern_helpers.star is an error. The 're'/'time'/'log' bindings move to a new 'builtins' module so they can be shared with the expression path, which must not import tavern._core.run. --- docs/source/scripting.md | 46 ++++++- tavern/_core/starlark/builtins.py | 125 ++++++++++++++++++ tavern/_core/starlark/expressions.py | 53 +++++++- tavern/_core/starlark/starlark_env.py | 103 ++------------- .../test_stage_conditions.tavern.yaml | 70 ++++++++++ tests/unit/starlark/test_expressions.py | 58 ++++++++ tests/unit/starlark/test_starlark_env.py | 20 +-- 7 files changed, 368 insertions(+), 107 deletions(-) create mode 100644 tavern/_core/starlark/builtins.py diff --git a/docs/source/scripting.md b/docs/source/scripting.md index 672a89c51..a7ac4e13a 100644 --- a/docs/source/scripting.md +++ b/docs/source/scripting.md @@ -77,8 +77,8 @@ pytest --tavern-experimental-starlark-pipeline ## Per-stage expressions Rewriting a whole test as a `control_flow` script is a lot of ceremony if all you want is "only run this stage if the -last one returned something". For that, stages support two keys which are single Starlark _expressions_, evaluated by -the same embedded interpreter. There is no script, no `load()`, and stages do not need an `id`. These need the same +last one returned something". For that, stages support three keys which are Starlark _expressions_, evaluated by the +same embedded interpreter. There is no separate script and stages do not need an `id`. These need the same `--tavern-experimental-starlark-pipeline` flag as `control_flow`. Test variables - anything from `save`, `includes`, global config, fixtures, parametrisation, and the `tavern` box - are @@ -99,6 +99,46 @@ Two things follow from this which are worth being aware of: Because interpolation happens before evaluation, variable names which are not valid Starlark identifiers - anything with a dash in it, or a Starlark reserved word - work fine. +### Multiline expressions + +An 'expression' does not have to be one line. Using a YAML block scalar, any of these keys can be a short script, and +the value of its **last statement** is what decides the result - it still has to be `True` or `False`. Helper modules +have to be `load()`ed just like in a `control_flow` script, so this is the way to use `re` in a condition: + +```yaml +stages: + - name: Only upgrade if the server is on a v2 release + if: | + load("@tavern_helpers.star", "re") + + match = re.search("v(\\d+)\\.", "{server_banner}") + match != None and int(match.groups[0]) == 2 + request: + url: "{global_host}/upgrade" + method: POST + response: + status_code: 200 +``` + +The same works for `retry_until` and `fail_if`, which additionally have `response` in scope: + +```yaml + retry_until: | + load("@tavern_helpers.star", "re") + + terminal = re.match("(SUCCESS|FAILED)", response.body["status"]) + terminal != None +``` + +> **Be careful with this.** A condition which needs several statements to express is a sign the test is doing quite a +> lot of thinking, and it is easy to end up with something which is hard to read, hard to debug (Starlark errors are +> not very helpful, see [Error Messages](#error-messages)), and effectively untested. Prefer a single expression, or a +> stage which asserts on the response in its `response` block, and only reach for a script when there is no reasonable +> alternative. If it is getting long, it probably wants to be a [`control_flow` script](#basic-usage) instead. + +`run_stage()` is deliberately **not** available - there is already a stage being run - and loading anything other than +`@tavern_helpers.star` is an error. + ### Running a stage conditionally with `if` The stage only runs if the expression evaluates to `True`. This is an alternative to the @@ -643,5 +683,3 @@ includes, regex extraction, retry patterns, and the per-stage `if`/`retry_until` - Let users import their own functions into starlark? - Add a new CLI/ini flag to say "run 'finally' stages when using starlark script" - Allow `if` on `finally` stages, and give it access to the previous stage's response. -- Make `re`/`time` and any other helper modules available in per-stage `if`/`retry_until` expressions - currently only - the Starlark builtins and `struct` are in scope. diff --git a/tavern/_core/starlark/builtins.py b/tavern/_core/starlark/builtins.py new file mode 100644 index 000000000..662749dba --- /dev/null +++ b/tavern/_core/starlark/builtins.py @@ -0,0 +1,125 @@ +"""Bindings for the helper 'library' modules loaded from tavern_helpers.star. + +These are the parts of the Starlark environment which do not need a pipeline runner - +'re', 'time' and 'log'. They are shared between a full 'control_flow' script and the +per-stage expressions, which can also load them but cannot run stages. + +This module must not import anything from tavern._core.run, so that it can be imported +from the per-stage expression path. +""" + +import functools +import importlib.resources +import logging +import re +import time +from typing import TYPE_CHECKING, Any + +from .types import from_starlark, to_starlark + +if TYPE_CHECKING: + import starlark + +logger: logging.Logger = logging.getLogger(__name__) + + +def wrap_callable(fn): + """Decorator that converts all arguments from starlark→Python before + calling *fn*, and converts the return value from Python→starlark.""" + + @functools.wraps(fn) + def wrapper(*args, **kwargs): + converted_args = [from_starlark(a) for a in args] + converted_kwargs = {k: from_starlark(v) for k, v in kwargs.items()} + result = fn(*converted_args, **converted_kwargs) + return to_starlark(result) + + return wrapper + + +def get_starlark_builtins() -> str: + """Load the Starlark builtins from the tavern_helpers.star file. + + Returns: + The Starlark code for built-in helper functions + """ + return ( + importlib.resources.files(__package__) + .joinpath("tavern_helpers.star") + .read_text() + ) + + +def _match_to_dict(result: "re.Match | None") -> dict | None: + if result is None: + return None + return { + "group0": result.group(0), + "groups": list(result.groups()), + "start": result.start(), + "end": result.end(), + } + + +def add_library_callables(module: "starlark.Module") -> None: + """Add the dunder bindings which the 're', 'time' and 'log' helpers wrap + + Args: + module: the starlark module to add them to + """ + + @wrap_callable + def log(s: str) -> None: + """log a string to stdout.""" + logger.info(s) + + module.add_callable("log", log) + + @wrap_callable + def re_match(pattern: str, string: str | bytes) -> dict | None: + if isinstance(string, bytes): + string = string.decode("utf-8") + return _match_to_dict(re.match(pattern, string)) + + module.add_callable("__re_match", re_match) + + @wrap_callable + def re_search(pattern: str, string: str | bytes) -> dict | None: + if isinstance(string, bytes): + string = string.decode("utf-8") + return _match_to_dict(re.search(pattern, string)) + + module.add_callable("__re_search", re_search) + + @wrap_callable + def re_sub(pattern: str, repl: str, string: str | bytes) -> str: + if isinstance(string, bytes): + return re.sub(pattern, repl, string.decode("utf-8")) + return re.sub(pattern, repl, string) + + module.add_callable("__re_sub", re_sub) + + @wrap_callable + def time_sleep(seconds: float) -> None: + time.sleep(seconds) + + module.add_callable("__time_sleep", time_sleep) + + +def add_unavailable_run_stage(module: "starlark.Module", reason: str) -> None: + """Bind a 'run_stage' which just explains why it can't be used + + tavern_helpers.star always defines 'run_stage', so somewhere which can't run stages + still has to bind something for it to call. + + Args: + module: the starlark module to add it to + reason: message explaining why running a stage isn't possible here + """ + from tavern._core import exceptions + + @wrap_callable + def run_stage_unavailable(*args: Any, **kwargs: Any) -> Any: + raise exceptions.StarlarkError(reason) + + module.add_callable("__run_stage", run_stage_unavailable) diff --git a/tavern/_core/starlark/expressions.py b/tavern/_core/starlark/expressions.py index 092beb52c..626fcd309 100644 --- a/tavern/_core/starlark/expressions.py +++ b/tavern/_core/starlark/expressions.py @@ -5,6 +5,10 @@ simpleeval based ``skip`` key, expressions here are format-string interpolated before being evaluated, so ``if: "{var_x} > 2"`` is the way to refer to a test variable. +An 'expression' can also be several statements long, in which case the value of the last +one is what decides the result. The helper modules can be loaded as in a ``control_flow`` +script, but ``run_stage`` is not available - there is already a stage being run. + This module must not import anything from tavern._core.run, and must not import starlark at the top level, so that it can be imported (lazily) from the normal non-starlark test path. @@ -58,6 +62,51 @@ def _get_globals() -> "starlark.Globals": ) +def _get_file_loader( + module_globals: "starlark.Globals", dialect: "starlark.Dialect" +) -> "starlark.FileLoader": + """Get a loader which makes the tavern helper modules available to an expression + + This is the same set of helpers as in a 'control_flow' script, except that + 'run_stage' can't do anything - the stage the expression is attached to is already + being run. + + Args: + module_globals: globals to evaluate the helpers with + dialect: dialect to parse the helpers with + + Returns: + a loader which handles '@tavern_helpers.star' + """ + starlark = _import_starlark() + + from .builtins import ( + add_library_callables, + add_unavailable_run_stage, + get_starlark_builtins, + ) + + # The return type is a starlark.FrozenModule, but the name is shadowed by the + # local import above + def load(filename: str) -> Any: + if filename != "@tavern_helpers.star": + raise FileNotFoundError(filename) + + helpers = starlark.Module() + add_library_callables(helpers) + add_unavailable_run_stage( + helpers, + "'run_stage' is not available in a per-stage expression - use a " + "'control_flow' script if you need to run another stage", + ) + ast = starlark.parse(filename, get_starlark_builtins(), dialect=dialect) + starlark.eval(helpers, ast, module_globals) + + return helpers.freeze() + + return starlark.FileLoader(load) + + def eval_stage_expression( key: str, expr: str, @@ -213,7 +262,9 @@ def eval_expression( ) try: - result = starlark.eval(module, ast, module_globals) + result = starlark.eval( + module, ast, module_globals, _get_file_loader(module_globals, dialect) + ) except starlark.StarlarkError as e: raise exceptions.EvalError( f"Error evaluating Starlark expression for {description}: {formatted} " diff --git a/tavern/_core/starlark/starlark_env.py b/tavern/_core/starlark/starlark_env.py index 3eda323b4..56cfa3b92 100644 --- a/tavern/_core/starlark/starlark_env.py +++ b/tavern/_core/starlark/starlark_env.py @@ -2,11 +2,7 @@ import copy import dataclasses -import functools -import importlib.resources import logging -import re -import time from typing import Any, TypedDict import starlark @@ -18,6 +14,7 @@ from tavern._core.strict_util import StrictLevel from tavern._core.tincture import get_stage_tinctures +from .builtins import add_library_callables, get_starlark_builtins, wrap_callable from .response_struct import create_response_struct from .stage_registry import StageRegistry from .types import from_starlark, to_starlark @@ -25,20 +22,6 @@ logger: logging.Logger = logging.getLogger(__name__) -def _wrap_callable(fn): - """Decorator that converts all arguments from starlark→Python before - calling *fn*, and converts the return value from Python→starlark.""" - - @functools.wraps(fn) - def wrapper(*args, **kwargs): - converted_args = [from_starlark(a) for a in args] - converted_kwargs = {k: from_starlark(v) for k, v in kwargs.items()} - result = fn(*converted_args, **converted_kwargs) - return to_starlark(result) - - return wrapper - - class PipelineContext(TypedDict): """Context object passed between stages in starlark pipelines. @@ -88,19 +71,6 @@ def from_starlark(cls, obj: dict) -> "StageResponse": ) -def _get_starlark_builtins() -> str: - """Load the Starlark builtins from the tavern_helpers.star file. - - Returns: - The Starlark code for built-in helper functions - """ - return ( - importlib.resources.files(__package__) - .joinpath("tavern_helpers.star") - .read_text() - ) - - class StarlarkPipelineRunner: """Runner for executing starlark pipeline scripts. @@ -163,9 +133,7 @@ def load_and_run(self, script: str) -> Any: def load(filename: str) -> starlark.FrozenModule: """Implements the 'load' function in starlark. Currently only supports loading tavern helpers.""" if filename == "@tavern_helpers.star": - ast = starlark.parse( - filename, _get_starlark_builtins(), dialect=dialect - ) + ast = starlark.parse(filename, get_starlark_builtins(), dialect=dialect) mod = starlark.Module() self._setup_builtins(mod) starlark.eval(mod, ast, self.globals) @@ -289,14 +257,16 @@ def _re_sub(pattern, repl, s): re = struct(match=_re_match, sub=_re_sub) - 2. Add a wrapper function into this function and add it with module.add_callable. - dunder names are used to 'hide' the original function from the user. + 2. Add a wrapper function into builtins.add_library_callables (or into this + function, if it needs the pipeline runner) and add it with + module.add_callable. dunder names are used to 'hide' the original function + from the user. - @_wrap_callable + @wrap_callable def re_match(pattern, s): return re.match(pattern, s) - @_wrap_callable + @wrap_callable def re_sub(pattern, repl, s): return re.sub(pattern, repl, s) @@ -312,10 +282,12 @@ def re_sub(pattern, repl, s): if not re.match("(one_thing|another_thing)", resp.json["key"]): fail("No match found") """ + add_library_callables(module) + for stage_id, stage in self._stage_registry.get_all_stages().items(): module[stage_id] = to_starlark(stage) - @_wrap_callable + @wrap_callable def run_stage_binding( stage_id: str, continue_on_fail: bool, extra_vars: dict | None ) -> Any: @@ -337,56 +309,3 @@ def run_stage_binding( ) from e module.add_callable("__run_stage", run_stage_binding) - - @_wrap_callable - def log(s: str) -> None: - """log a string to stdout.""" - logger.info(s) - - module.add_callable("log", log) - - @_wrap_callable - def re_match(pattern: str, string: str | bytes) -> dict | None: - if isinstance(string, bytes): - string = string.decode("utf-8") - result = re.match(pattern, string) - if result is None: - return None - return { - "group0": result.group(0), - "groups": list(result.groups()), - "start": result.start(), - "end": result.end(), - } - - module.add_callable("__re_match", re_match) - - @_wrap_callable - def re_search(pattern: str, string: str | bytes) -> dict | None: - if isinstance(string, bytes): - string = string.decode("utf-8") - result = re.search(pattern, string) - if result is None: - return None - return { - "group0": result.group(0), - "groups": list(result.groups()), - "start": result.start(), - "end": result.end(), - } - - module.add_callable("__re_search", re_search) - - @_wrap_callable - def re_sub(pattern: str, repl: str, string: str | bytes) -> str: - if isinstance(string, bytes): - return re.sub(pattern, repl, string.decode("utf-8")) - return re.sub(pattern, repl, string) - - module.add_callable("__re_sub", re_sub) - - @_wrap_callable - def time_sleep(seconds: float) -> None: - time.sleep(seconds) - - module.add_callable("__time_sleep", time_sleep) diff --git a/tests/integration/starlark/test_stage_conditions.tavern.yaml b/tests/integration/starlark/test_stage_conditions.tavern.yaml index 84086ac8c..da0e1bcd1 100644 --- a/tests/integration/starlark/test_stage_conditions.tavern.yaml +++ b/tests/integration/starlark/test_stage_conditions.tavern.yaml @@ -97,6 +97,76 @@ stages: json: value: "ran" +--- +# An 'expression' can be several statements long, with the value of the last one +# deciding the result. The helper modules still have to be loaded, as in a +# 'control_flow' script +test_name: Test per-stage 'if' using a multiline script + +stages: + - name: Echo a version string to save + request: + url: "{global_host}/echo" + method: POST + json: + value: "server v2.5.1" + response: + status_code: 200 + save: + json: + saved_version: value + + - name: This stage should run because the major version is 2 + if: | + load("@tavern_helpers.star", "re") + + match = re.search("v(\\d+)\\.", "{saved_version}") + match != None and int(match.groups[0]) == 2 + request: + url: "{global_host}/echo" + method: POST + json: + value: "ran" + response: + status_code: 200 + json: + value: "ran" + + - name: This stage should not run because the major version is not 3 + if: | + load("@tavern_helpers.star", "re") + + match = re.search("v(\\d+)\\.", "{saved_version}") + match != None and int(match.groups[0]) == 3 + request: + url: "{global_host}/echo" + method: POST + json: + value: "should not have run" + response: + # If this stage is ever actually run it will fail here + status_code: 999 + +--- +test_name: Test 'retry_until' using a multiline script + +stages: + - name: Poll until the job reaches a state matching a regex + request: + url: "{global_host}/job/multiline-works" + method: GET + response: + status_code: 200 + json: + status: SUCCESS + max_retries: 5 + delay_after: 0.1 + retry_until: | + load("@tavern_helpers.star", "re") + + terminal = re.match("(SUCCESS|FAILED)", response.body["status"]) + terminal != None + --- test_name: Test per-stage 'if' referring to a variable that was never saved diff --git a/tests/unit/starlark/test_expressions.py b/tests/unit/starlark/test_expressions.py index fa26777fc..47449fba2 100644 --- a/tests/unit/starlark/test_expressions.py +++ b/tests/unit/starlark/test_expressions.py @@ -134,6 +134,64 @@ def test_response_missing_key(self): ) +class TestMultilineExpression: + """An 'expression' can be several statements, the last of which is the result""" + + def test_last_statement_is_the_result(self): + script = """ +x = {var_x} * 2 +y = x + 1 +y == 7 +""" + assert eval_expression(script, {"var_x": 3}, description="test") is True + + def test_last_statement_must_be_a_boolean(self): + script = """ +x = 1 +x +""" + with pytest.raises(exceptions.EvalError) as exc_info: + eval_expression(script, {}, description="test") + + assert "did not evaluate to True/False" in str(exc_info.value) + + def test_can_load_the_regex_helpers(self): + script = r""" +load("@tavern_helpers.star", "re") + +match = re.search("v(\\d+)\\.", response.body["version"]) +match != None and match.groups[0] == "{expected_major}" +""" + assert ( + eval_expression( + script, + {"expected_major": 25}, + response={"body": {"version": "v25.3.1"}}, + description="test", + ) + is True + ) + + def test_load_of_something_else_is_an_error(self): + with pytest.raises(exceptions.EvalError): + eval_expression( + 'load("@not_a_module.star", "thing")\nTrue', + {}, + description="test", + ) + + def test_run_stage_is_not_available(self): + script = """ +load("@tavern_helpers.star", "run_stage") + +run_stage("some_stage").failed +""" + with pytest.raises(exceptions.EvalError) as exc_info: + eval_expression(script, {}, description="test") + + assert "control_flow" in str(exc_info.value) + + class TestStageExpressionGuard: def test_requires_experimental_flag(self, fix_test_config): config = dataclasses.replace( diff --git a/tests/unit/starlark/test_starlark_env.py b/tests/unit/starlark/test_starlark_env.py index 1a57245e5..a47f3b965 100644 --- a/tests/unit/starlark/test_starlark_env.py +++ b/tests/unit/starlark/test_starlark_env.py @@ -12,11 +12,11 @@ from tavern._core import exceptions from tavern._core.run import _TestRunner +from tavern._core.starlark.builtins import wrap_callable from tavern._core.starlark.stage_registry import StageRegistry from tavern._core.starlark.starlark_env import ( StageResponse, StarlarkPipelineRunner, - _wrap_callable, ) from tavern._core.tincture import Tinctures @@ -57,12 +57,12 @@ def sample_stages(): class TestWrapCallable: - """Tests for the _wrap_callable decorator.""" + """Tests for the wrap_callable decorator.""" def test_wrap_callable_converts_args_to_starlark(self): - """Test that _wrap_callable converts Python args to starlark format.""" + """Test that wrap_callable converts Python args to starlark format.""" - @_wrap_callable + @wrap_callable def add(a, b): return a + b @@ -71,9 +71,9 @@ def add(a, b): assert result == 3 def test_wrap_callable_converts_kwargs_to_starlark(self): - """Test that _wrap_callable converts Python kwargs to starlark format.""" + """Test that wrap_callable converts Python kwargs to starlark format.""" - @_wrap_callable + @wrap_callable def format_url(base, path=""): return f"{base}{path}" @@ -81,9 +81,9 @@ def format_url(base, path=""): assert result == "http://example.com/api" def test_wrap_callable_converts_return_to_starlark(self): - """Test that _wrap_callable converts return value to starlark format.""" + """Test that wrap_callable converts return value to starlark format.""" - @_wrap_callable + @wrap_callable def get_dict(): return {"key": "value"} @@ -91,12 +91,12 @@ def get_dict(): assert result == {"key": "value"} def test_wrap_callable_converts_opaque_return_to_starlark(self): - """Test that _wrap_callable converts opaque return value to starlark format.""" + """Test that wrap_callable converts opaque return value to starlark format.""" class _boobllb: pass - @_wrap_callable + @wrap_callable def get_dict(): return _boobllb From 98068eaf548ee8f1dc9c30547891cb8b8a872625 Mon Sep 17 00:00:00 2001 From: Michael Boulton Date: Sun, 2 Aug 2026 15:43:29 +0100 Subject: [PATCH 12/12] test(starlark): Pin the value of a multiline expression ending in an assignment Returning the value of the last statement is behaviour of the starlark binding rather than something the language spec promises, and a script which ends on an assignment quietly evaluates to None instead. --- docs/source/scripting.md | 6 ++++-- tests/unit/starlark/test_expressions.py | 12 ++++++++++++ 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/docs/source/scripting.md b/docs/source/scripting.md index a7ac4e13a..409a2b9da 100644 --- a/docs/source/scripting.md +++ b/docs/source/scripting.md @@ -102,8 +102,10 @@ with a dash in it, or a Starlark reserved word - work fine. ### Multiline expressions An 'expression' does not have to be one line. Using a YAML block scalar, any of these keys can be a short script, and -the value of its **last statement** is what decides the result - it still has to be `True` or `False`. Helper modules -have to be `load()`ed just like in a `control_flow` script, so this is the way to use `re` in a condition: +the value of its **last statement** is what decides the result - it still has to be `True` or `False`. Note that only +an expression has a value, so a script which ends on an assignment (`result = x == 1`) fails rather than using what was +assigned. Helper modules have to be `load()`ed just like in a `control_flow` script, so this is the way to use `re` in +a condition: ```yaml stages: diff --git a/tests/unit/starlark/test_expressions.py b/tests/unit/starlark/test_expressions.py index 47449fba2..e2e7f1146 100644 --- a/tests/unit/starlark/test_expressions.py +++ b/tests/unit/starlark/test_expressions.py @@ -149,6 +149,18 @@ def test_last_statement_must_be_a_boolean(self): script = """ x = 1 x +""" + with pytest.raises(exceptions.EvalError) as exc_info: + eval_expression(script, {}, description="test") + + assert "did not evaluate to True/False" in str(exc_info.value) + + def test_ending_with_an_assignment_is_an_error(self): + """Only an expression statement has a value - anything else is None, which is + not a useful answer to 'should this stage run'""" + script = """ +x = 1 +result = x == 1 """ with pytest.raises(exceptions.EvalError) as exc_info: eval_expression(script, {}, description="test")