Skip to content

Tests should execute all sample code in docs #7

Description

@heikkitoivonen

Promote the doc-execution check suite-wide

tests/test_graphlib_complexity.py::TestDocumentedExamplesRun executes every
Python block on one page and fails if any raises. It should cover all of
docs/.

A first attempt to survey the whole tree hung and had to be killed; what it
taught is in "Safety requirements" below, which supersedes the earlier
version of this ticket.

Why

Nine module pages have now been checked by hand against their documentation.
In every case the formal complexity tables were broadly right. The defects
were in code nobody had executed:

page broken blocks (before fixes)
graphlib 5 of 11 prepare() + static_order() raised ValueError
compileall 2 of 2 both referenced paths that did not exist
py_compile 2 of 2 both raised FileNotFoundError
collections 1 of 6 NameError on an undefined d
bisect 1 of 11 NameError - found while writing this ticket
array 0 of 6
counter 0 of 10
ordereddict 0 of 10
struct 0 of 15

Five of nine pages shipped an example that could not run. The graphlib page's
first example was one of them. Neither make check nor several rounds of
external review caught any of these, because nothing executes the docs.

Two of the broken blocks were introduced by us, while adding complexity
annotations to blocks whose code we had not read. One reached three
translations before anyone noticed.

What exists today

The graphlib version is ~35 lines: pull ```python fences out of the markdown,
exec each in a fresh namespace, collect failures with line numbers, assert
the list is empty. Verified to catch the real bug - reintroducing the
`prepare()` call fails it with `line 21: ValueError`.

Survey of the full tree

English pages only (docs/fi|ja|zh excluded - code blocks are byte-identical
across locales by project rule, so English is sufficient):

  • 3,138 Python blocks total
  • 238 contain something that must never run in CI:
    • 118 exit/blocking (sys.exit, while True, input()
    • 52 processes (multiprocessing, threading, concurrent.futures)
    • 44 network (socket, urlopen, smtplib, ftplib)
    • 28 browser/system (import antigravity opens a web browser,
      webbrowser, subprocess, os.system, os.fork, signal)
    • 18 delete/move (shutil.rmtree, os.remove, Path.unlink)
  • 2,900 remaining candidates

How many of those 2,900 run clean is still unknown - the survey that was
measuring it is the one that hung.

Safety requirements - all learned by getting them wrong

  1. Run each block in a SUBPROCESS with a hard timeout. An in-process
    signal.setitimer alarm is NOT sufficient: the survey run sat for 87
    minutes having used 9 seconds of CPU, blocked in unix_stream_data_wait
    with fd 0 on a socket, and needed SIGKILL. Something in the blocking path
    swallowed the alarm.
  2. Close stdin, do not merely redirect it. The skip list caught input( but
    not breakpoint(), help(), pdb.set_trace(), sys.stdin.read() or
    getpass.getpass() - none of which match a naive filter, and all of which
    appear in docs/builtins/help.md, docs/stdlib/pdb.md and
    docs/stdlib/termios.md.
  3. textwrap.dedent every block before compiling. Blocks nested in !!!
    admonitions carry the admonition's indentation and raise IndentationError
    otherwise - a false positive on a widely used pattern in this repo. Worse,
    it MASKS real errors: bisect.md:209 reported IndentationError until it
    was dedented, and then revealed a genuine NameError underneath.
  4. Execute in a temp cwd, never the repo - blocks write files.
  5. Report failures AFTER restoring sys.stdout. A diagnostic that prints
    inside the except while stdout is still captured silently reports
    nothing. This cost an hour of confusion during the survey.
  6. Blocks that should raise, to demonstrate an exception, need a convention
    • an allowlist or a marker in the fence info string.

What this check cannot do

It catches blocks that crash. It does not catch blocks that lie. On
py_compile a block demonstrating doraise=True was written with
quiet=2 as well, which suppresses the raise; it executes cleanly and proves
nothing. Only the corresponding unit test caught that. Both mechanisms are
needed; neither subsumes the other.

Open question - the design depends on one number

Once the 2,900 candidates can actually be surveyed:

  1. If most run, record a baseline of known-unrunnable blocks and fail only on
    NEW breakage. This catches the d-style bug at the moment it is
    introduced, which is the main prize.
  2. If most fail, a baseline of thousands is worthless. Target only
    self-contained blocks instead, detected by an AST walk checking that every
    free name is bound within the block. Many blocks are deliberately partial
    (process(item), risky_operation()).
  3. Fallback: scope to pages that already have a complexity test file (array,
    bisect, collections, compileall, counter, difflib, graphlib, heapq,
    ordereddict, py_compile, range, struct) where the blocks are known to run,
    and grow page by page.

Runtime

Unmeasured, and possibly the deciding constraint. The suite is currently ~90s
for 954 tests. Subprocess-per-block over 2,900 blocks will not be free. If it
cannot be made fast: a separate make target rather than make test, or
option 3 above.

Acceptance criteria

  • All of docs/ covered, or an explicit documented scope with a reason
  • Fails with the file and line number of each broken block
  • Green on a clean tree; verified to fail when a known-broken block is
    reintroduced (mutation check - and assert the mutation applied, since
    one in this project silently matched nothing after a reformat)
  • Subprocess isolation, hard timeout, stdin closed, temp cwd, dedent
  • Suite runtime impact measured and acceptable
  • make check passes: ruff, pyright, pytest
  • The graphlib-local version is removed or folded into the shared one
  • AGENTS.md updated - its "Adding or Changing a Complexity Claim"
    section already tells contributors to run a page's blocks, but there is
    no tooling behind that instruction yet

Known bug to fix alongside

docs/stdlib/bisect.md:209 - the block inside the "Sorted Data Requirement"
admonition calls bisect.bisect(unsorted, 2) without importing bisect.
Found while gathering numbers for this ticket. The page is translated, so the
fix must be mirrored into fi/ja/zh with --update-hashes.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions