From ccc6c5d1fd986b43523068b97df8719155caabc4 Mon Sep 17 00:00:00 2001 From: Joshua Wink Date: Sat, 10 Oct 2026 08:54:32 -0500 Subject: [PATCH] feat(core)!: report a step by a label, the name of its class State and Hooks reported each step by the full path of its class (module.QualName). They now report the class name: DiscountFilter. The second use of a class in one pipeline is DiscountFilter#2. A Valve has the label of its Filter. Removed, because they existed only for the full path: - AnonymousStep, and the refusal of classes from a function or from the entry script. A step without the method of its role is refused with a TypeError when it is added. - Pipeline.identities (now Pipeline.labels) and Pipeline.step_sites. - Lint rule CUP-A006 (entry-script step). BREAKING CHANGE: the strings in state.executed, state.skipped, state.errors, state.timings and in Hook calls are class names, not module paths. AnonymousStep, Pipeline.identities and Pipeline.step_sites are removed. Co-Authored-By: Claude Opus 5.5 --- README.md | 7 +- conformance/API.md | 103 ++++----- conformance/README.md | 2 +- .../runners/python/cup_ops/nested_pipeline.py | 2 +- .../runners/python/cup_ops/record_hook.py | 2 +- conformance/runners/python/runner.py | 8 +- docs/static-export.md | 12 +- examples/pricing/print_steps_hook.py | 6 +- packages/codeupipe-lint/README.md | 23 +- .../anonymous_steps/python_step_visitor.py | 88 ++------ .../codeupipe_lint/anonymous_steps/rules.py | 15 +- packages/codeupipe/README.md | 5 +- packages/codeupipe/codeupipe/core/README.md | 15 +- packages/codeupipe/codeupipe/core/__init__.py | 3 +- .../codeupipe/core/anonymous_step.py | 13 -- packages/codeupipe/codeupipe/core/pipeline.py | 61 ++--- packages/codeupipe/codeupipe/core/state.py | 6 +- .../codeupipe/codeupipe/core/step_identity.py | 77 ------- .../codeupipe/codeupipe/core/step_label.py | 19 ++ packages/codeupipe/codeupipe/core/valve.py | 9 +- .../lower/filters/emit_orchestrator_filter.py | 2 +- .../lower/filters/load_pipeline_filter.py | 8 +- .../filters/rewrite_payload_ops_filter.py | 2 +- packages/codeupipe/codeupipe/lower/ir.py | 4 +- tests/core/test_add_valve.py | 14 +- tests/core/test_api_contract.py | 12 +- tests/core/test_execution_modes.py | 6 +- tests/core/test_pipeline.py | 26 +-- tests/core/test_step_identity.py | 213 ------------------ tests/core/test_step_labels.py | 170 ++++++++++++++ tests/core/test_sync_pipeline.py | 12 +- tests/core/test_sync_support.py | 12 +- tests/core/test_valve_sync.py | 2 +- tests/examples/test_pricing.py | 12 +- .../test_analyze_python_filter.py | 2 +- .../anonymous_steps/test_lint_pipeline.py | 10 +- .../anonymous_steps/test_repo_is_clean.py | 4 +- tests/lint/fixtures/python/entry_script.py | 38 ---- tests/lint/fixtures/python/probe.py | 4 +- tests/lower/test_export_observe.py | 16 +- tests/lower/test_export_pipeline.py | 10 +- tests/lower/test_load_pipeline_filter.py | 4 +- 42 files changed, 414 insertions(+), 645 deletions(-) delete mode 100644 packages/codeupipe/codeupipe/core/anonymous_step.py delete mode 100644 packages/codeupipe/codeupipe/core/step_identity.py create mode 100644 packages/codeupipe/codeupipe/core/step_label.py delete mode 100644 tests/core/test_step_identity.py create mode 100644 tests/core/test_step_labels.py delete mode 100644 tests/lint/fixtures/python/entry_script.py diff --git a/README.md b/README.md index 89e3b1e..bebed8c 100644 --- a/README.md +++ b/README.md @@ -123,10 +123,9 @@ A Valve is made in the pipeline file: `add_valve(predicate, filter)`. It joins a Predicate to a Filter. The Filter runs only when `predicate.test(payload)` is true. There is no Valve file. -A step must be a class in a module. A lambda, or a class in the script that -you run, is refused. The reason is step identity: a step is identified by its -type, for example `examples.pricing.discount_filter.DiscountFilter`. State, -Hooks and the export use that identity. +A step is a class. State and Hooks report each step by the name of its +class, for example `DiscountFilter`. A lambda is refused, because it does not +have the method of a step. ```bash python -m examples.pricing diff --git a/conformance/API.md b/conformance/API.md index 3caaa39..d909af5 100644 --- a/conformance/API.md +++ b/conformance/API.md @@ -94,7 +94,7 @@ A Valve is added with one call, the condition first: - When `predicate.test(payload)` is true, the Filter runs. When it is false, the Payload passes through unchanged and the step is recorded in `state.skipped`, not `state.executed`. -- The step is identified by its Filter (see "Step identity"). +- The step has the label of its Filter (see "Step labels"). - `add_valve` is the one way to add a Valve. There is no Valve class for a user to make, and `add_filter` does not accept one. @@ -102,7 +102,7 @@ A Valve is added with one call, the condition first: `observe(payload)`, no return value. A Tap reads the Payload and does not write to it. **A Tap failure never halts the pipeline:** the error is -recorded in `state.errors` under the Tap's identity, the Tap still counts as +recorded in `state.errors` under the Tap's label, the Tap still counts as executed, and hooks are not notified. ## Hook — sees the lifecycle @@ -110,7 +110,7 @@ executed, and hooks are not notified. `before(step, payload)`, `after(step, payload)`, `on_error(step, error, payload)`. A Hook reads the Payload and does not write to it. -- `step` is the **step's identity** (see "Step identity"). Pipeline-level +- `step` is the **step's label** (see "Step labels"). Pipeline-level events pass absent (`None`). - `error` is the error itself (never just its message). - Order on success: `before(pipeline)`, then for each filter or Valve @@ -121,8 +121,8 @@ A Hook reads the Payload and does not write to it. ## State — what happened in the last run -`executed` (identities, in order), `skipped` (identities), `errors` -(identity + error), `timings` (identity → seconds, when timing is on). A run +`executed` (labels, in order), `skipped` (labels), `errors` +(label + error), `timings` (label → seconds, when timing is on). A run starts with a new State. **A failed run keeps the steps that completed before the failure.** @@ -140,66 +140,57 @@ before the failure.** | run (async, opt-in) | `run_async(p)` | | last run | `state` | | timing and lineage | `observe(timing=True, lineage=False)` | -| each step's identity, in order | `identities` | -| where each step was added (`file:line`) | `step_sites` | +| each step's label, in order | `labels` | -- **Steps have no names.** State and Hooks report each step's identity. +- **Steps are not named by hand.** State and Hooks report each step's label. - **`run` is synchronous** and never starts an event loop. If it meets an async step it raises `AsyncStepInSyncRun` rather than silently paying for a loop. -- A nested pipeline is one step: its identity appears once in the parent's +- A nested pipeline is one step: its label appears once in the parent's State. - **Timing and lineage are off by default.** -## Step identity: the filter, not a name - -A step's identity is a pointer to the code that runs it, derived -automatically: `add_filter(f)`, not `add_filter(f, name)`. State, Hooks and -exported code all report this identity. - -Why: in a codebase maintained by agents and reviewed by humans, an identity -taken from the code cannot drift from it. A hand-written name is an -unchecked label that can describe code which no longer matches it; a pointer -to the filter is always true, more specific, and needs no one to maintain -it. - -### The identity - -- **The filter's fully qualified symbol** (`app.filters.Validate`), plus an - **occurrence number only when the same identity appears more than once** - in one pipeline: the second is `...Validate#2`. Stable across unrelated - edits. -- **Extra detail, not identity**: the source location of the `add_*` call - (`file:line`), for agents and reviewers. -- **Valves**: the Filter's identity; the predicate's symbol is extra detail. -- **Pipelines used as a step are named types**: `class UserOnboarding(Pipeline)`, - identified by that type's symbol exactly as a filter is. A plain, unnamed - `Pipeline` can run at the top level but cannot be nested. -- Renames and moves change the identity. That is honest (it is different - code); cross-version comparisons follow renames from version control. -- A filter defined in the entry script (`__main__.X`) is refused like an - anonymous one: its identity changes with how the program is started, and - tests cannot import it. - -### Anonymous filters are unsupported - -Lambdas, closures, classes defined inside a function, inline functions: as -filters, taps, valve predicates or nested pipelines. They are not -independently testable and have no stable identity. Enforced in two layers: - -1. **The runtime refuses them** at `add_*` time (`AnonymousStep`), with a - message naming the fix. -2. **The linter flags them** at authoring time: - `python -m codeupipe_lint.anonymous_steps `. One line per - anonymous step, `path:line:col CUP-A00N slug message`; exit 1 when it - finds any. An intentional one (a test of the refusal) is suppressed with - a `cup: allow-anonymous` comment. Rules and limits: - [packages/codeupipe-lint/README.md](../packages/codeupipe-lint/README.md). +## Step labels + +A step is reported by a label. The label is the name of the step's class: +`add_filter(ValidateFilter())` gives the label `ValidateFilter`. State and +Hooks report this label. A step is not given a name by hand: +`add_filter(f)`, not `add_filter(f, name)`. + +- **A second use gets a number.** When the same label is in one pipeline + more than once, the second is `ValidateFilter#2`, the third + `ValidateFilter#3`. +- **A Valve** has the label of its Filter. +- **A nested pipeline** has the label of its class. Make it a named type + (`class UserOnboarding(Pipeline)`), so that the label says what it is. A + plain `Pipeline` that is nested has the label `Pipeline`. +- A label is for people and for records. It does not say where the class is + defined, and two classes with the same name in different modules have the + same label. + +### What is refused when a step is added + +A step must have the method of its role. `add_filter` needs `call(payload)`, +`add_tap` needs `observe(payload)`, `add_valve` needs a predicate with +`test(payload)` and a Filter with `call(payload)`, and `add_pipeline` needs a +Pipeline. A value without that method (a lambda, a plain function) is +refused with a `TypeError` that names the method. + +### What the linter flags + +A step that is a lambda, a function, a class defined inside a function, or a +plain nested `Pipeline` cannot be imported and tested alone, and its label +says little. The linter flags these when the code is written: +`python -m codeupipe_lint.anonymous_steps `. One line per finding, +`path:line:col CUP-A00N slug message`; exit 1 when it finds any. An +intentional one is suppressed with a `cup: allow-anonymous` comment. Rules +and limits: +[packages/codeupipe-lint/README.md](../packages/codeupipe-lint/README.md). **Conformance.** The shared cases name their steps; the runner builds every -step, so it maps each reported identity back to the case's step name. An -identity that is wrong, or differs between the dynamic run and an export, -does not map and fails the comparison. +step, so it maps each reported label back to the case's step name. A label +that is wrong, or differs between the dynamic run and an export, does not +map and fails the comparison. ## Not in the contract yet diff --git a/conformance/README.md b/conformance/README.md index f9a852e..26cb6bd 100644 --- a/conformance/README.md +++ b/conformance/README.md @@ -89,7 +89,7 @@ Numbers compare as JSON numbers (`1 == 1.0`), but `true` is never `1`. | tap | `fail` message | raise | | valve `when` | `equals` key, value | `get(key) == value` | | valve `when` | `truthy` key | `get(key)` is truthy (missing ⇒ false) | -| hooks | `"hooks": true` | record `before:`, `after:`, `error:`; pipeline-level events use an empty step name. On a filter failure: `error:` then `error:`. The runner maps each reported step identity (API.md) back to the case's step name | +| hooks | `"hooks": true` | record `before:`, `after:`, `error:`; pipeline-level events use an empty step name. On a filter failure: `error:` then `error:`. The runner maps each reported step label (API.md) back to the case's step name | ## Runner protocol diff --git a/conformance/runners/python/cup_ops/nested_pipeline.py b/conformance/runners/python/cup_ops/nested_pipeline.py index ab078c3..5812f5d 100644 --- a/conformance/runners/python/cup_ops/nested_pipeline.py +++ b/conformance/runners/python/cup_ops/nested_pipeline.py @@ -5,4 +5,4 @@ class NestedPipeline(Pipeline): """The runner's named pipeline type: nested pipelines must be named - types (conformance/API.md, step identity).""" + types (conformance/API.md, step labels).""" diff --git a/conformance/runners/python/cup_ops/record_hook.py b/conformance/runners/python/cup_ops/record_hook.py index cb6216e..679266f 100644 --- a/conformance/runners/python/cup_ops/record_hook.py +++ b/conformance/runners/python/cup_ops/record_hook.py @@ -5,7 +5,7 @@ class RecordHook: """Records ``before:``, ``after:``, ``error:``; the - pipeline-level events use an empty step. Hooks receive step identities; + pipeline-level events use an empty step. Hooks receive step labels; the runner maps them back to the case's step names (runner.labels).""" def before(self, step, payload): diff --git a/conformance/runners/python/runner.py b/conformance/runners/python/runner.py index 759395f..323c2be 100644 --- a/conformance/runners/python/runner.py +++ b/conformance/runners/python/runner.py @@ -130,15 +130,15 @@ def build(case, cls, predicates, names): def labels(p, names): - """identity -> the case's step name, for the steps of ``p``. + """label -> the case's step name, for the steps of ``p``. - Steps are identified by their filter (conformance/API.md), while the + Steps are reported by label (conformance/API.md), while the shared cases name them. The runner built every step, so it knows which - case step each identity belongs to. An identity reported that is not in + case step each label belongs to. A label reported that is not in this map (wrong, or inconsistent between modes) stays as is and fails the comparison. """ - return {identity: names[id(step)] for identity, step, _type in p._steps} + return {label: names[id(step)] for label, step, _type in p._steps} def _named(steps, label): diff --git a/docs/static-export.md b/docs/static-export.md index b032f8d..ca02191 100644 --- a/docs/static-export.md +++ b/docs/static-export.md @@ -52,13 +52,13 @@ Exit code: **0** exported, **1** refused (the message names the step and the fix), **2** the command could not run. Nothing is written on 1 or 2. Tests: [tests/lower/test_cli.py](../tests/lower/test_cli.py). -## Step identity in exported code +## Step labels in exported code -Steps are identified by their filter (`app.filters.Validate`, `#2` from the -second use; conformance/API.md). An identity is a label, not an identifier, -so exported code never uses it as a name: each step gets a generated module -and function, `step_01_validate`, `step_02_validate`, and the identity -appears only as a string in State and in Hook calls. That also rules out +A step has a label, the name of its class (`Validate`, `Validate#2` from the +second use; conformance/API.md). A label is not an identifier, so exported +code never uses it as a name: each step gets a generated module and +function, `step_01_validate`, `step_02_validate`, and the label appears only +as a string in State and in Hook calls. That also rules out every collision between a step and a builtin, a module global or the generated `run()` (scorecard findings F3–F8). diff --git a/examples/pricing/print_steps_hook.py b/examples/pricing/print_steps_hook.py index 099cb87..f8e20b8 100644 --- a/examples/pricing/print_steps_hook.py +++ b/examples/pricing/print_steps_hook.py @@ -4,12 +4,12 @@ class PrintStepsHook(Hook): - """Sees the lifecycle. ``step`` is the step's identity; it is None for - the pipeline itself.""" + """Sees the lifecycle. ``step`` is the label of the step, which is the + name of its class. It is None for the pipeline itself.""" def before(self, step, payload): if step is not None: - print(" " + step.replace("examples.pricing.", "")) + print(" " + step) def after(self, step, payload): pass diff --git a/packages/codeupipe-lint/README.md b/packages/codeupipe-lint/README.md index a3660e4..fb9237a 100644 --- a/packages/codeupipe-lint/README.md +++ b/packages/codeupipe-lint/README.md @@ -26,10 +26,10 @@ in the list: each linter uses the ids that are its own. ## Anonymous-step linter -Step identity (conformance/API.md, "Step identity") makes a step's identity a -pointer to its filter. Anonymous steps have no such pointer and are -unsupported. The runtime refuses them when the step is added; this linter -flags them at authoring time. +A step is reported by the name of its class (conformance/API.md, "Step +labels"). A step that is a lambda, a function, a class defined inside a +function, or a plain nested `Pipeline` has no useful name and cannot be +imported and tested alone. This linter flags them when the code is written. ```bash python -m codeupipe_lint.anonymous_steps # files or directories @@ -40,7 +40,7 @@ python -m codeupipe_lint.anonymous_steps --rules # list the rule i Each finding is one line, and the message names the fix: ``` -path:line:col CUP-A001 lambda-step Valve predicate is a lambda. Define it at module level (...) so it has a stable identity and can be tested on its own. +path:line:col CUP-A001 lambda-step Valve predicate is a lambda. Define it at module level (...) so it has a name in State and can be tested on its own. ``` With no paths, both linters read them from `cup.config.json` @@ -61,7 +61,6 @@ returns the result Payload (`findings`, `suppressed`, `errors`, `report`, `exit_ | CUP-A002 | local-function-step | a named function declared inside a function | | CUP-A003 | local-type-step | a class declared inside a function, or an instance of one | | CUP-A005 | unnamed-pipeline-step | a plain, unnamed `Pipeline` nested as a step; nested pipelines must be named Pipeline types | -| CUP-A006 | entry-script-step | a class or function of a script (`__main__`), added from its `if __name__ == "__main__":` block | | CUP-A008 | function-predicate | a named function as a Valve predicate. Predicates are objects with `test(payload)` (conformance/API.md, "Valve") | A step is any argument of `add_filter`, `add_tap` or `add_pipeline` (on any @@ -75,19 +74,15 @@ A comment containing `# cup: allow-anonymous` (any text may follow) suppresses a finding when it is on the finding's line, on the line of the enclosing add or Valve call, or alone on the line directly above. Suppressed findings are counted in the summary. Use it only where the anonymous step is -the point, as in the tests that exercise the runtime's refusal -(`tests/core/test_step_identity.py`). +the point, as in a test of what the runtime does with one +(`tests/core/test_step_labels.py`). ### Limits Names resolve lexically (innermost function scope out to the module; a parameter, loop target or import shadows). A name bound two different ways -in one scope is not flagged. CUP-A006 is deliberately conservative: only a -module with a `__main__` guard, only steps added from that block or from a -module-level function it calls (transitively); a script without a guard is -not flagged (the runtime still refuses it). Not seen: `functools.partial`, -`type(...)`-built classes, steps built in another module (all refused at -runtime). +in one scope is not flagged. Not seen: `functools.partial`, +`type(...)`-built classes, steps built in another module. ### Layout diff --git a/packages/codeupipe-lint/codeupipe_lint/anonymous_steps/python_step_visitor.py b/packages/codeupipe-lint/codeupipe_lint/anonymous_steps/python_step_visitor.py index 2a94505..074f7fe 100644 --- a/packages/codeupipe-lint/codeupipe_lint/anonymous_steps/python_step_visitor.py +++ b/packages/codeupipe-lint/codeupipe_lint/anonymous_steps/python_step_visitor.py @@ -12,10 +12,7 @@ - as a Valve predicate, a named function (CUP-A008): predicates are objects with ``test(payload)``; - the result of calling a function in this module that returns one of the - above (a closure factory), reported with the same rule; -- a module-level class or function of a script, added from its - ``if __name__ == "__main__":`` block or a function that block calls - (CUP-A006). Only then: a module that is merely importable is fine. + above (a closure factory), reported with the same rule. Names are resolved lexically: innermost function scope outwards to the module, stopping at the first scope that binds the name. A name bound more @@ -28,7 +25,7 @@ from typing import Dict, List, Optional, Set, Tuple from .finding import Finding -from .rules import (ENTRY_SCRIPT, FUNCTION_PREDICATE, LAMBDA, LOCAL_FUNCTION, LOCAL_TYPE, UNNAMED_PIPELINE, +from .rules import (FUNCTION_PREDICATE, LAMBDA, LOCAL_FUNCTION, LOCAL_TYPE, UNNAMED_PIPELINE, make_finding) _STEP_METHODS = { @@ -56,18 +53,6 @@ def _is_plain_pipeline_call(node: ast.AST) -> bool: isinstance(func, ast.Attribute) and func.attr == "Pipeline") -def _is_main_guard(node: ast.AST) -> bool: - if not isinstance(node, ast.If) or not isinstance(node.test, ast.Compare): - return False - test = node.test - if len(test.ops) != 1 or not isinstance(test.ops[0], ast.Eq): - return False - sides = [test.left, test.comparators[0]] - names = [s for s in sides if isinstance(s, ast.Name) and s.id == "__name__"] - consts = [s for s in sides if isinstance(s, ast.Constant) and s.value == "__main__"] - return bool(names) and bool(consts) - - def _own_nodes(body: List[ast.stmt]): """Every node in ``body``, not descending into nested functions, classes or lambdas.""" stack = list(reversed(body)) @@ -103,13 +88,6 @@ def __init__(self, path: str, tree: ast.Module): self._findings: List[Finding] = [] self._seen: Set[Tuple[int, int, str]] = set() self._module_scope = self._scope(tree.body, None, is_module=True) - self._main_functions: Set[str] = set() - self._main_block: Optional[ast.If] = None - for stmt in tree.body: - if _is_main_guard(stmt): - self._main_block = stmt - if self._main_block is not None: - self._main_functions = self._reached_from_main() # -- scopes --------------------------------------------------------- @@ -191,40 +169,24 @@ def _resolve(name: str, chain: List[Dict[str, Binding]]) -> Binding: return scope[name] return _OTHER - # -- __main__ ------------------------------------------------------- - - def _reached_from_main(self) -> Set[str]: - defs = {s.name: s for s in self._tree.body if isinstance(s, _FUNCS)} - reached: Set[str] = set() - frontier = [self._main_block] - while frontier: - node = frontier.pop() - for sub in ast.walk(node): - if isinstance(sub, ast.Call) and isinstance(sub.func, ast.Name): - name = sub.func.id - if name in defs and name not in reached: - reached.add(name) - frontier.append(defs[name]) - return reached - # -- classification ------------------------------------------------- - def _classify(self, expr: ast.AST, chain: List[Dict[str, Binding]], in_main: bool, + def _classify(self, expr: ast.AST, chain: List[Dict[str, Binding]], visiting: Set[int]) -> Optional[Tuple[str, str]]: """(rule, what) when ``expr`` is an anonymous step, else None. ``what`` starts with a verb.""" - got = self._describe(expr, chain, in_main, visiting) + got = self._describe(expr, chain, visiting) if got is None: return None rule, subject, noun = got return rule, ("`%s` is %s" % (subject, noun)) if subject else ("is " + noun) - def _describe(self, expr: ast.AST, chain: List[Dict[str, Binding]], in_main: bool, + def _describe(self, expr: ast.AST, chain: List[Dict[str, Binding]], visiting: Set[int]) -> Optional[Tuple[str, Optional[str], str]]: """(rule, subject name or None, noun phrase).""" if isinstance(expr, ast.Lambda): return LAMBDA, None, "a lambda" if isinstance(expr, ast.Name): - got = self._describe_binding(self._resolve(expr.id, chain), in_main) + got = self._describe_binding(self._resolve(expr.id, chain)) return None if got is None else (got[0], expr.id, got[1]) if _is_plain_pipeline_call(expr): return UNNAMED_PIPELINE, None, "a plain Pipeline()" @@ -234,8 +196,6 @@ def _describe(self, expr: ast.AST, chain: List[Dict[str, Binding]], in_main: boo kind = binding[0] if kind == "local_class": return LOCAL_TYPE, None, "an instance of %s, a class defined inside %s()" % (name, binding[2]) - if kind == "module_class" and in_main: - return ENTRY_SCRIPT, None, "an instance of %s, defined in the entry script" % name if kind in ("local_def", "module_def"): parent = chain if kind == "local_def" else [self._module_scope] returned = self._returned(binding[3], parent, visiting) @@ -246,7 +206,7 @@ def _describe(self, expr: ast.AST, chain: List[Dict[str, Binding]], in_main: boo return None @staticmethod - def _describe_binding(binding: Binding, in_main: bool) -> Optional[Tuple[str, str]]: + def _describe_binding(binding: Binding) -> Optional[Tuple[str, str]]: kind, name, owner = binding[0], binding[1], binding[2] if kind == "lambda": return LAMBDA, "a lambda" @@ -258,10 +218,6 @@ def _describe_binding(binding: Binding, in_main: bool) -> Optional[Tuple[str, st return LOCAL_TYPE, "an instance of %s, a class defined inside %s()" % (name, owner) if kind == "plain_pipeline": return UNNAMED_PIPELINE, "a plain Pipeline()" - if kind == "module_def" and in_main: - return ENTRY_SCRIPT, "a function defined in the entry script" - if kind == "module_class" and in_main: - return ENTRY_SCRIPT, "a class defined in the entry script" return None def _returned(self, func: ast.AST, parent_chain: List[Dict[str, Binding]], @@ -273,7 +229,7 @@ def _returned(self, func: ast.AST, parent_chain: List[Dict[str, Binding]], chain = self._chain_for(func, parent_chain) for node in _own_nodes(func.body): if isinstance(node, ast.Return) and node.value is not None: - got = self._describe(node.value, chain, False, visiting) + got = self._describe(node.value, chain, visiting) if got is not None: return got return None @@ -281,41 +237,35 @@ def _returned(self, func: ast.AST, parent_chain: List[Dict[str, Binding]], # -- walk ----------------------------------------------------------- def findings(self) -> List[Finding]: - self._walk(self._tree, [self._module_scope], False) + self._walk(self._tree, [self._module_scope]) return self._findings - def _walk(self, node: ast.AST, chain: List[Dict[str, Binding]], in_main: bool) -> None: + def _walk(self, node: ast.AST, chain: List[Dict[str, Binding]]) -> None: for child in ast.iter_child_nodes(node): - child_main = in_main - if child is self._main_block: - child_main = True if isinstance(child, _FUNCS): - if chain[0] is self._module_scope and child.name in self._main_functions: - child_main = True - self._walk(child, self._chain_for(child, chain), child_main) + self._walk(child, self._chain_for(child, chain)) continue if isinstance(child, ast.Call): - self._check_call(child, chain, child_main) - self._walk(child, chain, child_main) + self._check_call(child, chain) + self._walk(child, chain) - def _check_call(self, call: ast.Call, chain: List[Dict[str, Binding]], in_main: bool) -> None: + def _check_call(self, call: ast.Call, chain: List[Dict[str, Binding]]) -> None: func = call.func if isinstance(func, ast.Attribute) and func.attr in _STEP_METHODS: role = _STEP_METHODS[func.attr] for arg in list(call.args) + [k.value for k in call.keywords]: elements = arg.elts if isinstance(arg, (ast.List, ast.Tuple, ast.Set)) else [arg] for element in elements: - self._report(element, role, call, chain, in_main) + self._report(element, role, call, chain) elif isinstance(func, ast.Attribute) and func.attr == "add_valve": for index, arg in enumerate(call.args): - self._report(arg, _VALVE_ROLES[min(index, 1)], call, chain, in_main) + self._report(arg, _VALVE_ROLES[min(index, 1)], call, chain) for keyword in call.keywords: if keyword.arg in _VALVE_KEYWORDS: - self._report(keyword.value, _VALVE_KEYWORDS[keyword.arg], call, chain, in_main) + self._report(keyword.value, _VALVE_KEYWORDS[keyword.arg], call, chain) - def _report(self, expr: ast.AST, role: str, call: ast.Call, chain: List[Dict[str, Binding]], - in_main: bool) -> None: - got = self._classify(expr, chain, in_main, set()) + def _report(self, expr: ast.AST, role: str, call: ast.Call, chain: List[Dict[str, Binding]]) -> None: + got = self._classify(expr, chain, set()) if got is None and role == "Valve predicate" and isinstance(expr, ast.Name) \ and self._resolve(expr.id, chain)[0] == "module_def": got = FUNCTION_PREDICATE, "`%s` is a function, not a predicate object" % expr.id diff --git a/packages/codeupipe-lint/codeupipe_lint/anonymous_steps/rules.py b/packages/codeupipe-lint/codeupipe_lint/anonymous_steps/rules.py index 4b3c0d9..759aa4c 100644 --- a/packages/codeupipe-lint/codeupipe_lint/anonymous_steps/rules.py +++ b/packages/codeupipe-lint/codeupipe_lint/anonymous_steps/rules.py @@ -1,7 +1,7 @@ """Rule ids, and the messages that name the fix (packages/codeupipe-lint/README.md). -The wording mirrors the runtime's AnonymousStep refusals -(packages/codeupipe/codeupipe/core/step_identity.py). +The runtime reports a step by the name of its class +(packages/codeupipe/codeupipe/core/step_label.py). """ from typing import Optional @@ -9,13 +9,12 @@ from .finding import Finding __all__ = ["RULES", "LAMBDA", "LOCAL_FUNCTION", "LOCAL_TYPE", "UNNAMED_PIPELINE", - "ENTRY_SCRIPT", "FUNCTION_PREDICATE", "PARSE_ERROR", "make_finding"] + "FUNCTION_PREDICATE", "PARSE_ERROR", "make_finding"] LAMBDA = "CUP-A001" LOCAL_FUNCTION = "CUP-A002" LOCAL_TYPE = "CUP-A003" UNNAMED_PIPELINE = "CUP-A005" -ENTRY_SCRIPT = "CUP-A006" FUNCTION_PREDICATE = "CUP-A008" PARSE_ERROR = "CUP-A000" @@ -25,11 +24,10 @@ LOCAL_FUNCTION: "local-function-step", LOCAL_TYPE: "local-type-step", UNNAMED_PIPELINE: "unnamed-pipeline-step", - ENTRY_SCRIPT: "entry-script-step", FUNCTION_PREDICATE: "function-predicate", } -_TAIL = " so it has a stable identity and can be tested on its own." +_TAIL = " so it has a name in State and can be tested on its own." # Steps and Valve predicates are named types (predicates are objects with # test(payload), conformance/API.md, "Valve"). @@ -44,11 +42,8 @@ def _fix(lang: str, rule: str) -> str: if rule == UNNAMED_PIPELINE: - return ("A nested pipeline must be a named pipeline type (%s), identified like a filter. " + return ("A nested pipeline must be a named pipeline type (%s), named like a filter. " "A plain Pipeline can only run at the top level." % _PIPELINE_EXAMPLE[lang]) - if rule == ENTRY_SCRIPT: - return ("Its identity would be __main__.X, which changes with how the program is started, " - "and tests cannot import it. Move it into a module.") if rule == FUNCTION_PREDICATE: return ("Valve predicates are objects (conformance/API.md, \"Valve\"): wrap the condition in a " "named class with test(payload), configuration on the instance, and pass an instance.") diff --git a/packages/codeupipe/README.md b/packages/codeupipe/README.md index 6888435..cef9763 100644 --- a/packages/codeupipe/README.md +++ b/packages/codeupipe/README.md @@ -72,8 +72,9 @@ PricingPipeline().run(Payload({"price": 100.0, "qty": 12})).to_dict() # {'price': 90.0, 'qty': 12, 'bulk': True} ``` -A step must be a class in a module. A lambda is refused, because a step is -identified by its type, and State, Hooks and the export use that identity. +A step is a class. State and Hooks report each step by the name of its +class, for example `DiscountFilter`. A lambda is refused, because it does not +have the method of a step. ## What is in the package diff --git a/packages/codeupipe/codeupipe/core/README.md b/packages/codeupipe/codeupipe/core/README.md index c2099bb..e4cecd4 100644 --- a/packages/codeupipe/codeupipe/core/README.md +++ b/packages/codeupipe/codeupipe/core/README.md @@ -16,14 +16,13 @@ library only. `add_valve`, `add_tap`, `add_pipeline`, `use_hook`, `run`, `run_async`, `state`. `run()` is synchronous and never starts an event loop; `run_async()` is the opt-in - entry point for async steps. Steps have no - names: each is identified by its filter (`step_identity.py`), `#n` from - the second use; State and Hooks report that identity. Nested pipelines are - named `Pipeline` subclasses. See conformance/API.md, "Step identity". -- `step_identity.py` — derives a step's identity (`module.QualName`) and the - `file:line` it was added at; refuses anonymous steps. -- `anonymous_step.py` — `AnonymousStep` (a `TypeError`): lambdas, closures, - classes defined in a function, `__main__` filters, unnamed nested pipelines. + entry point for async steps. Steps are not named by + hand: each has a label, the name of its class (`step_label.py`), `#n` from + the second use; State and Hooks report that label. A step without the + method of its role is refused with a `TypeError` when it is added. See + conformance/API.md, "Step labels". +- `step_label.py` — `label_of(step)`: the class name of a step; for a Valve, + of its Filter. - `async_step_in_sync_run.py` — `AsyncStepInSyncRun`: raised when the sync `Pipeline.run()` meets async work (it never starts an event loop). - `predicate.py` — `Predicate`: a yes-or-no question about the payload, an diff --git a/packages/codeupipe/codeupipe/core/__init__.py b/packages/codeupipe/codeupipe/core/__init__.py index 5d17fd2..aea534f 100644 --- a/packages/codeupipe/codeupipe/core/__init__.py +++ b/packages/codeupipe/codeupipe/core/__init__.py @@ -9,7 +9,6 @@ from .payload import Payload from .filter import AsyncFilter, Filter from .async_step_in_sync_run import AsyncStepInSyncRun -from .anonymous_step import AnonymousStep from .pipeline import Pipeline from .predicate import Predicate from .tap import AsyncTap, Tap @@ -25,5 +24,5 @@ "Tap", "AsyncTap", "Hook", "AsyncHook", "State", - "AnonymousStep", "AsyncStepInSyncRun", + "AsyncStepInSyncRun", ] diff --git a/packages/codeupipe/codeupipe/core/anonymous_step.py b/packages/codeupipe/codeupipe/core/anonymous_step.py deleted file mode 100644 index e78d547..0000000 --- a/packages/codeupipe/codeupipe/core/anonymous_step.py +++ /dev/null @@ -1,13 +0,0 @@ -"""AnonymousStep: a step whose identity cannot be derived from the code. - -Step identity is a pointer to the filter (conformance/API.md, "Step -identity"). Lambdas, closures, classes defined inside a function, filters -defined in the entry script and unnamed nested pipelines have no stable -pointer, and are not independently testable, so the runtime refuses them. -""" - -__all__ = ["AnonymousStep"] - - -class AnonymousStep(TypeError): - """Raised when a step has no stable identity to report.""" diff --git a/packages/codeupipe/codeupipe/core/pipeline.py b/packages/codeupipe/codeupipe/core/pipeline.py index 3d9b63a..767ee24 100644 --- a/packages/codeupipe/codeupipe/core/pipeline.py +++ b/packages/codeupipe/codeupipe/core/pipeline.py @@ -13,7 +13,7 @@ from .tap import AsyncTap, Tap from .hook import Hook from .state import State -from .step_identity import add_site, identify +from .step_label import label_of from .async_step_in_sync_run import AsyncStepInSyncRun from .valve import Valve @@ -37,11 +37,9 @@ class Pipeline(Generic[TInput, TOutput]): def __init__(self): self._preflight_key: Any = None self._preflight_cache: Any = None - self._steps: List[Tuple[str, Any, str]] = [] # (identity, step, type) - # Step identity (conformance/API.md): derived from the code, #n from - # the second use of the same filter; the add call's file:line as detail. - self._occurrences: Dict[str, int] = {} - self._step_sites: Dict[str, str] = {} + self._steps: List[Tuple[str, Any, str]] = [] # (label, step, type) + # A step's label is its class name, with #n from the second use. + self._uses: Dict[str, int] = {} self._hooks: List[Hook] = [] self._state: State = State() self._observe_timing: bool = False @@ -53,50 +51,53 @@ def state(self) -> State: return self._state @property - def identities(self) -> List[str]: - """Each step's identity, in order.""" - return [identity for identity, _step, _type in self._steps] - - @property - def step_sites(self) -> Dict[str, str]: - """identity -> ``file:line`` of the ``add_*`` call that added it.""" - return dict(self._step_sites) - - def _identity(self, step: Any, kind: str, base: Optional[str] = None) -> str: - """Derive the step's identity; number it from its second use.""" - base = base or identify(step, kind) - count = self._occurrences.get(base, 0) + 1 - self._occurrences[base] = count - identity = base if count == 1 else f"{base}#{count}" - self._step_sites[identity] = add_site(3) # the caller of add_* + def labels(self) -> List[str]: + """Each step's label, in order.""" + return [label for label, _step, _type in self._steps] + + def _label(self, step: Any) -> str: + """The step's class name, numbered from its second use in this pipeline.""" + base = label_of(step) + count = self._uses.get(base, 0) + 1 + self._uses[base] = count self._preflight_key = None # shape changed - return identity + return base if count == 1 else f"{base}#{count}" + + @staticmethod + def _require(step: Any, method: str, what: str) -> None: + """Refuse a step that does not have the method of its kind.""" + if not callable(getattr(step, method, None)): + raise TypeError(f"{what} has {method}(payload). {step!r} does not.") def add_filter(self, filter: Union[Filter[TInput, TOutput], AsyncFilter[TInput, TOutput]]) -> None: """Add a Filter. A filter whose ``call`` is a coroutine needs ``run_async()``.""" if isinstance(filter, Valve): raise TypeError("A Valve is added with add_valve(predicate, filter), not with add_filter.") + self._require(filter, "call", "A Filter") step_type = "async_filter" if self._is_async_method(filter, "call") else "filter" - self._steps.append((self._identity(filter, "filter"), filter, step_type)) + self._steps.append((self._label(filter), filter, step_type)) def add_valve(self, predicate: Any, filter: Union[Filter[TInput, TOutput], AsyncFilter[TInput, TOutput]]) -> None: """Add a Valve: ``filter`` runs only when ``predicate.test(payload)`` is true. The condition comes first, then the Filter that it controls. When the test is false the Payload passes through unchanged and the step is - recorded as skipped. The step is identified by its Filter. + recorded as skipped. The step has the label of its Filter. """ if not callable(getattr(predicate, "test", None)) and callable(getattr(predicate, "call", None)) \ and callable(getattr(filter, "test", None)): raise TypeError("The arguments are in the wrong order: add_valve(predicate, filter). " "The condition comes first, then the Filter.") + self._require(predicate, "test", "A Predicate") + self._require(filter, "call", "A Filter") valve = Valve(filter, predicate) - self._steps.append((self._identity(valve, "filter"), valve, "async_filter")) + self._steps.append((self._label(valve), valve, "async_filter")) def add_tap(self, tap: Union[Tap, AsyncTap]) -> None: """Add a Tap. A tap whose ``observe`` is a coroutine needs ``run_async()``.""" + self._require(tap, "observe", "A Tap") step_type = "async_tap" if self._is_async_method(tap, "observe") else "tap" - self._steps.append((self._identity(tap, "tap"), tap, step_type)) + self._steps.append((self._label(tap), tap, step_type)) def use_hook(self, hook: Hook) -> None: """Attach a lifecycle hook. A hook with coroutine methods needs ``run_async()``.""" @@ -104,8 +105,10 @@ def use_hook(self, hook: Hook) -> None: self._hooks.append(hook) def add_pipeline(self, pipeline: 'Pipeline') -> None: - """Nest a named Pipeline type as a single step inside this Pipeline.""" - self._steps.append((self._identity(pipeline, "pipeline"), pipeline, "pipeline")) + """Nest a Pipeline as a single step inside this Pipeline.""" + if not isinstance(pipeline, Pipeline): + raise TypeError(f"add_pipeline takes a Pipeline. {pipeline!r} is not one.") + self._steps.append((self._label(pipeline), pipeline, "pipeline")) def observe(self, *, timing: bool = True, lineage: bool = False) -> None: """Enable observation features (timing, lineage tracking).""" diff --git a/packages/codeupipe/codeupipe/core/state.py b/packages/codeupipe/codeupipe/core/state.py index 08f1256..0ba7967 100644 --- a/packages/codeupipe/codeupipe/core/state.py +++ b/packages/codeupipe/codeupipe/core/state.py @@ -13,10 +13,10 @@ class State: """The record of one run. A run starts with a new State. - - ``executed``: step identities, in order + - ``executed``: step labels, in order - ``skipped``: steps a Valve did not run - - ``errors``: (identity, error) pairs - - ``timings``: identity -> seconds, when timing is on + - ``errors``: (label, error) pairs + - ``timings``: label -> seconds, when timing is on """ def __init__(self): diff --git a/packages/codeupipe/codeupipe/core/step_identity.py b/packages/codeupipe/codeupipe/core/step_identity.py deleted file mode 100644 index 5ccdc92..0000000 --- a/packages/codeupipe/codeupipe/core/step_identity.py +++ /dev/null @@ -1,77 +0,0 @@ -"""Derive a step's identity from its code (conformance/API.md, "Step identity"). - -A step is identified by the fully qualified symbol of the code that runs it: -``module.QualName`` of a filter's or tap's class, of a Valve's inner filter, -of a named Pipeline type, or of a module-level function. Nothing about it is -written by hand, so it cannot drift from the code it names. -""" - -import sys -import types -from typing import Any - -from .anonymous_step import AnonymousStep - -__all__ = ["identify", "symbol_of", "predicate_symbol", "add_site"] - -_FIX = "Define it at module level (a named class or a module-level def) so it has a stable identity and can be tested on its own." - - -def symbol_of(target: Any, what: str) -> str: - """``module.QualName`` of a class or function, or AnonymousStep.""" - module = getattr(target, "__module__", None) - qualname = getattr(target, "__qualname__", None) - if not module or not qualname: - raise AnonymousStep(f"{what} {target!r} has no module and qualified name. {_FIX}") - if "" in qualname: - raise AnonymousStep(f"{what} is a lambda ({module}.{qualname}). {_FIX}") - if "" in qualname: - raise AnonymousStep(f"{what} {module}.{qualname} is defined inside a function. {_FIX}") - if module == "__main__": - raise AnonymousStep( - f"{what} {qualname} is defined in the entry script (__main__), so its identity changes with " - f"how the program is started and tests cannot import it. Move it into a module.") - return f"{module}.{qualname}" - - -def predicate_symbol(predicate: Any) -> str: - """The identity of a Valve predicate: an object with ``test(payload) -> bool``. - - Predicates are objects (conformance/API.md, "Valve"), identified by - their class, with any configuration on the instance. A function (named, - lambda or closure) is not a predicate object. - """ - if isinstance(predicate, (types.FunctionType, types.BuiltinFunctionType, types.MethodType, type)) \ - or not callable(getattr(predicate, "test", None)): - what = getattr(predicate, "__qualname__", None) or type(predicate).__qualname__ - raise AnonymousStep( - f"Valve predicate {what} is not a predicate object. Declare a module-level class with " - f"test(self, payload) -> bool (configuration on the instance) and pass an instance.") - return symbol_of(type(predicate), "Valve predicate") - - -def identify(step: Any, kind: str) -> str: - """The identity of a step, before any ``#n`` occurrence number. - - kind: "filter", "tap" or "pipeline". - """ - from .pipeline import Pipeline # local: pipeline imports this module - from .valve import Valve - - if kind == "pipeline" or isinstance(step, Pipeline): - if type(step) is Pipeline: - raise AnonymousStep( - "A nested pipeline must be a named Pipeline type (class UserOnboarding(Pipeline): ...), " - "identified like a filter. A plain Pipeline() can only run at the top level.") - return symbol_of(type(step), "Nested pipeline") - if isinstance(step, Valve): - return identify(step._inner, "filter") - if isinstance(step, (type, types.FunctionType, types.BuiltinFunctionType, types.MethodType)): - return symbol_of(step, kind.capitalize()) - return symbol_of(type(step), kind.capitalize()) - - -def add_site(depth: int = 2) -> str: - """``file:line`` of the caller ``depth`` frames up: where the step was added.""" - frame = sys._getframe(depth) - return f"{frame.f_code.co_filename}:{frame.f_lineno}" diff --git a/packages/codeupipe/codeupipe/core/step_label.py b/packages/codeupipe/codeupipe/core/step_label.py new file mode 100644 index 0000000..bac8e34 --- /dev/null +++ b/packages/codeupipe/codeupipe/core/step_label.py @@ -0,0 +1,19 @@ +"""The label of a step: the name of its class. + +State and Hooks report a step by this label. It comes from the code, so it +cannot differ from the code. A Valve has the label of its Filter. +""" + +from typing import Any + +__all__ = ["label_of"] + + +def label_of(step: Any) -> str: + """The class name of ``step``; for a Valve, of its Filter.""" + inner = getattr(step, "_inner", None) + if inner is not None and hasattr(step, "_predicate"): # a Valve + return label_of(inner) + if isinstance(step, type): + return step.__name__ + return type(step).__name__ diff --git a/packages/codeupipe/codeupipe/core/valve.py b/packages/codeupipe/codeupipe/core/valve.py index 595e0fc..c3f4c5a 100644 --- a/packages/codeupipe/codeupipe/core/valve.py +++ b/packages/codeupipe/codeupipe/core/valve.py @@ -58,11 +58,8 @@ def __init__( inner: Filter[TInput, TOutput], predicate: "Predicate", ): - # A Valve is identified by its inner filter (conformance/API.md); its - # predicate is an object with test(payload) -> bool, identified by its - # class, configuration on the instance. Functions are not predicates. - from .step_identity import predicate_symbol - self.predicate_symbol = predicate_symbol(predicate) + # The predicate is an object with test(payload) -> bool. Its settings + # are on the instance. self._inner = inner self._predicate_object = predicate self._predicate = predicate.test # bound once: called per run @@ -111,4 +108,4 @@ def call_sync(self, payload: Payload[TInput]) -> Payload[TOutput]: return result def __repr__(self) -> str: - return f"Valve({type(self._inner).__qualname__}, {self.predicate_symbol})" + return f"Valve({type(self._predicate_object).__name__}, {type(self._inner).__name__})" diff --git a/packages/codeupipe/codeupipe/lower/filters/emit_orchestrator_filter.py b/packages/codeupipe/codeupipe/lower/filters/emit_orchestrator_filter.py index 74f2e74..655be73 100644 --- a/packages/codeupipe/codeupipe/lower/filters/emit_orchestrator_filter.py +++ b/packages/codeupipe/codeupipe/lower/filters/emit_orchestrator_filter.py @@ -38,7 +38,7 @@ def _import_lines(steps) -> List[str]: # import it. Relative imports keep step names (``json``, ``truncate``) # from shadowing top-level modules on the customer's sys.path. # Code uses each step's generated identifier (``step_03_validate``), never - # its identity: identities are labels, reported in State and to Hooks. + # its label, reported in State and to Hooks. lines = [] for step in steps: names = [step.ident] diff --git a/packages/codeupipe/codeupipe/lower/filters/load_pipeline_filter.py b/packages/codeupipe/codeupipe/lower/filters/load_pipeline_filter.py index d8cc10b..b6e373b 100644 --- a/packages/codeupipe/codeupipe/lower/filters/load_pipeline_filter.py +++ b/packages/codeupipe/codeupipe/lower/filters/load_pipeline_filter.py @@ -45,14 +45,14 @@ def _check_pipeline(pipeline, name: str, allow_hooks: bool = False) -> None: "(top-level Hooks only), or remove them.") -def _ident(index: int, identity: str) -> str: +def _ident(index: int, label: str) -> str: """A safe, readable Python identifier for a step: ``step_03_validate``. - Generated code never uses the identity itself as a name: it is a label - (``app.filters.Validate#2``), and anything derived from user code could + Generated code never uses the label itself as a name: it is a string + (``Validate#2``), and anything derived from user code could collide with a builtin, a module global or the generated run(). """ - short = identity.rsplit("/", 1)[-1].split("#", 1)[0].rsplit(".", 1)[-1] + short = label.rsplit("/", 1)[-1].split("#", 1)[0].rsplit(".", 1)[-1] snake = re.sub(r"(? the dict variable it becomes diff --git a/packages/codeupipe/codeupipe/lower/ir.py b/packages/codeupipe/codeupipe/lower/ir.py index c90426a..bd09b1e 100644 --- a/packages/codeupipe/codeupipe/lower/ir.py +++ b/packages/codeupipe/codeupipe/lower/ir.py @@ -113,8 +113,8 @@ class StepIR: emitted file runs without that module. ident: The Python identifier the generated code uses for this step's module and function (``step_03_validate``). Generated, never taken - from ``name``: ``name`` is the step's identity (a label such as - ``app.filters.Validate#2``), which is not an identifier. + from ``name``: ``name`` is the step's label (such as + ``Validate#2``), which is not an identifier. group: For a step flattened out of a nested Pipeline, the name of the top-level step that nested it (``inner`` for ``inner__greet``). Observable export uses it to report the nested pipeline as one diff --git a/tests/core/test_add_valve.py b/tests/core/test_add_valve.py index 3a10008..f274dae 100644 --- a/tests/core/test_add_valve.py +++ b/tests/core/test_add_valve.py @@ -42,7 +42,7 @@ def test_the_filter_runs_when_the_predicate_is_true(): pipeline = Pipeline() pipeline.add_valve(IsOn(), SetA()) assert pipeline.run(Payload({"on": True})).to_dict() == {"on": True, "a": 1} - assert pipeline.state.executed == [f"{HERE}.SetA"] + assert pipeline.state.executed == ["SetA"] assert pipeline.state.skipped == [] @@ -51,20 +51,14 @@ def test_the_payload_passes_through_when_the_predicate_is_false(): pipeline.add_valve(IsOn(), SetA()) assert pipeline.run(Payload({"on": False})).to_dict() == {"on": False} assert pipeline.state.executed == [] - assert pipeline.state.skipped == [f"{HERE}.SetA"] + assert pipeline.state.skipped == ["SetA"] def test_a_valve_is_identified_by_its_filter(): pipeline = Pipeline() pipeline.add_valve(IsOn(), SetA()) pipeline.add_valve(AtLeast("n", 2), SetA()) - assert pipeline.identities == [f"{HERE}.SetA", f"{HERE}.SetA#2"] - - -def test_the_step_site_is_the_add_valve_line(): - pipeline = Pipeline() - pipeline.add_valve(IsOn(), SetA()) - assert "test_add_valve.py:" in pipeline.step_sites[f"{HERE}.SetA"] + assert pipeline.labels == ["SetA", "SetA#2"] def test_a_predicate_keeps_its_settings(): @@ -91,7 +85,7 @@ def test_an_async_filter_behind_a_valve_needs_run_async(): def test_a_function_is_not_a_predicate(): def is_on(payload): return True - with pytest.raises(codeupipe.AnonymousStep): + with pytest.raises(TypeError, match=r"test\(payload\)"): Pipeline().add_valve(is_on, SetA()) # cup: allow-anonymous (exercises the refusal) diff --git a/tests/core/test_api_contract.py b/tests/core/test_api_contract.py index 35c7d9a..95e17ce 100644 --- a/tests/core/test_api_contract.py +++ b/tests/core/test_api_contract.py @@ -21,8 +21,8 @@ async def call(self, payload): return payload.insert("n", payload.get("n", 0) + 1) -INC = f"{__name__}.Inc" -BOOM = f"{__name__}.Boom" +INC = "Inc" +BOOM = "Boom" class _SetsX: @@ -75,11 +75,11 @@ def test_nested_pipeline_runs_synchronously_as_one_step(self): outer.add_filter(Inc()) outer.add_pipeline(IncTwice()) assert outer.run(Payload({})).get("n") == 2 - assert outer.state.executed == [INC, f"{__name__}.IncTwice"] + assert outer.state.executed == [INC, "IncTwice"] class TestStepsHaveNoNames: - """Identity is derived from the filter (tests/core/test_step_identity.py).""" + """The label is derived from the step (tests/core/test_step_labels.py).""" @pytest.mark.parametrize("add", ["add_filter", "add_tap"]) def test_a_name_argument_is_rejected(self, add): @@ -93,8 +93,8 @@ def call(self, payload): raise RuntimeError('boom') -class TestHooksReceiveStepIdentities: - def test_before_after_and_on_error_get_identities(self): +class TestHooksReceiveStepLabels: + def test_before_after_and_on_error_get_labels(self): events = [] class Record(Hook): diff --git a/tests/core/test_execution_modes.py b/tests/core/test_execution_modes.py index 2f1ff0a..c0d5e74 100644 --- a/tests/core/test_execution_modes.py +++ b/tests/core/test_execution_modes.py @@ -20,7 +20,7 @@ def test(self, payload): class SubPipeline(Pipeline): - """A named pipeline type, so it can be nested (conformance/API.md, step identity).""" + """A named pipeline type, so it can be nested (conformance/API.md, step labels).""" # ── Helpers ────────────────────────────────────────────────── @@ -123,7 +123,7 @@ def test_run_sync_updates_state(self): pipe = Pipeline() pipe.add_filter(AddTenFilter()) result = pipe.run(Payload({"value": 0})) - assert f"{__name__}.AddTenFilter" in pipe.state.executed + assert "AddTenFilter" in pipe.state.executed # ═══════════════════════════════════════════════════════════════ @@ -184,7 +184,7 @@ async def test_nested_pipeline_state_tracked(self): outer.add_pipeline(inner) await outer.run_async(Payload({"value": 0})) - assert f"{__name__}.SubPipeline" in outer.state.executed + assert "SubPipeline" in outer.state.executed @pytest.mark.asyncio async def test_nested_pipeline_error_propagates(self): diff --git a/tests/core/test_pipeline.py b/tests/core/test_pipeline.py index c5e800e..0f1df60 100644 --- a/tests/core/test_pipeline.py +++ b/tests/core/test_pipeline.py @@ -22,11 +22,11 @@ def __init__(self): self.log = [] async def before(self, step: Optional[str], payload: Payload) -> None: - # hooks receive the step's identity (API contract), None for the pipeline + # hooks receive the step's label (API contract), None for the pipeline self.log.append(f"before_{step if step is not None else 'pipeline_start'}") async def after(self, step: Optional[str], payload: Payload) -> None: - # hooks receive the step's identity (API contract), None for the pipeline + # hooks receive the step's label (API contract), None for the pipeline self.log.append(f"after_{step if step is not None else 'pipeline_end'}") async def on_error(self, step: Optional[str], error: Exception, payload: Payload) -> None: @@ -117,7 +117,7 @@ def test_pipeline_state_tracking(self): async def run_test(): await pipeline.run_async(Payload()) - assert pipeline.state.executed == [f"{__name__}.TestFilter2", f"{__name__}.TestFilter2#2"] + assert pipeline.state.executed == ["TestFilter2", "TestFilter2#2"] asyncio.run(run_test()) @@ -222,8 +222,8 @@ def test_valve_blocks_when_predicate_false(self): async def run_test(): result = await pipeline.run_async(Payload()) - assert pipeline.state.executed == [f"{__name__}.MarkFirst"] - assert pipeline.state.skipped == [f"{__name__}.MarkGated"] + assert pipeline.state.executed == ["MarkFirst"] + assert pipeline.state.skipped == ["MarkGated"] assert result.get("first_ran") is True assert result.get("gated_ran") is None @@ -289,7 +289,7 @@ def test_tap_tracked_in_state(self): async def run_test(): await pipeline.run_async(Payload()) - assert f"{__name__}.NoopTap" in pipeline.state.executed + assert "NoopTap" in pipeline.state.executed asyncio.run(run_test()) @@ -326,9 +326,9 @@ async def run_test(): await pipeline.run_async(Payload()) assert "before_pipeline_start" in hook.log - # hooks receive the step's identity (API contract) - assert f"before_{__name__}.NamedFilter3" in hook.log - assert f"after_{__name__}.NamedFilter3" in hook.log + # hooks receive the step's label (API contract) + assert "before_NamedFilter3" in hook.log + assert "after_NamedFilter3" in hook.log assert "after_pipeline_end" in hook.log asyncio.run(run_test()) @@ -436,7 +436,7 @@ def test_run_sync_records_a_closed_valve_as_skipped(self): result = p.run(Payload({})) assert result.get("marked") is None - assert p.state.skipped == [f"{__name__}.TestRunSyncWithValves._Mark"] + assert p.state.skipped == ["_Mark"] assert p.state.executed == [] @pytest.mark.unit @@ -450,7 +450,7 @@ def test_a_valve_over_an_async_filter_needs_run_async(self): p.run(Payload({})) result = asyncio.run(p.run_async(Payload({}))) assert result.get("marked") is True - assert p.state.executed == [f"{__name__}.TestRunSyncWithValves._AsyncMark"] + assert p.state.executed == ["_AsyncMark"] class _Predicate5: @@ -503,11 +503,11 @@ def _pipeline(self, tap, hook=None): def _assert_contained(self, p, result): assert result.get("score") == 2 - tap = next(i for i in p.identities if i.endswith(("._Boom", "._AsyncBoom"))) + tap = next(i for i in p.labels if i in ("_Boom", "_AsyncBoom")) assert p.state.errors[-1][0] == tap assert isinstance(p.state.errors[-1][1], ConnectionError) assert tap in p.state.executed - assert f"{__name__}.TestTapFailureIsolation._Score#2" in p.state.executed + assert "_Score#2" in p.state.executed @pytest.mark.unit @pytest.mark.core diff --git a/tests/core/test_step_identity.py b/tests/core/test_step_identity.py deleted file mode 100644 index a6c2d9c..0000000 --- a/tests/core/test_step_identity.py +++ /dev/null @@ -1,213 +0,0 @@ -"""Step identity: a pointer to the filter, derived from the code (conformance/API.md). - -No step names. A step is identified by its filter's fully qualified symbol, -``#n`` when the same one is used again in a pipeline; the add site -(file:line) is recorded as extra detail. Anonymous steps are refused. -""" - -import pytest - -from codeupipe import Hook, Payload, Pipeline -from codeupipe.core.valve import Valve -from codeupipe.core import AnonymousStep - -MODULE = __name__ - - -class Add: - def call(self, payload): - return payload.insert("n", payload.get("n", 0) + 1) - - -class Boom: - def call(self, payload): - raise RuntimeError("boom") - - -class Audit: - def observe(self, payload): - pass - - -class UserOnboarding(Pipeline): - """A named pipeline type: nestable, identified like a filter.""" - - def __init__(self): - super().__init__() - self.add_filter(Add()) - - -class IsOn: - """A predicate object: test(payload) -> bool, identified by its class.""" - - def test(self, payload): - return bool(payload.get("on")) - - -class AtLeast: - """A configurable predicate: its setting lives on the instance.""" - - def __init__(self, n): - self.n = n - - def test(self, payload): - return payload.get("n", 0) >= self.n - - -def is_on(payload): - return bool(payload.get("on")) - - -class Record(Hook): - def __init__(self): - self.events = [] - - def before(self, step, payload): - self.events.append(("before", step)) - - def after(self, step, payload): - self.events.append(("after", step)) - - def on_error(self, step, error, payload): - self.events.append(("error", step)) - - -def _pipeline(*steps): - p = Pipeline() - for step in steps: - p.add_filter(step) - return p - - -class TestIdentityIsTheFiltersSymbol: - def test_state_reports_the_qualified_symbol(self): - p = _pipeline(Add()) - p.run(Payload({})) - assert p.state.executed == [f"{MODULE}.Add"] - - def test_reuse_is_numbered_from_the_second_occurrence(self): - p = _pipeline(Add(), Add(), Add()) - p.run(Payload({})) - assert p.state.executed == [f"{MODULE}.Add", f"{MODULE}.Add#2", f"{MODULE}.Add#3"] - - def test_taps_are_identified_the_same_way(self): - p = _pipeline(Add()) - p.add_tap(Audit()) - p.run(Payload({})) - assert p.state.executed == [f"{MODULE}.Add", f"{MODULE}.Audit"] - - def test_hooks_receive_the_identity(self): - p = _pipeline(Add(), Boom()) - hook = Record() - p.use_hook(hook) - with pytest.raises(RuntimeError): - p.run(Payload({})) - assert hook.events == [("before", None), ("before", f"{MODULE}.Add"), ("after", f"{MODULE}.Add"), - ("before", f"{MODULE}.Boom"), ("error", f"{MODULE}.Boom"), ("error", None)] - - def test_a_valve_is_identified_by_its_inner_filter(self): - p = Pipeline() - p.add_valve(IsOn(), Add()) - p.run(Payload({})) - assert p.state.skipped == [f"{MODULE}.Add"] - - def test_a_valve_and_a_plain_use_of_the_same_filter_are_numbered(self): - p = _pipeline(Add()) - p.add_valve(IsOn(), Add()) - p.run(Payload({"on": True})) - assert p.state.executed == [f"{MODULE}.Add", f"{MODULE}.Add#2"] - - def test_a_named_pipeline_type_nests_as_one_step(self): - outer = _pipeline(Add()) - outer.add_pipeline(UserOnboarding()) - assert outer.run(Payload({})).get("n") == 2 - assert outer.state.executed == [f"{MODULE}.Add", f"{MODULE}.UserOnboarding"] - - def test_identities_lists_steps_in_order(self): - p = _pipeline(Add(), Add()) - assert p.identities == [f"{MODULE}.Add", f"{MODULE}.Add#2"] - - -class TestPredicateObjects: - def test_the_predicate_is_identified_by_its_class(self): - assert Valve(Add(), IsOn()).predicate_symbol == f"{MODULE}.IsOn" - - def test_a_configurable_predicate(self): - p = Pipeline() - p.add_valve(AtLeast(5), Add()) - assert p.run(Payload({"n": 5})).get("n") == 6 - assert p.run(Payload({"n": 1})).get("n") == 1 - - -class TestAddSite: - def test_the_add_call_location_is_recorded(self): - p = Pipeline() - p.add_filter(Add()) # this line - site = p.step_sites[f"{MODULE}.Add"] - assert site.endswith("test_step_identity.py:%d" % (TestAddSite.line_of_add_call(),)) - - @staticmethod - def line_of_add_call(): - import inspect - source, start = inspect.getsourcelines(TestAddSite.test_the_add_call_location_is_recorded) - return start + next(i for i, line in enumerate(source) if "# this line" in line) - - -def _make_closure_filter(): - class Local: - def call(self, payload): - return payload - return Local() - - -def _make_closure_predicate(): - def inner(payload): - return True - return inner - - -class TestAnonymousStepsAreRefused: - def test_class_defined_in_a_function(self): - with pytest.raises(AnonymousStep, match="Local"): - Pipeline().add_filter(_make_closure_filter()) # cup: allow-anonymous (exercises the refusal) - - def test_a_function_is_not_a_predicate(self): - with pytest.raises(AnonymousStep, match="predicate object"): - Valve(Add(), is_on) # cup: allow-anonymous (exercises the refusal) - - def test_lambda_predicate(self): - with pytest.raises(AnonymousStep, match="lambda"): - Valve(Add(), lambda payload: True) # cup: allow-anonymous (exercises the refusal) - - def test_closure_predicate(self): - with pytest.raises(AnonymousStep, match="inner"): - Valve(Add(), _make_closure_predicate()) # cup: allow-anonymous (exercises the refusal) - - def test_filter_defined_in_the_entry_script(self): - Main = type("Main", (), {"call": lambda self, payload: payload, "__module__": "__main__"}) - with pytest.raises(AnonymousStep, match="__main__"): - Pipeline().add_filter(Main()) - - def test_plain_pipeline_cannot_be_nested(self): - with pytest.raises(AnonymousStep, match="named Pipeline type"): - Pipeline().add_pipeline(_pipeline(Add())) # cup: allow-anonymous (exercises the refusal) - - def test_the_message_names_the_fix(self): - with pytest.raises(AnonymousStep) as info: - Pipeline().add_filter(_make_closure_filter()) # cup: allow-anonymous (exercises the refusal) - assert "module level" in str(info.value) - - def test_anonymous_step_is_a_type_error(self): - assert issubclass(AnonymousStep, TypeError) - - -class TestNamesAreGone: - def test_add_filter_takes_no_name(self): - with pytest.raises(TypeError): - Pipeline().add_filter(Add(), "add") # type: ignore[call-arg] - - def test_valve_takes_no_name(self): - with pytest.raises(TypeError): - Valve("gate", Add(), IsOn()) # type: ignore[call-arg] - - diff --git a/tests/core/test_step_labels.py b/tests/core/test_step_labels.py new file mode 100644 index 0000000..ee2a0c8 --- /dev/null +++ b/tests/core/test_step_labels.py @@ -0,0 +1,170 @@ +"""A step is reported by a label: the name of its class. + +State and Hooks use the label. Nothing is refused for its name: a class in a +script, a class in a function and a plain nested Pipeline all work. The +linter is the guard for names. +""" + +import pytest + +import codeupipe +from codeupipe import Hook, Payload, Pipeline + + +class AddFilter: + def call(self, payload): + return payload.insert("n", payload.get("n", 0) + 1) + + +class FailFilter: + def call(self, payload): + raise RuntimeError("boom") + + +class SeeTap: + def observe(self, payload): + pass + + +class IsOnPredicate: + def test(self, payload): + return payload.get("on") is True + + +class InnerPipeline(Pipeline): + def __init__(self): + super().__init__() + self.add_filter(AddFilter()) + + +class RecordHook(Hook): + def __init__(self): + self.events = [] + + def before(self, step, payload): + self.events.append(("before", step)) + + def after(self, step, payload): + self.events.append(("after", step)) + + def on_error(self, step, error, payload): + self.events.append(("error", step)) + + +def test_the_label_is_the_class_name(): + pipeline = Pipeline() + pipeline.add_filter(AddFilter()) + pipeline.add_tap(SeeTap()) + assert pipeline.labels == ["AddFilter", "SeeTap"] + + +def test_state_reports_labels(): + pipeline = Pipeline() + pipeline.add_filter(AddFilter()) + pipeline.add_tap(SeeTap()) + pipeline.run(Payload({})) + assert pipeline.state.executed == ["AddFilter", "SeeTap"] + + +def test_the_second_use_of_a_class_gets_a_number(): + pipeline = Pipeline() + for _ in range(3): + pipeline.add_filter(AddFilter()) + assert pipeline.labels == ["AddFilter", "AddFilter#2", "AddFilter#3"] + assert pipeline.run(Payload({})).get("n") == 3 + assert pipeline.state.executed == ["AddFilter", "AddFilter#2", "AddFilter#3"] + + +def test_a_valve_has_the_label_of_its_filter(): + pipeline = Pipeline() + pipeline.add_valve(IsOnPredicate(), AddFilter()) + assert pipeline.labels == ["AddFilter"] + pipeline.run(Payload({"on": False})) + assert pipeline.state.skipped == ["AddFilter"] + + +def test_a_nested_pipeline_has_the_label_of_its_class(): + outer = Pipeline() + outer.add_pipeline(InnerPipeline()) + assert outer.labels == ["InnerPipeline"] + outer.run(Payload({})) + assert outer.state.executed == ["InnerPipeline"] + + +def test_a_plain_nested_pipeline_is_accepted(): + inner = Pipeline() + inner.add_filter(AddFilter()) + outer = Pipeline() + outer.add_pipeline(inner) # cup: allow-anonymous (a plain nested Pipeline works at run time) + assert outer.labels == ["Pipeline"] + assert outer.run(Payload({})).get("n") == 1 + + +def test_hooks_receive_labels(): + hook = RecordHook() + pipeline = Pipeline() + pipeline.add_filter(AddFilter()) + pipeline.use_hook(hook) + pipeline.run(Payload({})) + assert hook.events == [("before", None), ("before", "AddFilter"), ("after", "AddFilter"), ("after", None)] + + +def test_an_error_names_the_label(): + hook = RecordHook() + pipeline = Pipeline() + pipeline.add_filter(FailFilter()) + pipeline.use_hook(hook) + with pytest.raises(RuntimeError): + pipeline.run(Payload({})) + assert ("error", "FailFilter") in hook.events + + +def test_a_class_defined_in_a_function_works(): + class LocalFilter: + def call(self, payload): + return payload.insert("local", True) + + pipeline = Pipeline() + pipeline.add_filter(LocalFilter()) # cup: allow-anonymous (a local class works at run time) + assert pipeline.labels == ["LocalFilter"] + assert pipeline.run(Payload({})).get("local") is True + + +def test_a_class_from_the_entry_script_works(): + script_class = type("ScriptFilter", (), {"__module__": "__main__", + "call": lambda self, payload: payload.insert("s", 1)}) + pipeline = Pipeline() + pipeline.add_filter(script_class()) + assert pipeline.labels == ["ScriptFilter"] + assert pipeline.run(Payload({})).get("s") == 1 + + +@pytest.mark.parametrize("add, bad, says", [ + ("add_filter", lambda payload: payload, "call(payload)"), + ("add_filter", object(), "call(payload)"), + ("add_tap", object(), "observe(payload)"), + ("add_pipeline", object(), "Pipeline"), +]) +def test_a_step_with_the_wrong_shape_is_refused_when_it_is_added(add, bad, says): + with pytest.raises(TypeError) as error: + getattr(Pipeline(), add)(bad) # cup: allow-anonymous (exercises the refusal) + assert says in str(error.value) + + +def test_a_predicate_with_no_test_method_is_refused(): + def is_on(payload): + return True + with pytest.raises(TypeError) as error: + Pipeline().add_valve(is_on, AddFilter()) # cup: allow-anonymous (exercises the refusal) + assert "test(payload)" in str(error.value) + + +def test_a_name_argument_is_not_accepted(): + with pytest.raises(TypeError): + Pipeline().add_filter(AddFilter(), "a_name") + + +def test_the_identity_names_are_gone(): + assert "AnonymousStep" not in codeupipe.__all__ + assert not hasattr(Pipeline(), "identities") + assert not hasattr(Pipeline(), "step_sites") diff --git a/tests/core/test_sync_pipeline.py b/tests/core/test_sync_pipeline.py index 1353264..e370b35 100644 --- a/tests/core/test_sync_pipeline.py +++ b/tests/core/test_sync_pipeline.py @@ -100,7 +100,7 @@ def test_state_tracks_execution(self): sp.add_filter(AddOneFilter()) sp.add_filter(UpperFilter()) sp.run(Payload({"x": 1, "name": "alice"})) - assert sp.state.executed == [f"{__name__}.AddOneFilter", f"{__name__}.UpperFilter"] + assert sp.state.executed == ["AddOneFilter", "UpperFilter"] def test_returns_payload_type(self): sp = Pipeline() @@ -129,7 +129,7 @@ def test_tap_state_tracked(self): sp = Pipeline() sp.add_tap(RecordTap()) sp.run(Payload({"x": 1})) - assert f"{__name__}.RecordTap" in sp.state.executed + assert "RecordTap" in sp.state.executed # ── Hooks ─────────────────────────────────────────────────────────── @@ -171,8 +171,8 @@ def test_timing_recorded(self): sp.add_filter(AddOneFilter()) sp.observe(timing=True) sp.run(Payload({"x": 0})) - assert f"{__name__}.AddOneFilter" in sp.state.timings - assert sp.state.timings[f"{__name__}.AddOneFilter"] > 0 + assert "AddOneFilter" in sp.state.timings + assert sp.state.timings["AddOneFilter"] > 0 # ── Govern contracts ──────────────────────────────────────────────── @@ -264,7 +264,7 @@ def test_run_contains_a_tap_failure(self): result = p.run(Payload({})) assert result.get("score") == 2 - assert p.state.errors[-1][0] == f"{__name__}.TestTapFailureIsolation._Boom" - assert f"{__name__}.TestTapFailureIsolation._Boom" in p.state.executed + assert p.state.errors[-1][0] == "_Boom" + assert "_Boom" in p.state.executed assert hook.errors == [] diff --git a/tests/core/test_sync_support.py b/tests/core/test_sync_support.py index ef0768b..5afbcea 100644 --- a/tests/core/test_sync_support.py +++ b/tests/core/test_sync_support.py @@ -71,7 +71,7 @@ async def go(): return pipeline.state state = run(go()) - assert f"{__name__}.SyncPass" in state.executed + assert "SyncPass" in state.executed # =========================================================================== @@ -128,7 +128,7 @@ async def go(): return pipeline.state state = run(go()) - assert f"{__name__}.SyncNopTap" in state.executed + assert "SyncNopTap" in state.executed # =========================================================================== @@ -167,8 +167,8 @@ async def go(): run(go()) assert log == [ "before:pipeline", - f"before:{__name__}.PassFilter", - f"after:{__name__}.PassFilter", + "before:PassFilter", + "after:PassFilter", "after:pipeline", ] @@ -192,7 +192,7 @@ async def go(): run(go()) # the failing step first, then the pipeline - assert errors == [f"{__name__}.BoomFilter:boom", "pipeline:boom"] + assert errors == ["BoomFilter:boom", "pipeline:boom"] # =========================================================================== @@ -230,7 +230,7 @@ async def go(): return pipeline.state state = run(go()) - assert f"{__name__}.SyncDiscount2" in state.skipped + assert "SyncDiscount2" in state.skipped # =========================================================================== diff --git a/tests/core/test_valve_sync.py b/tests/core/test_valve_sync.py index 9dd72f5..9fe8f63 100644 --- a/tests/core/test_valve_sync.py +++ b/tests/core/test_valve_sync.py @@ -132,7 +132,7 @@ def test_skipped_valve_is_marked_skipped(self): pipeline = Pipeline() pipeline.add_valve(_Predicate8(), Append()) pipeline.run(Payload({"s": "a"})) - assert f"{__name__}.Append" in pipeline.state.skipped + assert "Append" in pipeline.state.skipped def test_valve_is_no_longer_far_slower_than_a_plain_step(self): """The regression guard. Not a benchmark — an order-of-magnitude diff --git a/tests/examples/test_pricing.py b/tests/examples/test_pricing.py index 78f147a..f262714 100644 --- a/tests/examples/test_pricing.py +++ b/tests/examples/test_pricing.py @@ -47,17 +47,17 @@ def test_dynamic_run(): pipeline = PricingPipeline() out = pipeline.run(Payload({"price": 100.0, "qty": 12})) assert out.to_dict() == {"price": 90.0, "qty": 12, "bulk": True} - assert pipeline.identities == ["examples.pricing.discount_filter.DiscountFilter", - "examples.pricing.mark_bulk_filter.MarkBulkFilter", - "examples.pricing.audit_tap.AuditTap"] - assert [name for name, _ in pipeline.state.errors] == ["examples.pricing.audit_tap.AuditTap"] + assert pipeline.labels == ["DiscountFilter", + "MarkBulkFilter", + "AuditTap"] + assert [name for name, _ in pipeline.state.errors] == ["AuditTap"] def test_valve_skips_small_orders(): pipeline = PricingPipeline() out = pipeline.run(Payload({"price": 100.0, "qty": 2})) assert out.to_dict() == {"price": 90.0, "qty": 2} - assert pipeline.state.skipped == ["examples.pricing.mark_bulk_filter.MarkBulkFilter"] + assert pipeline.state.skipped == ["MarkBulkFilter"] def test_a_hook_sees_each_step(capsys): @@ -65,7 +65,7 @@ def test_a_hook_sees_each_step(capsys): pipeline.use_hook(PrintStepsHook()) pipeline.run(Payload({"price": 100.0, "qty": 12})) lines = capsys.readouterr().out.splitlines() - assert lines == [" discount_filter.DiscountFilter", " mark_bulk_filter.MarkBulkFilter"] + assert lines == [" DiscountFilter", " MarkBulkFilter"] def test_the_example_runs_as_a_module(): diff --git a/tests/lint/anonymous_steps/test_analyze_python_filter.py b/tests/lint/anonymous_steps/test_analyze_python_filter.py index 7c49c6c..9d31963 100644 --- a/tests/lint/anonymous_steps/test_analyze_python_filter.py +++ b/tests/lint/anonymous_steps/test_analyze_python_filter.py @@ -10,7 +10,7 @@ from ._support import by_line, expectations, fixture, run_analyzer -@pytest.mark.parametrize("name", ["positive.py", "negative.py", "entry_script.py", "probe.py"]) +@pytest.mark.parametrize("name", ["positive.py", "negative.py", "probe.py"]) def test_fixture_findings_match_expectations(name): path = fixture("python", name) findings, errors = run_analyzer(AnalyzePythonFilter(), "python", [path]) diff --git a/tests/lint/anonymous_steps/test_lint_pipeline.py b/tests/lint/anonymous_steps/test_lint_pipeline.py index 0c8e01e..9727cb3 100644 --- a/tests/lint/anonymous_steps/test_lint_pipeline.py +++ b/tests/lint/anonymous_steps/test_lint_pipeline.py @@ -6,11 +6,11 @@ def test_the_linter_is_a_pipeline_of_named_filters(): - identities = build_lint_pipeline().identities - assert identities[0] == "codeupipe_lint.anonymous_steps.collect_files_filter.CollectFilesFilter" - assert "codeupipe_lint.anonymous_steps.analyze_python_filter.AnalyzePythonFilter" in identities - assert identities[-2:] == ["codeupipe_lint.anonymous_steps.apply_suppressions_filter.ApplySuppressionsFilter", - "codeupipe_lint.anonymous_steps.format_report_filter.FormatReportFilter"] + labels = build_lint_pipeline().labels + assert labels[0] == "CollectFilesFilter" + assert "AnalyzePythonFilter" in labels + assert labels[-2:] == ["ApplySuppressionsFilter", + "FormatReportFilter"] def test_lint_paths_reports_and_sets_the_exit_code(): diff --git a/tests/lint/anonymous_steps/test_repo_is_clean.py b/tests/lint/anonymous_steps/test_repo_is_clean.py index aee12eb..02e2a3e 100644 --- a/tests/lint/anonymous_steps/test_repo_is_clean.py +++ b/tests/lint/anonymous_steps/test_repo_is_clean.py @@ -9,8 +9,8 @@ CHECKED = ["packages/codeupipe/codeupipe", "conformance/runners/python", "scorecard", "tests/core", "tests/lower", "tests/scorecard", "tests/conformance"] -INTENTIONAL = {"tests/core/test_step_identity.py", "tests/lower/test_export_refusals.py", - "tests/core/test_add_valve.py"} +INTENTIONAL = {"tests/lower/test_export_refusals.py", + "tests/core/test_add_valve.py", "tests/core/test_step_labels.py"} def test_repo_python_has_no_anonymous_steps(): diff --git a/tests/lint/fixtures/python/entry_script.py b/tests/lint/fixtures/python/entry_script.py deleted file mode 100644 index cb0fa57..0000000 --- a/tests/lint/fixtures/python/entry_script.py +++ /dev/null @@ -1,38 +0,0 @@ -"""A script: its classes are __main__.X when run, so steps defined here and -added from the __main__ block (directly or via a function it calls) are flagged.""" - -from codeupipe import Payload, Pipeline - - -class Local: - def call(self, payload): - return payload - - -def is_on(payload): - return True - - -def library_builder(): - # Not reached from the __main__ block: an importer gets a real module name. - p = Pipeline() - p.add_filter(Local()) - return p - - -def main(): - p = Pipeline() - p.add_filter(Local()) # expect: CUP-A006 - p.add_valve(is_on, Local()) # expect: CUP-A006 CUP-A006 - helper(p) - return p.run(Payload({})) - - -def helper(p): - p.add_tap(Local()) # expect: CUP-A006 - - -if __name__ == "__main__": - main() - q = Pipeline() - q.add_filter(Local()) # expect: CUP-A006 diff --git a/tests/lint/fixtures/python/probe.py b/tests/lint/fixtures/python/probe.py index e67532f..fbdf9e2 100644 --- a/tests/lint/fixtures/python/probe.py +++ b/tests/lint/fixtures/python/probe.py @@ -1,4 +1,4 @@ -"""Step-identity probe cases, with describe(label, f) replaced by +"""Step probe cases, with describe(label, f) replaced by p.add_filter(f): the probe's cases as lint fixtures.""" from codeupipe import Pipeline @@ -31,4 +31,4 @@ def call(self, payload): p.add_filter(lambda payload: payload) # expect: CUP-A001 p.add_filter(make_closure("x")) # expect: CUP-A002 p.add_filter(make_local_class()) # expect: CUP-A003 -p.add_filter(DefinedInEntryScript()) # not flagged: no __main__ guard, so the linter stays conservative (the runtime refuses it) +p.add_filter(DefinedInEntryScript()) # not flagged: a module-level class diff --git a/tests/lower/test_export_observe.py b/tests/lower/test_export_observe.py index 42f096c..f3f417e 100644 --- a/tests/lower/test_export_observe.py +++ b/tests/lower/test_export_observe.py @@ -15,15 +15,15 @@ from codeupipe.lower import build_export_pipeline_v2 -ADD = f"{__name__}._Add" -BOOM = f"{__name__}._Boom" -FAILING_TAP = f"{__name__}._FailingTap" -TAP = f"{__name__}._Tap" -SUB = f"{__name__}.SubPipeline" +ADD = "_Add" +BOOM = "_Boom" +FAILING_TAP = "_FailingTap" +TAP = "_Tap" +SUB = "SubPipeline" class SubPipeline(Pipeline): - """A named pipeline type, so it can be nested (conformance/API.md, step identity).""" + """A named pipeline type, so it can be nested (conformance/API.md, step labels).""" class _Add: @@ -317,8 +317,8 @@ def test_inline_layout_cannot_observe(self, tmp_path): _export(p, tmp_path, lowering="inline") -def test_identities_are_labels_not_code_names(tmp_path): - """Generated code names steps step_01_... itself; the identity (which is +def test_labels_are_not_code_names(tmp_path): + """Generated code names steps step_01_... itself; the label (which is not an identifier) is only ever a string in State and Hooks.""" p = Pipeline() p.add_filter(_Add()) diff --git a/tests/lower/test_export_pipeline.py b/tests/lower/test_export_pipeline.py index bad81ff..d1e4cc0 100644 --- a/tests/lower/test_export_pipeline.py +++ b/tests/lower/test_export_pipeline.py @@ -15,7 +15,7 @@ class SubPipeline(Pipeline): - """A named pipeline type, so it can be nested (conformance/API.md, step identity).""" + """A named pipeline type, so it can be nested (conformance/API.md, step labels).""" class _CleanText: @@ -141,10 +141,8 @@ def test_predicate_parameter_need_not_be_named_payload(self, tmp_path): assert package.run({"text": "a", "price": 10, "member": False})["price"] == 10 def test_lambda_predicate_is_refused_before_export(self): - """Anonymous predicates never reach export: the Valve refuses them.""" - from codeupipe import AnonymousStep - - with pytest.raises(AnonymousStep, match="lambda"): + """A lambda is not a Predicate: it has no test method.""" + with pytest.raises(TypeError, match=r"test\(payload\)"): self._pipeline(lambda payload: payload.get("member") is True) @@ -182,7 +180,7 @@ def test_base_exceptions_from_a_tap_still_propagate(self, tmp_path): class TestNestedPipelineExport: """A nested Pipeline lowers by flattening its steps into the parent (conformance: pipeline/nested-pipeline-is-a-step). Generated modules are - numbered in flattened order; identities stay labels.""" + numbered in flattened order; labels stay as they are.""" def test_nested_pipeline_exports_and_matches_dynamic(self, tmp_path): inner = SubPipeline() diff --git a/tests/lower/test_load_pipeline_filter.py b/tests/lower/test_load_pipeline_filter.py index 8ed41e0..84c6cfc 100644 --- a/tests/lower/test_load_pipeline_filter.py +++ b/tests/lower/test_load_pipeline_filter.py @@ -30,14 +30,14 @@ def test(self, payload): class TestLoadPipeline: - def test_extracts_step_identities_idents_and_kinds(self): + def test_extracts_step_labels_idents_and_kinds(self): p = Pipeline() p.add_filter(_ExFilter()) p.add_tap(_ExTap()) result = LoadPipelineFilter().call(Payload({"pipeline": p})) ir = result.get("pipeline_ir") assert isinstance(ir, PipelineIR) - assert [s.name for s in ir.steps] == [f"{__name__}._ExFilter", f"{__name__}._ExTap"] + assert [s.name for s in ir.steps] == ["_ExFilter", "_ExTap"] assert [s.ident for s in ir.steps] == ["step_01_ex_filter", "step_02_ex_tap"] assert [s.kind for s in ir.steps] == ["filter", "tap"]