Skip to content

Add docs-tests.yml; remove test_doc_snippets.py from rust-test.yml#399

Merged
igerber merged 3 commits intomainfrom
feature/docs-tests-workflow
Apr 26, 2026
Merged

Add docs-tests.yml; remove test_doc_snippets.py from rust-test.yml#399
igerber merged 3 commits intomainfrom
feature/docs-tests-workflow

Conversation

@igerber
Copy link
Copy Markdown
Owner

@igerber igerber commented Apr 26, 2026

Summary

  • Adds .github/workflows/docs-tests.yml — a separate workflow that owns tests/test_doc_snippets.py and runs on docs/**, diff_diff/**, tests/test_doc_snippets.py, pyproject.toml, and the workflow file
  • Removes tests/test_doc_snippets.py from .github/workflows/rust-test.yml via --ignore on all three pytest invocations so doc-snippet tests stop riding the code workflow

Why

PR #389 (Batch A) shipped six broken HAD doc snippets in 2026-04 that no CI run caught because rust-test.yml only triggers on rust/**, diff_diff/**, tests/**, pyproject.toml, and its own workflow file — none of which include docs/. PR #396 patched the snippets but did not address the structural gap. This PR addresses it by mirroring the precedent already set by notebooks.yml (separate workflow scoped to docs/tutorials/** + diff_diff/** for notebook validation).

Trigger behavior

Change Triggers rust-test.yml? Triggers docs-tests.yml?
diff_diff/** only yes yes (defense-in-depth: API change can break snippets)
docs/** only no yes
tests/test_doc_snippets.py only yes (collection-only; pytest --ignore skips it) yes
pyproject.toml yes yes

Subtlety on the rust-test.yml edits

Lines 158/165 (the Rust matrix) are defensive consistency edits, not behavior changes. The matrix copies tests/ to /tmp/tests (rust-test.yml:138, 142) without docs/, so tests/test_doc_snippets.py:23 DOCS_DIR = Path(__file__).resolve().parent.parent / "docs" resolves to /tmp/docs/ which doesn't exist; the collector silently skips every RST file via the guard at tests/test_doc_snippets.py:129. Adding --ignore there prevents the no-op from becoming a real run if anyone later adds cp -r docs ... to the copy steps.

Line 193 (Pure Python Fallback) is the only edit that changes CI signal — that job runs from the repo root and was the only place the snippet test actually executed. Each invocation now carries an in-YAML comment documenting which case applies.

Methodology references

  • N/A — no methodology changes. Touches only CI workflow YAML.

Validation

  • python -c "import yaml; yaml.safe_load(open('.github/workflows/docs-tests.yml')); yaml.safe_load(open('.github/workflows/rust-test.yml'))" — both files well-formed
  • pytest tests/ --ignore=tests/test_doc_snippets.py --ignore=tests/test_rust_backend.py --collect-only shows 0 occurrences of test_doc_snippets in the collected set (was 115 cases when not ignored), confirming the new --ignore composes with the existing --ignore=tests/test_rust_backend.py
  • The workflow file is itself in the trigger paths, so the new workflow runs on this PR's own commit — first end-to-end CI validation happens here

Security / privacy

  • Confirm no secrets/PII in this PR: Yes

…est.yml

PR #389 (Batch A) shipped six broken HAD doc snippets in 2026-04 that
no CI run caught because rust-test.yml only triggers on
rust/, diff_diff/, tests/, pyproject.toml, and the workflow file —
none of which include docs/. PR #396 patched the snippets but did not
address the structural gap. This PR addresses it.

Two changes:

1. New .github/workflows/docs-tests.yml — separate workflow that
   runs `pytest tests/test_doc_snippets.py -v` on a single
   ubuntu-latest / py3.14 / pure-Python runner. Triggers on
   docs/, diff_diff/, tests/test_doc_snippets.py, pyproject.toml,
   and the workflow file itself; same ready-for-ci label gate as
   rust-test.yml / notebooks.yml. Mirrors notebooks.yml's shape
   (the existing precedent for `pytest`-validated docs assets) so
   the two doc-validation workflows look like siblings.

2. .github/workflows/rust-test.yml: add
   --ignore=tests/test_doc_snippets.py to all three pytest
   invocations so doc snippets stop riding the code workflow.

   The Pure Python Fallback edit (line 193) is the only one that
   changes CI signal: that job runs from the repo root and was the
   ONLY place where test_doc_snippets.py actually executed. The two
   Rust-matrix edits (lines 158, 165) are defensive consistency: the
   matrix copies tests/ to /tmp/tests (rust-test.yml:138, 142)
   without docs/, so DOCS_DIR resolves to /tmp/docs/ which doesn't
   exist; the test collector silently skips every RST file via the
   guard at tests/test_doc_snippets.py:129. Adding --ignore there
   prevents the no-op from becoming a real run if anyone later adds
   `cp -r docs ...` to the copy steps. Each invocation now carries
   an in-YAML comment documenting which case it's the defensive vs
   behavior-changing edit.

Verification:
- python -c "import yaml; yaml.safe_load(open('.github/workflows/
  docs-tests.yml')); yaml.safe_load(open('.github/workflows/
  rust-test.yml'))" — both files well-formed.
- pytest tests/ --ignore=tests/test_doc_snippets.py
  --ignore=tests/test_rust_backend.py --collect-only — 0 occurrences
  of test_doc_snippets in the collected set (was 115 cases collected
  when not ignored), confirming pytest accepts repeated --ignore
  flags as the existing line-193 pattern with --ignore=tests/
  test_rust_backend.py already showed.

After this PR opens, the workflow file itself triggers docs-tests.yml
on its own change, providing the first end-to-end CI validation.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@github-actions
Copy link
Copy Markdown

Overall Assessment

✅ Looks good — no unmitigated P0/P1 findings.

Executive Summary

  • No methodology surface changed. The diff is limited to CI workflow ownership and invocation paths, so the Methodology Registry is not implicated for estimator correctness.
  • docs-tests.yml closes the main structural gap by running tests/test_doc_snippets.py from repo root on docs/**, diff_diff/**, tests/test_doc_snippets.py, and pyproject.toml changes (.github/workflows/docs-tests.yml:L3-L56).
  • Removing test_doc_snippets.py from the Rust matrix does not drop meaningful Rust-path signal: those jobs run from copied tests/ trees without docs/, and the snippet collector already skips missing RST targets (tests/test_doc_snippets.py:L23-L23, tests/test_doc_snippets.py:L127-L130).
  • P3: rust-test.yml still path-matches tests/**, so edits to tests/test_doc_snippets.py will continue to fan out into the full Rust/pure-Python matrix even though that file is now ignored everywhere it used to run.
  • P3: the new workflow copies the repo’s known broad labeled/unlabeled trigger pattern, so unrelated label churn can still re-run docs CI.

Methodology

  • No findings. The PR changes only .github/workflows/docs-tests.yml:L1-L56 and .github/workflows/rust-test.yml:L155-L201; no estimator, weighting, variance/SE, identification, assumption checks, or defaults are modified.

Code Quality

  • No findings.

Performance

  • Severity: P3. Impact: rust-test.yml still subscribes to tests/** (.github/workflows/rust-test.yml:L3-L20), while all relevant pytest invocations now ignore tests/test_doc_snippets.py (.github/workflows/rust-test.yml:L155-L201). A harness-only edit will still trigger the full Rust/pure-Python matrix with no added signal. Concrete fix: exclude tests/test_doc_snippets.py from the Rust workflow trigger surface, or relocate that harness outside tests/ so ownership and trigger scope match.

Maintainability

  • No findings.

Tech Debt

  • Severity: P3. Impact: docs-tests.yml repeats the broad labeled/unlabeled trigger pattern (.github/workflows/docs-tests.yml:L12-L20) that TODO already tracks as avoidable CI churn for other workflows (TODO.md:L123-L123). Once ready-for-ci is present, unrelated label edits can re-run docs CI. Concrete fix: restrict label-driven runs to ready-for-ci add/remove transitions only, or drop labeled/unlabeled from this workflow.

Security

  • No findings. Both workflows remain contents: read only (.github/workflows/docs-tests.yml:L26-L27, .github/workflows/rust-test.yml:L22-L23).

Documentation/Tests

  • No findings. The new workflow is a net improvement in CI signal for docs changes because it executes tests/test_doc_snippets.py from repo root (.github/workflows/docs-tests.yml:L52-L56), and the removal from the /tmp Rust matrix does not eliminate real snippet execution for those jobs given the collector’s missing-file behavior (tests/test_doc_snippets.py:L23-L23, tests/test_doc_snippets.py:L127-L130).

P3 (Performance): rust-test.yml's `tests/**` trigger still fans out
into the full Rust matrix on harness-only edits to
tests/test_doc_snippets.py, even though every pytest invocation in
that workflow now ignores the file. Added the GitHub Actions
negative-pattern `!tests/test_doc_snippets.py` to both the push and
pull_request paths blocks so a snippet-test-only edit no longer
spins up the matrix. The file is owned exclusively by docs-tests.yml
which still triggers (since its own `paths:` lists the file
explicitly).

P3 (Tech Debt): docs-tests.yml repeats the broad `labeled` /
`unlabeled` trigger pattern that TODO.md:123 already tracks as
avoidable CI churn for rust-test.yml + notebooks.yml under PR #269.
Rather than diverge by pre-fixing the new workflow alone, extended
the tracking row to include `docs-tests.yml` so the planned
cross-cutting fix covers all three uniformly.

Verification: python -c "import yaml; yaml.safe_load(open(
'.github/workflows/rust-test.yml'))" succeeds; em-dash sweep clean.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@igerber
Copy link
Copy Markdown
Owner Author

igerber commented Apr 26, 2026

/ai-review

@github-actions
Copy link
Copy Markdown

🔁 AI review rerun (requested by @igerber)

Head SHA: 131f6e4bb3bde586e851ce2cb4a2d5c24b110301


Overall Assessment

✅ Looks good — no unmitigated P0/P1 findings.

Executive Summary

  • No methodology surface changed; the diff is confined to CI workflow ownership and TODO tracking, so no estimator in docs/methodology/REGISTRY.md:L1-L43 is implicated.
  • Re-review: the earlier non-blocking Rust-matrix fan-out issue is addressed. rust-test.yml now excludes tests/test_doc_snippets.py from both trigger paths and pytest execution (.github/workflows/rust-test.yml:L6-L13, .github/workflows/rust-test.yml:L159-L205).
  • The new docs workflow runs tests/test_doc_snippets.py from repo root, which is the context where DOCS_DIR resolves to the real docs/ tree (.github/workflows/docs-tests.yml:L52-L56, tests/test_doc_snippets.py:L23-L23, tests/test_doc_snippets.py:L124-L130).
  • The remaining label-churn rerun issue is now explicitly tracked in TODO.md, so it is mitigated under repo policy (.github/workflows/docs-tests.yml:L12-L20, TODO.md:L117-L124).
  • P2: docs-tests.yml does not trigger on tests/conftest.py, even though that file is auto-loaded for the snippet run and currently mutates import path/environment (.github/workflows/docs-tests.yml:L12-L20, tests/conftest.py:L1-L18).

Methodology

  • Severity: None. Impact: Method(s) affected: none. No estimator, weighting, variance/SE, identification, assumption-check, or default-behavior code changed; the registry review confirms this PR is CI-only (docs/methodology/REGISTRY.md:L1-L43). Concrete fix: None.

Code Quality

  • Severity: None. Impact: No code-quality defect identified in the YAML/TODO edits. Concrete fix: None.

Performance

  • Severity: None. Impact: No unmitigated CI-performance regression remains in the changed files; the prior harness-only Rust-matrix rerun issue appears resolved by the new path exclusion plus pytest --ignore changes (.github/workflows/rust-test.yml:L6-L13, .github/workflows/rust-test.yml:L159-L205). Concrete fix: None.

Maintainability

  • Severity: None. Impact: No maintainability defect beyond the trigger-scope gap noted under Documentation/Tests. Concrete fix: None.

Tech Debt

  • Severity: P3-informational (tracked in TODO.md). Impact: docs-tests.yml still uses broad labeled/unlabeled PR triggers, so unrelated label churn can re-run docs CI when ready-for-ci is already present, but this PR explicitly updates the existing deferred-work entry to include the new workflow, which mitigates it under policy (.github/workflows/docs-tests.yml:L12-L20, TODO.md:L117-L124). Concrete fix: Optional follow-up to restrict label-driven runs to ready-for-ci add/remove transitions.

Security

  • Severity: None. Impact: No new secret handling or privilege expansion; both workflows remain contents: read only (.github/workflows/docs-tests.yml:L26-L27, .github/workflows/rust-test.yml:L26-L27). Concrete fix: None.

Documentation/Tests

  • Severity: P2. Impact: docs-tests.yml only path-matches tests/test_doc_snippets.py, but pytest auto-loads tests/conftest.py for that run, and tests/conftest.py:L13-L18 currently mutates helper import path and environment. A tests/conftest.py-only change can therefore break doc-snippet execution without triggering the new workflow, leaving a residual CI blind spot (.github/workflows/docs-tests.yml:L12-L20). Concrete fix: add tests/conftest.py to the push and pull_request paths list in .github/workflows/docs-tests.yml; if more shared pytest config is added later, include those files as well.

P2: docs-tests.yml only path-matched tests/test_doc_snippets.py, but
pytest auto-loads tests/conftest.py for any test under tests/ — and
that file mutates sys.path (line 14: prepends tests/helpers/) and
sets MPLBACKEND=Agg (line 18) before any test imports it. A change
to tests/conftest.py that breaks either of those load-time steps
would silently break the snippet test on every doc PR until a
docs-tests.yml-triggering file happened to change alongside it,
leaving a real CI blind spot.

Fix: added 'tests/conftest.py' to both the push and pull_request
`paths:` blocks in docs-tests.yml. An inline comment on the push
block explains the dependency; the pull_request block doesn't repeat
the prose to avoid drift between the two copies.

Verified the workflow YAML is well-formed via yaml.safe_load.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@igerber
Copy link
Copy Markdown
Owner Author

igerber commented Apr 26, 2026

/ai-review

@github-actions
Copy link
Copy Markdown

🔁 AI review rerun (requested by @igerber)

Head SHA: d9ad1e0efd9697d59044526999d3b46556f7f464


Overall Assessment

✅ Looks good — no unmitigated P0/P1 findings.

Executive Summary

  • Re-review: the prior docs-trigger blind spot is fixed. docs-tests.yml now path-matches tests/conftest.py, which pytest auto-loads for the snippet run and which mutates sys.path/MPLBACKEND. .github/workflows/docs-tests.yml:L9-L16, .github/workflows/docs-tests.yml:L23-L26, tests/conftest.py:L13-L18
  • No methodology surface changed. The diff is limited to CI workflow ownership and TODO tracking, so no estimator/method in the Methodology Registry is implicated. docs/methodology/REGISTRY.md:L1-L32, .github/workflows/docs-tests.yml:L1-L62, .github/workflows/rust-test.yml:L1-L205, TODO.md:L117-L123
  • The new docs workflow runs tests/test_doc_snippets.py from repo root in pure-Python mode, which is the execution context where DOCS_DIR resolves to the real docs/ tree and snippet cases are actually collected. .github/workflows/docs-tests.yml:L58-L62, tests/test_doc_snippets.py:L23-L23, tests/test_doc_snippets.py:L124-L139
  • rust-test.yml now excludes tests/test_doc_snippets.py both at workflow-trigger and pytest-invocation levels, removing the previous duplicate/no-op coverage from the Rust matrix and leaving one clear owner for this surface. .github/workflows/rust-test.yml:L7-L14, .github/workflows/rust-test.yml:L19-L24, .github/workflows/rust-test.yml:L159-L173, .github/workflows/rust-test.yml:L200-L205
  • The remaining label-churn rerun behavior is explicitly tracked in TODO.md, so it is mitigated under repo policy as P3-informational. .github/workflows/docs-tests.yml:L17-L18, .github/workflows/docs-tests.yml:L38-L40, TODO.md:L123-L123

Methodology

  • Severity: None. Impact: Method(s) affected: none. No estimator, weighting, variance/SE, identification, assumption-check, or default-behavior code changed; no Registry note/deviation review was required beyond confirming the diff is CI-only. Concrete fix: None. docs/methodology/REGISTRY.md:L1-L32

Code Quality

  • Severity: None. Impact: The changed workflow wiring is internally consistent with the actual snippet harness behavior; the prior tests/conftest.py omission has been corrected. Concrete fix: None. .github/workflows/docs-tests.yml:L9-L16, .github/workflows/docs-tests.yml:L23-L26, tests/conftest.py:L13-L18

Performance

  • Severity: None. Impact: The PR reduces redundant CI work by preventing test_doc_snippets.py from riding the Rust matrix and pure-Python fallback suite, while preserving a real repo-root execution in the dedicated docs workflow. Concrete fix: None. .github/workflows/rust-test.yml:L7-L14, .github/workflows/rust-test.yml:L159-L173, .github/workflows/rust-test.yml:L200-L205, .github/workflows/docs-tests.yml:L58-L62

Maintainability

  • Severity: None. Impact: Workflow ownership is clearer: doc-snippet validation now has a single dedicated job instead of being partially present in multiple places, including a /tmp/tests context that lacked the docs/ tree. Concrete fix: None. .github/workflows/rust-test.yml:L140-L166, .github/workflows/docs-tests.yml:L35-L62, tests/test_doc_snippets.py:L23-L23

Tech Debt

  • Severity: P3-informational (tracked in TODO.md). Impact: Unrelated labeled/unlabeled events can still rerun CI when ready-for-ci is already present, but this PR updates the existing deferred-work entry to include docs-tests.yml, so it is mitigated under project policy. Concrete fix: Optional follow-up to filter label-triggered runs to ready-for-ci add/remove transitions only. .github/workflows/docs-tests.yml:L17-L18, .github/workflows/docs-tests.yml:L38-L40, .github/workflows/rust-test.yml:L15-L17, TODO.md:L123-L123

Security

  • Severity: None. Impact: No new secret handling, token scope, or privilege expansion; the new workflow stays read-only. Concrete fix: None. .github/workflows/docs-tests.yml:L32-L33, .github/workflows/rust-test.yml:L26-L27

Documentation/Tests

  • Severity: None. Impact: Re-review: the previous documentation-test trigger gap is resolved. The new workflow now covers the actual snippet execution surface, including tests/conftest.py, and runs in the only context where DOCS_DIR points at the real docs tree. Concrete fix: None. .github/workflows/docs-tests.yml:L7-L16, .github/workflows/docs-tests.yml:L21-L26, .github/workflows/docs-tests.yml:L54-L62, tests/test_doc_snippets.py:L23-L23, tests/test_doc_snippets.py:L124-L139, tests/test_doc_snippets.py:L377-L413

@igerber igerber added the ready-for-ci Triggers CI test workflows label Apr 26, 2026
@igerber igerber merged commit 9dcf09f into main Apr 26, 2026
26 of 27 checks passed
@igerber igerber deleted the feature/docs-tests-workflow branch April 26, 2026 22:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-for-ci Triggers CI test workflows

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant