diff --git a/.github/CODE_OF_CONDUCT.md b/.github/CODE_OF_CONDUCT.md index ed2ca01..c9ed370 100644 --- a/.github/CODE_OF_CONDUCT.md +++ b/.github/CODE_OF_CONDUCT.md @@ -2,7 +2,7 @@ nodrill is not a [Python Software Foundation](https://www.python.org/psf-landing/) project, but everyone interacting in its issues, pull requests and discussions is expected to follow the [PSF Code of Conduct](https://policies.python.org/python.org/code-of-conduct/). -In short: be open, considerate and respectful, whatever anyone's position in the project is. +In short, be open, considerate and respectful, whatever anyone's position in the project is. ## Enforcement diff --git a/.github/CONTRIBUTING.md b/.github/CONTRIBUTING.md index 072937a..7d6d25f 100644 --- a/.github/CONTRIBUTING.md +++ b/.github/CONTRIBUTING.md @@ -16,23 +16,23 @@ make install ``` `make install` syncs the locked environment and installs the pre-commit hooks. -Everything else runs through the `Makefile`; `make help` lists the targets. +Everything else runs through the `Makefile`, and `make help` lists the targets. ## The gate ```bash -make # lint, typecheck, coverage, docs, workflow audit — the same order CI runs +make # lint, typecheck, coverage, docs, workflow audit, in the order CI runs them make -k # same, but keep going after a failure so one run reports everything ``` -Individual pieces, when you want a faster loop: +Individual pieces, for when you want a faster loop. | Command | What it does | | --- | --- | | `make format` | ruff format plus the safe ruff fixes | | `make lint` | ruff format `--check` and `ruff check` | | `make typecheck` | mypy and pyright | -| `make test` | pytest; `make test ARGS="-k inject -x"` to narrow it | +| `make test` | pytest, which `make test ARGS="-k inject -x"` narrows | | `make testcov` | pytest under coverage with the 100 percent gate | | `make docs` | Sphinx with warnings as errors | | `make audit` | zizmor over the GitHub Actions workflows | @@ -44,8 +44,8 @@ A pull request is expected to pass all of it. - **Coverage is 100 percent on branches.** New code arrives with the tests that cover it. A `# pragma: no cover` is not the fix. -- **Two type checkers.** mypy runs strict over `src` and `tests`; pyright checks `src`. - Both must be clean — a few API shapes exist only because the two disagree. +- **Two type checkers.** mypy runs strict over `src` and `tests`, and pyright checks `src` plus `tests/cycle`. + Both must be clean, and a few API shapes exist only because the two disagree. - **Ruff with `select = ["ALL"]`.** A new ignore goes in `pyproject.toml` with a comment saying why, rather than a bare `# noqa` at the call site. - **Docs build with `-W`.** @@ -53,16 +53,16 @@ A pull request is expected to pass all of it. ## House style -Docstrings are plain PEP 257 prose: an imperative first line, no reStructuredText roles, no bullet lists. -The reference pages carry the detailed descriptions; docstrings stay terse. +Docstrings are plain PEP 257 prose, an imperative first line, no reStructuredText roles, no bullet lists. +The reference pages carry the detailed descriptions, so docstrings stay terse. Comments explain why, not what. -Prose in `.md` and `.rst` files uses semantic line breaks: a new sentence starts a new line, so rewording a paragraph shows up as a one-line diff. -There is no column limit: a sentence stays on one line however long it runs, and nothing is re-wrapped by hand. -`make lint-md` checks the rest of the Markdown — headings, lists, blank lines — but no tool can check the sentence rule, so that one rides on review. +Prose in `.md` and `.rst` files uses semantic line breaks, so a new sentence starts a new line and rewording a paragraph shows up as a one-line diff. +There is no column limit, so a sentence stays on one line however long it runs and nothing is re-wrapped by hand. +`make lint-md` checks the rest of the Markdown, headings and lists and blank lines, but no tool can check the sentence rule, so that one rides on review. -Public names are load-bearing: `provider`, `use`, `wrap`, `Executor`, `set_default` and `from_ctx` were reviewed and are fixed. -Propose a rename in an issue; please do not perform one in a pull request. +Public names are load-bearing, and `provider`, `use`, `wrap`, `Executor`, `set_default` and `from_ctx` were reviewed and are fixed. +Propose a rename in an issue rather than performing one in a pull request. ## Reporting a bug @@ -74,5 +74,5 @@ For a security issue, follow [SECURITY.md](SECURITY.md) instead of opening an is ## AI-assisted contributions Using an assistant to write a patch is fine. -Submitting one you have not read, run and understood is not: you are the author of the pull request, and review time is the scarce resource here. +Submitting one you have not read, run and understood is not, since you are the author of the pull request and review time is the scarce resource here. Say so in the description if a change was largely machine-generated, and make sure the tests genuinely exercise the behaviour rather than restating the implementation. diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md index 0419435..1728146 100644 --- a/.github/PULL_REQUEST_TEMPLATE.md +++ b/.github/PULL_REQUEST_TEMPLATE.md @@ -4,9 +4,9 @@ ## Checklist - + -- [ ] `make -k` passes: lint, mypy, pyright, 100 percent branch coverage, docs, workflow audit. +- [ ] `make -k` passes, meaning lint, mypy, pyright, 100 percent branch coverage, docs and the workflow audit. - [ ] There are tests for the new or changed behaviour. - [ ] Documentation is updated, including the reference page if the public API changed. - [ ] Prose uses semantic line breaks, one sentence per line. diff --git a/.github/SECURITY.md b/.github/SECURITY.md index 7972f47..cacfa17 100644 --- a/.github/SECURITY.md +++ b/.github/SECURITY.md @@ -5,12 +5,14 @@ Only the latest released version is supported. Fixes ship in a new release rather than as patches to older ones. -nodrill has no runtime dependencies and does not parse untrusted input, so its realistic security surface is narrow: values leaking across a context boundary they should not cross — between threads, between asyncio tasks, or out of a `provider` block that has exited. +nodrill has no runtime dependencies and does not parse untrusted input, so its realistic security surface is narrow. +It is a value leaking across a context boundary it should not cross, between threads, between asyncio tasks, or out of a `provider` block that has exited. Reports in that shape are treated as security issues, not ordinary bugs. ## Reporting a vulnerability -Report privately through GitHub: [open a draft security advisory](https://github.com/paqstd-dev/nodrill/security/advisories/new). Please do not open a public issue for a suspected vulnerability. +Report privately through GitHub by [opening a draft security advisory](https://github.com/paqstd-dev/nodrill/security/advisories/new). +Please do not open a public issue for a suspected vulnerability. Include the smallest program that reproduces the leak, the Python version, and whether threads or asyncio are involved. diff --git a/README.md b/README.md index 3e5862c..0046032 100644 --- a/README.md +++ b/README.md @@ -62,6 +62,9 @@ When it goes wrong, `debug()` turns a miss into a diagnosis naming the thread, t `isolate()` gives a test fresh context state and rolls it back afterwards. Everything above is importable from the top-level package, and there is nothing else to import. +Before it goes wrong, running a suite under `NODRILL_CONTRACT` records which keys each entry point actually read and which of those a fallback answered rather than a provider, and `python -m nodrill contract` renders that into a file a pull request reviews. +It is a command rather than an import, so it adds no name to the package and no script to your PATH. + ## Overhead A lookup is one dict read on a single `ContextVar`, and nothing is constructed, resolved or cached along the way. @@ -91,7 +94,8 @@ Start with the [tutorial](https://nodrill.readthedocs.io/en/latest/content/intro ## Contributing -Bug reports and small focused pull requests are welcome. See [CONTRIBUTING.md](https://github.com/paqstd-dev/nodrill/blob/main/.github/CONTRIBUTING.md). +Bug reports and small focused pull requests are welcome. +See [CONTRIBUTING.md](https://github.com/paqstd-dev/nodrill/blob/main/.github/CONTRIBUTING.md). `make install` sets up the environment, and `make` runs the same gate CI does. Security issues go through a [private advisory](https://github.com/paqstd-dev/nodrill/security/advisories/new) rather than the issue tracker. diff --git a/benchmarks/bench.py b/benchmarks/bench.py index 10327e8..02ecc3d 100644 --- a/benchmarks/bench.py +++ b/benchmarks/bench.py @@ -4,21 +4,31 @@ one in docs/content/misc/performance.rst between the markers, so the published numbers and the script that produced them cannot drift apart. -The first rows are one function doing one read, reached five ways, so the -rows are comparable to each other and to handing the value in as a -parameter, which is what nodrill replaces. The rest price the things the -prose claims: entering a scope, entering it with a stack already open, and -crossing into a thread. - -Absolute nanoseconds move with the machine and a rerun lands within a few -percent; the ratios are the part worth reading. Nothing here runs in CI: -timing on a shared runner measures the runner. +The first rows are one function doing one read, reached six ways, so the rows +are comparable to each other and to handing the value in as a parameter, which +is what nodrill replaces. Then the lookups on their own, what a scope costs +to open, and what carrying one to a worker costs. + +Absolute nanoseconds move with the machine, so the whole table is timed +several times over and every row keeps its own best pass. Timing one row to +completion before starting the next made each row hostage to whatever the +machine did during its own second, which moved the ratios as well, since the +baseline every ratio divides by is one of the rows. + +A published number is only replaced when it moved further than a rerun moves +it, so running this on an unchanged tree writes nothing and a diff means a +real change. Two revisions are told apart with --save on one and --against +on the other, which prints the deltas and says which of them a rerun would +not explain. Nothing here runs in CI, because timing on a shared runner +measures the runner. """ from __future__ import annotations import argparse +import json import platform +import re import sys import timeit from collections.abc import Mapping, Sequence @@ -28,7 +38,7 @@ from datetime import date from pathlib import Path -from nodrill import FromCtx, inject, injected, lazy, provider, ref, use, wrap +from nodrill import Executor, FromCtx, debug, inject, injected, lazy, provider, ref, use, wrap PAGE = Path(__file__).resolve().parent.parent / "docs/content/misc/performance.rst" START = ".. benchmarks generated by benchmarks/bench.py, do not edit by hand" @@ -37,13 +47,22 @@ # Deep enough that a per-provider cost would show, shallow enough to stay realistic. STACK_DEPTH = 8 +# Deep enough that the slope shows rather than hiding under the fixed cost of a block. +DEEP_STACK = 64 + # What a request scope carries by the time the layers are done accumulating. NAMESPACE_WIDTH = 8 +# Enough passes that a row unlucky in one of them is measured fairly in another. +PASSES = 5 + +# What a rerun moves a row by, below which the published number is left alone. +NOISE = 0.08 + @dataclass class Config: - """The provided value; a dataclass because that is what callers use.""" + """The provided value, a dataclass because that is what callers use.""" dsn: str = "postgres://" @@ -69,10 +88,10 @@ def read_passed(_request: str, cfg: Config) -> str: def noop() -> None: - """Do nothing, so the wrap() row prices wrap() and not its target.""" + """Do nothing, so the wrap() and Executor rows price the carry and not its target.""" -# Labels are the README's row headings, so changing one rewrites the published table. +# Labels are the performance page's row headings, so changing one rewrites the published table. PASSED = "one read in a function, value passed in as a parameter" USED = "the same read through `use()`" INJECTED = "the same read through `@inject`" @@ -80,19 +99,24 @@ def noop() -> None: SEALED = "the same read through a `sealed=True` provider" LAZY = "the same read through a resolved `lazy` provider" ALONE = "`use(Config)` on its own, without the call frame" +ATTRIBUTE = "`use('scope').field`, one attribute off a namespace" REF = "the same lookup through a `ref()` key" +FALLBACK = "`use('absent', default=...)`, a miss that falls back" +COUNTED = "`use(Config)` under `debug(unused=True)`" REFERENCE = "bare `ContextVar.get()`, for reference" ENTER = "`with provider(...)`, enter and exit" STACKED = f"the same with {STACK_DEPTH} providers already open" +DEEP = f"the same with {DEEP_STACK} providers already open" SEALED_ENTER = "`with provider(..., sealed=True)`, entered and exited" LAZY_ENTER = "`with provider(lazy(...))`, entered and exited unread" EXTEND = f"`with provider(..., extend=True)`, over an {NAMESPACE_WIDTH}-attribute namespace" -THREAD = "`wrap(fn)()`, per call into a thread" +REPLAY = "`wrap(fn)()`, the context replay per call" +WORKER = "`Executor.submit(fn).result()`, a round trip through a worker" # Handing the value in is the alternative nodrill replaces, so it is what the ratios divide by. BASELINE = PASSED -# The published order: the comparable reads, then the floor, then the scope costs. +# The published order, comparable reads first, then the lookups, the scopes and the handoffs. ORDER = ( PASSED, USED, @@ -101,14 +125,19 @@ def noop() -> None: SEALED, LAZY, ALONE, + ATTRIBUTE, REF, + FALLBACK, + COUNTED, REFERENCE, ENTER, STACKED, + DEEP, SEALED_ENTER, LAZY_ENTER, EXTEND, - THREAD, + REPLAY, + WORKER, ) ENTER_STATEMENT = "\nwith provider(config):\n pass\n" @@ -123,25 +152,56 @@ def noop() -> None: (INJECTED, "read_injected('r')"), (ALONE, "use(Config)"), (REF, "use(CONFIG_REF)"), + (FALLBACK, "use('absent', default=None)"), (REFERENCE, "reference.get()"), (ENTER, ENTER_STATEMENT), (SEALED_ENTER, SEALED_ENTER_STATEMENT), (LAZY_ENTER, LAZY_ENTER_STATEMENT), - (THREAD, "bound()"), + (REPLAY, "bound()"), ) -def measure(statement: str, namespace: dict[str, object]) -> float: - """Return nanoseconds per loop for statement, letting timeit pick the count.""" +def measure( + label: str, statement: str, namespace: dict[str, object], loops: dict[str, int] +) -> float: + """Return nanoseconds per loop, reusing the loop count this row settled on in pass one.""" timer = timeit.Timer(statement, globals=namespace) - loops, total = timer.autorange() - # Best of five rather than one mean, since a single run trails whatever the machine did. - best = min(timer.repeat(repeat=5, number=loops)) - return min(total, best) / loops * 1e9 - - -def run() -> dict[str, float]: - """Time every case, each under the context its row describes.""" + count = loops.get(label) + if count is None: + count, total = timer.autorange() + loops[label] = count + return total / count * 1e9 + return timer.timeit(count) / count * 1e9 + + +def best_of(passes: int) -> dict[str, float]: + """Time the whole table repeatedly and keep each row's best pass. + + Noise only ever adds time, so the minimum is the estimate, and taking it + across passes rather than within one row is what stops a row that was + timed during a bad second from being the published number. + """ + loops: dict[str, int] = {} + best: dict[str, float] = {} + for number in range(passes): + timings = run(loops) + if not number: + checked(timings) + for label, timing in timings.items(): + best[label] = min(best.get(label, timing), timing) + return best + + +def checked(timings: Mapping[str, float]) -> None: + """Refuse a run and an ORDER that disagree, after one pass rather than at render.""" + missing = [label for label in ORDER if label not in timings] + unpublished = [label for label in timings if label not in ORDER] + if missing or unpublished: + raise KeyError(f"ORDER and run() disagree, missing {missing}, unpublished {unpublished}") + + +def run(loops: dict[str, int]) -> dict[str, float]: + """Time every case once, each under the context its row describes.""" config = Config() reference: ContextVar[Config] = ContextVar("reference") reference.set(config) @@ -149,57 +209,100 @@ def run() -> dict[str, float]: with provider(config): bound = wrap(noop) namespace = {**globals(), **locals()} - timings = {label: measure(statement, namespace) for label, statement in PROVIDED} + timings = {label: measure(label, stmt, namespace, loops) for label, stmt in PROVIDED} # The frozen row reuses read_used, so its delta is the proxy and nothing else. with provider(config, frozen=True): - timings[FROZEN] = measure("read_used('r')", {**globals(), **locals()}) + timings[FROZEN] = measure(FROZEN, "read_used('r')", {**globals(), **locals()}, loops) # The sealed row is the same read again, so its delta is the liveness check and nothing else. with provider(config, sealed=True): - timings[SEALED] = measure("read_used('r')", {**globals(), **locals()}) + timings[SEALED] = measure(SEALED, "read_used('r')", {**globals(), **locals()}, loops) # And the lazy row prices the cell after the first read has already resolved it. with provider(lazy(Config, Config)): - timings[LAZY] = measure("read_used('r')", {**globals(), **locals()}) + timings[LAZY] = measure(LAZY, "read_used('r')", {**globals(), **locals()}, loops) + + # Counting is what a recorded contract installs too, so this row prices both. + with debug(unused=True), provider(config): + timings[COUNTED] = measure(COUNTED, "use(Config)", {**globals(), **locals()}, loops) # An extending layer copies the enclosing namespace too, so it is priced over a full one. with provider("scope", **{f"field{i}": i for i in range(NAMESPACE_WIDTH)}): - timings[EXTEND] = measure(EXTEND_STATEMENT, {**globals(), **locals()}) + namespace = {**globals(), **locals()} + timings[EXTEND] = measure(EXTEND, EXTEND_STATEMENT, namespace, loops) + # The shape extend=True and adopt() both produce, which is a lookup and one getattr. + timings[ATTRIBUTE] = measure(ATTRIBUTE, "use('scope').field0", namespace, loops) # Entering copies the registry, so the claim that the copy scales with depth is priced here. - with ExitStack() as stack: - for layer in range(STACK_DEPTH): - stack.enter_context(provider(f"layer{layer}")) - timings[STACKED] = measure(ENTER_STATEMENT, {**globals(), **locals()}) + for label, depth in ((STACKED, STACK_DEPTH), (DEEP, DEEP_STACK)): + with ExitStack() as stack: + for layer in range(depth): + stack.enter_context(provider(f"layer{layer}")) + timings[label] = measure(label, ENTER_STATEMENT, {**globals(), **locals()}, loops) + + # A real handoff, so the replay row above is not read as the price of a thread. + with provider(config), Executor(max_workers=1) as pool: + pool.submit(noop).result() + timings[WORKER] = measure( + WORKER, "pool.submit(noop).result()", {**globals(), **locals()}, loops + ) return timings -def render(timings: Mapping[str, float]) -> str: - """Format timings as the reStructuredText block the performance page carries.""" - base = timings[BASELINE] - header = ("operation", "ns", "×") # noqa: RUF001 — the sign is the published column heading - rows = [(label, f"{round(timings[label])}", ratio(timings[label] / base)) for label in ORDER] +def line(cells: Sequence[str], widths: Sequence[int]) -> str: + """Format one row, padded to the column widths the whole table shares.""" + return " ".join(cell.ljust(width) for cell, width in zip(cells, widths, strict=True)).rstrip() + + +def table(header: Sequence[str], rows: Sequence[Sequence[str]]) -> str: + """Format rows as the reStructuredText simple table both outputs share.""" widths = [max(len(cell) for cell in column) for column in zip(header, *rows, strict=True)] rule = " ".join("=" * width for width in widths) + return "\n".join([rule, line(header, widths), rule, *(line(row, widths) for row in rows), rule]) - def line(cells: tuple[str, str, str]) -> str: - return " ".join( - cell.ljust(width) for cell, width in zip(cells, widths, strict=True) - ).rstrip() - lines = [rule, line(header), rule, *(line(row) for row in rows), rule] - return "\n".join(lines) + f"\n\n{stamp()}\n" +def render(timings: Mapping[str, float]) -> str: + """Format timings as the reStructuredText block the performance page carries.""" + base = timings[BASELINE] + # The multiplication sign, since that is the heading the published table carries. + header = ("operation", "ns", "×") # noqa: RUF001 + # Divided after rounding, so the table is a function of its own ns column and a rerun settles. + rows = [ + (label, f"{round(timings[label])}", ratio(round(timings[label]) / round(base))) + for label in ORDER + ] + return table(header, rows) + + +def compared(fresh: Mapping[str, float], old: Mapping[str, float]) -> str: + """Format this run against a saved one, so a real change reads apart from the weather.""" + rows = [] + for label in ORDER: + now = fresh[label] + was = old.get(label) + if was is None: + rows.append((label, "-", f"{round(now)}", "-", "new")) + continue + change = (now - was) / was + verdict = "moved" if abs(change) > NOISE else "settled" + rows.append((label, f"{round(was)}", f"{round(now)}", f"{change:+.0%}", verdict)) + return table(("operation", "was", "now", "change", "verdict"), rows) + + +def machine() -> str: + """Describe the interpreter and the machine, which a comparison has to hold fixed.""" + where = platform.platform(terse=True).replace("-", " ") + return ( + f"{platform.python_implementation()} {platform.python_version()} " + f"on {where}, {platform.machine()}" + ) def stamp() -> str: """Describe the interpreter, the machine and the day that produced these numbers.""" - where = platform.platform(terse=True).replace("-", " ") - return ( - f"{platform.python_implementation()} {platform.python_version()} on {where}, " - f"{platform.machine()}, measured {date.today().isoformat()}." # noqa: DTZ011 - ) + return f"{machine()}, measured {date.today().isoformat()}." # noqa: DTZ011 def ratio(times: float) -> str: @@ -207,33 +310,126 @@ def ratio(times: float) -> str: return f"{times:.1f}" if times < 10 else str(round(times)) # noqa: PLR2004 -def splice(document: str, table: str) -> str: - """Return document with the region between the markers replaced by table.""" +# One row of the rendered table, which is how the page hands its numbers back. +ROW = re.compile(r"^(\S.*?)\s{2,}(\d+)\s{2,}[\d.]+$") + + +def carried(document: str) -> str: + """Return the table the page carries right now, rules included, so it can be compared.""" + start = document.index(START) + end = document.index(END, start) + block = document[start:end].splitlines() + rules = [number for number, line in enumerate(block) if line.startswith("==")] + if not rules: + return "" + return "\n".join(block[rules[0] : rules[-1] + 1]) + + +def published(table_text: str) -> dict[str, float]: + """Read the numbers the page already carries, so a rerun can leave them where they are.""" + found = (ROW.match(line) for line in table_text.splitlines()) + return {row[1]: float(row[2]) for row in found if row is not None and row[1] != "operation"} + + +def steadied(fresh: Mapping[str, float], old: Mapping[str, float]) -> dict[str, float]: + """Keep every published number a rerun would only have jittered. + + A row moves when it moved further than a rerun moves it, so the diff of + this page is a signal rather than the weather on the machine that ran it. + """ + kept = {} + for label, timing in fresh.items(): + was = old.get(label) + settled = was is not None and abs(timing - was) <= was * NOISE + kept[label] = was if settled else timing + return kept + + +def splice(document: str, table_text: str) -> str: + """Return document with the region between the markers replaced by the table.""" start = document.index(START) + len(START) end = document.index(END, start) - return f"{document[:start]}\n\n{table}\n{document[end:]}" + return f"{document[:start]}\n\n{table_text}\n{document[end:]}" + + +def write_page(timings: Mapping[str, float]) -> int: + """Rewrite the rows of the performance page that moved, and report what changed.""" + document = PAGE.read_text(encoding="utf-8") + if START not in document or END not in document: + sys.stderr.write(f"{PAGE}: markers {START} and {END} not found\n") + return 1 + was = carried(document) + old = published(was) + steady = steadied(timings, old) + fresh = render(steady) + if set(published(fresh)) != set(ORDER): + sys.stderr.write(f"{PAGE}: the table does not read back as its own rows, nothing written\n") + return 1 + if fresh == was: + sys.stderr.write(f"{PAGE}: every row is within {NOISE:.0%} of what it says, left alone\n") + return 0 + PAGE.write_text(splice(document, f"{fresh}\n\n{stamp()}\n"), encoding="utf-8") + moved = [label for label in ORDER if round(steady[label]) != old.get(label)] + sys.stderr.write(f"{PAGE}: rewrote {len(moved)} of {len(ORDER)} rows\n") + for label in moved: + sys.stderr.write(f" {old.get(label)} -> {round(steady[label])} {label}\n") + return 0 + + +def against(timings: Mapping[str, float], source: str) -> int: + """Print this run against the one saved in source, refusing another machine's numbers.""" + saved = json.loads(Path(source).read_text(encoding="utf-8")) + if saved.get("machine") != machine(): + sys.stderr.write( + f"{source} was saved on {saved.get('machine')} and this is {machine()}, " + f"so the deltas would mean nothing\n" + ) + return 1 + sys.stdout.write(f"{compared(timings, saved['timings'])}\n\n{stamp()}\n") + return 0 def main(argv: Sequence[str] | None = None) -> int: - """Write the table to stdout, or into the performance page with --write.""" + """Write the table to stdout, into the performance page, or against a saved run.""" parser = argparse.ArgumentParser(description=__doc__.splitlines()[0]) parser.add_argument( + "--passes", + type=int, + default=PASSES, + metavar="N", + help="how many times to time the whole table, keeping each row's best pass", + ) + parser.add_argument( + "--save", + metavar="FILE", + help="also write the raw timings as JSON, for another revision to compare against", + ) + output = parser.add_mutually_exclusive_group() + output.add_argument( "--write", action="store_true", - help="replace the table in the performance page instead of writing to stdout", + help="update the rows of the performance page that moved, instead of writing to stdout", + ) + output.add_argument( + "--against", + metavar="FILE", + help="print the deltas against timings --save wrote, instead of a table of ns", ) args = parser.parse_args(argv) - - table = render(run()) - if not args.write: - sys.stdout.write(table) - return 0 - - document = PAGE.read_text(encoding="utf-8") - if START not in document or END not in document: - sys.stderr.write(f"{PAGE}: markers {START} and {END} not found\n") - return 1 - PAGE.write_text(splice(document, table), encoding="utf-8") + if args.passes < 1: + parser.error("--passes takes a positive count") + + timings = best_of(args.passes) + if args.save: + saved = {"machine": machine(), "timings": timings} + Path(args.save).write_text(json.dumps(saved, indent=2), encoding="utf-8") + sys.stderr.write(f"{args.save}: saved {len(timings)} rows\n") + + if args.against: + return against(timings, args.against) + if args.write: + return write_page(timings) + sys.stdout.write(f"{render(timings)}\n\n{stamp()}\n") return 0 diff --git a/docs/conf.py b/docs/conf.py index c16cfed..829b0dd 100644 --- a/docs/conf.py +++ b/docs/conf.py @@ -9,8 +9,7 @@ author = "Pavel Kutsenko" copyright = "2026, Pavel Kutsenko" -# nodrill is installed alongside the docs group, so the built docs always -# report the version they were built from. +# Installed alongside the docs group, so a build reports the version it was built from. release = package_version("nodrill") version = ".".join(release.split(".")[:2]) @@ -23,8 +22,7 @@ templates_path = ["_templates"] exclude_patterns = ["_build", ".DS_Store", "Thumbs.db"] -# Single backticks mean inline code, the way they read in every other file -# in the repo; broken explicit roles still fail the build under nitpicky. +# Single backticks are inline code here, as they read everywhere else in the repo. default_role = "literal" nitpicky = True @@ -48,15 +46,13 @@ html_favicon = "_static/img/favicon.svg" html_copy_source = False -# Read the Docs exports the canonical URL of the version being built, and a URL -# hardcoded here would point every version at latest. +# From Read the Docs, since a URL hardcoded here would point every version at latest. html_baseurl = os.environ.get("READTHEDOCS_CANONICAL_URL", "") html_theme_options = { "accent_color": "teal", "color_mode": "auto", - # The card GitHub shows for the repository, as an absolute URL a crawler can - # fetch. index.rst repeats it in :cover:, which asks for the large preview. + # Absolute, since a crawler fetches it, and index.rst repeats it for the large preview. "og_image_url": ( "https://raw.githubusercontent.com/paqstd-dev/nodrill/main" "/.github/assets/social-preview.png" @@ -78,6 +74,5 @@ ], } -# intersphinx already resolves every CPython target on each build, so linkcheck -# skips the host rather than re-requesting it and collecting HTTP 429s. +# Resolved by intersphinx on every build already, so re-requesting it only collects 429s. linkcheck_ignore = [r"https://docs\.python\.org/.*"] diff --git a/docs/content/howto/find-out-why-the-context-is-missing.rst b/docs/content/howto/find-out-why-the-context-is-missing.rst index 4ed9c90..12f6d9e 100644 --- a/docs/content/howto/find-out-why-the-context-is-missing.rst +++ b/docs/content/howto/find-out-why-the-context-is-missing.rst @@ -117,7 +117,7 @@ Finding a provider nothing reads That is usually a key that moved or a layer whose readers went away, and nothing else makes it visible. Reads are counted per block, so a shadowed provider is reported even when something read the inner one under the same key. -It is off by default even inside debug mode, since a warning changes what a program prints and a counting read costs roughly three times a plain one, and a block whose body raised is never blamed. +It is off by default even inside debug mode, since a warning changes what a program prints and a counting read costs roughly three and a half times a plain one, and a block whose body raised is never blamed. The warning is an :exc:`~nodrill.UnusedProviderWarning`, so `warnings.filterwarnings` can silence it by category. A miss inside an adopt block diff --git a/docs/content/howto/index.rst b/docs/content/howto/index.rst index 282259a..ac129ec 100644 --- a/docs/content/howto/index.rst +++ b/docs/content/howto/index.rst @@ -20,6 +20,7 @@ Each one is a complete program you can paste into a file and run, with the reaso test-injected-code refer-to-a-key-you-cannot-import find-out-why-the-context-is-missing + record-what-a-handler-reads see-the-context-in-a-traceback add-context-to-every-log-record replace-a-contextvar diff --git a/docs/content/howto/record-what-a-handler-reads.rst b/docs/content/howto/record-what-a-handler-reads.rst new file mode 100644 index 0000000..c57820e --- /dev/null +++ b/docs/content/howto/record-what-a-handler-reads.rst @@ -0,0 +1,179 @@ +.. _howto-record-what-a-handler-reads: + +Record what a handler reads +=========================== + +A parameter is visible in a signature and a context lookup is not, so the question a reviewer actually asks, which is whether this handler can miss in production, is answered today by deploying. + +Recording answers it from a test run instead. +Run the suite with ``NODRILL_CONTRACT`` pointing at a directory, render what it recorded into a file, and commit the file. +From then on every pull request that changes what a handler reads changes one line of that file, in the diff, where somebody can see it. + +.. code-block:: python + :caption: app.py + + from collections.abc import Iterator + from contextlib import contextmanager + + from nodrill import provider, set_default, use + + + class Settings: + def __init__(self, dsn: str = "sqlite://") -> None: + self.dsn = dsn + + + class User: + def __init__(self, name: str = "anonymous") -> None: + self.name = name + + + class Origin: + def __init__(self, label: str = "system") -> None: + self.label = label + + + set_default(Origin, Origin) + + + def record_write() -> str: + return f"{use(User).name} from {use(Origin).label} on {use(Settings).dsn}" + + + @contextmanager + def running() -> Iterator[None]: + """The process-wide layer, opened once the way a main function does.""" + with provider(Settings()): + yield + + + def serve_http(name: str) -> str: + """The web entry point, which opens both keys the handler reads.""" + with provider("http request"), provider(User(name)), provider(Origin("http")): + return record_write() + + + def run_job(name: str) -> str: + """The queue entry point, which opens the user and leaves the origin to fall back.""" + with provider("celery worker"), provider(User(name)): + return record_write() + +Record a run, then render it. +Anything that exercises the entry points will do, and a test suite is the usual one. + +.. code-block:: console + + $ NODRILL_CONTRACT=.nodrill python -c " + import app + with app.running(): + app.serve_http('ada'); app.run_job('grace')" + $ python -m nodrill contract --from .nodrill + nodrill: 4 facts under 1 entry point, recorded from 1 process. A contract is only as complete as the run that recorded it. + +.. code-block:: text + + # nodrill contract 1 + app:Settings requires app:Origin + app:Settings requires app:Settings + app:Settings requires app:User + app:Settings set_default app:Origin + +Four facts under one entry point, and the entry point is the configuration this process opened in ``running()``. +That is correct and it is useless. + +Name the boundaries +------------------- + +An entry point is the outermost provider block open above a read. +An application that opens configuration, a database handle or a settings object above its server loop makes that block the entry point for everything underneath, and the first column stops distinguishing anything. + +``NODRILL_CONTRACT_ENTRY`` names the blocks that are boundaries, as rendered keys separated by commas, so a block whose key is in that list mints its own entry point even when something is open above it. + +.. code-block:: console + + $ NODRILL_CONTRACT=.nodrill NODRILL_CONTRACT_ENTRY="'http request','celery worker'" python -c " + import app + with app.running(): + app.serve_http('ada'); app.run_job('grace')" + $ python -m nodrill contract --from .nodrill + nodrill: 6 facts under 2 entry points, recorded from 1 process. A contract is only as complete as the run that recorded it. + +.. code-block:: text + + # nodrill contract 1 + 'celery worker' requires app:Settings + 'celery worker' requires app:User + 'celery worker' set_default app:Origin + 'http request' requires app:Origin + 'http request' requires app:Settings + 'http request' requires app:User + +Now the file says something. +The two boundaries read the same three keys, except that the queue never opens `Origin`, so a :func:`~nodrill.set_default` factory answers for it and every row it writes is labelled `system`. +That is a bug the code cannot show you and no test fails on, and it is one line of a diff. + +Pass the same value to the command, so it can tell you about a boundary you named that no block opened, which is what a renamed key looks like. +A boundary that opened and read nothing is a row of the file rather than that message. + +Reading the file +---------------- + +The second field is the one to read. +A `requires` row is a provider answering, and a `set_default` or a `default` row is a key the boundary never opened, which is the row this file exists for. +:ref:`ref-contract-file` has the whole vocabulary, the entry point `(none)` among it. + +Wiring it into CI +----------------- + +Two steps, recording and reviewing. + +.. code-block:: yaml + :caption: .github/workflows/ci.yml + + env: + NODRILL_CONTRACT_ENTRY: "'http request','celery worker'" + steps: + - run: NODRILL_CONTRACT=.nodrill pytest + - run: python -m nodrill contract --from .nodrill --write nodrill.contract + - run: git diff --exit-code nodrill.contract + +The variable is on the job rather than on the recording step, because the command reads it too, and that is what lets it report a boundary you named that no block opened. +The contract file has to be committed for the third step to compare anything, since ``git diff`` says nothing about a path git does not track. + +Recording is off unless ``NODRILL_CONTRACT`` is set, and the variable is read once when `nodrill` is imported, which is also why a subprocess your suite spawns records too. +Each process writes its own file into the directory and the command merges them, so a suite that shells out or one using a :class:`~concurrent.futures.ProcessPoolExecutor` needs nothing extra. +The directory is resolved once and written back into the environment, so a relative ``.nodrill`` means the same place to a child your suite starts in another directory. +A directory reused by a later run is not a problem either, since every process of one run shares a run id and the command reads the newest run and says how many older files it left out. + +A run id is inherited through the environment, so processes of one run share it only when the process that started them imported `nodrill` itself. +A runner that starts its workers directly is the case where that does not hold, and `pytest -n` from a controller whose `conftest.py` never imports the library is the one you are most likely to meet. +Either import `nodrill` in `conftest.py`, or set ``NODRILL_CONTRACT_RUN`` yourself alongside ``NODRILL_CONTRACT``, and the summary will then report one run rather than shards left out. + +What the contract is worth +-------------------------- + +Exactly as much as the run that recorded it. + +A contract lists what the run observed and nothing else, so a key only one untested branch reads is a key the file does not mention. +The summary line says how many facts under how many entry points the conclusion rests on, and it says it every time rather than only when the number is small, because a guarantee that overstates itself is worse than no guarantee. + +Five limits are worth knowing before you rely on it. + +The entry point is a key, so two boundaries that open the same one are one row set, and a boundary you have not named is whatever is open above it. + +A read that raises :exc:`~nodrill.NoProviderError` is not recorded, because it is already loud. +The file is about what an entry point needs and gets, and a miss that reaches a traceback needs no file to be noticed. + +A value read through :func:`~nodrill.inject` is recorded exactly like one read through :func:`~nodrill.use`, but a value a handler receives as an ordinary argument is not context and never appears. +The file describes the context a boundary depends on, which is the part of its input that no signature shows. + +An ambient read through `nodrill.context` is not recorded, because the ambient namespace is unscoped and has no entry point to be credited to. +A handler that reaches for `context.request_id` shows nothing in the file, and a provider block is what makes a dependency reviewable. + +A :func:`~nodrill.lazy` factory runs under the context its own block was opened in, so the keys it reads are credited to whatever entry point was current then rather than to the boundary whose request forced the build. +Read the factory's dependencies under the boundary as well if the row matters, or open the lazy block inside the boundary. + +.. rubric:: See also + +- :doc:`find-out-why-the-context-is-missing` for a miss that is happening now rather than one that might. +- :doc:`/content/topics/declaring` for naming, in the code, which boundary was meant to provide a key. diff --git a/docs/content/howto/see-the-context-in-a-traceback.rst b/docs/content/howto/see-the-context-in-a-traceback.rst index d3b2693..1d475d1 100644 --- a/docs/content/howto/see-the-context-in-a-traceback.rst +++ b/docs/content/howto/see-the-context-in-a-traceback.rst @@ -78,7 +78,7 @@ A ``repr`` that raises an :exc:`Exception` is replaced by `` frozenset[str]: + """Read NODRILL_CONTRACT_ENTRY, whose value is rendered keys separated by commas. + + A key holding a comma cannot be named this way, which is the price of a + spelling somebody types into a CI file by hand. + """ + return frozenset(entry for entry in (part.strip() for part in value.split(",")) if entry) + + +def _new_run() -> str: + """Mint an id for this run, so a directory reused tomorrow does not merge into today.""" + return f"{time.time_ns()}-{uuid.uuid4().hex}" + + +def _render(reads: _Reads) -> str: + """Render a contract, sorted so the file is a property of the run and not of its order.""" + lines = [_HEADER, *(_GAP.join(read) for read in sorted(reads))] + return "".join(f"{line}{_NEWLINE}" for line in lines) + + +def _visible(reads: _Reads) -> _Reads: + """Drop the opened row of a boundary that went on to read, since its reads already say so. + + What survives is the boundary a run opened and read nothing under, which + is a fact about that entry point and not the absence of one. + """ + read = {entry for entry, verb, _ in reads if verb != _OPENED} + return {fact for fact in reads if fact[1] != _OPENED or fact[0] not in read} + + +def _refuse(source: str, saw: str, expected: str) -> ValueError: + """Build the one refusal, so a caller can say which file and what it expected.""" + return ValueError( + f"{source} is not a nodrill contract this version reads. {expected}, found {saw}" + ) + + +def _parse(text: str, source: str) -> _Reads: + """Read a contract or a shard back, refusing anything this reader does not know.""" + lines = text.splitlines() + if not lines or lines[0] != _HEADER: + opening = repr(lines[0]) if lines else "an empty file" + raise _refuse(source, opening, f"Expected {_HEADER!r} on the first line") + found: _Reads = set() + for number, line in enumerate(lines[1:], start=2): + fields = line.split(_GAP) + if len(fields) != 3: # noqa: PLR2004 + raise _refuse(source, repr(line), f"Expected three fields on line {number}") + entry, verb, key = fields + if verb not in _VERBS: + raise _refuse(source, repr(verb), f"Expected one of {sorted(_VERBS)} on line {number}") + found.add((entry, verb, key)) + return found + + +def _write(target: Path, text: str) -> None: + """Write one file of the format, with nothing about it left to the platform.""" + target.write_text(text, encoding=_ENCODING, newline=_NEWLINE) + + +def _dump(directory: str, run: str, reads: _Reads) -> None: + """Write this process's records into its own shard of the run, then forget them. + + Forgetting is what makes a second call a no-op, which matters because a + multiprocessing worker is finalized as well as registered at exit. Taken + before the write, so a thread still recording during shutdown cannot + change the set the render is walking. + """ + if not reads: + return + facts = set(reads) + reads.clear() + target = Path(directory) + # The run first so a merge can group by it, then one token, since rpartition recovers the run. + shard = target / f"{run}-{uuid.uuid4().hex}{_SUFFIX}" + try: + target.mkdir(parents=True, exist_ok=True) + _write(shard, _render(facts)) + except OSError as error: + # A message rather than a traceback out of an exit hook, which exits 0 either way. + sys.stderr.write(f"nodrill: cannot record to {directory}, {error.strerror}\n") + + +def _merge(directory: Path) -> tuple[_Reads, int, int]: + """Read the newest run in a directory, and say how many shards it left behind. + + A directory reused across runs holds both, and a contract built from + yesterday's reads describes a program that no longer exists. + """ + shards = sorted(directory.glob(f"*{_SUFFIX}")) + if not shards: + return set(), 0, 0 + runs: dict[str, list[Path]] = {} + for shard in shards: + runs.setdefault(shard.name.rpartition("-")[0], []).append(shard) + # By when a run last wrote rather than by its id, since a run id may be one a CI system chose. + current = max(runs.values(), key=lambda group: max(shard.stat().st_mtime for shard in group)) + found: _Reads = set() + for shard in current: + found |= _parse(shard.read_text(encoding=_ENCODING), str(shard)) + return found, len(current), len(shards) - len(current) + + +def _summary(reads: _Reads, shards: int, stale: int) -> str: + """Say what the contract rests on, since a guarantee that overstates itself is worse than none. + + The figures are what this stage can honestly own, which is what a run + observed rather than what a tree contains. + """ + entries = len({entry for entry, _, _ in reads} - {_NO_ENTRY}) + said = ( + f"nodrill: {_counted(len(reads), 'fact')} under " + f"{_counted(entries, 'entry point')}, " + f"recorded from {_counted(shards, 'process', 'processes')}. " + f"A contract is only as complete as the run that recorded it." + ) + if stale: + said += f" Left out {_counted(stale, 'shard')} from before this run." + return said + + +def _unseen(reads: _Reads, declared: frozenset[str]) -> str | None: + """Report a declared entry point no block opened, since a renamed key would go quiet. + + Read off the whole run rather than off the rendered contract, because a + boundary that opened and read nothing is recorded and not rendered. + """ + missing = sorted(declared - {entry for entry, _, _ in reads}) + if not missing: + return None + return f"nodrill: no block opened {', '.join(missing)}, named by {_ENTRY_VAR}" + + +def _say(message: str) -> None: + """Put one diagnostic on standard error, so the artefact on standard output stays the file.""" + sys.stderr.write(f"{message}\n") + + +def _contract(source: str, target: str | None, declared: frozenset[str]) -> int: + """Render the contract a recorded run left, to a file or to stdout.""" + directory = Path(source) + if not directory.is_dir(): + _say(f"nodrill: nothing recorded at {source}, so there is no contract") + return 1 + try: + reads, shards, stale = _merge(directory) + except (OSError, ValueError) as error: + _say(f"nodrill: cannot read the run at {source}, {error}") + return 1 + if not shards: + _say( + f"nodrill: {source} holds no shards, so nothing armed the recorder. " + f"Run the suite with NODRILL_CONTRACT={source} first" + ) + return 1 + facts = _visible(reads) + text = _render(facts) + if target is None: + # Through the buffer, so neither the locale nor the platform edits the artefact. + sys.stdout.flush() + sys.stdout.buffer.write(text.encode(_ENCODING)) + sys.stdout.buffer.flush() + else: + try: + _write(Path(target), text) + except OSError as error: + _say(f"nodrill: cannot write {target}, {error.strerror}") + return 1 + _say(_summary(facts, shards, stale)) + unseen = _unseen(reads, declared) + if unseen is not None: + _say(unseen) + return 0 + + +def main(argv: list[str] | None = None) -> int: + """Run one subcommand and return the code the interpreter should exit with.""" + # Deferred, so arming a process does not pay for the command line it will never run. + import argparse # noqa: PLC0415 + + from . import __version__ # noqa: PLC0415 + + parser = argparse.ArgumentParser( + prog="python -m nodrill", + description="Record and review what each entry point reads out of the context.", + allow_abbrev=False, + ) + parser.add_argument("--version", action="version", version=f"nodrill {__version__}") + commands = parser.add_subparsers(dest="command", required=True) + contract = commands.add_parser( + "contract", + help="render the contract a run recorded under NODRILL_CONTRACT", + description=( + "Run the suite with NODRILL_CONTRACT set to a directory, then render what it " + "recorded into a file a pull request can review." + ), + allow_abbrev=False, + ) + contract.add_argument( + "--from", + dest="source", + required=True, + metavar="DIR", + help="the directory NODRILL_CONTRACT named during the run", + ) + contract.add_argument( + "--write", + dest="target", + metavar="FILE", + help="the contract file to write, where the default is stdout", + ) + args = parser.parse_args(argv) + return _contract(args.source, args.target, _declared(os.environ.get(_ENTRY_VAR, ""))) diff --git a/src/nodrill/_concurrency.py b/src/nodrill/_concurrency.py index cf7e18f..75cabe8 100644 --- a/src/nodrill/_concurrency.py +++ b/src/nodrill/_concurrency.py @@ -20,11 +20,10 @@ def wrap(fn: Callable[P, R]) -> Callable[P, R]: """Bind fn to a snapshot of the context active when wrap() was called. - Each invocation runs under a fresh copy of the snapshot, so the result - is safe to call concurrently and callee writes stay local. The - snapshot is taken at wrap() time, so wrapping at import binds - import-time state. Async functions are rejected, because asyncio - propagates context itself. + Each invocation runs under a fresh copy of the snapshot, so the result is + safe to call concurrently and callee writes stay local. The snapshot is + taken at wrap() time, so wrapping at import binds import-time state. + Async functions are rejected, because asyncio propagates context itself. """ if inspect.iscoroutinefunction(fn) or inspect.isasyncgenfunction(fn): raise TypeError( @@ -50,9 +49,8 @@ def restore_and_call() -> R: class Executor(ThreadPoolExecutor): """ThreadPoolExecutor whose tasks see the submit-time context. - Each task runs under its own context copy, so worker-side writes never - leak between tasks or back to the submitter. map() inherits the - behavior via submit(). + Each task runs under its own context copy, so worker-side writes never leak + between tasks or back to the submitter. map() inherits it through submit(). """ def submit(self, fn: Callable[P, R], /, *args: P.args, **kwargs: P.kwargs) -> Future[R]: diff --git a/src/nodrill/_core.py b/src/nodrill/_core.py index 3ff33cf..369357c 100644 --- a/src/nodrill/_core.py +++ b/src/nodrill/_core.py @@ -15,7 +15,14 @@ from typing import TYPE_CHECKING, Any, ClassVar, Generic, TypeVar, overload from ._ambient import _ambient -from ._debug import _diagnose, _record_enter, _record_exit, _recount, _user_site +from ._debug import ( + _diagnose, + _record_enter, + _record_exit, + _record_fallback, + _reinstrument, + _user_site, +) from ._debug import _state as _debug_state from ._declare import _expected_at, _fired, _note_fallback, _pending from ._declare import _restore as _restore_declared @@ -35,6 +42,8 @@ _registry: ContextVar[dict[str | type[Any], Any]] = ContextVar( "nodrill_registry", default=_EMPTY_REGISTRY ) +# Bound once for the two hot readers, the way the @inject wrapper already binds it at decoration. +_registry_get = _registry.get # Configuration rather than per-context state, so deliberately not a ContextVar. _defaults: dict[type[Any], Callable[[], Any]] = {} @@ -110,16 +119,20 @@ def _repaired( """ repaired = dict(current) key = leaving._key # noqa: SLF001 + restored: tuple[str | type[Any], dict[str | type[Any], Any]] | None = None # Innermost first, so the first block still open under the key is the one that owns it now. for open_block in reversed(chain): entered = open_block._entered # noqa: SLF001 if open_block is not leaving and entered is not None and open_block._key == key: # noqa: SLF001 - repaired[key] = entered[key] + # Unhooked, since the instrumentation counts a subscript as a read a consumer made. + repaired[key] = dict.__getitem__(entered, key) + restored = (key, entered) break else: repaired.pop(key, None) - repaired[_Open] = tuple(block for block in chain if block is not leaving) - return _recount(repaired, current) + surviving = tuple(block for block in chain if block is not leaving) + repaired[_Open] = surviving + return _reinstrument(repaired, current, surviving, restored=restored) class _Provider(Generic[T]): @@ -173,13 +186,14 @@ def __enter__(self) -> T: # A fresh scope per entry, which is what stops a re-entry reviving the last one. self._scope = scope = _Scope(self._key, _user_site()[0]) value, public = _sealed_views(value, public, scope) - enclosing = _registry.get() + enclosing = _registry_get() updated = dict(enclosing) updated[self._key] = public - if _debug_state.recording: - self._block, updated = _record_enter(self._key, enclosing, updated) + chain = enclosing.get(_Open, ()) + if _debug_state.watching: + self._block, updated = _record_enter(self._key, enclosing, updated, outermost=not chain) # After the ledger, so the chain lands on the mapping actually installed. - updated[_Open] = (*enclosing.get(_Open, ()), self) + updated[_Open] = (*chain, self) self._entered = updated self._token = _registry.set(updated) return value @@ -380,8 +394,7 @@ class _Sealing: _sealed = True - # What the mixin reads off whichever provider it sits in front of, declared because - # a self typed as that host would leave super() with nothing to resolve against. + # Declared here, since a self typed as the host provider leaves super() nothing to resolve. _scope: _Scope _token: Token[dict[str | type[Any], Any]] | None @@ -415,20 +428,29 @@ class _SealedExtendingProvider(_Sealing, _ExtendingProvider): __slots__ = () -def _refuse_data_flags(**flags: Any) -> None: +def _data_flag_error(name: str, value: Any) -> TypeError: + """Report a flag handed data, naming the namespace spelling that wanted it.""" + return TypeError( + f"provider({name}=...) is a flag and cannot carry data, and " + f"{value!r} would turn it on as well as vanish. For a namespace " + f"attribute of that name write " + f"provider(Namespace({name}={value!r}, ...), key=)" + ) + + +def _refuse_data_flags(frozen: Any, extend: Any, sealed: Any) -> None: """Refuse a flag carrying data, which would otherwise eat a namespace attribute. provider("plan", extend="v1") reads as an attribute and binds the parameter, so the value disappears and the feature turns itself on. """ - for name, value in flags.items(): - if value is not True and value is not False: - raise TypeError( - f"provider({name}=...) is a flag and cannot carry data, and " - f"{value!r} would turn it on as well as vanish. For a namespace " - f"attribute of that name write " - f"provider(Namespace({name}={value!r}, ...), key=)" - ) + # Spelled out rather than looped over **flags, which packed a dict on every provider() call. + if frozen is not True and frozen is not False: + raise _data_flag_error("frozen", frozen) + if extend is not True and extend is not False: + raise _data_flag_error("extend", extend) + if sealed is not True and sealed is not False: + raise _data_flag_error("sealed", sealed) @overload @@ -490,7 +512,7 @@ def provider( once the block has exited, so a value captured by a closure or a background task reports the escape where it happens. """ - _refuse_data_flags(frozen=frozen, extend=extend, sealed=sealed) + _refuse_data_flags(frozen, extend, sealed) target = _target_of(args, values) if isinstance(target, str): if key is not None: @@ -604,7 +626,7 @@ def use(key: Any, *, default: Any = _MISSING) -> Any: instance typed as that class. A miss tries a set_default() factory, then the default argument, then raises NoProviderError. """ - registry = _registry.get() + registry = _registry_get() try: return registry[key] except KeyError: @@ -615,6 +637,12 @@ def use(key: Any, *, default: Any = _MISSING) -> Any: return _resolve_miss(key, default) +def _open_chain() -> tuple[_Provider[Any], ...]: + """Return the blocks open right now, unhooked so asking is not itself a read.""" + chain: tuple[_Provider[Any], ...] = dict.get(_registry_get(), _Open, ()) + return chain + + def _resolve_miss(key: Any, default: Any = _MISSING) -> Any: """Finish a lookup that missed the registry. @@ -642,13 +670,17 @@ def _resolve_miss(key: Any, default: Any = _MISSING) -> Any: # A suspicious class pays the count, and a pending declaration one resolution check. if _pending or target in _fired: _note_fallback(target) + if _debug_state.auditing: + _record_fallback(target, "set_default", _open_chain()) return factory() if default is not _MISSING: + if _debug_state.auditing: + _record_fallback(target, "default", _open_chain()) return default # The resolved target, since that is what a provider registered under. recording = _debug_state.recording diagnosis = _diagnose(target) if recording else None - available = [k for k in _registry.get() if k is not _Open] + available = [k for k in _registry_get() if k is not _Open] raise NoProviderError( key, available, diagnosis, provided_by=_expected_at(target), offer_debug=not recording ) @@ -663,7 +695,7 @@ def active() -> Mapping[str | type[Any], Any]: """ registry = _registry.get() if _Open in registry: - # A counting registry always carries _Open, so the filter is also the uncounting copy. + # An instrumented registry always carries _Open, so the filter is also the plain copy. registry = {key: value for key, value in registry.items() if key is not _Open} return MappingProxyType(registry) diff --git a/src/nodrill/_debug.py b/src/nodrill/_debug.py index 8ad647f..b1aaecb 100644 --- a/src/nodrill/_debug.py +++ b/src/nodrill/_debug.py @@ -2,28 +2,35 @@ A miss usually means the provider is open somewhere this frame cannot see, because the call crossed a boundary that does not carry context. The -evidence for that sits in another context, which is where a lookup cannot -look. +evidence sits in another context, which is where a lookup cannot look. -While debug mode is on, every provider block records where it was entered in -a module-level ledger, and a miss reads the ledger to report a cause. A -ContextVar could not hold it, since one would only ever show the scopes this -frame already sees. +While debug mode is on, every provider block records where it was entered +in a module-level ledger that a miss reads to report a cause. A ContextVar +could not hold it, since one shows only the scopes this frame already sees. + +The contract recorder rides the same instrumentation. NODRILL_CONTRACT arms +it at import, every read under an entry point becomes a fact, and the process +writes its shard at exit through _audit. """ from __future__ import annotations +import atexit import inspect import itertools import os +import sys import threading import warnings +from collections.abc import Iterable, Iterator, MutableMapping +from contextlib import contextmanager from types import TracebackType from typing import Any, NamedTuple from weakref import WeakKeyDictionary from ._declare import _report_lines -from ._errors import UnusedProviderWarning, _describe_key, _Key +from ._errors import _NO_ENTRY, UnusedProviderWarning, _counted, _describe_key, _Key, _key_path +from ._refs import _key_target _Registry = dict[_Key, Any] @@ -83,17 +90,34 @@ class _State: recording and counting mirror the two depths rather than being read off them, since the provider path tests one of them on every block entered. + watching is recording or auditing, so that path still tests one thing. """ - __slots__ = ("counting", "depth", "recording", "seq", "unused_depth") + __slots__ = ( + "auditing", + "counting", + "depth", + "reads_full", + "recording", + "seq", + "unused_depth", + "watching", + ) def __init__(self) -> None: self.depth = 0 self.unused_depth = 0 self.recording = False self.counting = False + self.auditing = False + self.watching = False + self.reads_full = False self.seq = 0 + def watch(self) -> None: + """Restate what the provider path tests, so the two flags behind it live in one place.""" + self.watching = self.recording or self.auditing + _state = _State() @@ -105,6 +129,43 @@ def __init__(self) -> None: # Keys the cap above dropped, which a miss reports as gone rather than as absent. _forgotten: dict[_Key, None] = {} +# Never rolled back, since a run is the unit, and capped since an entry point may carry data. +_reads: set[tuple[str, str, str]] = set() +# High enough that no honest run reaches it, low enough to stay an answer rather than a heap. +_READS_LIMIT = 100_000 +# Keys NODRILL_CONTRACT_ENTRY names as boundaries, which mint a label even when nested. +_declared_entries: set[str] = set() + + +def _note(fact: tuple[str, str, str]) -> None: + """Record one fact, and say once when a run stopped being one the contract can rest on.""" + if fact in _reads: + return + if len(_reads) >= _READS_LIMIT: + if not _state.reads_full: + _state.reads_full = True + sys.stderr.write( + f"nodrill: {_READS_LIMIT} facts recorded, so this run stopped recording. " + f"An entry point carrying a request id mints one per request, and " + f"NODRILL_CONTRACT_ENTRY names the block that is the boundary\n" + ) + return + _reads.add(fact) + + +def _entry_for(chain: tuple[Any, ...]) -> str: + """Return the entry point a chain of open blocks answers to. + + A declared boundary wins over the block above it, innermost first, and an + empty chain is no entry point at all rather than the last one to close. + """ + labels = [_key_path(_key_target(block._key)) for block in chain] # noqa: SLF001 + for label in reversed(labels): + if label in _declared_entries: + return label + return labels[0] if labels else _NO_ENTRY + + # Serials rather than id(), which the interpreter hands on as soon as a task dies. _task_serials: WeakKeyDictionary[Any, int] = WeakKeyDictionary() _next_task_serial = itertools.count(1).__next__ @@ -113,54 +174,160 @@ def __init__(self) -> None: _from_env = os.environ.get("NODRILL_DEBUG", "") not in {"", "0"} _state.depth = 1 if _from_env else 0 _state.recording = _from_env +_state.watch() -class _CountingRegistry(dict[_Key, Any]): - """Registry that marks which block's value a lookup read. +def _arm(environ: MutableMapping[str, str]) -> None: + """Turn the audit on from the environment, and arrange for this process to write its shard. - Installed only while debug(unused=True) is on, which is what keeps read - counting out of use() itself. owners maps a key to the block providing - it, so a read credits that block and not every block sharing the key. + A variable rather than a call, because a child interpreter inherits one + and a call would have to be made again in every process a suite spawns. + Takes the mapping rather than reading os.environ, so what it sets can be + tested without a child interpreter. """ + # Spelled here rather than imported, since it decides whether _audit is loaded at all. + directory = environ.get("NODRILL_CONTRACT", "") + if not directory: + return + _state.auditing = True + _state.watch() + # Deferred, so a process that never audits pays for none of the tool's imports. + from multiprocessing.util import Finalize, register_after_fork # noqa: PLC0415 + from pathlib import Path # noqa: PLC0415 + + from ._audit import _ENTRY_VAR, _RUN_VAR, _declared, _dump, _new_run # noqa: PLC0415 + + # Resolved now, since the hooks below run at exit and a program may have moved by then. + directory = str(Path(directory).resolve()) + # Written back too, so a child that starts elsewhere records here and not beside itself. + environ["NODRILL_CONTRACT"] = directory + _declared_entries.update(_declared(environ.get(_ENTRY_VAR, ""))) + run = environ.get(_RUN_VAR) or _new_run() + # Written back so every child joins this run rather than starting one of its own. + environ[_RUN_VAR] = run + + def _finalize(_: object = None) -> None: + """Arrange the exit a worker takes when it never runs atexit.""" + Finalize(None, _dump, args=(directory, run, _reads), exitpriority=0) + + atexit.register(_dump, directory, run, _reads) + # A multiprocessing worker exits through os._exit, which runs finalizers and not atexit. + _finalize() + # A fork clears that registry before the worker body runs, so the child registers again. + register_after_fork(sys.modules[__name__], _finalize) - __slots__ = ("owners",) - def __init__(self, registry: _Registry, owners: dict[_Key, _Reads]) -> None: +_arm(os.environ) + + +@contextmanager +def _recording(entries: Iterable[str] = ()) -> Iterator[set[tuple[str, str, str]]]: + """Record a contract for the extent of a block, which nothing public does on purpose. + + Saved and restored rather than switched off at the end, since the process + may be recording for real. Meant for a test that wants to assert what a + handler read without spawning a child interpreter. + """ + saved = (_state.auditing, set(_reads), set(_declared_entries), _state.reads_full) + _reads.clear() + _declared_entries.clear() + _declared_entries.update(entries) + _state.auditing = True + _state.reads_full = False + _state.watch() + try: + yield _reads + finally: + _state.auditing = saved[0] + _state.reads_full = saved[3] + _state.watch() + _reads.clear() + _reads.update(saved[1]) + _declared_entries.clear() + _declared_entries.update(saved[2]) + + +class _InstrumentedRegistry(dict[_Key, Any]): + """Registry that watches lookups, for read counting and for the audit. + + Installed instead of branching in use(), which is what keeps both + features out of the hot path when neither is on. owners maps a key to + the block providing it, so a read credits that block and not every block + sharing the key, and entry names the outermost block open above it. + """ + + __slots__ = ("entry", "owners") + + def __init__(self, registry: _Registry, owners: dict[_Key, _Reads], entry: str) -> None: super().__init__(registry) self.owners = owners + self.entry = entry def _mark(self, key: _Key) -> None: - """Note that something read the block providing key.""" + """Mark the block providing key as read.""" reads = self.owners.get(key) if reads is not None: reads.hit = True - def __getitem__(self, key: _Key) -> Any: + def __getitem__(self, key: Any) -> Any: + # Typed loosely because this sees what a caller passed, not what the registry stores. value = super().__getitem__(key) - self._mark(key) + if self.owners: + self._mark(key) + # A consumer read is a subscript, which is what leaves the chain key and a merge out. + if _state.auditing: + _note((self.entry, "requires", _key_path(_key_target(key)))) return value - def get(self, key: _Key, default: Any = None) -> Any: - """Return the value for key, marking the read, the way @inject reads it.""" + def get(self, key: Any, default: Any = None) -> Any: + """Return the value for key, marking the read, the way an extending layer reads it.""" value = super().get(key, _MISS) if value is _MISS: return default - self._mark(key) + if self.owners: + self._mark(key) return value -def _recount(registry: _Registry, replaced: _Registry) -> _Registry: - """Return registry as a counting one when the mapping it replaces was counting.""" - if isinstance(replaced, _CountingRegistry): - return _CountingRegistry(registry, replaced.owners) - return registry +def _reinstrument( + registry: _Registry, + replaced: _Registry, + chain: tuple[Any, ...], + *, + restored: tuple[_Key, _Registry] | None = None, +) -> _Registry: + """Return registry instrumented the way the mapping it replaces was. + + The label is derived from the chain rather than carried over, since a + repaired mapping outlives the block that minted it. A key restored from + a block still open is credited to that block, or the next read of it + would count for the block that just left. + """ + if not isinstance(replaced, _InstrumentedRegistry): + return registry + owners = replaced.owners + if restored is not None: + key, entered = restored + reads = entered.owners.get(key) if isinstance(entered, _InstrumentedRegistry) else None + if reads is not None: + owners = {**owners, key: reads} + return _InstrumentedRegistry(registry, owners, _entry_for(chain)) + + +def _record_fallback(key: _Key, source: str, chain: tuple[Any, ...]) -> None: + """Note a miss a registration answered, which is the read a raise would never report. + + A set_default factory and a use(key, default=...) both return before + anything reports a miss, so a NoProviderError a registration is hiding + would otherwise never appear in a contract. + """ + _note((_entry_for(chain), source, _key_path(_key_target(key)))) def _user_site() -> tuple[_Site, int]: """Return the innermost site outside this package, and how far up it is. - The distance is the stacklevel warnings.warn() wants, counted from the - caller. + The distance is the stacklevel warnings.warn() wants, counted from the caller. """ frame = inspect.currentframe() levels = 0 @@ -197,26 +364,42 @@ def _where() -> _Where: return _Where(ident, name, serial, task.get_name()) -def _record_enter(key: _Key, enclosing: _Registry, registry: _Registry) -> tuple[int, _Registry]: +def _record_enter( + key: _Key, enclosing: _Registry, registry: _Registry, *, outermost: bool +) -> tuple[int | None, _Registry]: """Note an entered provider block, and return its handle with the registry to install. - The handle is the block's serial, which the provider holds until it exits. - id() would be reused by the next provider at that address. + The handle is the block's serial, which the provider holds until it exits, + and it is None when only the audit is watching, since the ledger then has + nothing to forget. id() would be reused by whatever is allocated there next. """ - site, _ = _user_site() - where = _where() - reads = _Reads() if _state.counting else None - with _lock: - _state.seq += 1 - handle = _state.seq - _open[handle] = _Block(key, site, where, handle, reads) + handle: int | None = None + reads: _Reads | None = None + if _state.recording: + site, _ = _user_site() + where = _where() + reads = _Reads() if _state.counting else None + with _lock: + _state.seq += 1 + handle = _state.seq + _open[handle] = _Block(key, site, where, handle, reads) owners: dict[_Key, _Reads] = {} - if isinstance(enclosing, _CountingRegistry): + # Only the audit reads a label, and rendering a key is not free on the block path. + entry = _key_path(key) if _state.auditing else _NO_ENTRY + # Noted on the open, so a boundary that reads nothing stays apart from one that never ran. + if _state.auditing and entry in _declared_entries: + _note((entry, "opened", "nothing")) + if isinstance(enclosing, _InstrumentedRegistry): # Inherited whether or not counting is still on, since it is process-wide. owners = dict(enclosing.owners) + # Outermost is read off the chain, since a repaired mapping outlives the one that made it. + if not outermost and entry not in _declared_entries: + entry = enclosing.entry if reads is not None: owners[key] = reads - return handle, _CountingRegistry(registry, owners) if owners else registry + if not owners and not _state.auditing: + return handle, registry + return handle, _InstrumentedRegistry(registry, owners, entry) def _remember_closed(entry: _Block) -> None: @@ -257,8 +440,8 @@ def _record_exit(handle: int, *, failed: bool) -> None: def _rank(entry: _Block, here: _Where) -> tuple[int, int, int]: """Order the ledger by how likely a block is to explain this frame's miss. - Nearest frame first, then a block still open over one that exited, then - the innermost. + Nearest frame first, then a block still open over one that exited, and the + innermost of those last. """ if entry.where.thread != here.thread: near = 2 @@ -371,6 +554,7 @@ def __enter__(self) -> None: with _lock: _state.depth += 1 _state.recording = True + _state.watch() if self._unused: _state.unused_depth += 1 _state.counting = True @@ -384,6 +568,7 @@ def __exit__( with _lock: _state.depth -= 1 _state.recording = _state.depth > 0 + _state.watch() if self._unused: _state.unused_depth -= 1 _state.counting = _state.unused_depth > 0 @@ -420,12 +605,11 @@ def _codec_lines() -> list[str]: def explain() -> str: """Return a report of the provider blocks open right now, a thread at a time. - Written for a breakpoint, as print(nodrill.explain()). Blocks opened - on other threads and in other tasks are listed too, which is the reason - to read this rather than active(), and the reader's own thread comes - first with its own blocks innermost first. The codec and any suspicious - fallback that has fired are named above them, since nothing else in the - process reports either. + Written for a breakpoint, as print(nodrill.explain()). Blocks opened on + other threads and in other tasks are listed too, which is the reason to read + this rather than active(), and the reader's own thread comes first with its + own blocks innermost first. The codec and any suspicious fallback that has + fired are named above them, since nothing else in the process reports either. """ heading = [*_codec_lines(), *_report_lines()] if not _state.recording: @@ -443,7 +627,7 @@ def explain() -> str: blocks = sorted(_open.copy().values(), key=lambda entry: _listing(entry, here)) if not blocks: return "\n".join([*heading, "nodrill debug: no provider block is open."]) - counted = f"{len(blocks)} provider block{'' if len(blocks) == 1 else 's'}" + counted = _counted(len(blocks), "provider block") lines = [*heading, f"nodrill debug: {counted} open, innermost first within each thread."] lines += [ f" {_describe_key(entry.key)} opened at {entry.site.file}:{entry.site.line}, " diff --git a/src/nodrill/_declare.py b/src/nodrill/_declare.py index 057ba12..e1c6067 100644 --- a/src/nodrill/_declare.py +++ b/src/nodrill/_declare.py @@ -22,7 +22,7 @@ from types import MappingProxyType from typing import Any, Literal, TypeVar, overload -from ._errors import _KEY_TYPES, _describe_key, _Key +from ._errors import _KEY_TYPES, _counted, _describe_key, _Key from ._refs import _PENDING, _during_import, _is_ref, _Ref, _resolutions T = TypeVar("T") @@ -50,8 +50,7 @@ class _Scan: """Remembers the resolution count the last pending scan saw. - A holder rather than a module global, so no writer needs a global - statement. + A holder rather than a module global, so no writer needs a global statement. """ __slots__ = ("at",) @@ -77,10 +76,9 @@ class Declaration: class _Pending: """A declaration waiting for its ref to resolve. - Carries the metadata unassembled, since the Declaration is built with - the resolved key, and eq=False keeps identity comparison, since - comparing the ref inside would force the import this list exists to - avoid. + Carries the metadata unassembled, since the Declaration is built with the + resolved key, and eq=False keeps identity comparison, since comparing the + ref inside would force the import this list exists to avoid. """ ref: _Ref @@ -263,8 +261,7 @@ def keys() -> Mapping[str | type[Any], Declaration]: For a startup check, an admin page or a test. Nothing is imported by the call, so a declaration made through a ref() appears once the ref has - resolved, and the catalogue lists what the modules imported so far have - declared. + resolved, and the catalogue lists what the modules imported so far declared. """ _absorb() with _lock: @@ -300,7 +297,7 @@ def _report_lines() -> list[str]: counts = sorted(dict(_fired).items(), key=lambda item: _describe_key(item[0])) lines = [ f"nodrill declare: the 'suspicious' fallback for {_describe_key(target)} has fired " - f"{count} time{'' if count == 1 else 's'}." + f"{_counted(count, 'time')}." for target, count in counts if count ] @@ -325,7 +322,7 @@ def _restore(saved: _Saved) -> None: actually started from. The firings kept are the snapshot's, filtered to keys the merged catalogue still marks suspicious. A pre-block pending declaration whose ref resolved during the block is absorbed on the way - out, since it is pre-block configuration whose moment simply arrived, and + out, since it is pre-block configuration whose moment arrived, and leaving it pending would let it re-land over a later declaration. """ declared, pending, fired, dropped = saved diff --git a/src/nodrill/_errors.py b/src/nodrill/_errors.py index 73f48df..2aca21e 100644 --- a/src/nodrill/_errors.py +++ b/src/nodrill/_errors.py @@ -1,4 +1,4 @@ -"""Exceptions raised by nodrill.""" +"""Exceptions raised by nodrill, and the key vocabulary every message renders with.""" from __future__ import annotations @@ -13,11 +13,36 @@ # The same union for isinstance, as a tuple since `str | type` allocates a UnionType per evaluation. _KEY_TYPES = (str, type) +# What _key_path renders for no key at all, which only a read outside every block can be. +_NO_ENTRY = "(none)" + def _describe_key(key: Any) -> str: return repr(key) if isinstance(key, str) else getattr(key, "__qualname__", repr(key)) +def _key_path(key: _Key) -> str: + """Render a key the way ref() spells one, so two same-named classes stay apart. + + _describe_key renders a bare qualname, which reads well in a message and + is ambiguous in a file that is diffed, since two Config classes in two + modules render identically. Anything else falls back to its repr, since + instrumentation sees what a caller passed rather than what use() accepts. + """ + if isinstance(key, str): + return repr(key) + module = getattr(key, "__module__", None) + qualname = getattr(key, "__qualname__", None) + if module is None or qualname is None: + return repr(key) + return f"{module}:{qualname}" + + +def _counted(count: int, singular: str, plural: str | None = None) -> str: + """Render a count and its noun, since a figure in a sentence needs to agree with it.""" + return f"{count} {singular if count == 1 else plural or singular + 's'}" + + def _rebuilt( cls: type[BaseException], args: tuple[Any, ...], state: dict[str, Any] ) -> BaseException: @@ -36,9 +61,8 @@ def _reduced(error: BaseException) -> tuple[Any, tuple[Any, ...]]: class NoProviderError(LookupError): """Raised by use() when no provider is active for the requested key. - Carries the requested key, the active keys, the boundaries a declaration - named for it and, under debug mode, the diagnosis of where the value is, - as attributes. + Carries as attributes the requested key, the active keys, the boundaries a + declaration named for it and, under debug mode, where the value actually is. """ # A class-level default, so one pickled by a release without the field still answers. diff --git a/src/nodrill/_frozen.py b/src/nodrill/_frozen.py index 8faea63..bbba45b 100644 --- a/src/nodrill/_frozen.py +++ b/src/nodrill/_frozen.py @@ -70,8 +70,7 @@ def __hash__(self) -> int: def __reduce_ex__(self, protocol: SupportsIndex) -> Any: raise TypeError(_UNCOPYABLE) - # On the class, since copy looks these up on the instance and __getattr__ would - # hand back the target's own hook. + # On the class, since on the instance __getattr__ would hand copy the target's own hook. def __copy__(self) -> Any: raise TypeError(_UNCOPYABLE) diff --git a/src/nodrill/_inject.py b/src/nodrill/_inject.py index f2d350d..23067a4 100644 --- a/src/nodrill/_inject.py +++ b/src/nodrill/_inject.py @@ -29,7 +29,7 @@ ) from ._core import _registry, _resolve_miss -from ._errors import _describe_key +from ._errors import _counted, _describe_key from ._refs import _is_ref, _KeyArg _T = TypeVar("_T") @@ -292,8 +292,8 @@ def _missing_error(label: str, values: tuple[tuple[str, Any], ...]) -> TypeError # Two names join with a bare "and". Three or more take the serial comma. separator = " and " if count == 2 else ", and " # noqa: PLR2004 listed = ", ".join(repr(n) for n in names[:-1]) + separator + repr(names[-1]) - plural = "s" if count > 1 else "" - return TypeError(f"{label}() missing {count} required positional argument{plural}: {listed}") + counted = _counted(count, "required positional argument") + return TypeError(f"{label}() missing {counted}: {listed}") def _reserved(name: str) -> bool: @@ -417,7 +417,12 @@ def _missing_guard_lines(label: str, missing: list[str], ns: _WrapperSpace) -> l def _resolve_lines(target: str, key: str, ns: _WrapperSpace, indent: str) -> list[str]: """Render the one lookup template, an inlined registry hit with the miss path in _core.""" return [ - f"{indent}{target} = {ns.registry}().get({key}, {ns.omitted})", + # A subscript in a try beats get() plus an identity test, since a hit skips the handler. + f"{indent}try:", + f"{indent} {target} = {ns.registry}()[{key}]", + f"{indent}except KeyError:", + f"{indent} {target} = {ns.omitted}", + # The miss runs after the handler, so nothing it raises is chained onto the lookup's own. f"{indent}if {target} is {ns.omitted}:", f"{indent} {target} = {ns.miss}({key})", ] @@ -558,9 +563,8 @@ def _compile_wrapper( ) -> Callable[..., Any]: """Materialize the rendered wrapper and tie the lifetimes together. - The registered source lives exactly as long as the wrapper, and the - wrapper is popped out of its own globals so nothing needs the cycle - collector to die. + The registered source lives as long as the wrapper, and the wrapper is + popped out of its own globals so nothing needs the cycle collector to die. """ name, source, ns = _render_wrapper(func, sig, plan) filename = f"<@inject {plan.label}-{next(_SOURCE_IDS)}>" @@ -697,10 +701,9 @@ def inject(func: Any = None, /, *, from_: _KeyArg | None = None) -> Any: attribute of use("app"), defaults included, and skips self and cls. Explicitly passed arguments always win, an explicit None included. - Works on plain and async functions, methods, classmethods and - staticmethods in either decorator order. Generator functions are - rejected, because their bodies run after the call, possibly under - different providers. + Works on plain and async functions, methods, classmethods and staticmethods + in either decorator order. Generator functions are rejected, because their + bodies run after the call, possibly under different providers. """ if from_ is not None and not isinstance(from_, str | type) and not _is_ref(from_): raise TypeError( diff --git a/src/nodrill/_portable.py b/src/nodrill/_portable.py index 7aa77ed..a82a1a8 100644 --- a/src/nodrill/_portable.py +++ b/src/nodrill/_portable.py @@ -3,8 +3,7 @@ export() renders the providers you name as a plain dict that JSON can hold, and adopt() opens them again wherever that dict arrives. A codec registered with set_codec() maps what JSON cannot hold into what it can, and its result -is checked like any other, so the envelope stays JSON whatever the codec does -inside. +is checked like any other, so the envelope stays JSON whatever a codec does. """ from __future__ import annotations @@ -60,11 +59,10 @@ def export(*names: str) -> dict[str, Any]: Nothing travels unless it is named here, and every value has to be JSON-safe, meaning a str, int, float, bool, None, or a list or dict of - those. Anything else raises rather than being coerced, so a value - arrives on the other side as itself or not at all. Containers are - rebuilt rather than referenced, so a write to a provider never reaches - an envelope already handed on. The result carries a version that - adopt() checks. + those. Anything else raises rather than being coerced, so a value arrives + on the other side as itself or not at all. Containers are rebuilt rather + than referenced, so a write to a provider never reaches an envelope already + handed on. The result carries a version that adopt() checks. """ # Read once, so a set_codec() part way through cannot build one envelope out of two codecs. dump = _codec.dump @@ -87,9 +85,9 @@ def adopt( expects, which is what to reach for when the producer is not yours. annotate decides for these blocks what it decides for a provider() block, and annotate=False keeps a payload somebody else wrote out of a traceback - this process renders. What no check can say is - whether the values are true, and an adopted value is input with the same - trust as any other request field. + this process renders. What no check can say is whether the values are + true, and an adopted value is input with the same trust as any other + request field. """ return _adopting(_adopted(payload, only), annotate=annotate) @@ -116,11 +114,11 @@ def set_codec(*, dump: _Hook | None = None, load: _Hook | None = None) -> None: and never writes into it, since the containers below the top level are the exporting block's own. A load runs after the payload has been checked, never before, so a malformed one is refused without reaching - the codec at all. Each call states the whole codec, and - set_codec() with no arguments clears both, while a service that only - produces or only consumes registers the one half it needs. Both ends of - a boundary have to agree on the format, which is why this is startup - configuration rather than something a scope decides. + the codec at all. Each call states the whole codec, and set_codec() with + no arguments clears both, while a service that only produces or only + consumes registers the one half it needs. Both ends of a boundary have to + agree on the format, which is why this is startup configuration rather + than something a scope decides. """ for role, hook in (("dump", dump), ("load", load)): if hook is not None and not callable(hook): diff --git a/src/nodrill/_refs.py b/src/nodrill/_refs.py index 7805013..f205374 100644 --- a/src/nodrill/_refs.py +++ b/src/nodrill/_refs.py @@ -1,16 +1,14 @@ """The ref() key, its resolution, and the list of refs created so far. A ref names its target by import path and borrows that target's hash and -equality once it resolves, so the registry entry stored under the class is the -entry a lookup through the ref finds. Nothing branches on a ref. The dict -does the work, which leaves use() untouched and the compiled @inject wrappers -with it. +equality once it resolves, so the registry entry stored under the class is +the entry a lookup through the ref finds. Nothing branches on a ref, and +the dict doing the work is what leaves use() and @inject untouched. Resolution is deterministic and idempotent, so it runs unlocked. The module -lock guards only the lists of created refs that resolve_refs() walks. Holding a -lock across import_module() would order this module's lock against the import -system's per-module locks, which is the deadlock every lazy importer eventually -reports. +lock guards only the lists of created refs that resolve_refs() walks. A lock +held across import_module() would order this module's lock against the import +system's per-module locks, the deadlock every lazy importer eventually reports. """ from __future__ import annotations diff --git a/src/nodrill/_report.py b/src/nodrill/_report.py index 48f4f32..a89374e 100644 --- a/src/nodrill/_report.py +++ b/src/nodrill/_report.py @@ -100,16 +100,17 @@ def _boundary_note(exc: BaseException, where: str) -> None: def annotate_exceptions(*, enabled: bool = True) -> None: """Attach the scope to every exception leaving a provider block, process wide. - An exception passing out of a provider block gains a note naming what - that block provided, and comes out as the same object with __notes__ the - only thing about it that changed. Nested blocks each add their own as it - climbs, innermost first. One block decides for itself with - provider(..., annotate=True) or annotate=False, and enabled=False turns - the switch off again. Rendering a note calls the value's repr while the - block is unwinding, so whatever a provider holds can be printed into a - traceback, and a value carrying a secret should hide it in its own repr. - On Python 3.10 this warns and does nothing, since exception notes are - 3.11 and up. + An exception passing out of a provider block gains a note naming what that + block provided, and comes out as the same object with __notes__ the only + thing about it that changed, innermost block first where several nest. + + One block decides for itself with provider(..., annotate=True) or + annotate=False, and enabled=False turns the switch off again. + + Rendering a note calls the value's repr while the block is unwinding, so + whatever a provider holds can be printed into a traceback, and a value + carrying a secret should hide it in its own repr. On Python 3.10 this + warns and does nothing, since exception notes are 3.11 and up. """ if enabled and _add_note is _drop_note: warnings.warn(_UNSUPPORTED, RuntimeWarning, stacklevel=2) diff --git a/src/nodrill/_sealed.py b/src/nodrill/_sealed.py index 96bcbbf..574ec92 100644 --- a/src/nodrill/_sealed.py +++ b/src/nodrill/_sealed.py @@ -36,8 +36,7 @@ _UNCOPYABLE = "sealed context views cannot be pickled or copied" -# Generated rather than written out, since these three are off the hot path and report -# themselves, unlike the attribute members below which report the name they were given. +# Generated, since these three report themselves where the attribute members below report a name. _COMPARED: dict[str, Callable[..., Any]] = { "__eq__": operator.eq, "__ne__": operator.ne, @@ -110,8 +109,7 @@ def __class__(self) -> type[Any]: # pyright: ignore[reportIncompatibleMethodOve answer: type[Any] = self._nodrill_target.__class__ return answer - # Written out rather than generated, since a generator taking the name through *args - # costs half as much again on the operation a sealed value is read through most. + # Written out, since taking the name through *args costs half as much again on a read. def __getattr__(self, name: str) -> Any: scope = self._nodrill_scope if scope.exited is not None: @@ -137,8 +135,7 @@ def __dir__(self) -> list[str]: def __reduce_ex__(self, protocol: SupportsIndex) -> Any: raise TypeError(_UNCOPYABLE) - # On the class, since copy looks these up on the instance and __getattr__ would - # hand back the target's own hook. + # On the class, since on the instance __getattr__ would hand copy the target's own hook. def __copy__(self) -> Any: raise TypeError(_UNCOPYABLE) diff --git a/tests/__init__.py b/tests/__init__.py new file mode 100644 index 0000000..35d9bc0 --- /dev/null +++ b/tests/__init__.py @@ -0,0 +1 @@ +"""The suite, a package so both checkers agree on what a test module is called.""" diff --git a/tests/audit_app/__init__.py b/tests/audit_app/__init__.py new file mode 100644 index 0000000..fb034f9 --- /dev/null +++ b/tests/audit_app/__init__.py @@ -0,0 +1,7 @@ +"""A small application with three entry points, one of which cannot see what it reads. + +Written to be representative rather than flattering. The broken entry point +fails the way a real one does, by reading a key a boundary above it never +opened, and the two working ones read the same key through different shapes +so a contract has something to say about each. +""" diff --git a/tests/audit_app/app.py b/tests/audit_app/app.py new file mode 100644 index 0000000..65a5012 --- /dev/null +++ b/tests/audit_app/app.py @@ -0,0 +1,74 @@ +"""The application itself, a process-wide layer with three entry points under it. + +Shaped after a real service rather than after what flatters the tool. Config +is opened once in main and every boundary nests under it, which is the shape +that collapses to one entry point unless the boundaries are declared. +""" + +from __future__ import annotations + +from collections.abc import Iterator +from contextlib import contextmanager + +from nodrill import FromCtx, inject, injected, provider, set_default, use + + +class Settings: + """What the process is configured with, opened once above everything.""" + + def __init__(self, dsn: str = "sqlite://") -> None: + self.dsn = dsn + + +class User: + """Who the request is for.""" + + def __init__(self, name: str = "anonymous") -> None: + self.name = name + + +class Origin: + """Where a write came from, which an audit table records.""" + + def __init__(self, label: str = "system") -> None: + self.label = label + + +set_default(Origin, Origin) + + +def record_write() -> str: + """Write a row, naming the user and where the write came from.""" + return f"{use(User).name} from {use(Origin).label}" + + +@inject +def open_connection(settings: FromCtx[Settings] = injected) -> str: + """Read through a compiled wrapper, which is a different code path from use().""" + return settings.dsn + + +@contextmanager +def running() -> Iterator[None]: + """Open the process-wide layer the way a main function does.""" + with provider(Settings()): + yield + + +def serve_http(name: str) -> str: + """The web entry point, which opens every key the handler reads but the settings.""" + with provider("http request", route="/writes"), provider(User(name)): + with provider(Origin("http")): + return f"{record_write()} {open_connection()} {use('http request').route}" + + +def run_job(name: str) -> str: + """The queue entry point, which opens the user and leaves the origin to fall back.""" + with provider("celery worker"), provider(User(name)): + return record_write() + + +def run_report() -> str: + """The reporting entry point, which opens a boundary and forgets the user.""" + with provider("nightly report"): + return record_write() diff --git a/tests/cycle/__init__.py b/tests/cycle/__init__.py index 6c554e4..0429e4a 100644 --- a/tests/cycle/__init__.py +++ b/tests/cycle/__init__.py @@ -6,6 +6,5 @@ written with a plain import and cannot be imported at all, so the cycle here is a real one rather than a described one. -`at_import`, `alias` and `reloadable` carry a case each, described where they -stand. +`at_import`, `alias` and `reloadable` carry a case each, described in place. """ diff --git a/tests/cycle/at_import.py b/tests/cycle/at_import.py index 2e14ebd..649e058 100644 --- a/tests/cycle/at_import.py +++ b/tests/cycle/at_import.py @@ -22,8 +22,7 @@ class Scope: try: - # The module is still initialising here too, but the name that is missing is - # missing from the class, which has nothing to do with the import. + # Still initialising here too, but the missing name is the class's, not the import's. use(ref("tests.cycle.at_import:Scope.missing")) except KeyResolutionError as exc: NESTED_FAILURE = str(exc) diff --git a/tests/test_audit.py b/tests/test_audit.py new file mode 100644 index 0000000..2897794 --- /dev/null +++ b/tests/test_audit.py @@ -0,0 +1,823 @@ +"""What a run records, what the contract file says, and what the tool admits it cannot know.""" + +from __future__ import annotations + +import asyncio +import atexit +import importlib +import multiprocessing.util +import os +import runpy +import subprocess +import sys +from collections.abc import Iterator +from pathlib import Path +from typing import Any + +import pytest + +import nodrill +from nodrill import NoProviderError, provider, ref, set_default, use, wrap +from nodrill._audit import ( + _contract, + _declared, + _dump, + _merge, + _new_run, + _parse, + _render, + _summary, + _unseen, + _visible, + main, +) +from nodrill._debug import _arm, _declared_entries, _reads, _recording, _state +from nodrill._errors import _counted +from tests.audit_app.app import ( + Settings, + User, + open_connection, + run_job, + run_report, + running, + serve_http, +) + +_ROOT = Path(__file__).parent.parent +HEADER = "# nodrill contract 1" +APP = "tests.audit_app.app" +TAB = "\t" + + +@pytest.fixture +def recording() -> Iterator[set[tuple[str, str, str]]]: + """Turn the audit on for one test, through the seam the module owns.""" + with _recording() as reads: + yield reads + + +@pytest.fixture +def declaring() -> Iterator[set[tuple[str, str, str]]]: + """Name the app's two boundaries the way NODRILL_CONTRACT_ENTRY does.""" + with _recording({"'http request'", "'celery worker'"}) as reads: + yield reads + + +@pytest.fixture +def armed(recording: set[tuple[str, str, str]]) -> Iterator[list[tuple[Any, ...]]]: + """Collect what _arm registers, so a test never leaves a real hook on this process.""" + calls: list[tuple[Any, ...]] = [] + with pytest.MonkeyPatch.context() as patch: + patch.setattr(atexit, "register", lambda *call: calls.append(("atexit", call))) + patch.setattr( + multiprocessing.util, + "Finalize", + lambda *call, **kw: calls.append(("finalize", (call, kw))), + ) + patch.setattr( + multiprocessing.util, + "register_after_fork", + lambda obj, func: calls.append(("after fork", (obj, func))), + ) + yield calls + + +def _facts(reads: set[tuple[str, str, str]]) -> set[str]: + """Render what was recorded the way the contract file does, minus the header.""" + return {line for line in _render(_visible(reads)).splitlines() if line != HEADER} + + +def _entries(reads: set[tuple[str, str, str]]) -> set[str]: + """The first column, which is the whole question the entry point rule answers.""" + return {entry for entry, _, _ in reads} + + +class TestWhatARunRecords: + """A read is credited to the outermost block open above it.""" + + def test_a_read_is_credited_to_the_entry_point(self, declaring: Any) -> None: + with running(): + serve_http("ada") + assert f"'http request'{TAB}requires{TAB}{APP}:User" in _facts(declaring) + + def test_a_nested_block_does_not_become_an_entry_point(self, declaring: Any) -> None: + with running(): + serve_http("ada") + assert _entries(declaring) == {"'http request'"} + + def test_a_class_keyed_block_is_an_entry_point_like_any_other(self, recording: Any) -> None: + with running(): + use(User, default=None) + assert _entries(recording) == {f"{APP}:Settings"} + + def test_a_string_key_keeps_its_quotes_and_a_class_key_is_a_path(self, declaring: Any) -> None: + with running(): + serve_http("ada") + assert f"'http request'{TAB}requires{TAB}'http request'" in _facts(declaring) + assert f"'http request'{TAB}requires{TAB}{APP}:Origin" in _facts(declaring) + + def test_a_read_through_inject_is_recorded_like_any_other(self, declaring: Any) -> None: + with running(), provider("http request"): + open_connection() + assert _facts(declaring) == {f"'http request'{TAB}requires{TAB}{APP}:Settings"} + + def test_a_ref_key_records_what_it_resolves_to(self, recording: Any) -> None: + with provider("http request"), provider(User("ada")): + assert use(ref(f"{APP}:User")).name == "ada" + assert _facts(recording) == {f"'http request'{TAB}requires{TAB}{APP}:User"} + + def test_a_read_outside_every_block_has_no_entry_point(self, recording: Any) -> None: + class Loose: + pass + + set_default(Loose, Loose) + use(Loose) + assert _entries(recording) == {"(none)"} + + def test_a_fallback_after_every_block_closed_has_no_entry_point(self, recording: Any) -> None: + """A repair leaves its mapping installed, and a dead boundary must not be blamed.""" + + class Loose: + pass + + set_default(Loose, Loose) + + def tenant(slug: str) -> Iterator[None]: + with provider("tenant", slug=slug): + yield + yield + + with provider("http request", route="/"): + first, second = tenant("acme"), tenant("globex") + list(zip(first, second, strict=False)) + list(first) + list(second) + use(Loose) + assert f"(none){TAB}set_default{TAB}" in _facts(recording).pop() + + def test_two_classes_of_the_same_name_stay_apart(self, recording: Any) -> None: + class User: # the point is that this collides with the app's User + pass + + with provider("boundary"), provider(User()): + use(User) + recorded = {line for line in _facts(recording) if "User" in line} + assert len(recorded) == 1 + assert f"{APP}:User" not in next(iter(recorded)) + + +class TestWhatTheRecorderRefusesToCost: + """Instrumentation is passive, so it caps what it keeps and never raises on what it sees.""" + + def test_an_entry_point_carrying_data_stops_rather_than_growing_without_bound( + self, recording: Any, monkeypatch: pytest.MonkeyPatch, capsys: Any + ) -> None: + """A boundary keyed per request mints one entry point per request, which is unbounded.""" + monkeypatch.setattr("nodrill._debug._READS_LIMIT", 2) + for number in range(5): + with provider(f"request-{number}"), provider("db", dsn="x"): + use("db") + assert len(recording) == 2 + said = capsys.readouterr().err + assert "stopped recording" in said + assert "NODRILL_CONTRACT_ENTRY" in said + + def test_the_cap_says_so_once( + self, recording: Any, monkeypatch: pytest.MonkeyPatch, capsys: Any + ) -> None: + monkeypatch.setattr("nodrill._debug._READS_LIMIT", 1) + for number in range(4): + with provider(f"request-{number}"), provider("db", dsn="x"): + use("db") + assert capsys.readouterr().err.count("stopped recording") == 1 + + def test_a_key_the_recorder_did_not_expect_is_rendered_and_not_raised_on( + self, recording: Any + ) -> None: + """Turning the recorder on must not make a lookup that works in production raise.""" + + class Alias: + """Hashes and compares as the string key, which the registry answers on.""" + + def __hash__(self) -> int: + return hash("db") + + def __eq__(self, other: object) -> bool: + return other == "db" + + alias: Any = Alias() + with provider("http request"), provider("db", dsn="x"): + assert use(alias).dsn == "x" + assert any("Alias object at" in key for _, _, key in recording) + + +class TestTheCollapseAndTheDeclaration: + """A layer above the boundaries swallows them, which is why a boundary can be named.""" + + def test_a_process_wide_layer_swallows_every_boundary(self, recording: Any) -> None: + with running(): + serve_http("ada") + run_job("grace") + assert _entries(recording) == {f"{APP}:Settings"} + + def test_a_declared_key_mints_its_own_entry_point_under_that_layer( + self, declaring: Any + ) -> None: + with running(): + serve_http("ada") + run_job("grace") + assert _entries(declaring) == {"'http request'", "'celery worker'"} + + def test_the_fallback_lands_on_the_boundary_that_let_it_happen(self, declaring: Any) -> None: + with running(): + run_job("grace") + assert f"'celery worker'{TAB}set_default{TAB}{APP}:Origin" in _facts(declaring) + + def test_a_declared_key_nothing_opened_is_reported(self) -> None: + reads = {("'http request'", "requires", "x")} + assert _unseen(reads, frozenset({"'http request'"})) is None + message = _unseen(reads, frozenset({"'http request'", "'celery worker'"})) + assert message is not None + assert "no block opened 'celery worker'" in message + + def test_a_declared_boundary_that_reads_nothing_is_recorded_as_opened( + self, declaring: Any + ) -> None: + """A handler that reads nothing must not read as a boundary a rename took away.""" + with running(), provider("celery worker"): + pass + assert f"'celery worker'{TAB}opened{TAB}nothing" in _facts(declaring) + assert _unseen(declaring, frozenset({"'celery worker'"})) is None + + def test_the_opened_row_is_dropped_where_a_read_says_more(self) -> None: + reads = { + ("'http request'", "opened", "nothing"), + ("'http request'", "requires", "x"), + ("'celery worker'", "opened", "nothing"), + } + assert _visible(reads) == { + ("'http request'", "requires", "x"), + ("'celery worker'", "opened", "nothing"), + } + + @pytest.mark.parametrize( + ("value", "expected"), + [ + ("", frozenset()), + ("'a'", frozenset({"'a'"})), + ("'a', pkg:B ", frozenset({"'a'", "pkg:B"})), + ], + ids=["empty", "one", "several"], + ) + def test_the_variable_is_rendered_keys_separated_by_commas( + self, value: str, expected: frozenset[str] + ) -> None: + assert _declared(value) == expected + + +class TestWhatARaiseWouldNeverReport: + """A registration answering a miss is the case the audit exists for.""" + + def test_a_set_default_fallback_is_recorded_under_its_entry_point(self, recording: Any) -> None: + run_job("grace") + assert f"'celery worker'{TAB}set_default{TAB}{APP}:Origin" in _facts(recording) + + def test_a_default_argument_is_recorded_too(self, recording: Any) -> None: + with provider("celery worker"): + assert use(User, default=None) is None + assert f"'celery worker'{TAB}default{TAB}{APP}:User" in _facts(recording) + + def test_a_miss_that_actually_raises_records_nothing(self, recording: Any) -> None: + with pytest.raises(NoProviderError): + run_report() + assert not any("User" in line for line in _facts(recording)) + + @pytest.mark.parametrize("call", [lambda: use(User), open_connection], ids=["use", "inject"]) + def test_a_miss_carries_no_internal_exception(self, call: Any) -> None: + """The wrapper looks up in a try and must leave the handler before the miss runs.""" + with pytest.raises(NoProviderError) as raised: + call() + assert raised.value.__context__ is None + + @pytest.mark.parametrize( + "call", [lambda: use(Settings), open_connection], ids=["use", "inject"] + ) + def test_a_factory_that_raises_is_not_chained_onto_the_lookup(self, call: Any) -> None: + """A set_default factory runs on the miss path, and its failure is the whole story.""" + + def boom() -> Settings: + raise ValueError("the real failure") + + set_default(Settings, boom) + with pytest.raises(ValueError, match="the real failure") as raised: + call() + assert raised.value.__context__ is None + + def test_a_miss_keeps_the_exception_its_caller_was_handling(self) -> None: + """Suppressing every context would hide the error a cleanup path is recovering from.""" + + def cleanup() -> None: + try: + raise ValueError("the real failure") # noqa: TRY301 + except ValueError: + use(User) + + with pytest.raises(NoProviderError) as raised: + cleanup() + assert isinstance(raised.value.__context__, ValueError) + assert not raised.value.__suppress_context__ + + +class TestTheLabelSurvivesTheAwkwardPaths: + """The entry point rides the registry, so it goes wherever the registry goes.""" + + def test_a_block_closing_out_of_order_keeps_the_entry_point(self, recording: Any) -> None: + def tenant(slug: str) -> Iterator[None]: + with provider("tenant", slug=slug): + yield + yield + + with provider("http request", route="/"): + first, second = tenant("acme"), tenant("globex") + list(zip(first, second, strict=False)) + list(first) + list(second) + use("http request") + assert _facts(recording) == {f"'http request'{TAB}requires{TAB}'http request'"} + + def test_a_repair_does_not_hand_its_label_to_the_next_boundary(self, recording: Any) -> None: + """The mapping a repair leaves outlives its chain, and must not name what follows.""" + + def tenant(slug: str) -> Iterator[None]: + with provider("tenant", slug=slug): + yield + yield + + with provider("http request", route="/"): + first, second = tenant("acme"), tenant("globex") + list(zip(first, second, strict=False)) + list(first) + list(second) + with provider("celery worker"), provider(User("grace")): + use(User) + facts = _facts(recording) + assert f"'celery worker'{TAB}requires{TAB}{APP}:User" in facts + assert f"'http request'{TAB}requires{TAB}{APP}:User" not in facts + + def test_a_thread_carries_the_entry_point_it_was_wrapped_under( + self, recording: Any, in_thread: Any + ) -> None: + with provider("http request", route="/"), provider(User("ada")): + in_thread(wrap(lambda: use(User))) + assert f"'http request'{TAB}requires{TAB}{APP}:User" in _facts(recording) + + def test_a_thread_nobody_wrapped_reads_under_no_entry_point( + self, recording: Any, in_thread: Any + ) -> None: + """The bug the tool exists to surface, recorded as the fallback it becomes.""" + + class Loose: + pass + + set_default(Loose, Loose) + with provider("http request", route="/"): + in_thread(lambda: use(Loose)) + assert _entries(recording) == {"(none)"} + + async def test_a_sibling_task_reads_under_its_own_entry_point(self, recording: Any) -> None: + async def worker(label: str) -> None: + with provider(label), provider(User(label)): + await asyncio.sleep(0) + use(User) + + await asyncio.gather(worker("http request"), worker("celery worker")) + assert _facts(recording) == { + f"'http request'{TAB}requires{TAB}{APP}:User", + f"'celery worker'{TAB}requires{TAB}{APP}:User", + } + + +class TestTheSwitch: + """Off is the default, and arming is a function so it can be tested without a child.""" + + def test_an_unset_variable_arms_nothing(self) -> None: + environ: dict[str, str] = {} + before = (_state.auditing, _state.watching) + _arm(environ) + assert (_state.auditing, _state.watching) == before + assert environ == {} + + def test_arming_sets_the_switches_and_joins_a_run( + self, tmp_path: Path, armed: list[tuple[Any, ...]] + ) -> None: + environ = {"NODRILL_CONTRACT": str(tmp_path), "NODRILL_CONTRACT_ENTRY": "'a'"} + _arm(environ) + assert _state.auditing + assert _state.watching + assert "'a'" in _declared_entries + # Written back so a child interpreter joins this run rather than starting one. + assert environ["NODRILL_CONTRACT_RUN"] + # A pool worker exits through os._exit, and a fork clears what the parent registered. + assert [name for name, _ in armed] == ["atexit", "finalize", "after fork"] + + def test_a_forked_child_registers_the_finalizer_the_fork_cleared( + self, tmp_path: Path, armed: list[tuple[Any, ...]] + ) -> None: + """A fork clears the registry before a worker body runs, so the hook is registered again.""" + _arm({"NODRILL_CONTRACT": str(tmp_path)}) + [(_, (_, after_fork))] = [call for call in armed if call[0] == "after fork"] + armed.clear() + after_fork(None) + assert [name for name, _ in armed] == ["finalize"] + + def test_a_child_that_starts_elsewhere_records_where_the_parent_did( + self, tmp_path: Path, armed: list[tuple[Any, ...]], monkeypatch: pytest.MonkeyPatch + ) -> None: + """A relative directory names one place to the parent and another to a child that moved.""" + monkeypatch.chdir(tmp_path) + environ = {"NODRILL_CONTRACT": ".nodrill"} + _arm(environ) + assert environ["NODRILL_CONTRACT"] == str((tmp_path / ".nodrill").resolve()) + + def test_a_relative_directory_is_resolved_while_the_program_is_still_there( + self, tmp_path: Path, armed: list[tuple[Any, ...]], monkeypatch: pytest.MonkeyPatch + ) -> None: + """The hooks run at exit, by which time the program may have moved.""" + monkeypatch.chdir(tmp_path) + _arm({"NODRILL_CONTRACT": ".nodrill"}) + [(_, call)] = [entry for entry in armed if entry[0] == "atexit"] + assert Path(call[1]).is_absolute() + + def test_an_inherited_run_is_kept(self, tmp_path: Path, armed: list[tuple[Any, ...]]) -> None: + environ = {"NODRILL_CONTRACT": str(tmp_path), "NODRILL_CONTRACT_RUN": "given"} + _arm(environ) + assert environ["NODRILL_CONTRACT_RUN"] == "given" + registered = [call for name, call in armed if name in {"atexit", "finalize"}] + assert all("given" in repr(call) for call in registered) + + def test_a_run_with_the_switch_off_records_nothing( + self, monkeypatch: pytest.MonkeyPatch + ) -> None: + """Off is the state a lookup pays nothing for, so it is worth pinning as behaviour.""" + monkeypatch.setattr(_state, "auditing", False) + before = set(_reads) + with running(): + serve_http("ada") + assert set(_reads) == before + + +class TestTheContractFile: + """The file is the delivery mechanism, so its shape is the deliverable.""" + + def test_the_header_names_the_format(self) -> None: + assert _render(set()) == f"{HEADER}\n" + + def test_lines_are_sorted_so_the_file_is_not_a_property_of_the_run(self) -> None: + reads = {("'b'", "requires", "x"), ("'a'", "requires", "y"), ("'a'", "default", "z")} + assert _render(reads).splitlines()[1:] == [ + f"'a'{TAB}default{TAB}z", + f"'a'{TAB}requires{TAB}y", + f"'b'{TAB}requires{TAB}x", + ] + + def test_one_new_key_is_one_added_line(self) -> None: + before = _render({("'a'", "requires", "x"), ("'a'", "requires", "z")}) + after = _render({("'a'", "requires", spelling) for spelling in "xyz"}) + assert set(after.splitlines()) - set(before.splitlines()) == {f"'a'{TAB}requires{TAB}y"} + + def test_a_key_holding_two_spaces_is_still_one_line_of_three_fields( + self, recording: Any + ) -> None: + with provider("http request"), provider("a b", tag=1): + use("a b") + [line] = _facts(recording) + assert line.split(TAB) == ["'http request'", "requires", "'a b'"] + + def test_a_key_holding_a_newline_is_still_one_line(self, recording: Any) -> None: + with provider("http request"), provider("two\nlines", tag=1): + use("two\nlines") + [line] = _facts(recording) + assert line.split(TAB) == ["'http request'", "requires", "'two\\nlines'"] + + def test_a_contract_round_trips(self) -> None: + reads = {("'a'", "requires", "x"), ("'b'", "set_default", "y")} + assert _parse(_render(reads), "test") == reads + + @pytest.mark.parametrize( + ("text", "shown"), + [("", "an empty file"), ("# nodrill contract 2\n", "# nodrill contract 2")], + ids=["empty", "another version"], + ) + def test_a_version_this_reader_does_not_know_is_refused(self, text: str, shown: str) -> None: + with pytest.raises(ValueError, match="not a nodrill contract this version reads") as raised: + _parse(text, "somewhere") + assert shown in str(raised.value) + + @pytest.mark.parametrize( + ("line", "expected"), + [ + (f"'a'{TAB}requires", "Expected three fields on line 2"), + (f"'a'{TAB}requires{TAB}x{TAB}y", "Expected three fields on line 2"), + (f"'a'{TAB}invented{TAB}x", "Expected one of"), + ], + ids=["too few", "too many", "a verb nothing writes"], + ) + def test_a_line_the_format_does_not_allow_is_refused(self, line: str, expected: str) -> None: + """A shard is a file on disk, so a half-written one has to be an answer and not a crash.""" + with pytest.raises(ValueError, match="not a nodrill contract this version reads") as raised: + _parse(f"{HEADER}\n{line}\n", "somewhere") + assert expected in str(raised.value) + + def test_the_line_ending_belongs_to_the_file_and_not_to_the_platform( + self, tmp_path: Path + ) -> None: + """The whole workflow is a diff, so the bytes cannot depend on who rendered them.""" + _dump(str(tmp_path), "run", {("'a'", "requires", "x")}) + [shard] = tmp_path.glob("*.shard") + assert b"\r" not in shard.read_bytes() + + +class TestShards: + """One run is many processes, so the record is written per process and merged.""" + + def test_a_process_that_recorded_nothing_writes_no_shard(self, tmp_path: Path) -> None: + _dump(str(tmp_path / "missing"), "run", set()) + assert not (tmp_path / "missing").exists() + + def test_a_shard_round_trips(self, tmp_path: Path) -> None: + reads = {("'a'", "requires", "x"), ("'b'", "default", "y")} + _dump(str(tmp_path), "run", set(reads)) + assert _merge(tmp_path) == (reads, 1, 0) + + def test_dumping_twice_writes_one_shard(self, tmp_path: Path) -> None: + """A pool worker is finalized as well as registered, so a second dump is a no-op.""" + reads = {("'a'", "requires", "x")} + _dump(str(tmp_path), "run", reads) + _dump(str(tmp_path), "run", reads) + assert len(list(tmp_path.glob("*.shard"))) == 1 + + def test_shards_from_several_processes_merge(self, tmp_path: Path) -> None: + _dump(str(tmp_path), "run", {("'a'", "requires", "x")}) + _dump(str(tmp_path), "run", {("'b'", "requires", "y")}) + found, shards, stale = _merge(tmp_path) + assert found == {("'a'", "requires", "x"), ("'b'", "requires", "y")} + assert (shards, stale) == (2, 0) + + def test_an_earlier_run_in_the_same_directory_is_left_out(self, tmp_path: Path) -> None: + first, second = _new_run(), _new_run() + _dump(str(tmp_path), first, {("'a'", "requires", "gone")}) + for shard in tmp_path.glob("*.shard"): + os.utime(shard, (0, 0)) + _dump(str(tmp_path), second, {("'a'", "requires", "here")}) + assert _merge(tmp_path) == ({("'a'", "requires", "here")}, 1, 1) + + def test_a_run_id_a_ci_system_chose_does_not_outrank_a_later_one(self, tmp_path: Path) -> None: + """A run id is inheritable, so it may be any string and cannot be ordered as a number.""" + _dump(str(tmp_path), "build-42", {("'a'", "requires", "gone")}) + for shard in tmp_path.glob("*.shard"): + os.utime(shard, (0, 0)) + _dump(str(tmp_path), _new_run(), {("'a'", "requires", "here")}) + assert _merge(tmp_path) == ({("'a'", "requires", "here")}, 1, 1) + + def test_an_empty_directory_merges_to_nothing(self, tmp_path: Path) -> None: + assert _merge(tmp_path) == (set(), 0, 0) + + def test_a_run_id_is_unique(self) -> None: + assert _new_run() != _new_run() + + def test_a_directory_it_cannot_write_is_a_message_and_not_two_tracebacks( + self, tmp_path: Path, capsys: Any + ) -> None: + """The dump runs from an exit hook, where a raise is a traceback and never a failure.""" + blocked = tmp_path / "blocked" + blocked.write_text("not a directory", encoding="utf-8") + reads = {("'a'", "requires", "x")} + _dump(str(blocked), "run", reads) + assert "cannot record to" in capsys.readouterr().err + # Forgotten anyway, or the finalizer would reproduce the same failure a second time. + assert not reads + + def test_a_shard_the_reader_refuses_is_named(self, tmp_path: Path, capsys: Any) -> None: + (tmp_path / "1-x.shard").write_text("nonsense\n", encoding="utf-8") + assert _contract(str(tmp_path), None, frozenset()) == 1 + assert "cannot read the run at" in capsys.readouterr().err + + +class TestWhatTheToolAdmits: + """A guarantee that overstates itself is worse than no guarantee.""" + + def test_the_summary_counts_what_it_rests_on(self) -> None: + reads = {("'a'", "requires", "x"), ("'a'", "requires", "y"), ("'b'", "requires", "z")} + assert _summary(reads, 2, 0) == ( + "nodrill: 3 facts under 2 entry points, recorded from 2 processes. " + "A contract is only as complete as the run that recorded it." + ) + + def test_one_of_each_reads_as_a_sentence(self) -> None: + assert _summary({("'a'", "requires", "x")}, 1, 0).startswith( + "nodrill: 1 fact under 1 entry point, recorded from 1 process." + ) + + @pytest.mark.parametrize(("stale", "said"), [(1, "1 shard"), (3, "3 shards")]) + def test_shards_left_out_are_said_rather_than_dropped_quietly( + self, stale: int, said: str + ) -> None: + assert _summary(set(), 1, stale).endswith(f"Left out {said} from before this run.") + + @pytest.mark.parametrize( + ("count", "rendered"), [(0, "0 processes"), (1, "1 process"), (2, "2 processes")] + ) + def test_a_count_carries_its_noun(self, count: int, rendered: str) -> None: + assert _counted(count, "process", "processes") == rendered + + +class TestTheCommandLine: + """python -m nodrill is the whole surface, and it stays out of __all__.""" + + def test_a_directory_nothing_recorded_is_an_error(self, tmp_path: Path, capsys: Any) -> None: + assert _contract(str(tmp_path / "missing"), None, frozenset()) == 1 + assert "nothing recorded" in capsys.readouterr().err + + def test_a_directory_with_no_shards_says_the_recorder_never_armed( + self, tmp_path: Path, capsys: Any + ) -> None: + assert _contract(str(tmp_path), None, frozenset()) == 1 + assert "nothing armed the recorder" in capsys.readouterr().err + + def test_the_contract_goes_to_stdout_by_default(self, tmp_path: Path, capsys: Any) -> None: + _dump(str(tmp_path), "run", {("'a'", "requires", "x")}) + assert _contract(str(tmp_path), None, frozenset()) == 0 + captured = capsys.readouterr() + assert captured.out == f"{HEADER}\n'a'{TAB}requires{TAB}x\n" + assert "1 fact under 1 entry point" in captured.err + + def test_a_declared_key_nothing_opened_reaches_the_output( + self, tmp_path: Path, capsys: Any + ) -> None: + _dump(str(tmp_path), "run", {("'a'", "requires", "x")}) + assert _contract(str(tmp_path), None, frozenset({"'b'"})) == 0 + assert "no block opened 'b'" in capsys.readouterr().err + + def test_a_boundary_that_opened_and_read_nothing_is_a_row_and_not_a_diagnostic( + self, tmp_path: Path, capsys: Any + ) -> None: + _dump(str(tmp_path), "run", {("'a'", "requires", "x"), ("'b'", "opened", "nothing")}) + assert _contract(str(tmp_path), None, frozenset({"'a'", "'b'"})) == 0 + captured = capsys.readouterr() + assert f"'b'{TAB}opened{TAB}nothing" in captured.out + assert "no block opened" not in captured.err + + def test_write_names_the_file(self, tmp_path: Path, capsys: Any) -> None: + _dump(str(tmp_path), "run", {("'a'", "requires", "x")}) + target = tmp_path / "nodrill.contract" + assert main(["contract", "--from", str(tmp_path), "--write", str(target)]) == 0 + assert target.read_text(encoding="utf-8") == f"{HEADER}\n'a'{TAB}requires{TAB}x\n" + assert capsys.readouterr().out == "" + + def test_a_file_it_cannot_write_is_a_message_and_not_a_traceback( + self, tmp_path: Path, capsys: Any + ) -> None: + _dump(str(tmp_path), "run", {("'a'", "requires", "x")}) + target = tmp_path / "no" / "such" / "dir" / "out" + assert _contract(str(tmp_path), str(target), frozenset()) == 1 + assert "cannot write" in capsys.readouterr().err + + def test_a_subcommand_is_required(self) -> None: + with pytest.raises(SystemExit) as raised: + main([]) + # argparse owns 2, which is why nothing recorded is 1. + assert raised.value.code == 2 + + def test_a_flag_cannot_be_abbreviated(self, tmp_path: Path) -> None: + with pytest.raises(SystemExit): + main(["contract", "--fro", str(tmp_path)]) + + def test_the_command_says_which_nodrill_wrote_a_contract(self, capsys: Any) -> None: + """A format the reader refuses is the moment the version is worth asking for.""" + with pytest.raises(SystemExit) as raised: + main(["--version"]) + assert raised.value.code == 0 + assert capsys.readouterr().out.strip() == f"nodrill {nodrill.__version__}" + + +def _child( + program: str, directory: Path, entries: str = "", cwd: Path = _ROOT +) -> subprocess.CompletedProcess[str]: + """Run a program in a child interpreter with the recorder armed.""" + return subprocess.run( # the interpreter running this suite, with a program written above + [sys.executable, "-c", program], + check=True, + capture_output=True, + text=True, + cwd=str(cwd), + env={ + **os.environ, + "NODRILL_CONTRACT": str(directory), + "NODRILL_CONTRACT_ENTRY": entries, + "PYTHONPATH": str(_ROOT), + }, + ) + + +class TestARecordedRun: + """The end to end path, in a child interpreter, since the switch is read once at import.""" + + def test_the_environment_variable_arms_a_whole_process(self, tmp_path: Path) -> None: + program = f"from {APP} import running, serve_http\nwith running(): serve_http('ada')" + _child(program, tmp_path) + reads, _, _ = _merge(tmp_path) + assert f"{APP}:Settings{TAB}requires{TAB}{APP}:User" in _render(reads) + + def test_declaring_the_boundaries_splits_the_entry_points(self, tmp_path: Path) -> None: + _child( + f"from {APP} import running, serve_http, run_job\n" + "with running():\n serve_http('ada')\n run_job('grace')", + tmp_path, + entries="'http request','celery worker'", + ) + reads, _, _ = _merge(tmp_path) + assert _entries(reads) == {"'http request'", "'celery worker'"} + + def test_a_subprocess_the_run_spawns_joins_the_same_run(self, tmp_path: Path) -> None: + program = ( + "import subprocess, sys\n" + f"from {APP} import running, serve_http\n" + "with running(): serve_http('ada')\n" + "subprocess.run([sys.executable, '-c'," + f" 'from {APP} import run_job; run_job(\"grace\")'], check=True)\n" + ) + _child(program, tmp_path) + reads, shards, stale = _merge(tmp_path) + assert (shards, stale) == (2, 0) + assert f"'celery worker'{TAB}set_default{TAB}{APP}:Origin" in _render(reads) + + def test_a_subprocess_that_moves_still_records_into_the_same_directory( + self, tmp_path: Path + ) -> None: + """A relative directory is the whole point of the variable being resolved once.""" + elsewhere = tmp_path / "elsewhere" + elsewhere.mkdir() + program = ( + "import subprocess, sys\n" + f"from {APP} import running, serve_http\n" + "with running(): serve_http('ada')\n" + "subprocess.run([sys.executable, '-c'," + f" 'from {APP} import run_job; run_job(\"grace\")']," + f" check=True, cwd={str(elsewhere)!r})\n" + ) + _child(program, Path(".nodrill"), cwd=tmp_path) + assert not (elsewhere / ".nodrill").exists() + reads, shards, stale = _merge(tmp_path / ".nodrill") + assert (shards, stale) == (2, 0) + assert f"'celery worker'{TAB}requires{TAB}{APP}:User" in _render(reads) + + def test_a_process_pool_worker_records_its_own_shard(self, tmp_path: Path) -> None: + """A worker exits through os._exit, which runs finalizers and never atexit.""" + program = ( + "from concurrent.futures import ProcessPoolExecutor\n" + f"from {APP} import run_job\n" + "if __name__ == '__main__':\n" + " with ProcessPoolExecutor(max_workers=1) as pool:\n" + " pool.submit(run_job, 'grace').result()\n" + ) + _child(program, tmp_path) + reads, _, _ = _merge(tmp_path) + assert f"'celery worker'{TAB}requires{TAB}{APP}:User" in _render(reads) + + def test_two_runs_of_the_same_program_agree_byte_for_byte(self, tmp_path: Path) -> None: + program = f"from {APP} import running, serve_http\nwith running(): serve_http('ada')" + first, second = tmp_path / "first", tmp_path / "second" + _child(program, first) + _child(program, second) + assert _render(_merge(first)[0]) == _render(_merge(second)[0]) + + def test_the_module_runs_as_a_command(self, tmp_path: Path) -> None: + program = f"from {APP} import running, serve_http\nwith running(): serve_http('ada')" + _child(program, tmp_path) + result = subprocess.run( # the interpreter running this suite + [sys.executable, "-m", "nodrill", "contract", "--from", str(tmp_path)], + check=True, + capture_output=True, + text=True, + ) + assert result.stdout.startswith(f"{HEADER}\n") + assert "A contract is only as complete as the run that recorded it." in result.stderr + + def test_the_dispatch_exits_with_what_the_command_returned( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch, capsys: Any + ) -> None: + """__main__ is the two lines that turn a return code into an exit code.""" + argv = ["nodrill", "contract", "--from", str(tmp_path / "missing")] + monkeypatch.setattr(sys, "argv", argv) + with pytest.raises(SystemExit) as raised: + runpy.run_module("nodrill", run_name="__main__") + assert raised.value.code == 1 + assert "nothing recorded" in capsys.readouterr().err + + def test_importing_the_dispatch_does_not_exit_the_process_that_imported_it(self) -> None: + """A package walker imports every submodule, and __main__ is one of them.""" + # Popped again, so the next runpy of it starts from source the way a command line does. + try: + assert importlib.import_module("nodrill.__main__").main is main + finally: + sys.modules.pop("nodrill.__main__", None) diff --git a/tests/test_debug.py b/tests/test_debug.py index cb852e8..0fbecc0 100644 --- a/tests/test_debug.py +++ b/tests/test_debug.py @@ -191,7 +191,7 @@ def test_the_innermost_open_block_is_the_one_reported(self) -> None: assert f"{__file__}:{opened}" in str(error) def test_a_bare_pool_worker_is_diagnosed(self) -> None: - """The case the feature exists for: submitting to a pool that is not nodrill's.""" + """The case the feature exists for, submitting to a pool that is not nodrill's.""" with debug(), provider(Session()), ThreadPoolExecutor(max_workers=1) as pool: error = pool.submit(read_session).result() assert "which did not inherit that context" in str(error) @@ -505,7 +505,7 @@ def test_a_provider_nothing_read_warns_at_the_with_statement(self) -> None: assert record.lineno == opened def test_a_read_provider_is_silent(self) -> None: - """A provider something read is not warned about; warnings are errors here.""" + """A provider something read is not warned about, and warnings are errors here.""" with debug(unused=True), provider(Session()): use(Session) @@ -527,6 +527,16 @@ def test_an_extending_layer_counts_as_a_read_of_what_it_extends(self) -> None: [record] = records assert record.lineno == layered + def test_a_layer_over_nothing_credits_no_one(self) -> None: + """extend=True over a name nothing provided has no outer namespace to read.""" + with pytest.warns(UnusedProviderWarning, match="never read") as records, debug(unused=True): + with provider("db", dsn="x"): + use("db") + with provider("app", extend=True, tag="x"): + opened = line_above() + [record] = records + assert record.lineno == opened + def test_a_block_that_raised_is_not_warned_about(self) -> None: """A body that blew up never had the chance to read, so it is not blamed for it.""" with debug(unused=True): diff --git a/tests/test_errors.py b/tests/test_errors.py index 9bbaf50..98942bc 100644 --- a/tests/test_errors.py +++ b/tests/test_errors.py @@ -71,7 +71,7 @@ def test_no_suggestion_when_nothing_is_close(self) -> None: assert "Did you mean" not in str(exc_info.value) def test_hand_built_error_with_an_odd_key_still_builds_a_message(self) -> None: - """NoProviderError is public API: use() screens keys, a direct caller need not.""" + """NoProviderError is public API, and a direct caller need not screen keys as use() does.""" assert "42" in str(NoProviderError(42)) def test_use_rejects_non_key_types(self) -> None: diff --git a/tests/test_inject_async.py b/tests/test_inject_async.py index 1588c34..f3f547e 100644 --- a/tests/test_inject_async.py +++ b/tests/test_inject_async.py @@ -19,7 +19,7 @@ async def fetch(cfg: FromCtx[Config] = injected, retry: int = 0) -> str: class TestAsyncInjection: def test_wrapper_is_a_coroutine_function(self) -> None: - """The wrapper must be async — never a sync function returning a coroutine.""" + """The wrapper must be async, never a sync function returning a coroutine.""" assert inspect.iscoroutinefunction(fetch) async def test_injects_from_provider(self) -> None: diff --git a/tests/test_inject_binding.py b/tests/test_inject_binding.py index ccbc7aa..26a9cd4 100644 --- a/tests/test_inject_binding.py +++ b/tests/test_inject_binding.py @@ -1,7 +1,7 @@ -"""Argument binding in @inject: the compiled wrapper mirrors the signature. +"""Argument binding in @inject, where the compiled wrapper mirrors the signature. The wrapper's parameter list is generated from the function's own, so the -interpreter binds every call shape natively; these tests pin the shapes down. +interpreter binds every call shape natively, and these tests pin the shapes down. """ from collections.abc import Callable @@ -179,8 +179,7 @@ def handler(request: str, db: FromCtx[Db] = injected) -> str: handler("a", "b", "c") # type: ignore[call-arg, arg-type] def test_a_bad_call_fails_before_resolution(self) -> None: - # The wrapper mirrors the signature, so Python rejects the call exactly - # as it would reject the undecorated function, provider or no provider. + # The wrapper mirrors the signature, so Python rejects the call as it always would. @inject def handler(db: FromCtx[Db] = injected) -> str: return db.dsn @@ -233,8 +232,7 @@ def handler(db: FromCtx[Db], a: str, b: str) -> str: handler() # type: ignore[call-arg] def test_missing_required_argument_beats_the_provider_miss(self) -> None: - # The guard runs before any resolution, so the caller's mistake is - # reported even when no provider is active. + # The guard runs before any resolution, so no provider need be active to report it. @inject def handler(db: FromCtx[Db], tag: str) -> str: return tag diff --git a/tests/test_inject_codegen.py b/tests/test_inject_codegen.py index a670815..8d457a3 100644 --- a/tests/test_inject_codegen.py +++ b/tests/test_inject_codegen.py @@ -1,4 +1,4 @@ -"""The compiled wrapper as an artifact: its source, its names, its lifetime.""" +"""The compiled wrapper as an artifact, meaning its source, its names and its lifetime.""" import gc import linecache @@ -160,7 +160,10 @@ def handler(request: str, db: FromCtx[Db] = injected) -> str: assert generated_source(handler) == ( "def handler(request, db=_nd_injected):\n" " if db is _nd_injected:\n" - " db = _nd_registry().get(_nd_key_db, _nd_omitted)\n" + " try:\n" + " db = _nd_registry()[_nd_key_db]\n" + " except KeyError:\n" + " db = _nd_omitted\n" " if db is _nd_omitted:\n" " db = _nd_miss(_nd_key_db)\n" " return _nd_func(request, db)" @@ -175,7 +178,10 @@ def handler(dsn: Annotated[str, from_ctx(ref(f"{__name__}:Db"))] = injected) -> assert generated_source(handler) == ( "def handler(dsn=_nd_injected):\n" " if dsn is _nd_injected:\n" - " _nd_value = _nd_registry().get(_nd_key_dsn, _nd_omitted)\n" + " try:\n" + " _nd_value = _nd_registry()[_nd_key_dsn]\n" + " except KeyError:\n" + " _nd_value = _nd_omitted\n" " if _nd_value is _nd_omitted:\n" " _nd_value = _nd_miss(_nd_key_dsn)\n" " dsn = _nd_ref_attr(_nd_key_dsn, _nd_value, 'dsn')\n" @@ -193,7 +199,10 @@ def handler(db: FromCtx[Db], tag: str) -> str: " if tag is _nd_injected:\n" " raise _nd_missing_error(_nd_label, (('tag', tag),))\n" " if db is _nd_injected:\n" - " db = _nd_registry().get(_nd_key_db, _nd_omitted)\n" + " try:\n" + " db = _nd_registry()[_nd_key_db]\n" + " except KeyError:\n" + " db = _nd_omitted\n" " if db is _nd_omitted:\n" " db = _nd_miss(_nd_key_db)\n" " return _nd_func(db, tag)" @@ -211,7 +220,10 @@ def render(user: str, lang: str = "en") -> str: " _nd_unmet = []\n" " if user is _nd_injected:\n" " if _nd_source is _nd_omitted:\n" - " _nd_source = _nd_registry().get(_nd_from_key, _nd_omitted)\n" + " try:\n" + " _nd_source = _nd_registry()[_nd_from_key]\n" + " except KeyError:\n" + " _nd_source = _nd_omitted\n" " if _nd_source is _nd_omitted:\n" " _nd_source = _nd_miss(_nd_from_key)\n" " user = _nd_getattr(_nd_source, 'user', _nd_omitted)\n" @@ -219,7 +231,10 @@ def render(user: str, lang: str = "en") -> str: " _nd_unmet.append('user')\n" " if lang is _nd_injected:\n" " if _nd_source is _nd_omitted:\n" - " _nd_source = _nd_registry().get(_nd_from_key, _nd_omitted)\n" + " try:\n" + " _nd_source = _nd_registry()[_nd_from_key]\n" + " except KeyError:\n" + " _nd_source = _nd_omitted\n" " if _nd_source is _nd_omitted:\n" " _nd_source = _nd_miss(_nd_from_key)\n" " lang = _nd_getattr(_nd_source, 'lang', _nd_omitted)\n" diff --git a/tests/test_lazy.py b/tests/test_lazy.py index 5fee7ac..f6ee53d 100644 --- a/tests/test_lazy.py +++ b/tests/test_lazy.py @@ -58,7 +58,7 @@ def __call__(self) -> Config: class TestResolution: def test_unread_provider_never_builds(self) -> None: - """The whole point: a scope nothing reads costs nothing to open.""" + """The whole point, that a scope nothing reads costs nothing to open.""" factory = Counter() with provider(lazy(Config, factory)): pass @@ -202,7 +202,7 @@ def factory() -> Config: touch() def test_factory_returning_its_own_key_raises(self) -> None: - """The same mistake by return: the cell would otherwise become its own value.""" + """The same mistake by return, where the cell would otherwise become its own value.""" def factory() -> Config: returned: Config = use(Config) diff --git a/tests/test_lazy_hints.py b/tests/test_lazy_hints.py index 4e062f5..e6ae52a 100644 --- a/tests/test_lazy_hints.py +++ b/tests/test_lazy_hints.py @@ -10,7 +10,7 @@ @inject def refers_forward(cfg: FromCtx[DefinedLater] = injected) -> str: - """Decorated while `DefinedLater` does not exist yet — must not raise.""" + """Decorated while `DefinedLater` does not exist yet, which must not raise.""" return cfg.tag diff --git a/tests/test_refs.py b/tests/test_refs.py index 1e5622b..fd5871c 100644 --- a/tests/test_refs.py +++ b/tests/test_refs.py @@ -252,8 +252,7 @@ def test_path_must_be_a_string(self) -> None: ref(Config) # type: ignore[arg-type] def test_a_path_naming_a_module_is_only_refused_at_the_lookup(self) -> None: - # 'package.module' cannot be told from 'module.Name', so the path is accepted - # and the module it names is what fails, as any other non-key target does. + # 'package.module' cannot be told from 'module.Name', so the module it names fails. key = ref("json.decoder") with pytest.raises(TypeError, match="use\\(\\) expects a string name or a class"): use(key) diff --git a/tests/test_sealed.py b/tests/test_sealed.py index 9acbbee..17a901a 100644 --- a/tests/test_sealed.py +++ b/tests/test_sealed.py @@ -165,8 +165,7 @@ class TestGuardsAndCopies: def test_a_defaulting_getattr_does_not_swallow_the_expiry(self) -> None: with provider(Session(), sealed=True) as session: pass - # Not an AttributeError, since getattr(x, name, default) would answer the default - # and hand the caller the silent wrong value sealing exists to report. + # Not an AttributeError, or getattr(x, name, default) would answer with the default. assert not issubclass(ExpiredScopeError, AttributeError) with pytest.raises(ExpiredScopeError): getattr(session, "dsn", "fallback") diff --git a/tests/test_sealed_protocols.py b/tests/test_sealed_protocols.py index 584e862..d9037db 100644 --- a/tests/test_sealed_protocols.py +++ b/tests/test_sealed_protocols.py @@ -359,8 +359,7 @@ def expired(self) -> Everything: def test_every_generated_protocol_checks_the_scope( self, expired: Everything, name: str ) -> None: - # Called with no arguments, since the check runs before the delegation, so - # a missing one shows up as something other than ExpiredScopeError. + # Called with no arguments, since the check runs first and a missing one raises otherwise. args = (1,) if name in _REFLECTED or name in _INPLACE else () method = getattr(type(expired), name) with pytest.raises(ExpiredScopeError, match=rf"Everything\.{name} was used after"): diff --git a/tests/test_threads.py b/tests/test_threads.py index e5b3170..a317377 100644 --- a/tests/test_threads.py +++ b/tests/test_threads.py @@ -38,7 +38,7 @@ def pool() -> Iterator[Executor]: class TestPlainThreads: def test_plain_thread_does_not_see_context(self, in_thread: ThreadRunner) -> None: - """The documented behavior: threading.Thread starts with an empty context.""" + """The documented behavior, that threading.Thread starts with an empty context.""" with provider(Config(tag="main")): assert isinstance(in_thread(read_tag), NoProviderError)