🔧 Add sphinx-needs 8.0.0 compatibility#133
Merged
Merged
Conversation
Replace deprecated add_extra_option() with add_field() for sphinx-needs >= 8.0.0, using a version-gated shim that falls back to add_extra_option() for older versions. Also adds sphinx-needs 8.0.0 to the CI and nox test matrices, fixes the warning check in tests to also inspect stderr (where Sphinx writes warnings), adds a test asserting the test-file need node renders correctly, and removes the broken taplo pre-commit hook (taplo 0.9.3 PyPI package fails to build on this platform).
Add schema to string fields in _register_field (moved into use_schema branch for backwards compat with sphinx-needs < 6.0.0). Update all test fixture conf.py files to use needs_fields for sphinx-needs >= 8.0.0 instead of the deprecated needs_extra_options.
Move sphinx_needs and Version imports to top of file (fixes E402) and drop incorrect _V alias for Version (fixes N814).
🔧 Add sphinx-needs 8.0.0 compatibility
patdhlk
requested changes
Apr 1, 2026
patdhlk
left a comment
patdhlk
left a comment
Collaborator
There was a problem hiding this comment.
Thanks for the PR, some small nitpicks.
One general thing I noted: same version-gating pattern copy-pasted across 7 test conf.py files. A shared helper would reduce future maintenance. But this can be picked up in the future
# Changes ## Core Compatibility Field descriptions: Added proper semantic descriptions for sphinx-needs API fields instead of repeating field names Packaging dependency: Added explicit packaging>=20.0 dependency to avoid fragile transitive dependencies API compatibility: Updated field registration to work with both older and newer sphinx-needs versions ## Test Configuration Updated all test conf.py files to use proper version-specific configuration (needs_fields vs needs_extra_options) Added conditional logic for sphinx-needs version compatibility ## Test Improvements Enhanced count field verification: Test now properly verifies that integer count fields are rendered in need nodes Fixed warning check logic: Corrected test to properly check stderr for warnings (Sphinx writes warnings to stderr, not stdout)
Collaborator
|
@cpolzer only one ci issue then we are ready to go |
cpolzer
commented
May 7, 2026
patdhlk
self-requested a review
May 8, 2026 09:59
patdhlk
approved these changes
May 8, 2026
This was referenced Jun 16, 2026
Merged
ubmarco
added a commit
that referenced
this pull request
Jun 16, 2026
#137) ## What TDD regression test (no production code change) for the interaction between **sphinx-test-reports (STR)** and **sphinx-needs (SN)** schema validation. STR registers many extra fields with SN via the field API — `file`, `suite`, `case`, `case_name`, `case_parameter`, `classname`, `time`, `suites`, `cases`, `passed`, `skipped`, `failed`, `errors`, `result`. These are attached to **every** need in a project. This test builds a project with both extensions enabled and a strict schema (`schemas.json` with `unevaluatedProperties: false`, i.e. *no field beyond the core id/title/type is allowed*), defines plain `req` needs that leave all STR fields unpopulated, and asserts that **no schema violation** is reported (SN's `schema_violations.json` has empty `validation_warnings`). ## Why A strict need-type schema can wrongly "fail" for needs that don't populate STR's registered fields. If STR's fields are attached with non-null defaults (e.g. `""`/`0`), every plain need carries empty STR values and a strict schema reports `Unevaluated properties are not allowed ('file', 'suite', ... were unexpected)`. ## The bug, and why this is a regression test A sphinx-needs field registered **untyped** (legacy `add_extra_option` / `needs_extra_options`, no schema) defaults to `""`, which is **not** stripped before schema validation and therefore trips `unevaluatedProperties: false`. A field registered **typed** (`add_field(..., schema=...)`) defaults to `None` and is stripped. Released STR **1.3.2** registers `file`, `suite`, `case`, `case_name`, `case_parameter` and `classname` *untyped*, so a project using strict schema validation gets false-positive `Unevaluated properties are not allowed (...)` warnings for plain needs that never set those fields. STR `master` registers every field with a typed schema (#133) → they default to `None` → stripped → no warning. **This test locks that behavior in** and goes red if any field regresses to a non-null default. The fix ships in 1.4.0 (1.3.2 is still affected). ## Verification - **passes** on SN 6.0.0 / 6.3.0 / 8.0.0 / 8.1.1; **skips** on SN < 6.0.0 (predates `needs_schema_definitions`). - Confirmed meaningful: temporarily making STR register its string fields with a `""` default made the test go **red** with exactly the expected `unevaluatedProperties` violation for both needs, then reverted. - Heads-up for users: the literal JSON-Schema keyword **`additionalProperties: false` is *not* accepted** by SN's schema config (it is rejected at config-validation time). SN's supported way to forbid extra fields is **`unevaluatedProperties: false`**, which is what this test uses. - Full STR test suite green locally. ## Second, independent test `test_strict_schema_allows_constrained_field_in_local_schema` (fixture `schema_strictness_status/`): the core `status` field is constrained in the local schema (`const: "open"`, `required`) and set on the need, proving strict validation also works when a field *is* evaluated by the schema. Verified red when `status` diverges from the constrained value.
ubmarco
added a commit
that referenced
this pull request
Jun 16, 2026
## Release v1.4.0 Bumps the version (`pyproject.toml`, `docs/conf.py`, `test_reports.py`) and adds the 1.4.0 changelog for everything merged since 1.3.2. **Highlights** - Feature (#135): map JUnit XML `<properties>` to Sphinx-Needs fields/links. - Sphinx-Needs 8 support (#133): fields registered via the new `add_field` API. - Bugfix (#133): `file`, `suite`, `case`, `case_name`, `case_parameter` and `classname` are now registered with a typed schema → default to `None` → stripped before schema validation, fixing false-positive `Unevaluated properties are not allowed` warnings under `unevaluatedProperties: false`. Released 1.3.2 registered them untyped (default `""`), which leaked. - Bugfix (#134): JUnit `<error>` test cases were misclassified as `passed`; now handled correctly in `parse_testcase()`. - Testing (#130, #137): run against Sphinx-Needs 6.3.0; regression test that a strict schema ignores unpopulated Sphinx-Test-Reports fields. - Docs (#136, #139): clarify Sphinx-Needs type names vs. the hyphenated directives; note that numeric `cases` filtering requires Sphinx-Needs >= 6.
ubmarco
added a commit
that referenced
this pull request
Jun 19, 2026
The landing page showed two near-identical Sphinx-Needs cards (same icon). One linked to sphinx-needs.com, whose TLS handshake intermittently fails from CI runners (some edge nodes return a TLS internal_error alert), which made the Docs-Linkcheck job flaky; #133 had repointed it to readthedocs, leaving it a duplicate of the second card. Merge them into a single Sphinx-Needs card with two buttons: - Website @ useblocks.com -> https://useblocks.com/open-source/sphinx-needs (the working HTTPS redirect target of sphinx-needs.com) - Technical Docs -> https://sphinx-needs.readthedocs.io/en/latest/ This drops the duplicate card and keeps linkcheck pointed at hosts with working TLS.
ubmarco
added a commit
that referenced
this pull request
Jun 19, 2026
…140) ## Problem `Docs-Linkcheck` has been flaky — it failed on ~28% of recent runs (22 of the last 80), in outage-shaped clusters unrelated to the PR under test. Root cause: the landing page's ecosystem section linked to `https://sphinx-needs.com`, which has a partial **server-side TLS fault** — some edge nodes return a TLS `internal_error` alert during the handshake, so it resolves from ~70% of GitHub runners and fails from the rest. (Reproducible with both Python `ssl` and `openssl`, while `github.com` / `readthedocs.io` / `useblocks.com` all handshake fine.) Because linkcheck is a required gate, a single bad route blocked unrelated PRs. #133 swapped that link to `sphinx-needs.readthedocs.io`, which stopped the flakiness but left the section cluttered: two near-identical Sphinx-Needs cards, a redundant Sphinx-Test-Reports card (linking to these very docs), and full-bleed images of wildly different sizes (the small 154×225 Sphinx-Test-Reports logo upscaled to a blurry full-card height). ## Change Rework the landing-page ecosystem section in `docs/index.rst`: - **Merge** the two Sphinx-Needs cards into one, with two buttons: - **Website @ useblocks.com** → `https://useblocks.com/open-source/sphinx-needs` — the working HTTPS redirect target of `sphinx-needs.com` - **Technical Docs** → `https://sphinx-needs.readthedocs.io/en/latest/` - **Drop** the redundant Sphinx-Test-Reports card (it pointed at the documentation it was shown in). - **Collapse** the "Sphinx-Needs Ecosystem" and "Other Sphinx extensions" sections into a single "Other Sphinx extensions" section listing the three related projects (Sphinx-Needs, Sphinx Collections, Sphinx Bazel) in one 3-up row. - **Tidy the logos**: replace the full-bleed `:img-top:` images with centered, height-capped (120px) logos in the card body — crisp, equally sized, compact. Plain `.. image::`, no theme-specific CSS, so it survives a theme change. ## Verification `https://useblocks.com/open-source/sphinx-needs` → **HTTP 200, 0 redirects, TLS 1.3** (the exact request linkcheck makes). No `sphinx-needs.com` references remain in the repo. --- Out of scope for this PR: the underlying defect is the broken TLS edge node on the useblocks-owned `sphinx-needs.com` (it also affects real visitors), and linkcheck still gates merges on third-party availability — making the job non-blocking or scheduled would be the durable fix.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
add_extra_option()withadd_field()for sphinx-needs >= 8.0.0, using a version-gated shim that falls back toadd_extra_option()for older versions8.0.0to the CI matrix and nox test matrixstderr(where Sphinx actually writes warnings, notstdout)test_test_file_renders_need_with_countsto catch regressions where int kwargs toadd_need()are rejectedTest plan
nox -s "tests-3.12(sphinx='8.1.3', sphinx_needs='8.0.0')"and verify all tests pass6.3.0) to confirm backwards compatibilityReason:
Upgrading another sphinx-needs project showed errors:
"sphinx-test-reports passes integer field values (e.g. suites=1) which sphinx-needs 8.0 rejects with InvalidNeedException. "