✨ Add public sphinx_design.testing helpers#277
Conversation
Lift the test-suite's doctree-XML normalizer into a new semi-public `sphinx_design.testing` module so downstream extensions can reuse it for their doctree regression tests instead of re-implementing it. The `normalize_doctree_xml` helper and the `SphinxBuilder` wrapper are now importable with only `sphinx_design` installed (no pytest/test extras); the pytest fixtures in conftest become thin re-exporting wrappers.
6268fca to
2974a20
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #277 +/- ##
==========================================
+ Coverage 90.33% 90.40% +0.07%
==========================================
Files 12 13 +1
Lines 1086 1147 +61
==========================================
+ Hits 981 1037 +56
- Misses 105 110 +5
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
chrisjsewell
left a comment
There was a problem hiding this comment.
Review: Add public sphinx_design.testing helpers
Overview
Promotes the test suite's doctree-XML normalization (normalize_doctree_xml) and the SphinxBuilder wrapper from tests/conftest.py into a shipped, semi-public sphinx_design.testing module, so downstream extensions can reuse them instead of re-implementing. conftest.py shrinks to thin re-exports, a docs page and CHANGELOG entry are added, and unit tests cover both docutils serialization code paths. Nicely scoped, well-documented, and clean.
What's done well
- Import hygiene is correct.
sphinx.testingis underTYPE_CHECKINGonly andpytestisn't imported, soimport sphinx_design.testingworks with only the package installed — the stated goal.py.typedis already present, and since[tool.flit.sdist]only excludestests//docs//style/, the new in-package module ships correctly. - Version-floor handling. The
status/warningsgetattr(..., None)fallback to_status/_warningcorrectly papers overSphinxTestApp.status/warningbeing properties only from sphinx 7.3; the singularwarningattr vs. pluralwarningsproperty is matched right. - Global-lookup trick is deliberate and correct.
normalize_doctree_xmlreads the module global_DOCUTILS_0_22_PLUSat call time, which is what lets the unit tests monkeypatch both branches regardless of installed docutils. Good. - Regex safety.
re.escapeonextra_attributes, the leading-space +="1"/="0"anchors, and the known-attribute allowlist keep the substitution tight; the ordered-alternationtranslatable/translatedoverlap is safe because of the="…"anchor. The textual-heuristic caveat (text content mimickingattr="1") is honestly documented. - No stale references.
DOCUTILS_0_22_PLUSand the now-unusedre/nodes/SphinxTestApp/docutils_version_infoimports are removed from conftest, and nothing else intests/referenced them;from .conftest import SphinxBuilder(test_snippets.py) still resolves via the re-export.
Minor notes (non-blocking)
- Fallback branch is only covered on the 7.2 floor CI job. The
_status/_warningprivate-stream path instatus/warningsnever executes on the main matrix (7.3+ takes the property path), and there's no unit test forSphinxBuilder. That's the most fragile part of the diff, yet it's the least exercised. Consider a tiny test with a stub object exposing only_status/_warningto lock the fallback, so a regression wouldn't hide until the pinned-floor job runs. get_doctreerelative_tocan raiseValueErrorif a node'ssourceisn't undersrc_path. This is moved verbatim (not introduced here), so no action needed — just flagging that the behavior now lives in a public API where a downstream caller with an unexpected source path would hit it.- Doc example references
file_regressionwithout noting it comes frompytest-regressions; a one-line mention would help downstream readers copy-pasting it. - Micro:
normalize_doctree_xmlrebuilds/compiles two regexes per call. Irrelevant for test-time usage — not worth changing.
Correctness / risk
No correctness issues found. The refactor is behavior-preserving for existing tests (signatures unchanged, fixture still callable as normalize_doctree_xml(text)), the new module has no import-time side effects beyond a version check, and CI (pre-commit + RTD) is green.
Verdict: Looks good to merge. 👍
Generated by Claude Code
Ships the test suite's doctree-XML normalization as a semi-public module, so downstream extensions (e.g.
sphinx-design-elements) can stop re-implementing it.Changes
sphinx_design.testingmodule:normalize_doctree_xml(text, extra_attributes=())(papers over docutils 0.22's boolean-attribute serialization change; no-op on older docutils) and theSphinxBuilderwrapper used by our own fixtures. Module docstring states the support policy: signatures covered by deprecation policy, exact output not guaranteed stable across docutils versions.sphinx_designinstalled: nopytestimport, andsphinx.testingis imported only for type checking (verified empirically in a bare venv on sphinx 7.3.7, wheresphinx.testing.utilwould otherwise requiredefusedxml).SphinxTestApp's publicstatus/warningproperties only exist from sphinx 7.3, soSphinxBuilderfalls back to the private streams on the 7.2 floor.tests/conftest.pyshrinks to thin wrappers re-exporting the public helpers; existing test signatures unchanged.Verification
import sphinx_design.testingsucceedspy.typedships in the wheel, module fully annotated, both mypy configurations cleanCloses #260