Skip to content
This repository was archived by the owner on Sep 30, 2026. It is now read-only.

feat(pytest): plugin writing the XML shape this extension reads - #151

Merged
ubmarco merged 12 commits into
masterfrom
str-pytest-plugin
Sep 10, 2026
Merged

ubmarco merged 12 commits into
masterfrom
str-pytest-plugin

Conversation

@ubmarco

@ubmarco ubmarco commented Sep 7, 2026 •

Copy link
Copy Markdown
Member

Based on master. Independent of the needs.json converter (#148, merged); rebased on top of it.

Why

A stock pytest run writes a JUnit XML this extension can only partly use. Under pytest's default junit_family = xunit2, no <testcase> carries the file/line attributes that give a test case its source location (tr_source_file_option/tr_source_line_option, and the deterministic ID of tr_deterministic_case_ids). Under xunit1 pytest writes them, but counts the line from 0, keeps Bazel's runfiles prefix, and has no way to point a case at the file that drove it. And nothing records the requirements a test verifies.

What

-p sphinxcontrib.test_reports.pytest_plugin, with junit_family = xunit1 (warned about otherwise), does two things to every <testcase>, through hooks on the test reports rather than fixtures, so a case skipped or erroring during setup gets the same treatment and pytest-xdist works:

  • the source location becomes the one an editor shows: line counted from 1, path relative to the rootdir with Bazel's _main/ runfiles prefix cut at a whole path component;
  • the properties of an add_test_properties(...) decorator (or, for file-driven parameterised tests, the apply_test_metadata(...) runtime helper, which can also point the case at the driving file) are written as <properties>.

Which properties exist is configuration, not code. The test_reports_properties ini option declares them, one keyword [= XmlName] [, list] per line: the keyword a test writes, the <property> name it is written under, and whether it takes a list (joined with ", ", which tr_property_link_types splits again). A keyword not declared is written under its own name with a single value; a list under it, a list item that is not a string, or bytes, is a TypeError naming the option, so nothing is lost to a Python repr silently. The plugin ships no metamodel of its own.

S-CORE's model is the worked example in the docs:

[pytest]
addopts = -p sphinxcontrib.test_reports.pytest_plugin
junit_family = xunit1
test_reports_properties =
    partially_verifies = PartiallyVerifies, list
    fully_verifies = FullyVerifies, list
    test_type = TestType
    derivation_technique = DerivationTechnique

Ported from S-CORE docs-as-code's score_pytest attribute plugin: same two entry-point names, and with that example the XML comes out the same, so tests written against that plugin keep working when they import from here. Four deliberate differences in the XML: a case skipped or erroring at setup carries its location and properties here and pytest's stock values there; the properties keep the order of the keywords; the Bazel prefix is cut at a whole _main component only; and a file handed to apply_test_metadata is cut the same way. Two of that plugin's rules are not ported, as process rules of that project rather than properties of the data: the TestType/DerivationTechnique vocabularies are documented, not enforced, and a decorated test needs no docstring.

The package __init__ resolves setup lazily (PEP 562, #145), so loading the plugin into a pytest process imports neither Sphinx nor docutils; a test asserts it. A per-module mypy override lifts only the two Any checks the pytest API needs. A pytest extra (pytest>=7.0, where everything the plugin uses exists) pulls pytest in; it is not a dependency of the extension. pytest before 7.3.2 does not run on Python 3.12 by itself, so a plugin_floor nox session, wired into CI, runs the plugin's tests on the oldest pytest of each Python: 7.0.1 on 3.11, 7.3.2 on 3.12.

Tests

tests/test_pytest_plugin.py, 63 tests through pytester, each inner pytest a fresh subprocess: locations (a decorated function is located at its first decorator, as pytest does; skipped and setup-erroring cases included), the properties written from ini and pyproject declarations, the option grammar and its errors, the value rules, marker merging across class and function, the runtime helper's location override, nested in-process sessions (one failing to configure), a decorator applied before pytest_configure, a bad shape next to a fixture error, --strict-markers, the xunit2 start-up notice under strict warning policies, -p no:junitxml, legacy, an xdist run, the Bazel path cut, and that importing the plugin imports no Sphinx. Verified on pytest 7.0.1 and 7.4.4 (Python 3.11), 7.3.2, 7.4.4 and 8.4.2 (3.12) and 9.1.1 (3.14).

@codecov-commenter

codecov-commenter commented Sep 7, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 83.26446% with 81 lines in your changes missing coverage. Please review.
✅ Project coverage is 92.13%. Comparing base (9716a2e) to head (417cd34).

Files with missing lines Patch % Lines
sphinxcontrib/test_reports/pytest_plugin.py 56.91% 76 Missing and 5 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #151      +/-   ##
==========================================
- Coverage   93.23%   92.13%   -1.11%     
==========================================
  Files          43       45       +2     
  Lines        3712     4195     +483     
  Branches      341      384      +43     
==========================================
+ Hits         3461     3865     +404     
- Misses        158      233      +75     
- Partials       93       97       +4     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

ubmarco added a commit that referenced this pull request Sep 7, 2026
Addresses the second review of #145.

* Discovery: `find_project_config` stopped at any directory holding a
  `pyproject.toml`, the start directory included, so `docs/pyproject.toml`
  beside `conf.py` and every uv-workspace member layout
  (`packages/<dist>/pyproject.toml` with the docs below it, one
  `ubproject.toml` at the monorepo root) returned `None` without a word. A
  `pyproject.toml` marks a distribution, not the project, so it is no longer
  a marker: the search is bounded by the repository root (`.git`) and by
  nothing else. A fruitless search reports where it ended -- the bridge logs
  it at verbose level (`sphinx-build -v`), a converter passes its own logger
  -- and a relative start directory is made absolute first so it has parents
  to walk. Tests cover both layouts, the nested-repository boundary and the
  report, at the loader and in a real build.
* `tr_rootdir` declares `types=(str, os.PathLike)`. Its default is Sphinx's
  `confdir`, a `_StrPath`, so a plain string -- the only thing TOML or a
  string literal in `conf.py` can supply -- failed `check_confval_types` and
  took a `-W` build down on the documented example. The example is now built
  by a test that reads it from `configuration.rst` verbatim.
* Values inside the `file`/`suite`/`case` spellings are covered by tests: a
  non-string table value (`color = 999999`), a non-string list element and a
  short list each raise. Dropping either check left the suite green before.
* The package `__init__` resolves `setup` lazily (PEP 562), so importing
  `projectconfig` -- which runs the package `__init__` first -- no longer
  imports Sphinx, and the module's premise holds for a consumer without the
  documentation toolchain. An `ImportError` from the lazy import is re-raised
  as Sphinx's `ExtensionError` when Sphinx is importable: Sphinx fetches
  `setup` with `getattr()`, which only tolerates `AttributeError`, so a
  missing sphinx-needs would otherwise escape as a raw traceback. Taken from
  #151, which carried the same change and drops it on its next rebase.
  `setup` gains a return annotation so the lazily returned callable carries
  no `Any` under strict mypy.
* Text: the docs no longer claim that sub-tables of other tools are left
  alone -- only `[test_reports.convert]` is, any other sub-table is an unknown
  key like any other; both warning types (`test_reports.unknown_key`,
  `test_reports.missing_config`) are listed; the comment on
  `tr_config_from_toml` says an explicit file warns rather than "must exist";
  and the symlink claim of `_anchor_paths` is gone -- Sphinx resolves its
  `confdir` before the bridge runs, so the loader merely keeps the form of
  the path it is handed.
@ubmarco
ubmarco force-pushed the str-pytest-plugin branch 2 times, most recently from 76ff965 to f917bd0 Compare September 7, 2026 09:09
ubmarco added a commit that referenced this pull request Sep 7, 2026
Addresses the second review of #145.

* Discovery: `find_project_config` stopped at any directory holding a
  `pyproject.toml`, the start directory included, so `docs/pyproject.toml`
  beside `conf.py` and every uv-workspace member layout
  (`packages/<dist>/pyproject.toml` with the docs below it, one
  `ubproject.toml` at the monorepo root) returned `None` without a word. A
  `pyproject.toml` marks a distribution, not the project, so it is no longer
  a marker: the search is bounded by the repository root (`.git`) and by
  nothing else. A fruitless search reports where it ended -- the bridge logs
  it at verbose level (`sphinx-build -v`), a converter passes its own logger
  -- and a relative start directory is made absolute first so it has parents
  to walk. Tests cover both layouts, the nested-repository boundary and the
  report, at the loader and in a real build.
* `tr_rootdir` declares `types=(str, os.PathLike)`. Its default is Sphinx's
  `confdir`, a `_StrPath`, so a plain string -- the only thing TOML or a
  string literal in `conf.py` can supply -- failed `check_confval_types` and
  took a `-W` build down on the documented example. The example is now built
  by a test that reads it from `configuration.rst` verbatim.
* Values inside the `file`/`suite`/`case` spellings are covered by tests: a
  non-string table value (`color = 999999`), a non-string list element and a
  short list each raise. Dropping either check left the suite green before.
* The package `__init__` resolves `setup` lazily (PEP 562), so importing
  `projectconfig` -- which runs the package `__init__` first -- no longer
  imports Sphinx, and the module's premise holds for a consumer without the
  documentation toolchain. An `ImportError` from the lazy import is re-raised
  as Sphinx's `ExtensionError` when Sphinx is importable: Sphinx fetches
  `setup` with `getattr()`, which only tolerates `AttributeError`, so a
  missing sphinx-needs would otherwise escape as a raw traceback. Taken from
  #151, which carried the same change and drops it on its next rebase.
  `setup` gains a return annotation so the lazily returned callable carries
  no `Any` under strict mypy.
* Text: the docs no longer claim that sub-tables of other tools are left
  alone -- only `[test_reports.convert]` is, any other sub-table is an unknown
  key like any other; both warning types (`test_reports.unknown_key`,
  `test_reports.missing_config`) are listed; the comment on
  `tr_config_from_toml` says an explicit file warns rather than "must exist";
  and the symlink claim of `_anchor_paths` is gone -- Sphinx resolves its
  `confdir` before the bridge runs, so the loader merely keeps the form of
  the path it is handed.
Base automatically changed from str-ubproject-toml to master September 7, 2026 18:33
@ubmarco
ubmarco force-pushed the str-pytest-plugin branch 2 times, most recently from ae75128 to aaca208 Compare September 8, 2026 20:21
@ubmarco

ubmarco commented Sep 8, 2026

Copy link
Copy Markdown
Member Author

Nice shape overall — the lazy-setup isolation is properly asserted, the pytester coverage is real, and the "why xunit1" reasoning is written down where a user will hit it. The problem is concentrated in the one path the plugin exists for: writing properties. In several shapes a user will plausibly write, requirement links are silently corrupted or dropped — no error at test time, and on the build side they surface only as "link target not found" for IDs nobody wrote. I'd hold the merge for the High items.

Correctness ran through the review workflow engine at high (34 candidates → 22 verifiers → 2 refuted → 10 reported); every finding below is CONFIRMED, and I reproduced 1, 2, 3, 5, 7, 10 and 11 by hand. Architecture, compat and docs are my own. No prior review comments to check — CI is green across the whole matrix.

High

1. [High] The decorator vocabulary is hard-coded to S-CORE, and the generic escape hatch corrupts lists (architecture)

Only partially_verifies/fully_verifies are list-aware. Everything else falls through **properties and is written with str(), so a project whose metamodel uses different link fields — the explicit "metamodels with fields this plugin does not know" case — gets a Python repr in the XML:

properties_mapping(Satisfies=["REQ_1", "REQ_2"])   # -> value="['REQ_1', 'REQ_2']"
# tr_property_link_types splits on "," -> ["['REQ_1'", "'REQ_2']"]  = garbage link IDs, silently

mapping = {
PARTIALLY_VERIFIES: ", ".join(partially_verifies or ()),
FULLY_VERIFIES: ", ".join(fully_verifies or ()),
TEST_TYPE: test_type or "",
DERIVATION_TECHNIQUE: derivation_technique or "",
**properties,
}
cleaned = {name: value for name, value in mapping.items() if value}

Three further asymmetries push non-S-CORE projects onto a second-class path: the four blessed keywords are renamed snake_case→PascalCase while custom ones pass through verbatim (partially_verifies → PartiallyVerifies, but satisfies → satisfies), the ValueError message enumerates only the S-CORE names, and TEST_TYPES/DERIVATION_TECHNIQUES are S-CORE's vocabularies.

To my mind this is the thing to settle before shipping: S-CORE's names are a fine default, but they shouldn't be the only ergonomic path. The smallest fix that makes it generic is to join any sequence value uniformly, whichever keyword it arrived under — then the four named parameters become pure sugar over a mechanism that works for every metamodel, and finding 2 below disappears with it. Worth also deciding whether the snake_case→PascalCase alias map should be extensible (a module-level dict) rather than four constants.

2. [High] A bare string requirement ID is exploded character by character (correctness)

str satisfies Sequence[str], so this type-checks and is silently wrong:

properties_mapping(partially_verifies="REQ_1")   # -> {'PartiallyVerifies': 'R, E, Q, _, 1'}

PARTIALLY_VERIFIES: ", ".join(partially_verifies or ()),
FULLY_VERIFIES: ", ".join(fully_verifies or ()),

The build side then splits that into five nonexistent need IDs, and REQ_1 reads as unverified in traceability. _as_list on the runtime path (sphinxcontrib/test_reports/pytest_plugin.py:192) already handles exactly this — the decorator path just doesn't call it.

3. [High] Class-level and stacked markers are silently discarded (correctness)

get_closest_marker returns one marker, so only the innermost decorator survives. Reproduced:

@add_test_properties(test_type="requirements-based")
class TestThing:
    @add_test_properties(partially_verifies=["REQ_1"])
    def test_one(self): assert True
# XML: <property name="PartiallyVerifies" value="REQ_1"/>   -- TestType is gone

marker: pytest.Mark | None = node.get_closest_marker(MARKER)
if marker is None:
return
arguments: tuple[object, ...] = marker.args

Putting a classification on the class and links on each method is the natural way to use this. iter_markers plus merging (innermost winning per key) would fix it.

4. [High] filterwarnings = error turns the plugin into a hard abort (durability)

Two separate mechanisms, both reproduced on pytest 9.1:

  • The plugin's own PytestConfigWarning for xunit2 is raised via issue_config_time_warning and nothing ignores it, so a project with filterwarnings = error gets INTERNALERROR> pytest.PytestConfigWarning: ... junit_family is 'xunit2' instead of the intended advice — no tests collected, no report (sphinxcontrib/test_reports/pytest_plugin.py:233-242).
  • The record_xml_attribute experimental notice is silenced only through an appended ini filterwarnings line, which -W error on the command line and @pytest.mark.filterwarnings("error") both override — so every test errors at setup on a warning the user never opted into and cannot silence from their own config.

config.addinivalue_line(
"filterwarnings",
"ignore:record_xml_attribute is an experimental feature:"
"pytest.PytestExperimentalApiWarning",
)

A warnings.catch_warnings() around the fixture's use of record_xml_attribute (rather than an ini filter) fixes the second; the first wants a plain terminal message, or a warning the plugin also ignores.

5. [High] clean_source_path truncates any directory whose name ends in _main (correctness)

The marker is an unanchored substring plus rsplit(..., 1)[-1], not the documented ../_main/ prefix:

"""
if _RUNFILES_MARKER in path:
return path.rsplit(_RUNFILES_MARKER, 1)[-1]
return path

clean_source_path("services/app_main/tests/test_api.py")  -> "tests/test_api.py"
clean_source_path("domain_main/test_api.py")              -> "test_api.py"

Two different test files then write the same file, which with tr_deterministic_case_ids = True collides their generated IDs. Strip a leading ../_main/ / _main/ prefix, or match on a path-component boundary.

6. [High] Under pytest-xdist the location attributes are silently dropped (correctness)

pytest builds LogXML only on the controller (not hasattr(config, "workerinput")), so on workers record_xml_attribute resolves to a no-op. Verified: the same file run serially writes the plugin's 1-based line="3", and under -n 2 writes pytest's stock 0-based line="2" — no warning either way.

record_xml_attribute("file", clean_source_path(path))
if line_number is not None:
# pytest's line numbers are 0-based; editors and the report count
# from 1.
record_xml_attribute("line", str(line_number + 1))

<properties> still arrive (user_properties cross the worker boundary), which is what makes this invisible: the report looks fine and only the locations are wrong. Worth at least a start-up warning when xdist is active, plus a note in the docs.

Medium

7. [Medium] apply_test_metadata raises ValueError into the test body (durability)

The guard covers an empty mapping but not a mapping whose values are all empty — which is exactly what a spec file with an empty metadata block produces:

if not metadata:
return
properties = {

apply_test_metadata(record_property=..., metadata={"fully_verifies": []}) raises ValueError: no test properties given ... from sphinxcontrib/test_reports/pytest_plugin.py:175-181, failing a test whose code under test is fine. Given the documented call-it-first-thing pattern, this should return quietly like the empty case just above it.

8. [Medium] The docs' tr_property_link_types snippet breaks a build as written (docs)

.. code-block:: python
tr_property_link_types = {"PartiallyVerifies": "partially_verifies", "FullyVerifies": "fully_verifies"}

The mapped link fields must also be registered in sphinx-needs' needs_extra_links — _apply_property_links writes into extra_options, which sphinxcontrib/test_reports/directives/test_case.py:222 splats into add_need. Copying the snippet into conf.py without that errors out on the first test case carrying either property. One sentence and a needs_extra_links line in the example.

9. [Medium] The whole new module is excluded from mypy (architecture)

# pytest's marker API is Any-typed (MarkDecorator.__call__ on a
# Callable[..., object], whose `...` is itself an Any), which
# disallow_any_expr rejects for every decorator expression.
"^sphinxcontrib/test_reports/pytest_plugin\\.py$",

That list is otherwise a legacy backlog (# TODO: reactivate below files), and this adds a brand-new file to it under strict = true. It is also load-bearing for the bugs above: properties_mapping is annotated -> dict[str, str] but returns a raw list for custom keys (finding 1), which a type checker would flag.

The Any escape only has to cover the decorator expressions, not the module. A per-module override works — I verified this passes clean; note the module path is test_reports.* because sphinxcontrib is a namespace package:

[[tool.mypy.overrides]]
module = "test_reports.pytest_plugin"
disallow_any_expr = false
disallow_any_explicit = false

Low

10. [Low] -p no:junitxml crashes the session (durability)

xmlpath: object = config.option.xmlpath
family: object = config.getini("junit_family")
if xmlpath and family != "xunit1":

config.option.xmlpath only exists while pytest's junitxml plugin is registered; without it the run dies with INTERNALERROR> AttributeError: 'Namespace' object has no attribute 'xmlpath' before collection. getattr(config.option, "xmlpath", None) covers it.

11. [Low] Wrong ubproject.toml key in a module comment (docs)

#: Property names written to the XML, matching S-CORE's metamodel spelling so
#: a ``link_properties = { PartiallyVerifies = "partially_verifies" }`` in
#: ``ubproject.toml`` maps them without translation.

The key is property_link_types (sphinxcontrib/test_reports/projectconfig.py:88), not link_properties.

Compatibility

No concerns — purely additive. New module, new opt-in pytest extra, new docs page; the only shared edit is pytest_plugins in tests/conftest.py, which is test-only. Loading is explicit via -p rather than a pytest11 entry point, so no existing run changes behaviour just by upgrading. Good call.

@ubmarco

ubmarco commented Sep 8, 2026

Copy link
Copy Markdown
Member Author

@MaximilianSoerenPollak — could you weigh in on two design calls before I apply the review fixes? The other nine findings are mechanical and I'll just do them; these two change the public API, so I'd rather not pick alone.

Background: the plugin cannot know field types

Worth stating up front, because it constrains both questions. JUnit XML has exactly one property kind — <property name= value=>, a string. Whether a property becomes a link field or a plain field is decided entirely in conf.py, at import time:

conf.py result
tr_property_link_types = {"Name": "field"} link field — value split on ,, rejoined with ;
tr_extra_options = ["Name"] plain text field, raw value
both both
neither silently dropped

def _apply_property_links(self, properties):
"""Map JUnit <properties> values to sphinx-needs link fields via tr_property_link_types."""
tr_property_link_types = getattr(self.app.config, "tr_property_link_types", {})
for prop_name, link_field in tr_property_link_types.items():
prop_value = properties.get(prop_name, "")
if prop_value:
link_ids = ";".join(
id_val.strip() for id_val in prop_value.split(",") if id_val.strip()

So the plugin has no say in link-vs-plain and structurally can't. The only thing it controls is arity — whether a value is single- or multi-valued, i.e. whether it gets comma-joined on the way into that string.

Today arity is hard-coded to four names:

decorator kwarg annotation serialisation
partially_verifies, fully_verifies Sequence[str] ", ".join(...) — multi-valued
test_type, derivation_technique str passthrough — single-valued
**properties (anything else) str passthrough — single-valued only

mapping = {
PARTIALLY_VERIFIES: ", ".join(partially_verifies or ()),
FULLY_VERIFIES: ", ".join(fully_verifies or ()),
TEST_TYPE: test_type or "",
DERIVATION_TECHNIQUE: derivation_technique or "",
**properties,
}
cleaned = {name: value for name, value in mapping.items() if value}

Those two multi-valued names are S-CORE's link fields. A project with its own link field has no way to get list handling, so Satisfies=["REQ_1", "REQ_2"] is written as the Python repr "['REQ_1', 'REQ_2']", which tr_property_link_types then splits into the bogus IDs ['REQ_1' and 'REQ_2'] — silently. That is review finding 1.

Question A — how should arity be decided?

A1. Infer it. Any sequence value is joined with ", ", whichever keyword it arrived under; scalars pass through. No name list at all.

add_test_properties(Satisfies=["REQ_1", "REQ_2"])   # -> value="REQ_1, REQ_2"
add_test_properties(partially_verifies="REQ_1")     # -> value="REQ_1"      (also fixes finding 2)

Smallest change, fixes findings 1 and 2 together, keeps the score_pytest XML and entry points identical. Downside: test_type=["a", "b"] would silently become "a, b", which is nonsense for a single-valued enum — today the str annotation at least discourages that.

A2. Declare it. Same joining, but each property name carries an arity, and passing a list to a single-valued field is an error rather than a silent join. Costs a small table; catches the test_type=["a","b"] class of mistake. Still says nothing about link-ness.

A3. A2 plus an extensible alias map. Additionally let a project register its own snake_case → XML-name aliases, so custom link fields get the same ergonomics as the S-CORE four (satisfies=[...] → Satisfies) instead of having to write the PascalCase name themselves.

PROPERTY_ALIASES["satisfies"] = "Satisfies"
add_test_properties(satisfies=["REQ_1"])   # -> <property name="Satisfies" value="REQ_1"/>

A4. Drive the map from config (pytest ini / ubproject.toml). Most declarative; pulls config plumbing into a module that currently imports only pytest, and the plugin runs in the test process where ubproject.toml is not otherwise read.

My preference is A2, with A3 if we expect non-S-CORE metamodels soon — A1 is tempting for its size but trades away the only arity check we have.

Related hazard, independent of the choice: comma is the separator and there is no escaping. A free-text property containing commas (Owner="Smith, John") is fine until someone also link-maps that name, at which point it splits into two bogus IDs. Inherent to the wire format, not introduced here, but it argues for never link-mapping free-text fields.

Question B — pytest-xdist silently drops file/line

pytest builds LogXML only on the controller, so record_xml_attribute is a no-op on workers. Verified: the same file writes the plugin's 1-based line="3" serially, and pytest's stock 0-based line="2" under -n 2, with no warning. <properties> still arrive, since user_properties cross the worker boundary — which is what makes it invisible: the report looks complete and only the locations are wrong.

record_xml_attribute("file", clean_source_path(path))
if line_number is not None:
# pytest's line numbers are 0-based; editors and the report count
# from 1.
record_xml_attribute("line", str(line_number + 1))

B1. Warn at start-up when running under xdist and document it. Small and honest; xdist users still get no locations.
B2. Write the location through record_property and teach the build side to read it as a fallback. Actually works under -n, but changes the XML shape and breaks the "identical XML to score_pytest" promise.
B3. Document only.

B1 unless report runs are expected to be parallel — which is really the question: does anyone generate these reports under -n? If the report job runs serially, B1 is enough and B2 is wasted work.

Findings 2, 5, 7, 8, 9, 10, 11 and the marker-merge fix are unaffected by either answer and can land first.

@MaximilianSoerenPollak

Copy link
Copy Markdown
Collaborator

@ubmarco So regarding your questions.

For A I think your suggestion makes sense. We should hard error where possible, (having non allowed values) as well as allow for configuration for non S-CORE parts as I'm sure that will happen sooner or later.
So I would say ac ombination of 2 & 3 here would be best.

For B I think B1 is the easiest route. Pytest can run parrallelt ye and some do that already, but I have not seen anybody run xdist yet at least internally or in S-CORE. So I wouldn't worry to much about that one.

"equivalence-classes",
"fuzz-testing",
"error-guessing",
"explorative-testing",

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is not the entier list (at least from S-CORE process sight).
The one I had in my plugin I think were not complete.

Here you can find the complete list of the things:

https://eclipse-score.github.io/process_description/main/process_areas/verification/verification_concept.html#verification-concept-types-methods

I don't know why they have written it like this, it links from one place to another and then in the end links there.
Here is the original place:
https://eclipse-score.github.io/process_description/main/process_areas/verification/guidance/verification_specification.html#test-specification

BUt ye we probably want to have all allowed values in this list, not my incomplete one. Sorry for the bad pre-work there.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. I took the full list from the "Verification Methods" section you linked.

TEST_TYPES now has all 11 test types from that page:
control-flow-analysis, data-flow-analysis, fault-injection, inspection, interface-test, requirements-based, resource-usage, static-code-analysis, structural-statement-coverage, structural-branch-coverage, walkthrough.

DERIVATION_TECHNIQUES was already complete. It has the same 7 values as the page.

The docs page now links to that section, so people can look up what each value means.

One thing stays as it was: the plugin does not reject values outside these lists. It only documents them. The reason: S-CORE's own metamodel also accepts any text for test_type and derivation_technique (the pattern is ^.*$ in metamodel.yaml). If the plugin were stricter than the metamodel, a value that the docs build accepts would fail in pytest. If you want the plugin to reject unknown values anyway, tell me. It is a small change.

Commit 933737f.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes I don't think for this it makes sense to reject things outside as it's not S-CORE specific, but general. So that seems good for me.
One thing we could think of in the future is maybe a strict mode with an extensible list (that the user can extend or give) and then reject everything not in that list.

Comment thread pyproject.toml
[project.optional-dependencies]
# The pytest plugin (sphinxcontrib.test_reports.pytest_plugin); pytest is not
# a dependency of the extension or the converter.
pytest = ["pytest>=7.0"]

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you need some higher pytest version but I don't quiet remember.
we caught some issue swith pytest v 9.2 I think cause it changed stuff but maybe 7 and up should work.

9.2 might have changed things that were not relevant to the plugin.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But, if it works, it works, so don''t change it :D

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tested it instead of guessing. The plugin's own tests (35 tests, run through pytest's pytester, one of them with pytest-xdist) pass without any change on:

  • pytest 7.0.1 and 7.4.4 on Python 3.11
  • pytest 8.4.2 on Python 3.12
  • pytest 9.1.1 on Python 3.14

9.1.1 is the newest pytest on PyPI right now, so there is no 9.2 yet to test against.

Everything the plugin uses from pytest exists since version 7.0 or earlier: issue_config_time_warning, record_xml_attribute, iter_markers, Item.user_properties, and the public pytest.Config / pytest.Item / pytest.Mark names. So pytest>=7.0 stays.

ubmarco added a commit that referenced this pull request Sep 9, 2026
Only partially_verifies/fully_verifies were list-aware; everything else fell
through **properties and was written with str(), so a project's own link
field -- Satisfies=["REQ_1", "REQ_2"] -- ended up as the Python repr
"['REQ_1', 'REQ_2']", which tr_property_link_types then split into bogus
IDs, silently. And since str is a Sequence[str], partially_verifies="REQ_1"
was joined character by character into "R, E, Q, _, 1".

A PROPERTIES registry now decides how a keyword is written: its XML name
and whether it is multi-valued. Multi-valued properties join a sequence
with ", " and take a bare string as one value; single-valued ones reject a
sequence with a TypeError instead of joining it; a keyword the registry
does not know is written under its own name with a single value, and a
list under it is a TypeError pointing at register_property(), which adds a
project's own keywords (in conftest.py, before collection). The XML name
doubles as keyword. Sets and other objects are a TypeError rather than a
str() surprise; numbers are written as text.

apply_test_metadata goes through the same normalisation: metadata whose
values are all empty writes nothing and no longer raises into the test
body, and an explicit file/line override is applied regardless of the
metadata.

Also corrects the ubproject.toml key in the module comment
(property_link_types, not link_properties).

Review findings 1, 2, 7 and 11 of #151.
ubmarco added a commit that referenced this pull request Sep 9, 2026
get_closest_marker returns one marker, so a classification on the class
and links on each method -- the natural way to use the decorator -- lost
the class's properties silently, and of two stacked decorators on a
function only the one nearest the def survived. The fixture now merges
every marker on the node's chain (iter_markers), the one closest to the
function winning per property.

Review finding 3 of #151.
ubmarco added a commit that referenced this pull request Sep 9, 2026
Two mechanisms took a project with warnings-as-errors down:

* the xunit2 notice was a PytestConfigWarning through
  issue_config_time_warning, which `filterwarnings = error` raised straight
  out of pytest_configure -- an INTERNALERROR traceback, no tests, no
  report, instead of the advice;
* pytest's "record_xml_attribute is an experimental feature" notice was
  silenced by an appended ini filterwarnings line only, which `-W error`
  on the command line and a filterwarnings("error") mark both override, so
  every test errored at setup on a warning the user never opted into.

The notices are now a TestReportsConfigWarning of the plugin's own. Where
the project's filters make it an error, it is re-raised as a clean
UsageError ("ERROR: ..." and exit code 4) -- the project asked for
warnings to fail the run, and this one carries the fix. An
`ignore::sphinxcontrib.test_reports.pytest_plugin.TestReportsConfigWarning`
line silences it. The experimental-feature notice is dropped in process,
in a warnings.catch_warnings() around requesting the fixture, out of
reach of any filter setting.

With that, the fixture requests record_xml_attribute only when a report is
written under xunit1 (`legacy` is pytest's alias and no longer warned
about); under xunit2 pytest would drop the attributes again and warn per
test, which the one start-up notice covers. The properties go straight to
Item.user_properties, which is what the record_property fixture does, so
neither fixture is needed -- both belong to the junitxml plugin and are
gone under `-p no:junitxml`, where config.option.xmlpath does not exist
either and used to crash the session before collection.

Under pytest-xdist the XML writer exists on the controller only, so the
workers' record_xml_attribute is a no-op and the cases silently carry
pytest's stock 0-based locations while the properties still arrive. The
plugin now warns at start-up when tests are distributed; the docs say so.

The plugin's tests run the inner pytest as a subprocess from here on: the
test module has the plugin imported already, and an in-process run then
warns that the -p module cannot be assert-rewritten -- noise in the
warning counts and an error under the strict policies these tests set.

Review findings 4, 6 (B1) and 10 of #151.
ubmarco added a commit that referenced this pull request Sep 9, 2026
The marker was an unanchored substring, so any directory whose name ends
in `_main` lost its prefix: services/app_main/tests/test_api.py became
tests/test_api.py, and two different files then wrote the same `file` --
under tr_deterministic_case_ids, the same need ID. The cut now happens at
a whole `_main` component, on either separator, and at the last one:
under bzlmod the execroot's workspace directory is `_main` too, and an
absolute runfiles path handed to apply_test_metadata goes through both.

Review finding 5 of #151.
ubmarco added a commit that referenced this pull request Sep 9, 2026
The exclude list is a legacy backlog ("reactivate below files"), and the
new module went into it under strict = true. Only two checks need to give
way: disallow_any_expr, for pytest's Any-typed marker and config API, and
disallow_any_explicit, for the Callable[..., object] a decorator of
arbitrary test functions has to take. A per-module override lifts those
two and keeps everything else strict; it names the module both ways
because `packages` yields sphinxcontrib.test_reports.pytest_plugin while a
file on the command line resolves to test_reports.pytest_plugin, the
namespace package dropping its prefix.

Review finding 9 of #151.
ubmarco added a commit that referenced this pull request Sep 9, 2026
TEST_TYPES listed four of S-CORE's verification methods; the verification
concept in its process description names eleven (control-flow-analysis,
data-flow-analysis, inspection, static-code-analysis, the two structural
coverages and walkthrough were missing). DERIVATION_TECHNIQUES was already
complete. Still documented rather than enforced -- S-CORE's own metamodel
accepts any string for both fields -- and the docs now point at the
concept page and say so.

The build-side snippet mapped the properties to link fields without
registering those in needs_extra_links, which fails the build on the first
test case carrying either property; the snippet and a sentence now cover
it, and the needs_extra_options counterpart for plain fields.

Reviewer's comment on the vocabulary, and review finding 8 of #151.
@ubmarco

ubmarco commented Sep 9, 2026

Copy link
Copy Markdown
Member Author

All review findings are fixed. The fixes are six commits, one per topic, so each one can be read on its own. The two design questions were done the way @MaximilianSoerenPollak suggested: each property declares its own shape (A2 + A3), and pytest-xdist gets a warning (B1).

Here is what changed, finding by finding. "Before" is the bug, "Now" is the fix.

1. Lists under custom keywords were written as Python text (commit 105212a)
Before: Satisfies=["REQ_1", "REQ_2"] ended up as ['REQ_1', 'REQ_2'] in the XML. The docs build then split that into broken IDs, without any error.
Now: every property declares how it is written: its name in the XML, and whether it takes a list or a single value. Passing a list to a single-value property is an error, not a silent join. A new function teaches the plugin your own link fields, put it in conftest.py:
register_property("satisfies", "Satisfies", multi=True)
After that, @add_test_properties(satisfies=["REQ_1", "REQ_2"]) writes Satisfies with REQ_1, REQ_2. The XML name also works as keyword, so PartiallyVerifies=[...] is accepted too.

2. A single requirement ID was split into letters (commit 105212a)
Before: partially_verifies="REQ_1" was written as R, E, Q, _, 1, because a string is also a sequence.
Now: a plain string counts as one ID.

3. Decorators on classes were ignored (commit 77b3ee2)
Before: a decorator on the test class was dropped when the method also had one. Only the innermost decorator survived. The same for two decorators stacked on one function.
Now: the decorators on the function, its class and its module are merged. If two set the same property, the one closest to the function wins.

4. Strict warning settings crashed the run (commit 942a48b)
Before: with filterwarnings = error in the project config, the plugin's own "you use xunit2" warning crashed pytest with an INTERNALERROR trace, no tests ran. With -W error on the command line, pytest's "record_xml_attribute is an experimental feature" notice made every single test fail at setup.
Now: the plugin's notices have their own warning class, TestReportsConfigWarning. If a project turns warnings into errors, the notice becomes a normal, readable pytest error (ERROR: ..., exit code 4) instead of a crash. ignore::sphinxcontrib.test_reports.pytest_plugin.TestReportsConfigWarning in filterwarnings silences it. The "experimental feature" notice is dropped inside the plugin itself, so no warning setting can turn it into an error any more.

5. Folders ending in _main lost their name (commit e417c58)
Before: the Bazel prefix cut matched the text _main/ anywhere. services/app_main/tests/test_api.py became tests/test_api.py. Two different test files could get the same path, and with tr_deterministic_case_ids the same need ID.
Now: only a folder called exactly _main is cut, on either path separator. If there are several, the last one counts, because under bzlmod Bazel's execroot also has a _main folder.

6. pytest-xdist silently broke the source locations (commit 942a48b)
Before: with -n, pytest creates the XML writer only in the main process, so on the workers record_xml_attribute does nothing. The test cases then carried pytest's default locations (line numbers counted from 0), and nobody was told.
Now: the plugin warns at start-up when xdist distributes the tests, and the docs explain it. The locations are still lost on workers, so run the report job without -n.

7. Empty metadata raised an error inside the test (commit 105212a)
Before: apply_test_metadata(..., metadata={"fully_verifies": []}) raised ValueError and failed a test whose code was fine.
Now: metadata with only empty values writes nothing and is fine. A file/line given explicitly is still applied.

8. The docs example did not build (commit 933737f)
Before: the example set tr_property_link_types but did not register the link fields in sphinx-needs, so the build failed on the first test case with such a property.
Now: the example also sets needs_extra_links, and a sentence explains why.

9. The new module was excluded from mypy (commit 18d34f5)
Now: it is type-checked. Only the two "no Any" checks are switched off for this one module, because pytest's marker and config API is typed as Any. Everything else stays strict.

10. -p no:junitxml crashed pytest (commit 942a48b)
Before: the plugin read config.option.xmlpath, which only exists while pytest's junitxml plugin is loaded.
Now: it works. The plugin also no longer needs the record_property fixture, it writes the properties to Item.user_properties directly, which is what that fixture does.

11. Wrong key in a comment (commit 105212a)
The comment now names the real ubproject.toml key, property_link_types.

Review comment on the vocabulary (commit 933737f)
TEST_TYPES had 4 of S-CORE's 11 test types. Now it has all 11, and the docs link to the source page. The values are still documented, not enforced. Details in the thread.

Two more small things:

  • pytest's legacy value for junit_family is treated as xunit1 (it is an alias) and no longer warned about.
  • The plugin's tests now start the inner pytest as a separate process, like a real user would. There are 35 tests now, up from 10.

Checked with: the full test suite, mypy, pre-commit, a Sphinx build of the docs, and the plugin tests on pytest 7.0.1, 7.4.4, 8.4.2 and 9.1.1.

@ubmarco
ubmarco marked this pull request as ready for review September 9, 2026 22:21
A stock pytest run writes neither the `file`/`line` attributes a `<testcase>`
needs for a source location nor any `<properties>`, so the directives'
`tr_source_file_option`/`tr_source_line_option`, `tr_deterministic_case_ids`
and `tr_property_link_types` have nothing to work from. Enabled with
`-p sphinxcontrib.test_reports.pytest_plugin` under `junit_family = xunit1`,
the plugin records the source location of every test (Bazel's `_main/`
runfiles prefix cut off) and turns the metadata of `add_test_properties` into
`<properties>`: `PartiallyVerifies`, `FullyVerifies`, `TestType`,
`DerivationTechnique`, plus any further keyword under its own name. Lists are
joined with ", ", which `tr_property_link_types` splits again.
`apply_test_metadata` does the same at run time for parameterised tests whose
metadata lives in the files that drive them, and can point the case at that
file.

Ported from the `score_pytest` attribute plugin of S-CORE's docs-as-code; the
XML is shaped identically and the two entry points keep their names, so tests
written against that plugin keep working when they import from here. Two of
its rules are deliberately not ported -- the `TestType` / `DerivationTechnique`
vocabularies are documented, not enforced, and a decorated test needs no
docstring -- as process rules of that project rather than properties of the
data. Implemented as an autouse fixture rather than a `pytest_runtest_makereport`
hookwrapper: same XML, no hook plumbing.

The plugin warns at start-up when a report is requested under `xunit2` (no
attributes would be written) and silences pytest's per-test "experimental"
notice for `record_xml_attribute`, which it exists to use. It imports pytest
only; a `pytest` extra pulls it in. Tested through `pytester`.
Only partially_verifies/fully_verifies were list-aware; everything else fell
through **properties and was written with str(), so a project's own link
field -- Satisfies=["REQ_1", "REQ_2"] -- ended up as the Python repr
"['REQ_1', 'REQ_2']", which tr_property_link_types then split into bogus
IDs, silently. And since str is a Sequence[str], partially_verifies="REQ_1"
was joined character by character into "R, E, Q, _, 1".

A PROPERTIES registry now decides how a keyword is written: its XML name
and whether it is multi-valued. Multi-valued properties join a sequence
with ", " and take a bare string as one value; single-valued ones reject a
sequence with a TypeError instead of joining it; a keyword the registry
does not know is written under its own name with a single value, and a
list under it is a TypeError pointing at register_property(), which adds a
project's own keywords (in conftest.py, before collection). The XML name
doubles as keyword. Sets and other objects are a TypeError rather than a
str() surprise; numbers are written as text.

apply_test_metadata goes through the same normalisation: metadata whose
values are all empty writes nothing and no longer raises into the test
body, and an explicit file/line override is applied regardless of the
metadata.

Also corrects the ubproject.toml key in the module comment
(property_link_types, not link_properties).

Review findings 1, 2, 7 and 11 of #151.
get_closest_marker returns one marker, so a classification on the class
and links on each method -- the natural way to use the decorator -- lost
the class's properties silently, and of two stacked decorators on a
function only the one nearest the def survived. The fixture now merges
every marker on the node's chain (iter_markers), the one closest to the
function winning per property.

Review finding 3 of #151.
Two mechanisms took a project with warnings-as-errors down:

* the xunit2 notice was a PytestConfigWarning through
  issue_config_time_warning, which `filterwarnings = error` raised straight
  out of pytest_configure -- an INTERNALERROR traceback, no tests, no
  report, instead of the advice;
* pytest's "record_xml_attribute is an experimental feature" notice was
  silenced by an appended ini filterwarnings line only, which `-W error`
  on the command line and a filterwarnings("error") mark both override, so
  every test errored at setup on a warning the user never opted into.

The notices are now a TestReportsConfigWarning of the plugin's own. Where
the project's filters make it an error, it is re-raised as a clean
UsageError ("ERROR: ..." and exit code 4) -- the project asked for
warnings to fail the run, and this one carries the fix. An
`ignore::sphinxcontrib.test_reports.pytest_plugin.TestReportsConfigWarning`
line silences it. The experimental-feature notice is dropped in process,
in a warnings.catch_warnings() around requesting the fixture, out of
reach of any filter setting.

With that, the fixture requests record_xml_attribute only when a report is
written under xunit1 (`legacy` is pytest's alias and no longer warned
about); under xunit2 pytest would drop the attributes again and warn per
test, which the one start-up notice covers. The properties go straight to
Item.user_properties, which is what the record_property fixture does, so
neither fixture is needed -- both belong to the junitxml plugin and are
gone under `-p no:junitxml`, where config.option.xmlpath does not exist
either and used to crash the session before collection.

Under pytest-xdist the XML writer exists on the controller only, so the
workers' record_xml_attribute is a no-op and the cases silently carry
pytest's stock 0-based locations while the properties still arrive. The
plugin now warns at start-up when tests are distributed; the docs say so.

The plugin's tests run the inner pytest as a subprocess from here on: the
test module has the plugin imported already, and an in-process run then
warns that the -p module cannot be assert-rewritten -- noise in the
warning counts and an error under the strict policies these tests set.

Review findings 4, 6 (B1) and 10 of #151.
The marker was an unanchored substring, so any directory whose name ends
in `_main` lost its prefix: services/app_main/tests/test_api.py became
tests/test_api.py, and two different files then wrote the same `file` --
under tr_deterministic_case_ids, the same need ID. The cut now happens at
a whole `_main` component, on either separator, and at the last one:
under bzlmod the execroot's workspace directory is `_main` too, and an
absolute runfiles path handed to apply_test_metadata goes through both.

Review finding 5 of #151.
The exclude list is a legacy backlog ("reactivate below files"), and the
new module went into it under strict = true. Only two checks need to give
way: disallow_any_expr, for pytest's Any-typed marker and config API, and
disallow_any_explicit, for the Callable[..., object] a decorator of
arbitrary test functions has to take. A per-module override lifts those
two and keeps everything else strict; it names the module both ways
because `packages` yields sphinxcontrib.test_reports.pytest_plugin while a
file on the command line resolves to test_reports.pytest_plugin, the
namespace package dropping its prefix.

Review finding 9 of #151.
TEST_TYPES listed four of S-CORE's verification methods; the verification
concept in its process description names eleven (control-flow-analysis,
data-flow-analysis, inspection, static-code-analysis, the two structural
coverages and walkthrough were missing). DERIVATION_TECHNIQUES was already
complete. Still documented rather than enforced -- S-CORE's own metamodel
accepts any string for both fields -- and the docs now point at the
concept page and say so.

The build-side snippet mapped the properties to link fields without
registering those in needs_extra_links, which fails the build on the first
test case carrying either property; the snippet and a sentence now cover
it, and the needs_extra_options counterpart for plain fields.

Reviewer's comment on the vocabulary, and review finding 8 of #151.
The CI mypy job runs `uv sync --group dev` and then mypy. pytest is an
extra, not a dependency, so that environment had no pytest and mypy could
not follow the plugin's imports: "Cannot find implementation or library
stub for module named pytest", with every pytest type collapsing to Any.
The module was excluded from mypy until 18d34f5, which is why this never
showed. Adding pytest to the dev group mirrors the project venv, where the
check passes.
The plugin carried S-CORE's metamodel in code: the four property names as
constants, the four keywords as named parameters of add_test_properties
and properties_mapping, a pre-filled registry, and the two vocabularies.
Other metamodels got a code-level side door, register_property() in a
conftest.py. sphinx-test-reports is a generic project, and the plugin
should be shaped like one.

The model now comes from a pytest ini option the plugin registers,
test_reports_properties, one line per property:

    keyword [= XmlName] [, list]

keyword is what a test writes, XmlName the <property> name it is written
under (the keyword itself when omitted), and `list` marks a multi-valued
property. A keyword not declared is written under its own name with a
single value; a list under it is a TypeError that names the option. A
line outside the grammar, an unknown flag or a keyword declared twice stop
the run at start-up with a usage error quoting the line. Being pytest
configuration, the option lives in pytest.ini or pyproject.toml, reaches
xdist workers, and needs nothing outside pytest -- in particular not
ubproject.toml, which in S-CORE is an artifact of the Sphinx build that
runs after the tests.

The values are no longer serialised when the decorator runs but at test
setup, in the fixture, against the model pytest_configure installed: the
decorator stores the keywords as given and does not depend on
configuration having been read (a conftest may import test helpers before
pytest_configure). It still refuses a call with nothing in it at import
time, which needs no model. Merging stacked markers happens on the
serialised properties, so the XML name decides what "the same property"
is.

Gone from the code: the name constants, TEST_TYPES, DERIVATION_TECHNIQUES,
register_property and the named parameters. S-CORE's model is the worked
example of the docs page, in ini and pyproject form, with the two
vocabularies as a table; with it, tests written against score_pytest keep
working unchanged, since the parameters were keyword-only all along.
@ubmarco

ubmarco commented Sep 10, 2026

Copy link
Copy Markdown
Member Author

One more change after the review round, and it replaces something the review agreed on, so here is the why and the what.

Why. The plugin still carried S-CORE's metamodel in code: the four property names as constants, the four keywords as named parameters of add_test_properties, a pre-filled registry, and the two vocabularies. Other projects got a code-level side door, register_property() in a conftest.py. sphinx-test-reports is a generic project, so the plugin should be generic too, with S-CORE as one possible configuration. Reading ubproject.toml was not an option: in S-CORE that file is produced by the Sphinx build, which runs after the tests. So the configuration is pure pytest.

What. A new pytest ini option, test_reports_properties, declares the properties a test may carry, one per line:

keyword [= XmlName] [, list]
  • keyword is what a test author writes.
  • XmlName is the <property name> written to the report. Leave it out when it is the same as the keyword.
  • list marks a property that takes a list. Without it, a list is an error instead of a silent join.

A keyword that is not declared is written under its own name with a single value. A list under an undeclared keyword is a TypeError that names the option, so nothing is lost silently. A line with a typo stops the run at start-up with a clear usage error.

S-CORE's model is now the example in the docs, in pytest.ini:

[pytest]
addopts = -p sphinxcontrib.test_reports.pytest_plugin
junit_family = xunit1
test_reports_properties =
    partially_verifies = PartiallyVerifies, list
    fully_verifies = FullyVerifies, list
    test_type = TestType
    derivation_technique = DerivationTechnique

and as a list of the same four strings in pyproject.toml under [tool.pytest.ini_options]. With those four lines, tests written against score_pytest keep working unchanged: the parameters were keyword-only before, so add_test_properties(partially_verifies=[...], test_type=...) reads the same.

Removed: register_property, the name constants, TEST_TYPES and DERIVATION_TECHNIQUES. The two vocabularies are a table on the docs page, still documented and not enforced.

Also changed: the values are now written when a test is set up, not when the decorator runs, so the decorator never depends on configuration having been read yet. A decorator with nothing in it is still refused at import time.

Commit 72ff2dd. Checked with the full test suite, mypy, pre-commit, a docs build, and the plugin tests on pytest 7.0.1, 7.4.4, 8.4.2 and 9.1.1.

@chrisjsewell chrisjsewell left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed at 72ff2dde by reproducing the claims rather than reading them: the same test file run under this plugin and under the score_pytest origin with the XMLs diffed; the full suite on the 9.1/8.5.0 and 7.4.7/6.0.1 cells (378 passed, no skips, on each); the plugin's own module with no Sphinx installed (47 passed) and on pytest 7.0.1; the three loading routes under --strict-config and --strict-markers; the page's conf.py example built under -W; and eight mutations of the new code against its tests (seven caught; the survivor is the same blind spot as the second item under "Worth fixing"). Verdict: the redesign is right and the code is close. I would hold the merge for the four items under "Blocking", all small. The rest is your call.

What holds, measured. Every one of the eleven review findings is fixed and stays fixed under mutation: the arity check, the marker merge, the legacy alias, the _main component cut, the catch_warnings, the flag guard. The ini option is known at validation time on all three loading routes (addopts -p, PYTEST_PLUGINS, pytest_plugins in a root conftest), so the documented recipe is --strict-config clean. The claim in your last comment that the decorator no longer depends on configuration having been read is true and load-bearing: a decorated function in a module that a conftest.py imports runs during _preparse, before pytest_configure, and the head handles it where serialising at decorator time dies before collection. No test covers that shape; the eight-line probe (a conftest importing a helper module with a decorated function) is worth adding to TestPropertyModel. The warning policy is robust in every shape I tried, including a warnings.warn in a test body after the fixture under -W error, which still fails the test, so the catch_warnings block restores pytest's recorder. xdist warns exactly once and the model reaches the workers. The mypy override is minimal: removing either module name or either flag produces errors (12, 12 and 3). The vocabulary table matches the linked S-CORE page (11 and 7). The wheel carries the module, pytest sits behind the extra only, and [pytest] installs from the built wheel.

Blocking

1. The premise is false as written, in four places, and one of them becomes the commit message. The body's "Why", the module docstring, the first paragraph of docs/pytest.rst and the changelog all say pytest writes file/line "only through the record_xml_attribute fixture" and that no stock testcase carries them. Measured on pytest 9.1.1, and it is right there in _NodeReporter.record_testreport: under junit_family = xunit1 pytest writes file and a 0-based line on every testcase with no fixture at all, and record_xml_attribute merely overrides them. Only the default xunit2 drops them. Since the page's next section tells the user to set xunit1, the claim is false about the configuration it prescribes. What the plugin adds to the location is the 1-based line, the _main runfiles cut and the apply_test_metadata override, which is the honest sentence and still a useful one. directives/test_common.py:74 already attributes the absence to the family, correctly.

2. The body describes the design before 72ff2dde. It still lists PartiallyVerifies, FullyVerifies, TestType, DerivationTechnique as what the decorator writes, cites TEST_TYPES and DERIVATION_TECHNIQUES (deleted in that commit), says lists are joined (only for a property declared , list; undeclared, a list is a TypeError, which the head's own test asserts), and never mentions test_reports_properties. The squash commit copies it. Your 07:41Z comment is the right starting point for the rewrite; and "35 tests" is 47 by --collect-only.

3. str(item) inside the join re-opens findings 1 and 2, one level down. _serialise hoists str out of the sequence branch and then calls str() on every item of any other Sequence unchecked. Measured with the S-CORE profile, real runs, real XML:

fully_verifies=b"REQ_1"                        -> <property name="FullyVerifies" value="82, 69, 81, 95, 49">
partially_verifies=[["REQ_1", "REQ_2"]]        -> <property name="PartiallyVerifies" value="['REQ_1', 'REQ_2']">
partially_verifies=[[]], fully_verifies=["R"]  -> <property name="PartiallyVerifies" value="[]">

The build then splits them into 82;69;81;95;49, ['REQ_1';'REQ_2'] and [] as link IDs, silently, against the page's sentence that "a project cannot lose requirement IDs to a Python repr silently". The third row also shows _empty and _serialise disagreeing about [[]]: the import-time gate fires only when every value is empty, so it passes alongside a real value. apply_test_metadata is the realistic source, since a list of lists is one indentation away in a spec file. Fix: reject bytes/bytearray outright and require each item of a sequence to be a str, with a TypeError naming the item; that makes the two functions agree by construction. One test each.

4. The conf.py recipe on the page is deprecated on the sphinx-needs this repository just pinned CI to. docs/pytest.rst prescribes needs_extra_links and says each plain field "needs its needs_extra_options entry". Built verbatim against sphinx-needs 8.5.0 under -W:

WARNING: Config option "needs_extra_options" is deprecated. Please use "needs_fields" instead. [needs.deprecated]
WARNING: Config option "needs_extra_links" is deprecated. Please use "needs_links" instead. [needs.deprecated]

Both replacements arrived in sphinx-needs 7.0.0, the exact version this repository's own docs/conf.py switches on. The modern spelling (needs_links = {...}, needs_fields = {"TestType": {"nullable": True}, ...}) builds clean with the same project and report. The floor is 6.0.1, so either mirror the conf.py switch or add one clause: on sphinx-needs < 7 these are needs_extra_links and needs_extra_options. This is the successor of finding 8, whose fix added the deprecated spelling.

Worth fixing

  • Cases skipped at setup carry the other convention, and under Bazel that moves the deterministic ID. The location is recorded by a function-scoped autouse fixture, which never runs for @pytest.mark.skip, skipif(True), xfail(run=False) or a case whose module-scoped fixture errors; those keep pytest's stock 0-based, uncut file/line and carry no <properties>. So one report mixes 1-based executed cases with 0-based skipped ones, and in a tree with a component named _main the skipped case's file keeps the prefix: deterministic_case_id, which hashes the file, gives testcase__test_x__test_skipped_qyboc when the test runs and …_ijsjz when it is skipped. score_pytest has the same fixture, so this is inherited, not a regression. But this repository is the one shipping tr_deterministic_case_ids, and "Every test case now carries file and line" denies it. A pytest_runtest_makereport hookwrapper on the setup report reaches every case, because record_testreport restores already-set attributes over its own; measured as a 21-line conftest on both probe trees, every skipped case corrected, nothing else moved, the apply_test_metadata override still wins. It does reach into _pytest.junitxml.xml_key, which is private (present on 7.0.1), so that is your call. The docs sentence needs changing either way.
  • A nested in-process pytest session that loads the plugin empties the outer model. PROPERTIES is a module global cleared by pytest_unconfigure, so pytester.runpytest("-p", <plugin>) (in-process is runpytest's default) or a pytest.main() inside a test leaves every decorated test after it erroring at setup with "'partially_verifies' is not a configured property", a message that tells the user to declare what they declared. Narrow, since the inner session must itself load the plugin, but that is what a project testing its own pytest setup does, and the head's suite dodges it only by running its inner sessions as subprocesses. Put the model on config.stash (the fixture and _marked_properties already hold the config), or have pytest_configure save and pytest_unconfigure restore the previous value. My mutation that made pytest_unconfigure a no-op survived the whole suite; an in-process inner run between two decorated tests is the regression test for both.
  • "the XML is shaped identically" to score_pytest has four exceptions, and the two the texts name are the two that do not touch the XML. Same file under both plugins, diffed: a case that errors in a later fixture carries properties here and none there (they are written at setup now, and I think that is right); property order follows the keywords, not S-CORE's fixed order; the _main cut differs by design (finding 5); and apply_test_metadata(file=…) is cleaned here and passed through raw there. Name them in "Origin" and the docstring; three are improvements.
  • Two keywords may declare the same XmlName. parse_properties(['a = B', 'c = B']) is accepted and _normalise then keeps only the later value. The duplicate-keyword check applied to Property.name closes it.
  • "a wrong shape … fails that test": it errors at setup (the test asserts errors=1), and in this tool the need then carries result="error", not failure.
  • test_reports_properties in the ini without the -p line is ERROR: Unknown config option under --strict-config; one sentence next to the example saying the two blocks are a unit.
  • test_the_plugin_does_not_import_sphinx could also assert docutils is absent (it passes today).

Nits and for the record

A real keyword shadows another property's XmlName in _lookup (a = B and B = C: writing B=… gives <property name="C">); the precedence is right, the "doubles as keyword" sentence just does not say so. The comma-in-a-value hazard you raised in the design question is on cli.rst and configuration.rst but not on the page that produces the value. "The plugin tests pass on pytest 7.0.1" holds on Python 3.11 only: on 3.12 five fail on pytest's own ast.Str deprecation in the assertion rewriter, not on the plugin (7.0.1 predates 3.12; pytest>=7.4 would make the statement interpreter-independent). versionadded:: 1.5.0 is a release-time check if #159's breaking entry makes the release 2.0.

For #159, which lands second: the merge has two keep-both-sides conflicts (pyproject.toml extras, tests/conftest.py's pytest_plugins) and the changelog auto-merges. And tests/test_pytest_plugin.py is missing from its TOOLCHAIN_FREE_TESTS. That session is the only place "the plugin needs no Sphinx" is proven in CI's real shape; built as the nox session builds it (. plus pytest, no toolchain), the module passes 46 with the one xdist skip, and with the eight listed files 294 pass. One line in whichever PR lands second.

@MaximilianSoerenPollak MaximilianSoerenPollak left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From my side with the changes made etc. this now seems like a good candidate and starting point for further improvements.

💯

Review by chrisjsewell on 72ff2dd, all items.

The location and the properties were recorded by a function-scoped
autouse fixture, which never runs for a case skipped or erroring during
setup: those kept pytest's stock 0-based, uncut file/line and carried no
properties, so one report mixed the two conventions and, under Bazel, a
deterministic ID moved with the outcome. Both are hooks now. A
pytest_runtest_makereport wrapper attaches the markers' properties to the
item before its setup report is made (a wrong shape turns that report
into an error with the message), and a per-session pytest_runtest_logreport
hook writes file/line from the setup report on the process that owns
pytest's XML writer. That process is the xdist controller, so the
locations now hold under -n as well; the apply_test_metadata override
travels as two reserved user_properties entries that the hook takes out
before pytest writes the properties, and record_xml_attribute is no
longer used (still accepted by apply_test_metadata for score_pytest call
sites). The xdist notice and the experimental-notice suppression go with
the fixture. The XML writer sits behind _pytest.junitxml.xml_key; its
absence is handled, not assumed.

str(item) inside the join re-opened findings 1 and 2 one level down:
b"REQ_1" became "82, 69, 81, 95, 49", [["REQ_1", "REQ_2"]] its repr, and
[[]] the text "[]", all silently split into link IDs on the build side.
Every item of a list must be a str now, bytes/bytearray are rejected, and
the import-time emptiness check uses the writer's definition of empty so
the two cannot disagree. Two keywords declaring one XML name are an error
at start-up, where _normalise used to keep the later value.

PROPERTIES was cleared by pytest_unconfigure, so an in-process inner
session (pytest.main() in a test) left every later decorated test erroring
on "not a configured property". pytest_configure saves the outer model and
pytest_unconfigure restores it.

The premise was wrong in four places: pytest does write file/line under
xunit1, 0-based and with the runfiles prefix intact; only the default
xunit2 drops them. Module docstring, docs and changelog now say what the
plugin adds. The docs' conf.py recipe uses needs_links/needs_fields, which
sphinx-needs 7.0.0 introduced while deprecating needs_extra_links and
needs_extra_options, with a clause for sphinx-needs 6 -- built end to end
under -W against a plugin-written report. Also on the page: the four
deviations from score_pytest's XML, "errors at setup" instead of "fails",
the --strict-config unit of ini option and -p line, the comma hazard, the
keyword-over-XmlName precedence. The no-Sphinx test asserts docutils too.
The pytest extra requires 7.4, the first pytest that runs on Python 3.12.
…roperties

A conftest.py that imports a helper module runs during pre-parse, before
the model is installed; the decorator storing the keywords as given is
what makes that shape work, and nothing covered it. The probe from the
review: a conftest importing a helper with a decorated function, run with
the S-CORE profile, the properties written at setup.
@ubmarco

ubmarco commented Sep 10, 2026

Copy link
Copy Markdown
Member Author

@chrisjsewell on your review of 72ff2dd:

Thanks for measuring rather than reading. All four blocking items and the seven under "worth fixing" are in, on top of the rebased branch, plus the nits and the probe you suggested. Commits 5d044e8 and aace50d; 57 tests. In plain words:

Blocking

  1. The premise. Fixed in all four places (PR body, module docstring, first paragraph of the docs page, changelog). They now say what pytest actually does: under xunit2 there are no file/line attributes at all; under xunit1 pytest writes them, but counts the line from 0, keeps the Bazel prefix and cannot point a case at another file. That is what the plugin adds.
  2. The PR body is rewritten for the current design. It names test_reports_properties, no longer mentions the deleted constants, and gives the real test count.
  3. str(item) inside the join. Every item of a list now has to be a str; bytes and bytearray are rejected outright. Nested lists, [[]], [1, 2] and b"REQ_1" are all a TypeError naming the item. The import-time emptiness check uses the same definition as the writer: only None, "" and sequences of those are empty; everything else passes the decorator and is then judged by the writer. One test per case.
  4. The conf.py recipe uses needs_links and needs_fields, with one sentence for sphinx-needs 6 (needs_extra_links, needs_extra_options). I built the recipe end to end under -W with sphinx-needs 8.5.0: a report written by the plugin, imported by test-file with auto_cases, links landing on partially_verifies and fully_verifies, TestType and DerivationTechnique as fields.

Worth fixing

  • Skipped cases. Both the location and the properties are now written by hooks, not by a fixture. pytest_runtest_makereport on the setup phase attaches the properties to the item, so its report carries them, and a pytest_runtest_logreport hook writes file/line from the setup report on the process that owns the XML writer. A skipped, xfail(run=False) or setup-erroring case gets the same 1-based line, cut path and properties as one that ran; one test covers all three shapes. It does read _pytest.junitxml.xml_key, guarded: if that key is ever gone, the plugin writes no attributes rather than crashing.
  • This also makes pytest-xdist work, which is why the xdist warning is gone. The location is written on the controller from the report, the properties travel with it, and the apply_test_metadata override travels as two reserved user_properties entries that the hook removes before pytest writes the properties. Two tests run under -n 1 and check location, properties and override. record_xml_attribute is no longer used by the plugin; apply_test_metadata still accepts it for score_pytest call sites.
  • Nested in-process session. pytest_configure saves the outer model and pytest_unconfigure restores it. The test runs pytest.main() between two decorated tests, which also kills the pytest_unconfigure no-op mutant.
  • "Identically" has exceptions. Named in the module docstring and the Origin section: skipped cases, property order, the _main cut, the cleaned file of apply_test_metadata.
  • Two keywords, one XmlName is now a start-up error.
  • "fails that test" now reads "errors at setup", with the result="error" note.
  • The --strict-config sentence and the comma hazard are on the page.
  • test_the_plugin_does_not_import_sphinx also asserts docutils is absent.

The probe you suggested is a test now: a conftest.py imports a helper module with a decorated function, so the decorator runs at pre-parse time, before pytest_configure; the properties are written at setup as declared.

Nits: the keyword-over-XmlName precedence is stated on the page; the pytest extra is >=7.4, and the plugin's tests pass on 7.0.1 and 7.4.4 with Python 3.11, 7.4.4 and 8.4.2 with 3.12, 9.1.1 with 3.14.

Left open on purpose, two items: versionadded:: 1.5.0 is the release-time check you describe, decided when the release number is. The TOOLCHAIN_FREE_TESTS line for tests/test_pytest_plugin.py belongs to #159's list, so it goes into whichever PR lands second, as you say.

@chrisjsewell chrisjsewell left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-reviewed at aace50d4, the same way: every round-one finding re-measured on the new head, the new hook code put through its own battery, and the suites re-run. The four blocking items are closed, and fourteen of the sixteen round-one findings with them; the two left open (versionadded and the TOOLCHAIN_FREE_TESTS line) are the two we agreed to defer. Two small things remain before I approve, both below; everything else is your call.

The rewrite holds, measured. The eleven-shape file from round one (pass, fail, skip, skipif, xfail(run=False), a function-fixture error, a module-fixture error, a class-level decorator, a keyword-order variant, the parameterised pair) now gives every case the 1-based line, the cut path and its properties, skipped and setup-erroring ones included, and in the tree with a _main component the skipped case's deterministic ID now equals the executed one's. Under -n 2 the report is byte-equivalent to the serial run, override applied, with no reserved entry in any <properties>. The report.user_properties mechanics are the ones pytest actually has: each report holds its own copy, the setup report carries what the wrapper appended before yield, junitxml writes properties from the teardown report, and the tryfirst handler strips before it reads. Eight mutations of the hook code: seven red on the right tests (dropping tryfirst alone loses every attribute; dropping the worker guard makes the worker strip its own override, so the guard is load-bearing, not belt and braces) and the eighth, writing the location on every phase, byte-diffs identical, so nothing should pin it. Every repr input from round one is now a TypeError at setup with the case shown as <error>. The recipe on the page builds a real report end to end under -W on sphinx-needs 8.5.0 with zero warnings, links landing on both fields; the sphinx-needs 6 sentence is necessary and right. 388 passed on both cells with no skips, 57 in the module with no Sphinx installed, 55 on pytest 7.4.4, mypy and the -W docs build clean. The conftest-before-configure probe is a test now and it is the right one.

Before I approve

1. A score_pytest call site that still requests the record_xml_attribute fixture errors at setup under a strict warning policy, and it passed on 72ff2dde. The page and the docstring say such calls "may keep passing record_xml_attribute; it is accepted and not needed", but the problem is the fixture request in the test's signature, not the argument: pytest issues PytestExperimentalApiWarning for it, and under -W error or filterwarnings = error that is a setup error. On the previous head the plugin's own fixture requested it inside catch_warnings() first, so the test's request got the cached value silently; removing the fixture removed that cover. test_record_xml_attribute_is_still_accepted asserts warnings=1, so it encodes the notice rather than the strict case. Zero code: make the compatibility sentence say to drop the fixture from the signature and why, and add the strict-policy run to that test.

2. An empty list under a single-valued or undeclared keyword is a TypeError, and apply_test_metadata raises it into the test body. The arity check runs before the items are looked at, so test_type=[] or Owner=[] never reaches "nothing to write". With the documented file-driven pattern and a parser that returns [] for an absent field:

FAILED test_md.py::test_driven_by_a_file[a] - TypeError: 'test_type' takes a single value, not a sequence; …
FAILED test_md.py::test_driven_by_a_file[b] - TypeError: …

That is your review finding 7 returning through the list-valued door, against the page's "metadata without values … writes no properties and is not an error", and _empty still disagrees with _serialise on these shapes. Three lines: filter the None/"" items first, return None when nothing is left, then check arity. One test per direction.

At your discretion

  • An inner in-process session whose pytest_configure fails (a bad ini line) pops the outer model, because the push sits below the code that raises while pytest_unconfigure pops regardless; measured with your nested-session shape plus a broken ini. Move _OUTER_MODELS.append(dict(PROPERTIES)) to the first statement of pytest_configure.
  • Under -n 2 with xunit2 the start-up notice is issued three times (controller plus workers); pytest collapses the text, so the count reads 3 warnings rather than a flood. The old xdist notice had the workerinput guard and _notify has none now.
  • A bad property shape on a test whose fixture also raises at setup shows only the plugin's message; the fixture error is gone. Self-correcting, worth a line in the wrapper's docstring or an append rather than a replace.
  • The two reserved user_properties names are documented nowhere; a user who writes one has the property consumed as a location override with no message.
  • The body says 56 tests (57 by --collect-only), and "7.4, the first pytest that runs on Python 3.12" is not true, 7.3 runs and passes the module. The floor is fine; the reason, which is also a comment in pyproject.toml, is not.

One observation, not a finding: a skipped test now contributes its requirement links, with result="skipped" next to them. Strictly better than before, and honest, but a traceability query that does not look at result answers "verified" on a test that did not run. Worth a sentence on the page.

I will approve on the next push after checking the two items by hand; no further round is needed for the rest.

…n CI

Second review by chrisjsewell on aace50d.

An empty list under a single-valued or undeclared keyword was a
TypeError, because the arity check ran before the items were looked at;
a parser handing back [] for an absent field made apply_test_metadata
raise into the test body, against the documented "empty metadata is not
an error". Empty items are dropped first now, nothing left means nothing
written, and only then does the list-or-single rule apply.

The pytest extra is back at >=7.0, the widest range the plugin's use of
pytest allows, with an honest comment: pytest before 7.3.2 does not run
on Python 3.12 by itself, which is pytest's floor, not this one's. A
plugin_floor nox session, wired into CI as two jobs, runs the plugin's
tests on the oldest pytest of each Python -- 7.0.1 on 3.11, 7.3.2 on
3.12 -- so the claim is measured on every push.

Smaller things from the same review: pytest_configure pushes the outer
model as its first statement, so an inner session that fails to configure
(a bad ini line) pops its own entry and not the outer one; the xunit2
notice is issued on the controller only, not once more per xdist worker;
a bad property shape on a case whose fixture also failed at setup is
appended to that error instead of replacing it; the two reserved
user_properties names of the location override are documented, on the
page and in the docstring.

Two documented decisions. A score_pytest call site may keep passing
record_xml_attribute, but should drop the fixture from the test's
signature: requesting pytest's fixture is what draws its
experimental-feature notice, a setup error under -W error, and the plugin
neither requests it nor filters it any more, so that request and its
notice are the test's own business -- a test pins the strict-policy
behaviour. And a skipped test keeps its requirement links, next to
result="skipped": the plugin records the link, the project gives it its
meaning, and a traceability query that reads "verified" off a link has to
look at result as well.
@ubmarco

ubmarco commented Sep 10, 2026

Copy link
Copy Markdown
Member Author

@chrisjsewell on your second review: round two is in as 417cd34, and CI grew two jobs you may want to look at first.

The two before approval

record_xml_attribute under a strict policy. You are right that the previous head only hid this: our fixture asked for pytest's fixture first, inside catch_warnings, and the test's own request then got the cached value without the notice. We took your zero-code route, and I would rather say why than just do it. That fixture is pytest's, and with this plugin nothing needs it any more, because the location override travels with the report. So the page now says: keep passing the argument if you like, it is accepted and ignored, but drop the fixture from the signature, because requesting it is what draws the experimental notice, and under -W error that request is the setup error. The plugin neither requests it nor filters it, so the test owns both the request and the notice. The compat test asserts the notice on a normal run, and a second test pins the strict run: one error, pytest's message.

Empty lists under single-valued or undeclared keywords. Fixed the way you described. Empty items go first, nothing left means nothing written, and only then the list-or-single rule. test_type=[], Owner=[None, ""] and a metadata dict of nothing but []s write nothing; test_type=["a"] is still the TypeError. _empty and _serialise now agree on these shapes too.

At your discretion, all taken

  • The outer model is pushed as the first statement of pytest_configure. The nested-session test has a sibling with a broken inner ini: the inner run exits 4, and the decorated test after it still writes its properties.
  • The xunit2 notice is issued on the controller only. A test runs -n 1 with xunit2 and counts one warning.
  • A bad property shape on a case whose fixture also failed at setup is appended to the fixture's error instead of replacing it; the test looks for both messages.
  • The two reserved names, sphinxcontrib.test_reports:file and sphinxcontrib.test_reports:line, are on the page and in the docstring, with the sentence that a property recorded under either is read as an override.
  • The body says 63 tests, which is what --collect-only says.
  • The pytest floor. "7.4 is the first pytest that runs on 3.12" was false, as you said; 7.3.2 is, by classifier and changelog. Marco wants the widest range we can stand behind, so the extra is pytest>=7.0 again, which is where everything the plugin uses exists, and the comment now says that anything older than 7.3.2 not running on 3.12 is pytest's floor, not ours. To make that measured rather than stated, a plugin_floor nox session runs the plugin's tests on the oldest pytest of each Python, 7.0.1 on 3.11 and 7.3.2 on 3.12, and CI runs both cells. Those are the two new jobs.

Your observation about skipped tests is on the page now. We keep the links: the plugin records that this test claims to verify REQ_1, with result="skipped" right next to it, and what that is worth is the project's rule to make. The sentence says a traceability query that reads "verified" off a link has to look at result too, or a test that never ran counts as verification.

The two deferrals stand: versionadded at release time, and the TOOLCHAIN_FREE_TESTS line in whichever of #151 and #159 lands second.

@chrisjsewell chrisjsewell left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Checked 417cd34 by hand, as promised. Approving.

The two items hold. A spec parser that hands back [] for every absent field now writes nothing and the one real value survives (2 passed, FullyVerifies="REQ_9" alone in the XML); the documented call shape without the fixture passes under -W error with the override applied (file="specs/a.rst" line="3"); the old shape that still requests record_xml_attribute is now the test's own notice, and the page says to drop it and why. The empty-items-first order in _serialise is the fix I described, and _empty agrees with it by construction.

The discretionary items I re-read in the diff: the outer model is pushed as the first statement of pytest_configure, the xunit2 notice carries the workerinput guard, a property error is appended to a fixture error rather than replacing it, the two reserved names are on the page and in the docstring, and the skipped-test sentence is the right one. On the floor: pytest>=7.0 with the reason stated as pytest's own 3.12 boundary is honest, and the plugin_floor session makes it measured rather than claimed. Both new CI cells ran the real thing: 63 passed on pytest 7.0.1 / Python 3.11 and on 7.3.2 / 3.12, and all 29 legs are green at this head. Locally: 394 passed on the full suite, 63 in the module with no Sphinx installed, mypy clean, the docs build clean under -W, and the body's count is right.

The two deferrals stand as agreed: versionadded when the release number is decided, and tests/test_pytest_plugin.py into TOOLCHAIN_FREE_TESTS in whichever of #151 and #159 lands second.

@ubmarco
ubmarco merged commit 9652f22 into master Sep 10, 2026
28 checks passed
@ubmarco
ubmarco deleted the str-pytest-plugin branch September 10, 2026 20:31
ubmarco added a commit that referenced this pull request Sep 10, 2026
…xtra

#151 made the pytest plugin the `pytest` extra, so with Sphinx and
sphinx-needs moved to the `sphinx` extra, `pip install
"sphinx-test-reports[pytest]"` gives a test runner the plugin, pytest and
`lxml` -- nothing of the documentation toolchain. Nothing verified that:
the `toolchain_free` session ran the converter's tests only, and
`plugin_floor` installed `.[test]`, which now brings the toolchain back
in.

Both sessions install `.[pytest]` -- what a test runner installs -- and
assert that the toolchain is absent before running. `toolchain_free`
adds the plugin's test module (its three pytest-xdist tests skip there);
`plugin_floor` adds pytest-xdist so those keep running at the floor, and
forwards nox's posargs, which CI passes (`--full-trace`) and it ignored.

The install page lists the three install lines side by side -- the
`sphinx` extra for a documentation project, the `pytest` extra for a
test runner, the bare package for a build action running the command --
and the plugin's page, which said how to enable it but not how to
install it, names the extra. The changelog does too.
ubmarco added a commit that referenced this pull request Sep 10, 2026
… and `[pytest]` install without the toolchain (#159)

Closes #155.

## What changes

`pip install sphinx-test-reports` now installs `lxml` alone. Sphinx and
sphinx-needs move to a `sphinx` extra:

```toml
dependencies = ["lxml"]

[project.optional-dependencies]
sphinx = ["sphinx>=7.4", "sphinx-needs>=6.0.1"]
pytest = ["pytest>=7.0"]   # from #151, unchanged
```

So the install line of a documentation project becomes `pip install
"sphinx-test-reports[sphinx]"`, and a test runner or build action that
only wants `test-reports build needs` no longer pulls the toolchain. The
same goes for the pytest plugin #151 added as the `pytest` extra: `pip
install "sphinx-test-reports[pytest]"` now yields the plugin, pytest and
`lxml`, nothing of the toolchain. `[test]` gains the two floors it used
to inherit from the core list; `[docs]` already had them; the nox matrix
pins both explicitly and is unaffected.

The install page lists the three install lines side by side (`sphinx`
extra for a documentation project, `pytest` extra for a test runner,
bare package for a build action running the command), and the plugin's
page, which said how to enable the plugin but not how to install it,
names the extra.

## The version-constraint gap

An extra is opt-in, so a project that keeps installing the bare package
next to an already-installed older toolchain never shows pip the floors.
The lazy `setup` in `__init__.py` now enforces them at load time, before
the extension is imported (an outdated sphinx-needs may well import and
only fail later, inside a directive):

```
Extension error:
Could not load extension sphinxcontrib.test_reports: sphinx-needs 5.1.0 is installed, but sphinx-needs>=6.0.1 is required. Install the Sphinx extension's dependencies with: pip install "sphinx-test-reports[sphinx]"
```

A toolchain that is missing altogether keeps the existing "Could not
import extension" path, which now names the extra too:

```
Extension error:
Could not import extension sphinxcontrib.test_reports; the Sphinx extension's dependencies are an extra, install them with: pip install "sphinx-test-reports[sphinx]" (exception: No module named 'sphinx_needs')
```

One deviation from the sketch in the issue: instead of
`app.require_sphinx((7, 4))` plus a hard-coded sphinx-needs floor, the
new `toolchain` module reads the `sphinx` extra's requirements from the
package's own metadata (`importlib.metadata.requires`) and compares each
installed version against its specifier. The floors then live in
`pyproject.toml` alone -- #150 just moved them, and a copy in code would
need a lockstep test -- and a test guards that the extra the check reads
is the one `pyproject.toml` declares. `packaging` does the parsing; it
is imported lazily and the check stands down without it, since it
arrives with Sphinx (and pytest) wherever the extension can be loaded. A
distribution that is *not installed* is deliberately left to the import:
the import states it more precisely, and a toolchain importable from a
source tree without metadata must not be refused.

On Sphinx 9 the error appears inside Sphinx's crash report, as the
existing "Known" changelog entry already notes for the configuration
errors; the note now covers this case too.

## Guaranteeing the property in CI

A `toolchain_free` nox session (Python 3.11 and 3.12, new CI job, wired
into `all_good`) installs the package with the dependencies it declares
-- `pip install ".[pytest]"`, no `--no-deps`, so the declared lists
themselves are what is tested -- asserts that neither `sphinx`,
`sphinx_needs` nor `docutils` is importable, and runs the converter's
and the pytest plugin's test modules with `-m "not toolchain"`. The
tests that need a build carry the new `toolchain` mark (the issue
suggested `needs_sphinx`; `sphinx` itself is taken by Sphinx's own
testing fixtures). `tests/conftest.py` loads `sphinx.testing.fixtures`
only where Sphinx is importable, and one top-level import of the
package's Sphinx-dependent `exceptions` module in
`test_project_config.py` became local to the marked test that uses it.

#151's `plugin_floor` session (the plugin's tests on the oldest pytest
per Python) installed `.[test]`, which after this change brings the
toolchain back in. It now installs `.[pytest]` plus pytest-xdist and
runs the same toolchain-absent guard, so the floor run doubles as the
isolation proof on old pytest. It also forwards nox's posargs, which CI
passes (`--full-trace`) and it ignored.

The mypy CI job installs with `uv sync --group dev --extra sphinx`;
without the extra it would no longer see Sphinx and docutils.

## Open questions from the issue, as resolved here

- **Extra name:** `sphinx`, as proposed. It matches the
`[docs]`/`[test]` style and the `[pytest]` extra from #151.
- **1.5.0 or 2.0:** not decided here. The changelog carries a
"Breaking:" entry spelling out the install-line change; `version` is
untouched, so the release can decide.
- **Two-distribution split:** not done, per the suggested path.
- **`[pytest]` extra:** on master since #151; this PR is what makes it
toolchain-free, and the second commit proves and documents that.

## Verification

- `pytest -n auto tests/`: 407 passed (13 new in
`tests/test_toolchain.py`)
- `nox -s toolchain_free-3.11 toolchain_free-3.12`: 308 passed, 3
skipped (the pytest-xdist tests; xdist is not installed there), 24
`toolchain`-marked deselected, in a venv holding the package, `lxml` and
pytest only; the toolchain-absent guard passes
- `nox -s "plugin_floor(python='3.11', pytest_version='7.0.1')"
"plugin_floor(python='3.12', pytest_version='7.3.2')"`: 63 passed each,
toolchain absent
- `mypy`: clean (the new module is strict-checked, not excluded)
- `pre-commit run --all-files`: clean
- `sphinx-build -W` of the docs: clean

## Notes for the reviewer

- Rebased onto #151 (`pyproject.toml`, `noxfile.py`, `ci.yaml`,
`tests/conftest.py` conflicted; all resolved as a union of both sides).
The first commit is the original change; the second is the
plugin-isolation follow-up and can be reviewed, or dropped, on its own.
- `docs/install.rst` also fixes two pre-existing typos on the page it
rewrites ("must to be", "can be find").
pull Bot pushed a commit to boschglobal/sphinx-needs that referenced this pull request Sep 15, 2026
…locks/sphinx-test-reports#151)

Based on master. Independent of the needs.json converter (useblocks/sphinx-test-reports#148, merged);
rebased on top of it.

## Why

A stock pytest run writes a JUnit XML this extension can only partly
use. Under pytest's default `junit_family = xunit2`, no `<testcase>`
carries the `file`/`line` attributes that give a test case its source
location (`tr_source_file_option`/`tr_source_line_option`, and the
deterministic ID of `tr_deterministic_case_ids`). Under `xunit1` pytest
writes them, but counts the line from 0, keeps Bazel's runfiles prefix,
and has no way to point a case at the file that drove it. And nothing
records the requirements a test verifies.

## What

`-p sphinxcontrib.test_reports.pytest_plugin`, with `junit_family =
xunit1` (warned about otherwise), does two things to every `<testcase>`,
through hooks on the test reports rather than fixtures, so a case
skipped or erroring during setup gets the same treatment and
pytest-xdist works:

- the source location becomes the one an editor shows: line counted from
1, path relative to the rootdir with Bazel's `_main/` runfiles prefix
cut at a whole path component;
- the properties of an `add_test_properties(...)` decorator (or, for
file-driven parameterised tests, the `apply_test_metadata(...)` runtime
helper, which can also point the case at the driving file) are written
as `<properties>`.

Which properties exist is configuration, not code. The
`test_reports_properties` ini option declares them, one `keyword [=
XmlName] [, list]` per line: the keyword a test writes, the `<property>`
name it is written under, and whether it takes a list (joined with `",
"`, which `tr_property_link_types` splits again). A keyword not declared
is written under its own name with a single value; a list under it, a
list item that is not a string, or bytes, is a `TypeError` naming the
option, so nothing is lost to a Python `repr` silently. The plugin ships
no metamodel of its own.

S-CORE's model is the worked example in the docs:

```ini
[pytest]
addopts = -p sphinxcontrib.test_reports.pytest_plugin
junit_family = xunit1
test_reports_properties =
    partially_verifies = PartiallyVerifies, list
    fully_verifies = FullyVerifies, list
    test_type = TestType
    derivation_technique = DerivationTechnique
```

Ported from S-CORE docs-as-code's `score_pytest` attribute plugin: same
two entry-point names, and with that example the XML comes out the same,
so tests written against that plugin keep working when they import from
here. Four deliberate differences in the XML: a case skipped or erroring
at setup carries its location and properties here and pytest's stock
values there; the properties keep the order of the keywords; the Bazel
prefix is cut at a whole `_main` component only; and a `file` handed to
`apply_test_metadata` is cut the same way. Two of that plugin's rules
are not ported, as process rules of that project rather than properties
of the data: the `TestType`/`DerivationTechnique` vocabularies are
documented, not enforced, and a decorated test needs no docstring.

The package `__init__` resolves `setup` lazily (PEP 562, useblocks/sphinx-test-reports#145), so
loading the plugin into a pytest process imports neither Sphinx nor
docutils; a test asserts it. A per-module mypy override lifts only the
two `Any` checks the pytest API needs. A `pytest` extra (`pytest>=7.0`,
where everything the plugin uses exists) pulls pytest in; it is not a
dependency of the extension. pytest before 7.3.2 does not run on Python
3.12 by itself, so a `plugin_floor` nox session, wired into CI, runs the
plugin's tests on the oldest pytest of each Python: 7.0.1 on 3.11, 7.3.2
on 3.12.

## Tests

`tests/test_pytest_plugin.py`, 63 tests through `pytester`, each inner
pytest a fresh subprocess: locations (a decorated function is located at
its first decorator, as pytest does; skipped and setup-erroring cases
included), the properties written from ini and pyproject declarations,
the option grammar and its errors, the value rules, marker merging
across class and function, the runtime helper's location override,
nested in-process sessions (one failing to configure), a decorator
applied before `pytest_configure`, a bad shape next to a fixture error,
`--strict-markers`, the `xunit2` start-up notice under strict warning
policies, `-p no:junitxml`, `legacy`, an xdist run, the Bazel path cut,
and that importing the plugin imports no Sphinx. Verified on pytest
7.0.1 and 7.4.4 (Python 3.11), 7.3.2, 7.4.4 and 8.4.2 (3.12) and 9.1.1
(3.14).
pull Bot pushed a commit to boschglobal/sphinx-needs that referenced this pull request Sep 15, 2026
… and `[pytest]` install without the toolchain (useblocks/sphinx-test-reports#159)

Closes useblocks/sphinx-test-reports#155.

## What changes

`pip install sphinx-test-reports` now installs `lxml` alone. Sphinx and
sphinx-needs move to a `sphinx` extra:

```toml
dependencies = ["lxml"]

[project.optional-dependencies]
sphinx = ["sphinx>=7.4", "sphinx-needs>=6.0.1"]
pytest = ["pytest>=7.0"]   # from useblocks/sphinx-test-reports#151, unchanged
```

So the install line of a documentation project becomes `pip install
"sphinx-test-reports[sphinx]"`, and a test runner or build action that
only wants `test-reports build needs` no longer pulls the toolchain. The
same goes for the pytest plugin useblocks/sphinx-test-reports#151 added as the `pytest` extra: `pip
install "sphinx-test-reports[pytest]"` now yields the plugin, pytest and
`lxml`, nothing of the toolchain. `[test]` gains the two floors it used
to inherit from the core list; `[docs]` already had them; the nox matrix
pins both explicitly and is unaffected.

The install page lists the three install lines side by side (`sphinx`
extra for a documentation project, `pytest` extra for a test runner,
bare package for a build action running the command), and the plugin's
page, which said how to enable the plugin but not how to install it,
names the extra.

## The version-constraint gap

An extra is opt-in, so a project that keeps installing the bare package
next to an already-installed older toolchain never shows pip the floors.
The lazy `setup` in `__init__.py` now enforces them at load time, before
the extension is imported (an outdated sphinx-needs may well import and
only fail later, inside a directive):

```
Extension error:
Could not load extension sphinxcontrib.test_reports: sphinx-needs 5.1.0 is installed, but sphinx-needs>=6.0.1 is required. Install the Sphinx extension's dependencies with: pip install "sphinx-test-reports[sphinx]"
```

A toolchain that is missing altogether keeps the existing "Could not
import extension" path, which now names the extra too:

```
Extension error:
Could not import extension sphinxcontrib.test_reports; the Sphinx extension's dependencies are an extra, install them with: pip install "sphinx-test-reports[sphinx]" (exception: No module named 'sphinx_needs')
```

One deviation from the sketch in the issue: instead of
`app.require_sphinx((7, 4))` plus a hard-coded sphinx-needs floor, the
new `toolchain` module reads the `sphinx` extra's requirements from the
package's own metadata (`importlib.metadata.requires`) and compares each
installed version against its specifier. The floors then live in
`pyproject.toml` alone -- useblocks/sphinx-test-reports#150 just moved them, and a copy in code would
need a lockstep test -- and a test guards that the extra the check reads
is the one `pyproject.toml` declares. `packaging` does the parsing; it
is imported lazily and the check stands down without it, since it
arrives with Sphinx (and pytest) wherever the extension can be loaded. A
distribution that is *not installed* is deliberately left to the import:
the import states it more precisely, and a toolchain importable from a
source tree without metadata must not be refused.

On Sphinx 9 the error appears inside Sphinx's crash report, as the
existing "Known" changelog entry already notes for the configuration
errors; the note now covers this case too.

## Guaranteeing the property in CI

A `toolchain_free` nox session (Python 3.11 and 3.12, new CI job, wired
into `all_good`) installs the package with the dependencies it declares
-- `pip install ".[pytest]"`, no `--no-deps`, so the declared lists
themselves are what is tested -- asserts that neither `sphinx`,
`sphinx_needs` nor `docutils` is importable, and runs the converter's
and the pytest plugin's test modules with `-m "not toolchain"`. The
tests that need a build carry the new `toolchain` mark (the issue
suggested `needs_sphinx`; `sphinx` itself is taken by Sphinx's own
testing fixtures). `tests/conftest.py` loads `sphinx.testing.fixtures`
only where Sphinx is importable, and one top-level import of the
package's Sphinx-dependent `exceptions` module in
`test_project_config.py` became local to the marked test that uses it.

useblocks/sphinx-test-reports#151's `plugin_floor` session (the plugin's tests on the oldest pytest
per Python) installed `.[test]`, which after this change brings the
toolchain back in. It now installs `.[pytest]` plus pytest-xdist and
runs the same toolchain-absent guard, so the floor run doubles as the
isolation proof on old pytest. It also forwards nox's posargs, which CI
passes (`--full-trace`) and it ignored.

The mypy CI job installs with `uv sync --group dev --extra sphinx`;
without the extra it would no longer see Sphinx and docutils.

## Open questions from the issue, as resolved here

- **Extra name:** `sphinx`, as proposed. It matches the
`[docs]`/`[test]` style and the `[pytest]` extra from useblocks/sphinx-test-reports#151.
- **1.5.0 or 2.0:** not decided here. The changelog carries a
"Breaking:" entry spelling out the install-line change; `version` is
untouched, so the release can decide.
- **Two-distribution split:** not done, per the suggested path.
- **`[pytest]` extra:** on master since useblocks/sphinx-test-reports#151; this PR is what makes it
toolchain-free, and the second commit proves and documents that.

## Verification

- `pytest -n auto tests/`: 407 passed (13 new in
`tests/test_toolchain.py`)
- `nox -s toolchain_free-3.11 toolchain_free-3.12`: 308 passed, 3
skipped (the pytest-xdist tests; xdist is not installed there), 24
`toolchain`-marked deselected, in a venv holding the package, `lxml` and
pytest only; the toolchain-absent guard passes
- `nox -s "plugin_floor(python='3.11', pytest_version='7.0.1')"
"plugin_floor(python='3.12', pytest_version='7.3.2')"`: 63 passed each,
toolchain absent
- `mypy`: clean (the new module is strict-checked, not excluded)
- `pre-commit run --all-files`: clean
- `sphinx-build -W` of the docs: clean

## Notes for the reviewer

- Rebased onto useblocks/sphinx-test-reports#151 (`pyproject.toml`, `noxfile.py`, `ci.yaml`,
`tests/conftest.py` conflicted; all resolved as a union of both sides).
The first commit is the original change; the second is the
plugin-isolation follow-up and can be reviewed, or dropped, on its own.
- `docs/install.rst` also fixes two pre-existing typos on the page it
rewrites ("must to be", "can be find").
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants