From e1a36d623d2e1e67e4972b36c64973828234bbab Mon Sep 17 00:00:00 2001 From: Tobias Kaestner Date: Sun, 27 Sep 2026 06:40:34 +0200 Subject: [PATCH 1/5] fix: twister: correlate results to test cases by (suite, function) The test report found the spec test case for a twister result by function name alone. ZTEST function names are not unique across suites -- some 300 are reused in the Zephyr test tree -- so a result silently attached to whichever test case of that name was read last. load_spec_lookup now returns a SpecLookup keyed by (suite, test_function). It tries the name as reported and with ztest's test_ prefix, and falls back to the bare name only when exactly one test case carries it. The four inline lookups in test_module.py become one helper, which warns and links nothing when a result has no match or several, naming the candidates. That also covers one (suite, function) pair documented in several test modules, which the Zephyr tree has 26 of; resolving those needs the test module and is left for later. Five new tests drive load_spec_lookup and _build_results_rst together. Against the unchanged code 4 fail; the fifth pins the unique-name fallback, which already worked. Suite 158 -> 164. The safety test report is unchanged at queue/fifo scope: 90 results, same links. The acceptance suite (zdocs-tests) was not available and has not been run. Co-Authored-By: Claude Opus 5.5 Signed-off-by: Tobias Kaestner --- .../_tests/test_needtype_mapping.py | 30 ++-- sphinx/_extensions/_tests/test_test_module.py | 140 ++++++++++++++++-- .../_extensions/_tests/test_twister_reader.py | 16 +- sphinx/_extensions/test_module.py | 26 +++- sphinx/_extensions/twister_reader.py | 68 +++++++-- 5 files changed, 223 insertions(+), 57 deletions(-) diff --git a/sphinx/_extensions/_tests/test_needtype_mapping.py b/sphinx/_extensions/_tests/test_needtype_mapping.py index 470f6c3..5b5cfa3 100644 --- a/sphinx/_extensions/_tests/test_needtype_mapping.py +++ b/sphinx/_extensions/_tests/test_needtype_mapping.py @@ -43,6 +43,7 @@ import rst_builders as rb import test_module as tm +from twister_reader import SpecLookup # Override names chosen to share NO substring with the engine defaults # ("test_case", "test_procedure", "test_result", "verifies", "result_of", @@ -86,15 +87,14 @@ def _base_result(**kwargs): def _spec_lookup_single(): - return { - "test_queue_put": { - "id": "TSPEC-QUEUE-API-001", - "test_module": "tests/kernel/queue", - "suite": "kernel.queue", - "suite_title": "", - "req_ids": [], - }, - } + return SpecLookup([{ + "id": "TSPEC-QUEUE-API-001", + "test_function": "test_queue_put", + "test_module": "tests/kernel/queue", + "suite": "kernel.queue", + "suite_title": "", + "req_ids": [], + }]) # --------------------------------------------------------------------------- @@ -194,12 +194,12 @@ def test_summary_table_filter_honours_result_type_mapping_single_module(): def test_summary_table_filter_honours_result_type_mapping_multi_module(): - lookup = { - "test_a": {"id": "A", "test_module": "mod_a", "suite": "s", - "suite_title": "", "req_ids": []}, - "test_b": {"id": "B", "test_module": "mod_b", "suite": "s", - "suite_title": "", "req_ids": []}, - } + lookup = SpecLookup([ + {"id": "A", "test_function": "test_a", "test_module": "mod_a", "suite": "s", + "suite_title": "", "req_ids": []}, + {"id": "B", "test_function": "test_b", "test_module": "mod_b", "suite": "s", + "suite_title": "", "req_ids": []}, + ]) grouped = { ("s", "a"): [_base_result(function="a")], ("s", "b"): [_base_result(function="b")], diff --git a/sphinx/_extensions/_tests/test_test_module.py b/sphinx/_extensions/_tests/test_test_module.py index b83b61c..5a26172 100644 --- a/sphinx/_extensions/_tests/test_test_module.py +++ b/sphinx/_extensions/_tests/test_test_module.py @@ -4,6 +4,7 @@ """Tests for test_module.py — helper functions and Sphinx directive integration.""" import io +import json import xml.etree.ElementTree as ET from pathlib import Path from types import SimpleNamespace @@ -13,6 +14,7 @@ from conftest import FIXTURES from docutils import nodes from docutils.utils import Reporter +from twister_reader import SpecLookup _ROOTS = Path(__file__).parent / "roots" @@ -40,23 +42,29 @@ def _make_result(**kwargs): return defaults -def _spec_lookup(): - return { - "test_queue_put": { +def _spec_cases(): + return [ + { "id": "TSPEC-QUEUE-API-001", + "test_function": "test_queue_put", "test_module": "tests/kernel/queue", "suite": "kernel.queue", "suite_title": "Queue API ZTest suite", "req_ids": ["zep-srs-20-1"], }, - "test_queue_get": { + { "id": "TSPEC-QUEUE-API-002", + "test_function": "test_queue_get", "test_module": "tests/kernel/queue", "suite": "kernel.queue", "suite_title": "Queue API ZTest suite", "req_ids": [], }, - } + ] + + +def _spec_lookup(*extra): + return SpecLookup([*_spec_cases(), *extra]) # --------------------------------------------------------------------------- @@ -114,7 +122,7 @@ def test_build_results_rst_suite_heading_titlecase_fallback(): suite_order = ["queue_api_1cpu"] func_order = {"queue_api_1cpu": ["queue_put"]} grouped = {("queue_api_1cpu", "queue_put"): [_make_result(suite="queue_api_1cpu")]} - lookup = {"test_queue_put": {**_spec_lookup()["test_queue_put"], "suite_title": ""}} + lookup = SpecLookup([{**_spec_cases()[0], "suite_title": ""}]) lines = tm._build_results_rst(suite_order, func_order, grouped, lookup) assert any("Queue Api 1Cpu" in line for line in lines) @@ -147,16 +155,14 @@ def test_build_summary_table_single_module_filter(): def test_build_summary_table_multi_module_generic_filter(): - lookup = { - **_spec_lookup(), - "test_other": { - "id": "TSPEC-OTHER-001", - "test_module": "tests/kernel/other", - "suite": "s", - "suite_title": "", - "req_ids": [], - }, - } + lookup = _spec_lookup({ + "id": "TSPEC-OTHER-001", + "test_function": "test_other", + "test_module": "tests/kernel/other", + "suite": "s", + "suite_title": "", + "req_ids": [], + }) grouped = { ("s", "queue_put"): [_make_result()], ("s", "other"): [_make_result(function="other")], @@ -470,3 +476,105 @@ def test_testmodule_directive_missing_xml_dir_hard_fails(tmp_path): assert len(result) == 1 assert isinstance(result[0], nodes.system_message) assert result[0]["level"] == Reporter.ERROR_LEVEL + + +# --------------------------------------------------------------------------- +# spec <-> result correlation across suites (W1) +# +# ZTEST function names are not unique across suites — some 300 are reused in +# the Zephyr test tree (`test_sleep`, `test_remove`, ...). A lookup keyed by +# the bare function name attaches every such result to whichever test case of +# that name was read last. These tests drive load_spec_lookup and +# _build_results_rst together, so they pin the behaviour end to end. +# --------------------------------------------------------------------------- + +def _write_spec(tmp_path, cases): + """Write a minimal spec needs.json; `cases` is [(id, suite, test_function)].""" + needs = { + need_id: { + "id": need_id, "type": "test_case", "test_function": fn, "suite": suite, + "suite_title": f"{suite} title", "test_module": f"tests/{suite}", "verifies": [], + } + for need_id, suite, fn in cases + } + p = tmp_path / "needs.json" + p.write_text(json.dumps({"current_version": "1.0", "versions": {"1.0": {"needs": needs}}})) + return p + + +def _result_of(lines): + """Map each emitted test_result need id to the spec id in its :result_of:.""" + links, need_id = {}, None + for line in lines: + line = line.strip() + if line.startswith(":id:"): + need_id = line.split(":id:")[1].strip() + elif line.startswith(":result_of:"): + links[need_id] = line.split(":result_of:")[1].strip() + return links + + +def _report_lines(spec_path, results): + suite_order, func_order, grouped = tm._group_results(results) + lookup = tm.load_spec_lookup(spec_path) + return tm._build_results_rst(suite_order, func_order, grouped, lookup) + + +def _warnings(monkeypatch): + seen = [] + monkeypatch.setattr(tm.logger, "warning", lambda msg, *a, **k: seen.append(msg)) + return seen + + +def test_results_link_to_their_own_suites_test_case(tmp_path): + spec = _write_spec(tmp_path, [ + ("TSPEC-A-001", "suite_a", "test_shared"), + ("TSPEC-B-001", "suite_b", "test_shared"), + ]) + results = [ + _make_result(suite="suite_a", function="shared", scenario="sc.a"), + _make_result(suite="suite_b", function="shared", scenario="sc.b"), + ] + links = _result_of(_report_lines(spec, results)) + assert sorted(links.values()) == ["TSPEC-A-001", "TSPEC-B-001"] + assert all(need_id.endswith(spec_id) for need_id, spec_id in links.items()) + + +def test_suite_heading_comes_from_own_suite(tmp_path): + spec = _write_spec(tmp_path, [ + ("TSPEC-A-001", "suite_a", "test_shared"), + ("TSPEC-B-001", "suite_b", "test_shared"), + ]) + lines = _report_lines(spec, [_make_result(suite="suite_a", function="shared")]) + assert "suite_a title" in lines + assert "suite_b title" not in lines + + +def test_unique_bare_name_still_matches_across_suite_mismatch(tmp_path): + spec = _write_spec(tmp_path, [("TSPEC-A-001", "suite_a", "test_only_here")]) + lines = _report_lines(spec, [_make_result(suite="", function="only_here")]) + assert list(_result_of(lines).values()) == ["TSPEC-A-001"] + + +def test_ambiguous_bare_name_warns_and_links_nothing(tmp_path, monkeypatch): + warnings = _warnings(monkeypatch) + spec = _write_spec(tmp_path, [ + ("TSPEC-A-001", "suite_a", "test_shared"), + ("TSPEC-B-001", "suite_b", "test_shared"), + ]) + lines = _report_lines(spec, [_make_result(suite="suite_c", function="shared")]) + assert _result_of(lines) == {} + assert any("ambiguous" in w and "TSPEC-A-001" in w and "TSPEC-B-001" in w for w in warnings) + + +def test_duplicate_suite_and_function_warns_and_links_nothing(tmp_path, monkeypatch): + # The same (suite, function) pair in two test modules — 26 such pairs exist + # in the Zephyr tree. Neither may win silently. + warnings = _warnings(monkeypatch) + spec = _write_spec(tmp_path, [ + ("TSPEC-A-001", "basic", "test_shared"), + ("TSPEC-A-002", "basic", "test_shared"), + ]) + lines = _report_lines(spec, [_make_result(suite="basic", function="shared")]) + assert _result_of(lines) == {} + assert any("ambiguous" in w for w in warnings) diff --git a/sphinx/_extensions/_tests/test_twister_reader.py b/sphinx/_extensions/_tests/test_twister_reader.py index 7731953..5dcb264 100644 --- a/sphinx/_extensions/_tests/test_twister_reader.py +++ b/sphinx/_extensions/_tests/test_twister_reader.py @@ -78,20 +78,26 @@ def test_parse_results_strips_test_prefix(): # --------------------------------------------------------------------------- -def test_load_spec_lookup_keys_are_functions(): +def test_load_spec_lookup_finds_by_suite_and_function(): lookup = tw.load_spec_lookup(NEEDS_JSON) - assert "test_queue_put" in lookup - assert "test_queue_get" in lookup + assert lookup.find("kernel.queue", "test_queue_put")["id"] == "TSPEC-QUEUE-API-001" + assert lookup.find("kernel.queue", "test_queue_get")["id"] == "TSPEC-QUEUE-API-002" + + +def test_load_spec_lookup_accepts_stripped_test_prefix(): + # twister strips ztest's `test_` prefix from the function it reports + lookup = tw.load_spec_lookup(NEEDS_JSON) + assert lookup.find("kernel.queue", "queue_put")["id"] == "TSPEC-QUEUE-API-001" def test_load_spec_lookup_req_ids(): lookup = tw.load_spec_lookup(NEEDS_JSON) - assert lookup["test_queue_put"]["req_ids"] == ["zep-srs-20-1"] + assert lookup.find("kernel.queue", "queue_put")["req_ids"] == ["zep-srs-20-1"] def test_load_spec_lookup_suite_title(): lookup = tw.load_spec_lookup(NEEDS_JSON) - assert lookup["test_queue_put"]["suite_title"] == "Queue API Tests" + assert lookup.find("kernel.queue", "queue_put")["suite_title"] == "Queue API Tests" # --------------------------------------------------------------------------- diff --git a/sphinx/_extensions/test_module.py b/sphinx/_extensions/test_module.py index bcca573..8d4cf52 100644 --- a/sphinx/_extensions/test_module.py +++ b/sphinx/_extensions/test_module.py @@ -206,24 +206,38 @@ def _group_results(results): return suite_order, func_order, grouped +def _spec_info(spec_lookup, suite, fn): + """The spec test case for a result's (suite, fn), or None — warns when none or several.""" + hits = spec_lookup.candidates(suite, fn) + if len(hits) == 1: + return hits[0] + if not hits: + logger.warning(f"testreport: '{suite}.{fn}' not in spec needs.json — skipped") + else: + ids = ", ".join(sorted(h["id"] for h in hits)) + logger.warning( + f"testreport: '{suite}.{fn}' is ambiguous in spec needs.json ({ids}) — skipped" + ) + return None + + def _build_results_rst(suite_order, func_order, grouped, spec_lookup, need_names=None): """Build RST lines for all test_result needs, grouped into one section per suite.""" lines = [] for suite in suite_order: suite_title = next( ( - (spec_lookup.get(fn) or spec_lookup.get("test_" + fn) or {}).get("suite_title") + info["suite_title"] for fn in func_order[suite] - if (spec_lookup.get(fn) or spec_lookup.get("test_" + fn) or {}).get("suite_title") + if (info := spec_lookup.find(suite, fn)) and info.get("suite_title") ), None, ) heading = suite_title or suite.replace("_", " ").title() lines += [heading, "-" * len(heading), ""] for fn in func_order[suite]: - info = spec_lookup.get(fn) or spec_lookup.get("test_" + fn) + info = _spec_info(spec_lookup, suite, fn) if info is None: - logger.warning(f"testreport: '{fn}' not in spec needs.json — skipped") continue for r in grouped[(suite, fn)]: lines += build_result_rst( @@ -236,8 +250,8 @@ def _build_results_rst(suite_order, func_order, grouped, spec_lookup, need_names def _build_summary_table_rst(grouped, spec_lookup, need_names=None): """Build RST lines for the result summary needtable.""" modules = sorted({ - (spec_lookup.get(fn) or spec_lookup.get("test_" + fn) or {}).get("test_module", "") - for _, fn in grouped + (spec_lookup.find(suite, fn) or {}).get("test_module", "") + for suite, fn in grouped } - {""}) result_type = _need_name(need_names, "result") tbl_filter = ( diff --git a/sphinx/_extensions/twister_reader.py b/sphinx/_extensions/twister_reader.py index 7bf54a2..6184a47 100644 --- a/sphinx/_extensions/twister_reader.py +++ b/sphinx/_extensions/twister_reader.py @@ -18,6 +18,7 @@ __all__ = [ "parse_twister_results", + "SpecLookup", "load_spec_lookup", "find_handler_log", "load_twister_meta", @@ -83,8 +84,47 @@ def parse_twister_results(xml_path, module_filter=None, exact=False): return results +class SpecLookup: + """The spec's test cases, found by the (suite, function) of a twister result. + + ZTEST function names are not unique across suites (some 300 are reused in + the Zephyr test tree), so the suite is part of the key. Each entry is a dict + with at least `id`, `suite` and `test_function`. + + A result's function has ztest's ``test_`` prefix stripped, while the spec + records the C name as written, so both spellings are tried. When the + result's suite has no such case, the bare function name is used instead — + but only if exactly one case carries it. `candidates()` returns every + match, so an ambiguous one (several suites, or one (suite, function) pair + documented in several test modules) is visible to the caller; `find()` + returns a case only when there is exactly one. + """ + + def __init__(self, entries=()): + self._by_key = {} + self._by_name = {} + for info in entries: + fn = info["test_function"] + self._by_key.setdefault((info.get("suite", ""), fn), []).append(info) + self._by_name.setdefault(fn, []).append(info) + + def candidates(self, suite, fn): + names = (fn, "test_" + fn) + for name in names: + if (suite, name) in self._by_key: + return self._by_key[(suite, name)] + for name in names: + if name in self._by_name: + return self._by_name[name] + return [] + + def find(self, suite, fn): + hits = self.candidates(suite, fn) + return hits[0] if len(hits) == 1 else None + + def load_spec_lookup(json_path, need_names=None): - """Read spec needs.json; return {test_function: {id, test_module, suite, req_ids}}. + """Read spec needs.json; return a SpecLookup of its test cases. `need_names` is the same role->name mapping `rst_builders.py` emitters take (`testmodule_need_types`/`testmodule_need_links`, merged by the @@ -103,20 +143,18 @@ def load_spec_lookup(json_path, need_names=None): needs = versions.get(current, {}).get("needs", {}) case_type = _need_name(need_names, "case") verifies_link = _need_name(need_names, "verifies") - lookup = {} - for need_id, need in needs.items(): - if need.get("type") != case_type: - continue - fn = need.get("test_function", "") - if fn: - lookup[fn] = { - "id": need_id, - "test_module": need.get("test_module", ""), - "suite": need.get("suite", ""), - "suite_title": need.get("suite_title", ""), - "req_ids": need.get(verifies_link, []), - } - return lookup + return SpecLookup( + { + "id": need_id, + "test_function": need["test_function"], + "test_module": need.get("test_module", ""), + "suite": need.get("suite", ""), + "suite_title": need.get("suite_title", ""), + "req_ids": need.get(verifies_link, []), + } + for need_id, need in needs.items() + if need.get("type") == case_type and need.get("test_function") + ) def _out_dir_segment(test_path): From 82f372da615c33230fa421cf36e05d4e43c44c9c Mon Sep 17 00:00:00 2001 From: Tobias Kaestner Date: Tue, 29 Sep 2026 10:28:43 +0200 Subject: [PATCH 2/5] fix: twister: re-read the report when its inputs change testreport reads the twister XML and the spec's needs.json, and twisterinfo reads twister.json. All of them are outside the Sphinx source tree, and none were noted as dependencies. After a new twister run into the same ZDOCS_TWISTER_OUT, an incremental build kept the old report, or the "not found" one, and published it without a warning. Reproduced in safety-toolbox: 0 results before `west twister`, still 0 after. A Sphinx dependency alone is not enough. Sphinx re-reads a document only when a dependency is missing or NEWER than the document's last read. Twister output often arrives OLDER than that: a CI artifact, a cache restored with its timestamps, or a copy that keeps them. zdocs-tests step 31 reproduces it with shutil.copy2. Note each input with env.note_dependency, before the existence checks, so a report built ahead of its test run is re-read until the output appears. Also record each input's signature (mtime_ns, size, or None when missing) per document while it is read. An env-get-outdated handler re-reads any document whose signature differs. env-purge-doc and env-merge-info keep the record per document under parallel reads. Co-Authored-By: Claude Opus 5.5 Signed-off-by: Tobias Kaestner --- sphinx/_extensions/_tests/test_test_module.py | 75 ++++++++++++++++++- sphinx/_extensions/test_module.py | 69 +++++++++++++++++ 2 files changed, 143 insertions(+), 1 deletion(-) diff --git a/sphinx/_extensions/_tests/test_test_module.py b/sphinx/_extensions/_tests/test_test_module.py index 5a26172..e076350 100644 --- a/sphinx/_extensions/_tests/test_test_module.py +++ b/sphinx/_extensions/_tests/test_test_module.py @@ -5,6 +5,7 @@ """Tests for test_module.py — helper functions and Sphinx directive integration.""" import io import json +import os import xml.etree.ElementTree as ET from pathlib import Path from types import SimpleNamespace @@ -362,6 +363,16 @@ def test_testreport_directive_generates_result_ids(app): assert "TR-qemu-cortex-m3-ti-lm3s6965-kernel-queue-TSPEC-QUEUE-API-002" in html +@pytest.mark.sphinx("html", srcdir=str(_ROOTS / "test-testreport")) +def test_testreport_directive_notes_its_inputs_as_dependencies(app): + # Without these, an incremental build after a new twister run into the + # same directory keeps publishing the old report. + app.build() + deps = {str(Path(app.srcdir) / d) for d in app.env.dependencies["index"]} + assert str(Path(app.config.twister_output_dir) / "twister_report.xml") in deps + assert app.config.needs_external_needs[0]["json_path"] in deps + + @pytest.mark.sphinx("html", srcdir=str(_ROOTS / "test-testreport")) def test_testreport_directive_suite_heading(app): app.build() @@ -417,7 +428,13 @@ def test_testmodule_directive_procedure_ids(app): # --------------------------------------------------------------------------- def _fake_env(config, docname="index"): - return SimpleNamespace(app=SimpleNamespace(config=SimpleNamespace(**config)), docname=docname) + noted = [] + return SimpleNamespace( + app=SimpleNamespace(config=SimpleNamespace(**config)), + docname=docname, + noted=noted, + note_dependency=noted.append, + ) def test_testreport_directive_missing_xml_soft_fails(tmp_path): @@ -437,6 +454,10 @@ def test_testreport_directive_missing_xml_soft_fails(tmp_path): assert len(result) == 1 assert isinstance(result[0], nodes.paragraph) assert "not found" in result[0].astext() + # Still a dependency: Sphinx re-reads the report once the file appears. + env = directive.state.document.settings.env + assert str(tmp_path / "twister_report.xml") in env.noted + assert str(NEEDS / "needs.json") in env.noted def test_twisterinfo_directive_missing_json_soft_fails(tmp_path): @@ -453,6 +474,58 @@ def test_twisterinfo_directive_missing_json_soft_fails(tmp_path): assert len(result) == 1 assert isinstance(result[0], nodes.paragraph) assert "not found" in result[0].astext() + env = directive.state.document.settings.env + assert str(tmp_path / "twister.json") in env.noted + + +def _recorded_env(tmp_path, *names): + """A fake env that has read `index` with `names` (under tmp_path) as inputs.""" + env = SimpleNamespace(docname="index", note_dependency=lambda path: None) + for name in names: + tm._note_input(env, tmp_path / name) + return env + + +def test_an_input_appearing_with_an_older_mtime_outdates_the_report(tmp_path): + # The CI shape: the docs were read before the twister artifact arrived, and + # the artifact keeps its (older) timestamps. Sphinx's own mtime comparison + # misses this; the recorded signature does not. + env = _recorded_env(tmp_path, "twister_report.xml") + report = tmp_path / "twister_report.xml" + report.write_text("") + os.utime(report, ns=(1, 1)) + assert tm._outdated_by_input_change(None, env, set(), set(), set()) == ["index"] + + +def test_an_unchanged_input_does_not_outdate_the_report(tmp_path): + (tmp_path / "twister.json").write_text("{}") + env = _recorded_env(tmp_path, "twister.json") + assert tm._outdated_by_input_change(None, env, set(), set(), set()) == [] + + +def test_a_rewritten_input_outdates_the_report(tmp_path): + report = tmp_path / "twister_report.xml" + report.write_text("") + os.utime(report, ns=(10**18, 10**18)) + env = _recorded_env(tmp_path, "twister_report.xml") + report.write_text("") + os.utime(report, ns=(10**18, 10**18)) # same mtime, new size + assert tm._outdated_by_input_change(None, env, set(), set(), set()) == ["index"] + + +def test_a_removed_document_is_not_reported(tmp_path): + env = _recorded_env(tmp_path, "twister.json") + (tmp_path / "twister.json").write_text("{}") + assert tm._outdated_by_input_change(None, env, set(), set(), {"index"}) == [] + + +def test_purge_and_merge_keep_the_recorded_inputs_per_document(tmp_path): + env = _recorded_env(tmp_path, "twister.json") + worker = SimpleNamespace(zdocs_report_inputs={"report": {"x": None}, "other": {"y": None}}) + tm._merge_inputs(None, env, {"report"}, worker) + assert set(env.zdocs_report_inputs) == {"index", "report"} + tm._purge_inputs(None, env, "index") + assert set(env.zdocs_report_inputs) == {"report"} def test_testmodule_directive_missing_xml_dir_hard_fails(tmp_path): diff --git a/sphinx/_extensions/test_module.py b/sphinx/_extensions/test_module.py index 8d4cf52..86ee64f 100644 --- a/sphinx/_extensions/test_module.py +++ b/sphinx/_extensions/test_module.py @@ -3,6 +3,7 @@ # SPDX-License-Identifier: Apache-2.0 """Sphinx extension: testmodule and testreport directives (Route B — sphinx-needs).""" +import os import xml.etree.ElementTree as ET from collections import Counter, defaultdict from pathlib import Path @@ -508,6 +509,60 @@ def run(self): # TestReportDirective # --------------------------------------------------------------------------- +# --------------------------------------------------------------------------- +# Inputs outside the source tree +# +# testreport and twisterinfo read the twister XML/JSON and the spec's +# needs.json. Sphinx re-reads a document only when a tracked input is missing +# or NEWER than the document's last read. That is not enough here: twister +# output often arrives with an older mtime (a CI artifact or cache restored +# with its timestamps, a copy that preserves them). The report then stays +# stale without a warning. So each input's signature is recorded at read time, +# and a document is re-read whenever a signature differs, in either direction. +# --------------------------------------------------------------------------- + +def _input_signature(path): + """(mtime_ns, size) of ``path``, or None if it does not exist.""" + try: + st = os.stat(path) + except OSError: + return None + return (st.st_mtime_ns, st.st_size) + + +def _note_input(env, path): + """Track ``path`` as an input of the document being read.""" + path = str(path) + env.note_dependency(path) + inputs = getattr(env, "zdocs_report_inputs", None) + if inputs is None: + inputs = env.zdocs_report_inputs = {} + inputs.setdefault(env.docname, {})[path] = _input_signature(path) + + +def _outdated_by_input_change(app, env, added, changed, removed): + """``env-get-outdated``: documents whose recorded inputs changed.""" + return [ + docname + for docname, paths in getattr(env, "zdocs_report_inputs", {}).items() + if docname not in removed + and any(_input_signature(path) != sig for path, sig in paths.items()) + ] + + +def _purge_inputs(app, env, docname): + getattr(env, "zdocs_report_inputs", {}).pop(docname, None) + + +def _merge_inputs(app, env, docnames, other): + theirs = getattr(other, "zdocs_report_inputs", {}) + if not hasattr(env, "zdocs_report_inputs"): + env.zdocs_report_inputs = {} + for docname in docnames: + if docname in theirs: + env.zdocs_report_inputs[docname] = theirs[docname] + + class TestReportDirective(Directive): """ Emit sphinx-needs test_result nodes from a twister_report.xml. @@ -542,6 +597,14 @@ def run(self): # Fallback: first external-needs source (legacy behaviour). ext_needs = getattr(app.config, "needs_external_needs", []) spec_json = ext_needs[0].get("json_path", "") if ext_needs else "" + + # Both inputs live outside the source tree, so the report must be told + # to re-read when they change (see _note_input). Noted before the + # existence checks on purpose: a report built ahead of its test run + # has to pick the output up once it appears. + _note_input(env, xml_path) + if spec_json: + _note_input(env, spec_json) if not spec_json or not Path(spec_json).exists(): msg = f"[testreport: spec needs.json not found: {spec_json!r}]" logger.warning(f"testreport: {msg}") @@ -620,6 +683,9 @@ def run(self): ) json_path = str(Path(base) / json_path) + # See TestReportDirective.run: an input outside the source tree. + _note_input(env, json_path) + if not Path(json_path).exists(): msg = f"[twisterinfo: twister.json not found: {json_path!r}]" logger.warning(f"twisterinfo: {msg}") @@ -675,4 +741,7 @@ def setup(app): app.add_directive("testmodule", TestModuleDirective) app.add_directive("testreport", TestReportDirective) app.add_directive("twisterinfo", TwisterInfoDirective) + app.connect("env-get-outdated", _outdated_by_input_change) + app.connect("env-purge-doc", _purge_inputs) + app.connect("env-merge-info", _merge_inputs) return {"version": "0.2", "parallel_read_safe": True} From 2c9f6eb68267cacf77825dcecfa035b3558fa7d7 Mon Sep 17 00:00:00 2001 From: Tobias Kaestner Date: Tue, 29 Sep 2026 11:35:45 +0200 Subject: [PATCH 3/5] fix: testmodule: track its inputs; drop the pickled group index Two defects in `.. testmodule::`, both of which kept a stale test specification after the test sources changed. 1. None of its inputs were tracked. It reads Doxygen's index.xml, the module group, every inner group, and testcase.yaml, all outside the Sphinx source tree. An incremental build after a test change (a retagged status, a renamed test) therefore kept the old test cases. Reproduced in safety-toolbox: two tests retagged @draft/@obsolete, a fresh build shows 17/1/1 active/draft/obsolete, an incremental build kept 19 active. Now each input is noted through _note_input (4da0074), so any signature change re-reads the page. 2. The group index was cached as env._testmodule_group_index. env is pickled across builds, so a module group added or renamed later stayed "not found" until a full rebuild. It is now cached on the app, which lives for one build. testreport, twisterinfo and testmodule now all record their inputs through the one _note_input helper. Co-Authored-By: Claude Opus 5.5 Signed-off-by: Tobias Kaestner --- sphinx/_extensions/_tests/test_test_module.py | 19 ++++++++++++++ sphinx/_extensions/test_module.py | 25 ++++++++++++++++--- 2 files changed, 40 insertions(+), 4 deletions(-) diff --git a/sphinx/_extensions/_tests/test_test_module.py b/sphinx/_extensions/_tests/test_test_module.py index e076350..0dfba95 100644 --- a/sphinx/_extensions/_tests/test_test_module.py +++ b/sphinx/_extensions/_tests/test_test_module.py @@ -405,6 +405,25 @@ def test_testmodule_directive_suite_heading(app): assert "Queue API Tests" in html +@pytest.mark.sphinx("html", srcdir=str(_ROOTS / "test-testmodule")) +def test_testmodule_directive_notes_its_xml_and_testcase_yaml_as_inputs(app): + # Without these, an incremental build after the test sources change (a + # retagged status, a renamed test) keeps the old test cases. + app.build() + inputs = set(app.env.zdocs_report_inputs["index"]) + xml_dir = Path(app.config.testmodule_xml_dir) + assert str(xml_dir / "index.xml") in inputs + assert any(p.endswith(".xml") and "group__" in p for p in inputs) + assert any(p.endswith("testcase.yaml") for p in inputs) + + +@pytest.mark.sphinx("html", srcdir=str(_ROOTS / "test-testmodule")) +def test_testmodule_group_index_is_not_cached_across_builds(app): + # The pickled env must not carry the index into the next build. + app.build() + assert not hasattr(app.env, "_testmodule_group_index") + + @pytest.mark.sphinx("html", srcdir=str(_ROOTS / "test-testmodule")) def test_testmodule_directive_procedure_ids(app): app.build() diff --git a/sphinx/_extensions/test_module.py b/sphinx/_extensions/test_module.py index 86ee64f..c537870 100644 --- a/sphinx/_extensions/test_module.py +++ b/sphinx/_extensions/test_module.py @@ -464,14 +464,24 @@ def run(self): # project-specific env var name in a generic engine (decision 5). module_root = app.config.testmodule_root - if not hasattr(env, "_testmodule_group_index"): + # Every file read below lives outside the Sphinx source tree, so each + # is noted as an input (see _note_input): without that, an incremental + # build after the test sources change keeps the old test cases. + _note_input(env, xml_dir / "index.xml") + + # Parsed once per build and cached on the app, which lives for one + # build only. It used to be cached on env, which is pickled across + # builds, so a module group added later stayed "not found" until a + # full rebuild. + group_index = getattr(app, "_testmodule_group_index", None) + if group_index is None: try: - env._testmodule_group_index = load_group_index(xml_dir) + group_index = app._testmodule_group_index = load_group_index(xml_dir) except RuntimeError as exc: logger.warning(str(exc)) return [nodes.paragraph(text=str(exc))] - module_refid = env._testmodule_group_index.get(group_name) + module_refid = group_index.get(group_name) if module_refid is None: logger.warning(f"testmodule: Doxygen group '{group_name}' not found in index.xml") return [ @@ -479,6 +489,7 @@ def run(self): ] module_group_xml = xml_dir / f"{module_refid}.xml" + _note_input(env, module_group_xml) if not module_group_xml.exists(): logger.warning( f"testmodule: XML file not found for group '{group_name}': {module_group_xml}" @@ -486,10 +497,16 @@ def run(self): return [nodes.paragraph(text=f"[testmodule: XML missing for '{group_name}']")] module_cdef = ET.parse(module_group_xml).getroot().find("compounddef") + # _classify_inner_groups reads every inner group, and the suite and + # procedure builders read theirs again: all of them are inputs. + for inner in module_cdef.findall("innergroup"): + _note_input(env, xml_dir / f"{inner.get('refid')}.xml") suite_refids, proc_refids = _classify_inner_groups(module_cdef, xml_dir) need_names = _need_names_from_config(app) - scenario_lines = build_scenario_table(Path(module_root) / module_path / "testcase.yaml") + testcase_yaml = Path(module_root) / module_path / "testcase.yaml" + _note_input(env, testcase_yaml) + scenario_lines = build_scenario_table(testcase_yaml) all_rst = list(scenario_lines) for suite_refid in suite_refids: all_rst += _build_suite_rst( From b7aa62941ee90e6d40e46e33a736b689a5d29223 Mon Sep 17 00:00:00 2001 From: Tobias Kaestner Date: Tue, 29 Sep 2026 15:05:54 +0200 Subject: [PATCH 4/5] feat: twister: select a test report's results by test directory testreport selected results with :module:, a scenario-name prefix. Upstream scenario names do not follow module directories: tests/kernel/timer/timer_api runs as `kernel.timer`, which prefix-matches timer_error_case's `kernel.timer.error_case`, and tests/kernel/fatal/exception runs as `kernel.common.stack_protection*`. In the safety build 1431 of 3630 results rendered on another module's page; whichever page was built first won, and the others produced 3960 "A need with ID 'TR-...' already exists" warnings. The new :path: option names the test directory as twister.json records it (relative to ZEPHYR_BASE) and matches it exactly, after normalising slashes, a leading ./ and a trailing /. twister_report.xml has no path, so results are matched to twister.json's testsuites by (platform, scenario). The twister.json is the one beside the report XML; if it is missing the directive soft-fails to a "not found" paragraph like its other inputs, and it is tracked as an input so the page is re-read once it appears. :module: works as before. Given both, a run must match both. The execution logs use the same selection, so a page never shows a log whose results it does not show; the summary table already follows the selected results. On twister-out-b3, :path: tests/kernel/timer/timer_api selects 412 results (kernel.timer, kernel.timer.no_multitheading) where :module: kernel.timer took 508 from eight scenarios. Co-Authored-By: Claude Opus 5.5 Signed-off-by: Tobias Kaestner --- .../howto/render-test-specifications.rst | 6 + doc/manual/reference/directives-and-roles.rst | 31 +++- .../_tests/fixtures/twister-path/needs.json | 21 +++ .../_tests/fixtures/twister-path/twister.json | 30 ++++ .../fixtures/twister-path/twister_report.xml | 10 ++ .../_tests/roots/test-testreport-path/conf.py | 50 ++++++ .../roots/test-testreport-path/index.rst | 7 + .../roots/test-testreport-path/timer_api.rst | 5 + .../test-testreport-path/timer_error_case.rst | 5 + .../_tests/test_testreport_path.py | 149 ++++++++++++++++++ sphinx/_extensions/test_module.py | 55 ++++++- sphinx/_extensions/twister_reader.py | 76 ++++++++- 12 files changed, 426 insertions(+), 19 deletions(-) create mode 100644 sphinx/_extensions/_tests/fixtures/twister-path/needs.json create mode 100644 sphinx/_extensions/_tests/fixtures/twister-path/twister.json create mode 100644 sphinx/_extensions/_tests/fixtures/twister-path/twister_report.xml create mode 100644 sphinx/_extensions/_tests/roots/test-testreport-path/conf.py create mode 100644 sphinx/_extensions/_tests/roots/test-testreport-path/index.rst create mode 100644 sphinx/_extensions/_tests/roots/test-testreport-path/timer_api.rst create mode 100644 sphinx/_extensions/_tests/roots/test-testreport-path/timer_error_case.rst create mode 100644 sphinx/_extensions/_tests/test_testreport_path.py diff --git a/doc/manual/howto/render-test-specifications.rst b/doc/manual/howto/render-test-specifications.rst index 39c04f7..430d211 100644 --- a/doc/manual/howto/render-test-specifications.rst +++ b/doc/manual/howto/render-test-specifications.rst @@ -105,6 +105,12 @@ for the scenario table. .. twisterinfo:: twister.json +``:module:`` selects the runs by scenario-name prefix. Where one module's +scenario name is a prefix of another's (upstream Zephyr has many), select by +test directory instead: ``:path:`` takes the testsuite path as ``twister.json`` +records it, relative to ``ZEPHYR_BASE`` — for a test root outside the Zephyr +tree that starts with ``../`` (see :doc:`../reference/directives-and-roles`). + Point ``ZDOCS_TWISTER_OUT`` at a real ``west twister`` output directory (``-DZDOCS_TWISTER_OUT=$(west topdir)/twister-out``). Leaving it unset is supported: both directives render a "not found" note and the build still diff --git a/doc/manual/reference/directives-and-roles.rst b/doc/manual/reference/directives-and-roles.rst index 2d6555f..ef996e6 100644 --- a/doc/manual/reference/directives-and-roles.rst +++ b/doc/manual/reference/directives-and-roles.rst @@ -106,15 +106,40 @@ groups becomes one need each — nothing is written by hand per test case. .. code-block:: rst .. testreport:: twister_report.xml - :module: widget.probe + :path: tests/kernel/timer/timer_error_case .. twisterinfo:: twister.json ``testreport``'s and ``twisterinfo``'s arguments are filenames resolved against ``ZDOCS_TWISTER_OUT`` (or the including document's own directory, as a fallback, if that is unset) unless given as an absolute path. -``testreport``'s optional ``:module:`` prefix-matches against the JUnit -``classname``. Both directives **soft-fail** to a short "not found" paragraph + +``testreport`` selects which runs a page shows with two optional options: + +``:path:`` + A test directory, exactly as twister records it in ``twister.json``'s + testsuite ``path`` — relative to ``ZEPHYR_BASE``, e.g. + ``tests/kernel/timer/timer_error_case`` (a test root outside the Zephyr + tree reads ``..//tests/...``). Compared exactly after normalising + slashes, a leading ``./`` and a trailing ``/``; never as a prefix. The path + comes from the ``twister.json`` beside the report XML (the XML has none), and + each result is matched to its testsuite by platform and scenario name. If + that ``twister.json`` is missing, the directive soft-fails to a "not found" + paragraph like the other inputs. +``:module:`` + A scenario-name prefix, matched against the JUnit ``classname`` (the + scenario itself, or ``.`` followed by anything). + +With both, a run must match both. With neither, the page shows every result in +the report. ``:path:`` is the one that identifies a test module: scenario +names do not follow directories upstream — tests/kernel/timer/timer_api runs +as ``kernel.timer``, a prefix of timer_error_case's ``kernel.timer.error_case`` +— so ``:module:`` alone can put one module's results on another's page, where +the second page then fails with "A need with ID … already exists". The +execution-log section and the result summary follow the same selection, so a +page is consistent with itself. + +Both directives **soft-fail** to a short "not found" paragraph when their input is absent, rather than failing the build — a documentation build outrunning its test run is a normal pipeline state. ``testmodule`` does **not** soft-fail on a missing Doxygen group: annotated source is expected to diff --git a/sphinx/_extensions/_tests/fixtures/twister-path/needs.json b/sphinx/_extensions/_tests/fixtures/twister-path/needs.json new file mode 100644 index 0000000..7044ad9 --- /dev/null +++ b/sphinx/_extensions/_tests/fixtures/twister-path/needs.json @@ -0,0 +1,21 @@ +{ + "current_version": "1.0", + "versions": { + "1.0": { + "needs": { + "TSPEC-TA-001": { + "id": "TSPEC-TA-001", "type": "test_case", "title": "timer duration period", + "test_function": "test_timer_duration_period", + "test_module": "tests/kernel/timer/timer_api", + "suite": "timer_api", "suite_title": "Timer API", "verifies": [] + }, + "TSPEC-TE-001": { + "id": "TSPEC-TE-001", "type": "test_case", "title": "timer start null", + "test_function": "test_timer_start_null", + "test_module": "tests/kernel/timer/timer_error_case", + "suite": "timer_api_error", "suite_title": "Timer error cases", "verifies": [] + } + } + } + } +} diff --git a/sphinx/_extensions/_tests/fixtures/twister-path/twister.json b/sphinx/_extensions/_tests/fixtures/twister-path/twister.json new file mode 100644 index 0000000..ba79da4 --- /dev/null +++ b/sphinx/_extensions/_tests/fixtures/twister-path/twister.json @@ -0,0 +1,30 @@ +{ + "environment": { + "run_date": "2026-09-29T10:00:00+00:00", + "zephyr_version": "4.4.0", + "toolchain": "zephyr", + "os": "linux" + }, + "testsuites": [ + { + "name": "kernel.timer", + "platform": "mps2/an385", + "path": "tests/kernel/timer/timer_api", + "toolchain": "zephyr/gnu", + "status": "passed", + "testcases": [ + {"identifier": "kernel.timer.timer_api.timer_duration_period", "status": "passed"} + ] + }, + { + "name": "kernel.timer.error_case", + "platform": "mps2/an385", + "path": "tests/kernel/timer/timer_error_case", + "toolchain": "zephyr/gnu", + "status": "passed", + "testcases": [ + {"identifier": "kernel.timer.error_case.timer_api_error.timer_start_null", "status": "passed"} + ] + } + ] +} diff --git a/sphinx/_extensions/_tests/fixtures/twister-path/twister_report.xml b/sphinx/_extensions/_tests/fixtures/twister-path/twister_report.xml new file mode 100644 index 0000000..a1fab2a --- /dev/null +++ b/sphinx/_extensions/_tests/fixtures/twister-path/twister_report.xml @@ -0,0 +1,10 @@ + + + + + + + + diff --git a/sphinx/_extensions/_tests/roots/test-testreport-path/conf.py b/sphinx/_extensions/_tests/roots/test-testreport-path/conf.py new file mode 100644 index 0000000..9d05c35 --- /dev/null +++ b/sphinx/_extensions/_tests/roots/test-testreport-path/conf.py @@ -0,0 +1,50 @@ +# Copyright (c) 2026 inovex GmbH +# +# SPDX-License-Identifier: Apache-2.0 + +import sys +from pathlib import Path + +sys.path.insert(0, str(Path(__file__).resolve().parents[3])) # _extensions/ + +_FIXTURES = Path(__file__).resolve().parents[2] / "fixtures" + +extensions = ["sphinx_needs", "test_module"] +master_doc = "index" +exclude_patterns = ["_build"] + +needs_types = [ + dict(directive="test_result", title="Test Result", prefix="TRESULT_", + color="#FCE4D6", style="node"), + dict(directive="test_case", title="Test Case", prefix="TCASE_", + color="#E2EFDA", style="node"), +] +_str_field = {"schema": {"type": "string"}, "nullable": True} +needs_fields = { + "platform": {**_str_field}, + "scenario": {**_str_field}, + "twister_id": {**_str_field}, + "execution_time": {**_str_field}, + "reason": {**_str_field}, + "test_function": {**_str_field}, + "test_module": {**_str_field}, + "suite": {**_str_field}, + "suite_title": {**_str_field}, +} +needs_id_regex = r"^[A-Za-z][A-Za-z0-9_-]+" +needs_links = { + "result_of": {"description": "result of", "incoming": "has results", "outgoing": "result of"}, + "covers": {"description": "covers", "incoming": "covered by", "outgoing": "covers"}, + "verifies": {"description": "verifies", "incoming": "verified by", "outgoing": "verifies"}, +} +needs_external_needs = [{ + "json_path": str(_FIXTURES / "twister-path" / "needs.json"), + "base_url": "http://localhost/", + "version": "1.0", +}] +twister_output_dir = str(_FIXTURES / "twister-path") +testspec_needs_json = str(_FIXTURES / "twister-path" / "needs.json") +testspec_doxygen_url = "testspec" +api_doxygen_url = "api" +needs_build_json = True +suppress_warnings = ["needs.link_outgoing", "needs.external_link_outgoing", "config.cache"] diff --git a/sphinx/_extensions/_tests/roots/test-testreport-path/index.rst b/sphinx/_extensions/_tests/roots/test-testreport-path/index.rst new file mode 100644 index 0000000..2ef7542 --- /dev/null +++ b/sphinx/_extensions/_tests/roots/test-testreport-path/index.rst @@ -0,0 +1,7 @@ +Test Report +=========== + +.. toctree:: + + timer_api + timer_error_case diff --git a/sphinx/_extensions/_tests/roots/test-testreport-path/timer_api.rst b/sphinx/_extensions/_tests/roots/test-testreport-path/timer_api.rst new file mode 100644 index 0000000..81bec2a --- /dev/null +++ b/sphinx/_extensions/_tests/roots/test-testreport-path/timer_api.rst @@ -0,0 +1,5 @@ +Timer API +========= + +.. testreport:: twister_report.xml + :path: tests/kernel/timer/timer_api diff --git a/sphinx/_extensions/_tests/roots/test-testreport-path/timer_error_case.rst b/sphinx/_extensions/_tests/roots/test-testreport-path/timer_error_case.rst new file mode 100644 index 0000000..6dfb556 --- /dev/null +++ b/sphinx/_extensions/_tests/roots/test-testreport-path/timer_error_case.rst @@ -0,0 +1,5 @@ +Timer error cases +================= + +.. testreport:: twister_report.xml + :path: ./tests/kernel/timer/timer_error_case/ diff --git a/sphinx/_extensions/_tests/test_testreport_path.py b/sphinx/_extensions/_tests/test_testreport_path.py new file mode 100644 index 0000000..5897e32 --- /dev/null +++ b/sphinx/_extensions/_tests/test_testreport_path.py @@ -0,0 +1,149 @@ +# Copyright (c) 2026 inovex GmbH +# +# SPDX-License-Identifier: Apache-2.0 + +"""testreport ``:path:`` — select a module's results by its test directory. + +Upstream scenario names do not follow module directories. tests/kernel/timer/ +timer_api runs as ``kernel.timer``, which prefix-matches ``kernel.timer.error_case`` +from tests/kernel/timer/timer_error_case, so ``:module: kernel.timer`` put the +error-case results on the timer_api page too; the second page then failed with +"A need with ID ... already exists". The fixture reproduces exactly that pair. +""" + +from pathlib import Path +from types import SimpleNamespace + +import pytest +import test_module as tm +import twister_reader as tw +from conftest import FIXTURES +from docutils import nodes + +_ROOTS = Path(__file__).parent / "roots" +DATA = FIXTURES / "twister-path" +XML = DATA / "twister_report.xml" + +API = "tests/kernel/timer/timer_api" +ERR = "tests/kernel/timer/timer_error_case" + + +def _paths(): + return tw.testsuite_paths(tw.load_twister_meta(DATA / "twister.json")) + + +def _scenarios(results): + return sorted(r["scenario"] for r in results) + + +# --------------------------------------------------------------------------- +# twister_reader +# --------------------------------------------------------------------------- + + +def test_scenario_prefix_takes_the_colliding_module_too(): + # The defect, pinned: the prefix of timer_api's scenario selects both modules. + results = tw.parse_twister_results(XML, module_filter="kernel.timer") + assert _scenarios(results) == ["kernel.timer", "kernel.timer.error_case"] + + +def test_path_selects_only_its_own_module(): + paths = _paths() + api = tw.parse_twister_results(XML, path_filter=API, suite_paths=paths) + err = tw.parse_twister_results(XML, path_filter=ERR, suite_paths=paths) + assert _scenarios(api) == ["kernel.timer"] + assert _scenarios(err) == ["kernel.timer.error_case"] + + +@pytest.mark.parametrize("spelling", [ERR + "/", "./" + ERR, ERR.replace("/", "\\")]) +def test_path_is_normalised(spelling): + results = tw.parse_twister_results(XML, path_filter=spelling, suite_paths=_paths()) + assert _scenarios(results) == ["kernel.timer.error_case"] + + +def test_path_is_matched_exactly_not_as_a_prefix(): + results = tw.parse_twister_results(XML, path_filter="tests/kernel/timer", suite_paths=_paths()) + assert results == [] + + +def test_module_and_path_together_must_both_match(): + paths = _paths() + both = tw.parse_twister_results( + XML, module_filter="kernel.timer", path_filter=API, suite_paths=paths + ) + assert _scenarios(both) == ["kernel.timer"] + none = tw.parse_twister_results( + XML, module_filter="kernel.timer.error_case", path_filter=API, suite_paths=paths + ) + assert none == [] + + +def test_a_run_missing_from_twister_json_is_not_selected_by_path(): + results = tw.parse_twister_results(XML, path_filter=API, suite_paths={}) + assert results == [] + + +# --------------------------------------------------------------------------- +# Execution logs follow the same selection +# --------------------------------------------------------------------------- + + +def test_exec_logs_follow_the_path(): + api = "\n".join(tm._build_exec_logs_rst(str(DATA), None, API)) + assert "kernel.timer — mps2/an385" in api + assert "kernel.timer.error_case" not in api + # Control: the scenario prefix alone lists the other module's log as well. + prefix = "\n".join(tm._build_exec_logs_rst(str(DATA), "kernel.timer")) + assert "kernel.timer.error_case" in prefix + + +# --------------------------------------------------------------------------- +# Directive: soft-fail without twister.json +# --------------------------------------------------------------------------- + + +def test_path_without_twister_json_soft_fails(tmp_path, monkeypatch): + (tmp_path / "twister_report.xml").write_text(XML.read_text()) + warnings = [] + monkeypatch.setattr(tm.logger, "warning", lambda msg, *a, **k: warnings.append(msg)) + noted = [] + env = SimpleNamespace( + app=SimpleNamespace(config=SimpleNamespace( + testspec_needs_json=str(DATA / "needs.json"), + twister_output_dir=str(tmp_path), + )), + docname="index", + note_dependency=noted.append, + ) + directive = tm.TestReportDirective.__new__(tm.TestReportDirective) + directive.arguments = ["twister_report.xml"] + directive.options = {"path": API} + directive.state = SimpleNamespace(document=SimpleNamespace(settings=SimpleNamespace(env=env))) + + result = directive.run() + + assert len(result) == 1 and isinstance(result[0], nodes.paragraph) + text = result[0].astext() + assert "twister.json not found" in text and ":path:" in text + assert str(tmp_path) not in text # published node: no host path + assert any("twister.json" in w for w in warnings) + # Tracked, so the report is re-read once twister.json appears. + assert str(tmp_path / "twister.json") in noted + + +# --------------------------------------------------------------------------- +# Directive: two pages, each with only its own results +# --------------------------------------------------------------------------- + + +@pytest.mark.sphinx("html", srcdir=str(_ROOTS / "test-testreport-path")) +def test_each_page_gets_only_its_own_results(app, warning): + app.build() + out = Path(app.outdir) + api = (out / "timer_api.html").read_text() + err = (out / "timer_error_case.html").read_text() + assert "TR-mps2-an385-kernel-timer-TSPEC-TA-001" in api + assert "TSPEC-TE-001" not in api + assert "TR-mps2-an385-kernel-timer-error-case-TSPEC-TE-001" in err + assert "TSPEC-TA-001" not in err + assert "already exists" not in warning.getvalue() diff --git a/sphinx/_extensions/test_module.py b/sphinx/_extensions/test_module.py index c537870..0d8924f 100644 --- a/sphinx/_extensions/test_module.py +++ b/sphinx/_extensions/test_module.py @@ -25,6 +25,8 @@ load_spec_lookup, load_twister_meta, parse_twister_results, + scenario_selected, + testsuite_paths, ) logger = logging.getLogger(__name__) @@ -273,8 +275,12 @@ def _build_summary_table_rst(grouped, spec_lookup, need_names=None): ] -def _build_exec_logs_rst(twister_out_dir, module_filter): - """Build RST lines for the execution logs section; returns [] when unavailable.""" +def _build_exec_logs_rst(twister_out_dir, module_filter, path_filter=None): + """Build RST lines for the execution logs section; returns [] when unavailable. + + Selects the same runs as the results above (`scenario_selected`), so a page + never shows the log of a run whose results it does not show. + """ twister_json = Path(twister_out_dir) / "twister.json" if twister_out_dir else None if not twister_json or not twister_json.exists(): return [] @@ -284,11 +290,13 @@ def _build_exec_logs_rst(twister_out_dir, module_filter): logger.warning(f"testreport: could not load execution logs: {exc}") return [] + suite_paths = testsuite_paths(tw) log_entries = [] for ts in tw.get("testsuites", []): sname = ts["name"] - if module_filter and not ( - sname == module_filter or sname.startswith(module_filter + ".") + if not scenario_selected( + ts["platform"], sname, module_filter, + path_filter=path_filter, suite_paths=suite_paths, ): continue log_entries.append((sname, ts["platform"], ts.get("path", ""), ts.get("toolchain", ""))) @@ -587,7 +595,11 @@ class TestReportDirective(Directive): Usage:: .. testreport:: twister_report.xml - :module: kernel.queue + :path: tests/kernel/queue + + ``:path:`` selects the runs of one test directory, as twister.json records + it; ``:module:`` selects by scenario-name prefix. With both, a run must + match both. """ required_arguments = 1 @@ -595,11 +607,13 @@ class TestReportDirective(Directive): has_content = False option_spec = { "module": directives.unchanged, + "path": directives.unchanged, } def run(self): xml_path = self.arguments[0].strip() module_filter = self.options.get("module", "").strip() or None + path_filter = self.options.get("path", "").strip() or None env = self.state.document.settings.env app = env.app @@ -651,8 +665,31 @@ def run(self): display_msg = f"[testreport: twister XML not found: {_display_name(xml_path)}]" return [nodes.paragraph(text=display_msg)] + # twister_report.xml has no testsuite path; twister.json, written + # beside it by the same run, does. + suite_paths = None + if path_filter is not None: + twister_json = Path(xml_path).parent / "twister.json" + _note_input(env, twister_json) + if not twister_json.exists(): + logger.warning( + f"testreport: :path: needs twister.json beside the report, " + f"not found: {twister_json}" + ) + return [nodes.paragraph( + text=f"[testreport: twister.json not found: {twister_json.name} " + f"(needed for :path:)]" + )] + try: + suite_paths = testsuite_paths(load_twister_meta(twister_json)) + except Exception as exc: + logger.warning(f"testreport: cannot read {twister_json}: {exc}") + return [nodes.paragraph(text=f"[testreport: cannot read {twister_json.name}]")] + try: - results = parse_twister_results(xml_path, module_filter) + results = parse_twister_results( + xml_path, module_filter, path_filter=path_filter, suite_paths=suite_paths + ) except Exception as exc: logger.warning(str(exc)) return [nodes.paragraph(text=str(exc))] @@ -665,10 +702,12 @@ def run(self): all_rst = ( _build_results_rst(suite_order, func_order, grouped, spec_lookup, need_names=need_names) + _build_summary_table_rst(grouped, spec_lookup, need_names=need_names) - + _build_exec_logs_rst(twister_out_dir, module_filter) + + _build_exec_logs_rst(twister_out_dir, module_filter, path_filter) ) - _maybe_dump_rst(app, env.docname, "testreport", module_filter or "", "\n".join(all_rst)) + _maybe_dump_rst( + app, env.docname, "testreport", path_filter or module_filter or "", "\n".join(all_rst) + ) return _render_rst(all_rst, self.state, self.content_offset, match_titles=True) diff --git a/sphinx/_extensions/twister_reader.py b/sphinx/_extensions/twister_reader.py index 6184a47..5f41c93 100644 --- a/sphinx/_extensions/twister_reader.py +++ b/sphinx/_extensions/twister_reader.py @@ -18,6 +18,9 @@ __all__ = [ "parse_twister_results", + "normalise_test_path", + "scenario_selected", + "testsuite_paths", "SpecLookup", "load_spec_lookup", "find_handler_log", @@ -32,11 +35,70 @@ def _elem_text(elem): return " ".join("".join(elem.itertext()).split()) -def parse_twister_results(xml_path, module_filter=None, exact=False): +def normalise_test_path(path): + """A testsuite path in one spelling: forward slashes, no ``./``, no trailing ``/``. + + Twister writes ``path`` relative to ZEPHYR_BASE with forward slashes; a + consumer typing the same directory may add a trailing slash or a leading + ``./``, or come from a Windows checkout. None of that changes which + directory it is. + """ + p = str(path).replace("\\", "/") + while "//" in p: + p = p.replace("//", "/") + while p.startswith("./"): + p = p[2:] + return p.rstrip("/") + + +def testsuite_paths(twister_meta): + """``{(platform, scenario): normalised path}`` from a loaded twister.json. + + twister_report.xml carries no path, only the scenario (``classname``) per + platform, so the path a result came from is found here, by the same pair. + """ + return { + (ts.get("platform", ""), ts.get("name", "")): normalise_test_path(ts.get("path", "")) + for ts in twister_meta.get("testsuites", []) + } + + +def scenario_selected( + platform, scenario, module_filter=None, exact=False, path_filter=None, suite_paths=None +): + """Whether a (platform, scenario) run belongs to the report being built. + + ``module_filter`` matches the scenario name, as a dotted prefix (or exactly + with ``exact``). ``path_filter`` matches the testsuite directory exactly, + looked up in ``suite_paths`` (see `testsuite_paths`). With both, a run must + satisfy both. With neither, every run is selected. + + The scenario prefix alone is not a module: upstream scenario names do not + follow the directory layout (``kernel.timer`` is tests/kernel/timer/timer_api, + and prefixes ``kernel.timer.error_case`` from timer_error_case), so only + the path identifies a module's results reliably. + """ + if module_filter: + if exact: + if scenario != module_filter: + return False + elif not (scenario == module_filter or scenario.startswith(module_filter + ".")): + return False + if path_filter is not None: + path = (suite_paths or {}).get((platform, scenario)) + if path is None or path != normalise_test_path(path_filter): + return False + return True + + +def parse_twister_results( + xml_path, module_filter=None, exact=False, path_filter=None, suite_paths=None +): """Parse twister_report.xml into a list of result dicts. The 'function' field has any leading 'test_' prefix stripped so it matches - the keys used in spec_lookup. + the keys used in spec_lookup. Results are selected as `scenario_selected` + describes; ``path_filter`` needs ``suite_paths`` from the run's twister.json. """ root = ET.parse(xml_path).getroot() results = [] @@ -44,12 +106,10 @@ def parse_twister_results(xml_path, module_filter=None, exact=False): platform = ts.get("name", "") for tc in ts.findall("testcase"): classname = tc.get("classname", "") - if module_filter: - if exact: - if classname != module_filter: - continue - elif not (classname == module_filter or classname.startswith(module_filter + ".")): - continue + if not scenario_selected( + platform, classname, module_filter, exact, path_filter, suite_paths + ): + continue name = tc.get("name", "") scenario = classname suffix = name[len(scenario) + 1 :] if name.startswith(scenario + ".") else name From d7fb9feb4ab6a0299b2b589adf262d0b52c5aeff Mon Sep 17 00:00:00 2001 From: Tobias Kaestner Date: Tue, 29 Sep 2026 15:09:53 +0200 Subject: [PATCH 5/5] feat: twister: attach parameterized-test values to the test's result A ztest ZTEST_P function runs once per parameter value. Twister reports one aggregate .. from ztest's summary plus one result per value, .[/], with no suite segment. The report parsed a value as suite "" and function "sem_init_validity[cases/0]", found no spec case, and skipped it with a warning: 618 warnings in the safety build, and the per-value results were lost. Those are the results that matter when a value fails. ztest then summarises the function as FLAKY, which twister's summary parser does not know, so the aggregate becomes `blocked` in twister.json and a generic "Testsuite failed" in the XML. Only the failing value's result carries the assertion. Values are now recognised and attached to the aggregate of the same run (platform + scenario, matched by function name; the suite comes from the aggregate). The spec keeps one test case, and the report one result need per run. That need takes its status from the values (failed if any failed, skipped if all skipped, else passed), shows twister's own status when it disagrees ("Twister reported the test as `blocked`"), and renders "9 values: 8 passed, 1 failed" plus a table of the values that did not pass, with the assertion text instead of the suite message. A run without an aggregate takes the suite from other runs' aggregates of the same scenario and function, or from a unique spec case; values that match neither are skipped with one warning per function. twister.json is read, if present, for the statuses the XML cannot express. The fixture is cut from a real run with value 8 of sem_init_validity made to fail. On twister-out-b3 the 467 value results fold into their 21 aggregates, leaving the 3630 results the report shows. Co-Authored-By: Claude Opus 5.5 Signed-off-by: Tobias Kaestner --- .../explanation/testmodule-and-twister.rst | 28 +++ doc/manual/reference/directives-and-roles.rst | 6 + .../_tests/fixtures/twister-param/needs.json | 29 +++ .../fixtures/twister-param/twister.json | 77 +++++++ .../fixtures/twister-param/twister_report.xml | 34 +++ .../roots/test-testreport-param/conf.py | 50 ++++ .../roots/test-testreport-param/index.rst | 5 + .../_tests/test_testreport_param.py | 213 ++++++++++++++++++ sphinx/_extensions/rst_builders.py | 52 +++++ sphinx/_extensions/test_module.py | 49 ++-- sphinx/_extensions/twister_reader.py | 178 +++++++++++++-- 11 files changed, 690 insertions(+), 31 deletions(-) create mode 100644 sphinx/_extensions/_tests/fixtures/twister-param/needs.json create mode 100644 sphinx/_extensions/_tests/fixtures/twister-param/twister.json create mode 100644 sphinx/_extensions/_tests/fixtures/twister-param/twister_report.xml create mode 100644 sphinx/_extensions/_tests/roots/test-testreport-param/conf.py create mode 100644 sphinx/_extensions/_tests/roots/test-testreport-param/index.rst create mode 100644 sphinx/_extensions/_tests/test_testreport_param.py diff --git a/doc/manual/explanation/testmodule-and-twister.rst b/doc/manual/explanation/testmodule-and-twister.rst index c318ec7..0d281ee 100644 --- a/doc/manual/explanation/testmodule-and-twister.rst +++ b/doc/manual/explanation/testmodule-and-twister.rst @@ -71,6 +71,34 @@ are **derived from the registry** — the report's own ``testmodule:`` block already names the specification it reads — rather than hand-written by the consumer, which would duplicate what the registry knows. +Parameterized tests +------------------- + +A ztest ``ZTEST_P`` function runs once per parameter value, and twister +reports it twice over: one aggregate result, ``..``, from +ztest's summary line, and one result per value, +``.[/]`` — with no suite segment. The +specification documents the function once, so the report does too: each value +result is attached to the aggregate of the same run (platform and scenario), +and the aggregate's result need carries them. Its status comes from the +values — failed if any failed, skipped if all were skipped, passed if every +value that ran passed — and its body counts them ("9 values: 8 passed, 1 +failed") and tabulates only the values that did not pass, with the assertion +each one failed on. + +The aggregate's own status is not trusted for this. When one value fails, +ztest summarises the function as ``FLAKY``, which twister does not recognise: +it records the aggregate as ``blocked`` in ``twister.json`` and as a generic +"Testsuite failed" in the XML, and only the failing value's result carries the +real assertion. Where twister's status for the aggregate disagrees with the +values' verdict, the need says so ("Twister reported the test as +``blocked``"). + +A run whose values have no aggregate at all borrows the suite from other runs' +aggregates of the same scenario and function, or else from the specification +if exactly one test case has that function name. Values that match neither are +skipped with one warning per function, not one per value. + Where the test runner writes ---------------------------- diff --git a/doc/manual/reference/directives-and-roles.rst b/doc/manual/reference/directives-and-roles.rst index ef996e6..3a3211c 100644 --- a/doc/manual/reference/directives-and-roles.rst +++ b/doc/manual/reference/directives-and-roles.rst @@ -139,6 +139,12 @@ the second page then fails with "A need with ID … already exists". The execution-log section and the result summary follow the same selection, so a page is consistent with itself. +A parameterized test (``ZTEST_P``) gets one result need per run, not one per +parameter value: the values' results are attached to the test's aggregate +result, which takes its status from them and lists the values that did not +pass (:doc:`../explanation/testmodule-and-twister`). No need type or field is +added for this; the values render in the need's body. + Both directives **soft-fail** to a short "not found" paragraph when their input is absent, rather than failing the build — a documentation build outrunning its test run is a normal pipeline state. ``testmodule`` does diff --git a/sphinx/_extensions/_tests/fixtures/twister-param/needs.json b/sphinx/_extensions/_tests/fixtures/twister-param/needs.json new file mode 100644 index 0000000..af0415d --- /dev/null +++ b/sphinx/_extensions/_tests/fixtures/twister-param/needs.json @@ -0,0 +1,29 @@ +{ + "current_version": "1.0", + "versions": { + "1.0": { + "needs": { + "TSPEC-SEM-001": { + "id": "TSPEC-SEM-001", + "type": "test_case", + "title": "sem init validity", + "test_function": "test_sem_init_validity", + "test_module": "tests/kernel/semaphore/semaphore", + "suite": "semaphore", + "suite_title": "Semaphore", + "verifies": [] + }, + "TSPEC-SEM-002": { + "id": "TSPEC-SEM-002", + "type": "test_case", + "title": "sem count get", + "test_function": "test_sem_count_get", + "test_module": "tests/kernel/semaphore/semaphore", + "suite": "semaphore", + "suite_title": "Semaphore", + "verifies": [] + } + } + } + } +} \ No newline at end of file diff --git a/sphinx/_extensions/_tests/fixtures/twister-param/twister.json b/sphinx/_extensions/_tests/fixtures/twister-param/twister.json new file mode 100644 index 0000000..d1638a7 --- /dev/null +++ b/sphinx/_extensions/_tests/fixtures/twister-param/twister.json @@ -0,0 +1,77 @@ +{ + "environment": { + "run_date": "2026-09-29T12:18:23+00:00", + "zephyr_version": "v4.4.0-13457-g24e1b45a5d81", + "toolchain": "zephyr/gnu", + "os": "Linux" + }, + "testsuites": [ + { + "name": "kernel.semaphore", + "arch": "arm", + "platform": "mps2/an385", + "toolchain": "zephyr/gnu", + "status": "failed", + "reason": "Testsuite failed", + "path": "tests/kernel/semaphore/semaphore", + "testcases": [ + { + "identifier": "kernel.semaphore.semaphore.sem_count_get", + "execution_time": "0.01", + "status": "passed" + }, + { + "identifier": "kernel.semaphore.semaphore.sem_init_validity", + "execution_time": "0.00", + "status": "blocked", + "reason": "Testsuite failed" + }, + { + "identifier": "kernel.semaphore.sem_init_validity[cases/0]", + "execution_time": "0.00", + "status": "passed" + }, + { + "identifier": "kernel.semaphore.sem_init_validity[cases/1]", + "execution_time": "0.00", + "status": "passed" + }, + { + "identifier": "kernel.semaphore.sem_init_validity[cases/2]", + "execution_time": "0.00", + "status": "passed" + }, + { + "identifier": "kernel.semaphore.sem_init_validity[cases/3]", + "execution_time": "0.00", + "status": "passed" + }, + { + "identifier": "kernel.semaphore.sem_init_validity[cases/4]", + "execution_time": "0.00", + "status": "passed" + }, + { + "identifier": "kernel.semaphore.sem_init_validity[cases/5]", + "execution_time": "0.00", + "status": "passed" + }, + { + "identifier": "kernel.semaphore.sem_init_validity[cases/6]", + "execution_time": "0.00", + "status": "passed" + }, + { + "identifier": "kernel.semaphore.sem_init_validity[cases/7]", + "execution_time": "0.00", + "status": "passed" + }, + { + "identifier": "kernel.semaphore.sem_init_validity[cases/8]", + "execution_time": "0.01", + "status": "failed" + } + ] + } + ] +} \ No newline at end of file diff --git a/sphinx/_extensions/_tests/fixtures/twister-param/twister_report.xml b/sphinx/_extensions/_tests/fixtures/twister-param/twister_report.xml new file mode 100644 index 0000000..575ed66 --- /dev/null +++ b/sphinx/_extensions/_tests/fixtures/twister-param/twister_report.xml @@ -0,0 +1,34 @@ + + + + + + + *** Booting Zephyr OS build v4.4.0-13457-g24e1b45a5d81 *** +Running TESTSUITE semaphore +[...] +TESTSUITE semaphore failed. + + + + + + + + + + + + START - test_sem_init_validity[cases/8] + + Assertion failed at CMAKE_SOURCE_DIR/src/main.c:363: semaphore_test_sem_init_validity: (_act not equal to _exp) +k_sem_init incorrect return value: -22 != 0 + FAIL - test_sem_init_validity[cases/8] in 0.006 seconds + + + + diff --git a/sphinx/_extensions/_tests/roots/test-testreport-param/conf.py b/sphinx/_extensions/_tests/roots/test-testreport-param/conf.py new file mode 100644 index 0000000..992358c --- /dev/null +++ b/sphinx/_extensions/_tests/roots/test-testreport-param/conf.py @@ -0,0 +1,50 @@ +# Copyright (c) 2026 inovex GmbH +# +# SPDX-License-Identifier: Apache-2.0 + +import sys +from pathlib import Path + +sys.path.insert(0, str(Path(__file__).resolve().parents[3])) # _extensions/ + +_FIXTURES = Path(__file__).resolve().parents[2] / "fixtures" + +extensions = ["sphinx_needs", "test_module"] +master_doc = "index" +exclude_patterns = ["_build"] + +needs_types = [ + dict(directive="test_result", title="Test Result", prefix="TRESULT_", + color="#FCE4D6", style="node"), + dict(directive="test_case", title="Test Case", prefix="TCASE_", + color="#E2EFDA", style="node"), +] +_str_field = {"schema": {"type": "string"}, "nullable": True} +needs_fields = { + "platform": {**_str_field}, + "scenario": {**_str_field}, + "twister_id": {**_str_field}, + "execution_time": {**_str_field}, + "reason": {**_str_field}, + "test_function": {**_str_field}, + "test_module": {**_str_field}, + "suite": {**_str_field}, + "suite_title": {**_str_field}, +} +needs_id_regex = r"^[A-Za-z][A-Za-z0-9_-]+" +needs_links = { + "result_of": {"description": "result of", "incoming": "has results", "outgoing": "result of"}, + "covers": {"description": "covers", "incoming": "covered by", "outgoing": "covers"}, + "verifies": {"description": "verifies", "incoming": "verified by", "outgoing": "verifies"}, +} +needs_external_needs = [{ + "json_path": str(_FIXTURES / "twister-param" / "needs.json"), + "base_url": "http://localhost/", + "version": "1.0", +}] +twister_output_dir = str(_FIXTURES / "twister-param") +testspec_needs_json = str(_FIXTURES / "twister-param" / "needs.json") +testspec_doxygen_url = "testspec" +api_doxygen_url = "api" +needs_build_json = True +suppress_warnings = ["needs.link_outgoing", "needs.external_link_outgoing", "config.cache"] diff --git a/sphinx/_extensions/_tests/roots/test-testreport-param/index.rst b/sphinx/_extensions/_tests/roots/test-testreport-param/index.rst new file mode 100644 index 0000000..b3457e5 --- /dev/null +++ b/sphinx/_extensions/_tests/roots/test-testreport-param/index.rst @@ -0,0 +1,5 @@ +Test Report +=========== + +.. testreport:: twister_report.xml + :path: tests/kernel/semaphore/semaphore diff --git a/sphinx/_extensions/_tests/test_testreport_param.py b/sphinx/_extensions/_tests/test_testreport_param.py new file mode 100644 index 0000000..e4f2383 --- /dev/null +++ b/sphinx/_extensions/_tests/test_testreport_param.py @@ -0,0 +1,213 @@ +# Copyright (c) 2026 inovex GmbH +# +# SPDX-License-Identifier: Apache-2.0 + +"""Parameterized tests (ZTEST_P): one result per value, attached to the test's result. + +Twister reports a ZTEST_P function as an aggregate ``..`` +plus one result per value, ``.[/]``, with no +suite segment. The values used to be looked up in the spec as functions named +``sem_init_validity[cases/0]`` and skipped with a warning each, which lost the +one result that says which value failed. The fixture is a real run in which +value 8 fails: the aggregate is ``blocked`` in twister.json and "Testsuite +failed" in the XML, and only ``[cases/8]`` carries the assertion. +""" + +from pathlib import Path + +import pytest +import rst_builders as rb +import twister_reader as tw +from conftest import FIXTURES +from twister_reader import SpecLookup + +_ROOTS = Path(__file__).parent / "roots" +DATA = FIXTURES / "twister-param" +XML = DATA / "twister_report.xml" +AGGREGATE = "kernel.semaphore.semaphore.sem_init_validity" + + +def _statuses(): + return tw.testcase_statuses(tw.load_twister_meta(DATA / "twister.json")) + + +def _spec(): + return tw.load_spec_lookup(DATA / "needs.json") + + +def _result(platform="p", scenario="sc", suite="s", function="fn", status="passed", **kw): + r = { + "platform": platform, + "scenario": scenario, + "suite": suite, + "function": function, + "twister_id": f"{scenario}.{suite}.{function}", + "time": "0.01", + "status": status, + "reason": "", + } + r.update(kw) + return r + + +def _value(value, status="passed", platform="p", scenario="sc", function="fn", reason=""): + return _result( + platform, scenario, "", function, status, + twister_id=f"{scenario}.{function}[{value}]", instance=value, reason=reason, + ) + + +# --------------------------------------------------------------------------- +# parse_twister_results +# --------------------------------------------------------------------------- + + +def test_values_are_recognised(): + values = [r for r in tw.parse_twister_results(XML) if r.get("instance")] + assert [v["instance"] for v in values] == [f"cases/{i}" for i in range(9)] + assert {(v["suite"], v["function"]) for v in values} == {("", "sem_init_validity")} + + +def test_a_failed_value_carries_the_assertion_not_the_suite_message(): + failed = [r for r in tw.parse_twister_results(XML) if r["status"] == "failed" and r.get("instance")] + assert len(failed) == 1 + reason = failed[0]["reason"] + assert "Assertion failed at CMAKE_SOURCE_DIR/src/main.c:363" in reason + assert "-22 != 0" in reason + assert "START -" not in reason and "FAIL -" not in reason + + +def test_a_value_containing_dots_keeps_them(tmp_path): + tmp = tmp_path / "twister_report.xml" + tmp.write_text(XML.read_text().replace("[cases/1]", "[conversions/ms.to.cyc]")) + values = [r for r in tw.parse_twister_results(tmp) if r.get("instance")] + assert "conversions/ms.to.cyc" in [v["instance"] for v in values] + assert {v["function"] for v in values} == {"sem_init_validity"} + + +# --------------------------------------------------------------------------- +# fold_parameterized_results +# --------------------------------------------------------------------------- + + +def test_values_attach_to_their_aggregate(): + results, unmatched = tw.fold_parameterized_results( + tw.parse_twister_results(XML), _spec(), _statuses() + ) + assert unmatched == [] + assert [r["twister_id"] for r in results] == [ + "kernel.semaphore.semaphore.sem_count_get", + AGGREGATE, + ] + agg = results[1] + assert len(agg["values"]) == 9 + assert agg["status"] == "failed" + assert agg["reason"] == "9 values: 8 passed, 1 failed" + # twister.json says `blocked`, which disagrees with the values: kept visible. + assert agg["twister_status"] == "blocked" + + +def test_without_twister_json_the_xml_status_is_the_reference(): + results, _ = tw.fold_parameterized_results(tw.parse_twister_results(XML)) + agg = next(r for r in results if r["twister_id"] == AGGREGATE) + assert agg["status"] == "failed" + assert agg["twister_status"] == "" # the XML's `failed` agrees + + +def test_the_verdict_comes_from_the_values(): + agg = _result(status="failed") + results, _ = tw.fold_parameterized_results([agg, _value("v/0"), _value("v/1")]) + assert results == [agg] + assert agg["status"] == "passed" and agg["reason"] == "" + assert agg["twister_status"] == "failed" + + +@pytest.mark.parametrize( + "statuses, verdict", + [ + (["skipped", "skipped"], "skipped"), + (["passed", "skipped"], "passed"), + (["passed", "error"], "error"), + (["error", "failed"], "failed"), + ], +) +def test_verdict_rules(statuses, verdict): + agg = _result() + values = [_value(f"v/{i}", s) for i, s in enumerate(statuses)] + tw.fold_parameterized_results([agg, *values]) + assert agg["status"] == verdict + + +def test_a_run_without_aggregate_takes_the_suite_of_another_runs_aggregate(): + results, unmatched = tw.fold_parameterized_results([ + _result(platform="a", suite="suite_x"), + _value("v/0", platform="a"), + _value("v/0", platform="b", status="failed"), + ]) + assert unmatched == [] + b = next(r for r in results if r["platform"] == "b") + assert (b["suite"], b["function"], b["status"]) == ("suite_x", "fn", "failed") + assert len(b["values"]) == 1 + + +def test_a_run_without_any_aggregate_takes_a_unique_spec_case(): + spec = SpecLookup([{"id": "T-1", "suite": "suite_y", "test_function": "test_fn"}]) + results, unmatched = tw.fold_parameterized_results([_value("v/0")], spec) + assert unmatched == [] + assert [(r["suite"], r["function"]) for r in results] == [("suite_y", "fn")] + + +def test_unattachable_values_are_reported_once_per_function(): + spec = SpecLookup([ + {"id": "T-1", "suite": "a", "test_function": "test_fn"}, + {"id": "T-2", "suite": "b", "test_function": "test_fn"}, + ]) + results, unmatched = tw.fold_parameterized_results( + [_value("v/0"), _value("v/1"), _value("v/0", platform="q")], spec + ) + assert results == [] + assert unmatched == ["fn"] + + +def test_results_without_values_are_unchanged(): + plain = [_result(), _result(function="other")] + assert tw.fold_parameterized_results(plain) == (plain, []) + + +# --------------------------------------------------------------------------- +# Rendering +# --------------------------------------------------------------------------- + + +def test_result_need_lists_only_the_values_that_did_not_pass(): + results, _ = tw.fold_parameterized_results(tw.parse_twister_results(XML), None, _statuses()) + agg = next(r for r in results if r["twister_id"] == AGGREGATE) + rst = rb.build_result_rst(agg, "TSPEC-SEM-001", "tests/kernel/semaphore/semaphore") + assert ":status: failed" in rst + assert "9 values: 8 passed, 1 failed." in rst + assert "Twister reported the test as ``blocked``." in rst + assert "cases/8" in rst and "-22 != 0" in rst + assert "cases/0" not in rst + + +def test_reason_markup_is_escaped(): + assert rb._rst_text("a *b* `c` |d| x_y") == r"a \*b\* \`c\` \|d\| x\_y" + + +# --------------------------------------------------------------------------- +# Directive +# --------------------------------------------------------------------------- + + +@pytest.mark.sphinx("html", srcdir=str(_ROOTS / "test-testreport-param")) +def test_report_page_shows_the_failed_value(app, warning): + app.build() + html = (Path(app.outdir) / "index.html").read_text() + assert "TR-mps2-an385-kernel-semaphore-TSPEC-SEM-001" in html + assert "9 values: 8 passed, 1 failed" in html + assert "cases/8" in html and "Assertion failed" in html + assert "blocked" in html + assert "cases/3" not in html + # Recognised values are not "missing from the spec". + assert "not in spec needs.json" not in warning.getvalue() + assert "parameterized test" not in warning.getvalue() diff --git a/sphinx/_extensions/rst_builders.py b/sphinx/_extensions/rst_builders.py index 1325b4f..6ac8b60 100644 --- a/sphinx/_extensions/rst_builders.py +++ b/sphinx/_extensions/rst_builders.py @@ -11,6 +11,7 @@ # a role is absent from the mapping. import logging import re +from collections import Counter from pathlib import Path import yaml @@ -227,9 +228,60 @@ def build_result_rst(r, spec_id, test_module, req_ids=None, need_names=None): if r["reason"]: lines.append(f" :reason: {r['reason']}") lines.append("") + if r.get("values"): + lines += _values_rst(r) return "\n".join(lines) +def _values_summary(values): + """``"9 values: 8 passed, 1 failed"`` for a parameterized test's values.""" + counts = Counter(v["status"] for v in values) + order = ["passed", "failed", "error", "skipped"] + parts = [f"{counts[k]} {k}" for k in order if counts[k]] + parts += [f"{n} {k}" for k, n in sorted(counts.items()) if k not in order] + return f"{len(values)} values: {', '.join(parts)}" + + +_RST_INLINE = re.compile(r"([\\`*_|\[\]<>])") + + +def _rst_text(text): + """``text`` as literal RST prose: inline markup characters escaped.""" + return _RST_INLINE.sub(r"\\\1", " ".join(str(text).split())) + + +def _values_rst(r): + """The body of a parameterized test's result: counts, then what did not pass. + + Passed values are counted, not listed; a run of hundreds of values would + otherwise bury the one that failed. + """ + values = r["values"] + summary = _values_summary(values) + "." + if r.get("twister_status"): + summary += f" Twister reported the test as ``{r['twister_status']}``." + lines = [f" {summary}", ""] + others = [v for v in values if v["status"] != "passed"] + if others: + lines += [ + " .. list-table:: Values not passed", + " :header-rows: 1", + " :widths: 30 15 55", + "", + " * - Value", + " - Status", + " - Reason", + ] + for v in others: + lines += [ + f" * - {_rst_text(v['value'])}", + f" - {v['status']}", + f" - {_rst_text(v['reason']) if v['reason'] else '—'}", + ] + lines.append("") + return lines + + def build_scenario_table(testcase_yaml_path): """Return RST lines for a list-table of scenarios from testcase.yaml.""" try: diff --git a/sphinx/_extensions/test_module.py b/sphinx/_extensions/test_module.py index 0d8924f..ea851c3 100644 --- a/sphinx/_extensions/test_module.py +++ b/sphinx/_extensions/test_module.py @@ -22,10 +22,12 @@ from sphinx.util import logging from twister_reader import ( find_handler_log, + fold_parameterized_results, load_spec_lookup, load_twister_meta, parse_twister_results, scenario_selected, + testcase_statuses, testsuite_paths, ) @@ -665,26 +667,29 @@ def run(self): display_msg = f"[testreport: twister XML not found: {_display_name(xml_path)}]" return [nodes.paragraph(text=display_msg)] - # twister_report.xml has no testsuite path; twister.json, written - # beside it by the same run, does. - suite_paths = None - if path_filter is not None: - twister_json = Path(xml_path).parent / "twister.json" - _note_input(env, twister_json) - if not twister_json.exists(): - logger.warning( - f"testreport: :path: needs twister.json beside the report, " - f"not found: {twister_json}" - ) - return [nodes.paragraph( - text=f"[testreport: twister.json not found: {twister_json.name} " - f"(needed for :path:)]" - )] + # twister.json, written beside the XML by the same run, has what the + # XML lacks: each testsuite's path (needed for :path:) and statuses + # such as `blocked` (shown for a parameterized test, see below). + twister_json = Path(xml_path).parent / "twister.json" + _note_input(env, twister_json) + tw_meta = None + if twister_json.exists(): try: - suite_paths = testsuite_paths(load_twister_meta(twister_json)) + tw_meta = load_twister_meta(twister_json) except Exception as exc: logger.warning(f"testreport: cannot read {twister_json}: {exc}") - return [nodes.paragraph(text=f"[testreport: cannot read {twister_json.name}]")] + if path_filter is not None: + return [nodes.paragraph(text=f"[testreport: cannot read {twister_json.name}]")] + elif path_filter is not None: + logger.warning( + f"testreport: :path: needs twister.json beside the report, " + f"not found: {twister_json}" + ) + return [nodes.paragraph( + text=f"[testreport: twister.json not found: {twister_json.name} " + f"(needed for :path:)]" + )] + suite_paths = testsuite_paths(tw_meta) if tw_meta else None try: results = parse_twister_results( @@ -694,6 +699,16 @@ def run(self): logger.warning(str(exc)) return [nodes.paragraph(text=str(exc))] + # One result per parameter value becomes part of its test's result. + results, unmatched = fold_parameterized_results( + results, spec_lookup, testcase_statuses(tw_meta) if tw_meta else None + ) + for fn in unmatched: + logger.warning( + f"testreport: parameterized test '{fn}' has value results but no " + f"aggregate result and no unique spec case — its values are skipped" + ) + if not results: return [nodes.paragraph(text="[testreport: no matching results]")] diff --git a/sphinx/_extensions/twister_reader.py b/sphinx/_extensions/twister_reader.py index 5f41c93..861e5f2 100644 --- a/sphinx/_extensions/twister_reader.py +++ b/sphinx/_extensions/twister_reader.py @@ -6,6 +6,7 @@ import json import os +import re import xml.etree.ElementTree as ET from pathlib import Path @@ -14,13 +15,15 @@ # `rst_builders.py` carries the same "no Sphinx, no app.config" rule this # module follows — the mapping is passed in by the caller, never read from # config here — so importing its pure helper does not violate that rule. -from rst_builders import _need_name +from rst_builders import _need_name, _values_summary __all__ = [ "parse_twister_results", "normalise_test_path", "scenario_selected", "testsuite_paths", + "testcase_statuses", + "fold_parameterized_results", "SpecLookup", "load_spec_lookup", "find_handler_log", @@ -112,7 +115,15 @@ def parse_twister_results( continue name = tc.get("name", "") scenario = classname - suffix = name[len(scenario) + 1 :] if name.startswith(scenario + ".") else name + # A parameterized test (ZTEST_P) reports one result per value as + # `.[/]`: no suite segment, and + # the value may contain anything, dots included, so it is split + # off before the name is. + base, instance = name, None + if name.endswith("]") and "[" in name: + cut = name.index("[") + base, instance = name[:cut], name[cut + 1 : -1] + suffix = base[len(scenario) + 1 :] if base.startswith(scenario + ".") else base parts = suffix.rsplit(".", 1) suite = parts[0] if len(parts) == 2 else "" function = parts[-1] @@ -129,21 +140,160 @@ def parse_twister_results( status, reason = "skipped", skipped.get("message", "") or _elem_text(skipped) else: status, reason = "passed", "" - results.append( - { - "platform": platform, - "scenario": scenario, - "suite": suite, - "function": function, - "twister_id": name, - "time": tc.get("time", ""), - "status": status, - "reason": reason, - } - ) + if instance is not None and status in ("failed", "error"): + # Twister gives every value the suite's own message ("Testsuite + # failed"); the assertion is in the element's text. + reason = _assertion_text(failure if failure is not None else error) or reason + result = { + "platform": platform, + "scenario": scenario, + "suite": suite, + "function": function, + "twister_id": name, + "time": tc.get("time", ""), + "status": status, + "reason": reason, + } + if instance is not None: + result["instance"] = instance + results.append(result) return results +_ZTEST_MARKER = re.compile(r"^\s*(START|PASS|FAIL|SKIP) - ") + + +def _assertion_text(elem): + """The text of a failure element without ztest's START/PASS/FAIL marker lines.""" + lines = (elem.text or "").splitlines() if elem is not None else [] + kept = [line.strip() for line in lines if line.strip() and not _ZTEST_MARKER.match(line)] + return " ".join(kept) + + +def testcase_statuses(twister_meta): + """``{(platform, testcase identifier): status}`` from a loaded twister.json. + + twister.json keeps statuses the JUnit XML cannot express — ``blocked`` in + particular, which the XML reports as a failure. + """ + return { + (ts.get("platform", ""), tc.get("identifier", "")): tc.get("status", "") + for ts in twister_meta.get("testsuites", []) + for tc in ts.get("testcases", []) + } + + +def _attach_values(aggregate, instances, twister_statuses): + """Make ``aggregate`` the result of its parameter values. + + The verdict comes from the values: failed if any failed, error if any + errored, skipped if all were skipped, else passed. Twister's own status for + the aggregate is kept in ``twister_status`` when it disagrees: ztest + summarises a partly failing ZTEST_P as FLAKY, which twister does not + recognise and reports as ``blocked`` (twister.json) or "Testsuite failed" + (the XML). + """ + values = [ + {"value": r["instance"], "status": r["status"], "reason": r["reason"], "time": r["time"]} + for r in instances + ] + statuses = {v["status"] for v in values} + if "failed" in statuses: + verdict = "failed" + elif "error" in statuses: + verdict = "error" + elif statuses == {"skipped"}: + verdict = "skipped" + else: + verdict = "passed" + reported = aggregate.get("status", "") + if twister_statuses: + reported = twister_statuses.get((aggregate["platform"], aggregate["twister_id"]), reported) + aggregate["values"] = values + aggregate["twister_status"] = reported if reported and reported != verdict else "" + aggregate["status"] = verdict + aggregate["reason"] = "" if verdict == "passed" else _values_summary(values) + if not aggregate.get("time"): + aggregate["time"] = f"{sum(float(v['time'] or 0) for v in values):.2f}" + + +def fold_parameterized_results(results, spec_lookup=None, twister_statuses=None): + """Attach each parameterized test's value results to its aggregate result. + + Twister reports a ZTEST_P function once as the aggregate + ``..`` (from ztest's summary) and once per value as + ``.[/]`` (see `parse_twister_results`, + which marks the latter with ``instance``). The spec has one test case for + the function, so the values belong to the aggregate of the same run + (platform and scenario), found by the function name. + + A run without an aggregate gets one, with the suite taken from the + aggregates of other runs of the same scenario and function if they name + exactly one, else from the spec (``spec_lookup``) if exactly one case + carries the function. + + Returns ``(results, unmatched)``: the results without the value entries, + and the function names whose values could not be attached, once each. + """ + plain, by_run = [], {} + for r in results: + if r.get("instance") is None: + plain.append(r) + else: + by_run.setdefault((r["platform"], r["scenario"], r["function"]), []).append(r) + if not by_run: + return results, [] + + aggregates = {} + for r in plain: + aggregates.setdefault((r["platform"], r["scenario"], r["function"]), []).append(r) + + unmatched = [] + for (platform, scenario, fn), instances in by_run.items(): + hits = aggregates.get((platform, scenario, fn), []) + if len(hits) == 1: + aggregate = hits[0] + elif hits: + aggregate = None # several suites share the name in this run + else: + aggregate = _synthesized_aggregate( + platform, scenario, fn, aggregates, spec_lookup + ) + if aggregate is not None: + plain.append(aggregate) + if aggregate is None: + if fn not in unmatched: + unmatched.append(fn) + continue + _attach_values(aggregate, instances, twister_statuses) + return plain, unmatched + + +def _synthesized_aggregate(platform, scenario, fn, aggregates, spec_lookup): + suites = { + r["suite"] + for (_p, sc, f), rs in aggregates.items() + if sc == scenario and f == fn + for r in rs + } + if len(suites) == 1: + suite = suites.pop() + elif not suites and spec_lookup is not None and (info := spec_lookup.find("", fn)): + suite = info.get("suite", "") + else: + return None + return { + "platform": platform, + "scenario": scenario, + "suite": suite, + "function": fn, + "twister_id": f"{scenario}.{suite}.{fn}", + "time": "", + "status": "", + "reason": "", + } + + class SpecLookup: """The spec's test cases, found by the (suite, function) of a twister result.