From 470820d183bc60af3b5b6573239b70c043d09d35 Mon Sep 17 00:00:00 2001 From: Alexander Lanin Date: Thu, 3 Sep 2026 02:01:19 +0200 Subject: [PATCH] refactor: simplify Needs JSON project URL export --- .../score_metamodel/external_needs.py | 56 ++++++++++++------- .../tests/test_external_needs.py | 53 ++++++++++++++++++ 2 files changed, 88 insertions(+), 21 deletions(-) diff --git a/src/extensions/score_metamodel/external_needs.py b/src/extensions/score_metamodel/external_needs.py index 51f927a21..d51ca9e13 100644 --- a/src/extensions/score_metamodel/external_needs.py +++ b/src/extensions/score_metamodel/external_needs.py @@ -130,34 +130,48 @@ def parse_external_needs_sources_from_bazel_query() -> list[ExternalNeedsSource] return res -def extend_needs_json_exporter(config: Config, params: list[str]) -> None: +def extend_needs_json_exporter( + config: Config, + *, + exported_project_url: str | None = None, +) -> None: + """Add ``project_url`` to the Needs JSON export. + + By default, the value comes from the Sphinx configuration and an empty + value is reported as a configuration error. ``exported_project_url`` is an + explicit replacement for the JSON value; supplying it also makes an empty + replacement valid without changing the active Sphinx configuration. + + This function is intended to be called once during Sphinx configuration. """ - This will add each param to app.config as a config value. - Then it will overwrite the needs.json exporter to include these values. - """ - - for p in params: - # Note: we are currently addinig these values to config after config-inited. - # This is wrong. But good enough. - config.add(p, default="", rebuild="env", types=(), description="") + # ``project_url`` is a SCORE-specific configuration value, so register it + # before accessing it. + config.add("project_url", default="", rebuild="env", types=(), description="") + if exported_project_url is None and not config.project_url: + logger.error( + "Config value 'project_url' is not set. " + + "Please set it in your Sphinx config." + ) - if not getattr(config, p): - logger.error( - f"Config value '{p}' is not set. " - + "Please set it in your Sphinx config." - ) + # Keep export values on the Sphinx config object, which is also retained + # by each NeedsList instance, and leave room for adding more fields later. + config._score_metamodel_needs_json_export = {"project_url": exported_project_url} - # Patch json exporter to include our custom fields - # Note: yeah, NeedsList is the json exporter! + # Patch json exporter to include our custom field. + # Note: ``NeedsList`` is the sphinx-needs JSON exporter. orig_function = NeedsList._finalise # pyright: ignore[reportPrivateUsage] - def temp(self: NeedsList): - for p in params: - self.needs_list[p] = getattr(config, p) # pyright: ignore[reportUnknownMemberType] + def finalise_with_export_values(self: NeedsList): + for name, override in getattr( + self.config, "_score_metamodel_needs_json_export", {} + ).items(): + self.needs_list[name] = ( + getattr(self.config, name) if override is None else override + ) orig_function(self) - NeedsList._finalise = temp # pyright: ignore[reportPrivateUsage] + NeedsList._finalise = finalise_with_export_values # pyright: ignore[reportPrivateUsage] def get_external_needs_source(external_needs_source: str) -> list[ExternalNeedsSource]: @@ -228,7 +242,7 @@ def add_external_docs_sources(e: ExternalNeedsSource, config: Config): def connect_external_needs(app: Sphinx, config: Config): - extend_needs_json_exporter(config, ["project_url"]) + extend_needs_json_exporter(config) # Local external needs from DATA (e.g. :needs_json or :docs_sources) external_needs = get_external_needs_source(app.config.external_needs_source) diff --git a/src/extensions/score_metamodel/tests/test_external_needs.py b/src/extensions/score_metamodel/tests/test_external_needs.py index 5c8929b41..420092d82 100644 --- a/src/extensions/score_metamodel/tests/test_external_needs.py +++ b/src/extensions/score_metamodel/tests/test_external_needs.py @@ -19,6 +19,8 @@ import json from pathlib import Path +from types import SimpleNamespace +from typing import cast import pytest import score_metamodel.external_needs as ext_needs @@ -31,6 +33,57 @@ parse_external_needs_sources_from_DATA, ) from sphinx.config import Config +from sphinx_needs.needsfile import NeedsList + + +def _finalise(config: Config) -> NeedsList: + needs_list = cast(NeedsList, SimpleNamespace(needs_list={}, config=config)) + NeedsList._finalise(needs_list) # pyright: ignore[reportPrivateUsage] - white-box test + return needs_list + + +def test_extend_needs_json_exporter_reports_missing_config( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """The host export reports a missing project URL as a configuration error.""" + errors: list[str] = [] + monkeypatch.setattr(NeedsList, "_finalise", lambda _needs_list: None) + monkeypatch.setattr(ext_needs.logger, "error", errors.append) + + ext_needs.extend_needs_json_exporter(Config()) + + assert errors == [ + "Config value 'project_url' is not set. Please set it in your Sphinx config." + ] + + +@pytest.mark.parametrize( + ("exported_project_url", "expected_project_url"), + [(None, "https://example.test/after"), ("", "")], +) +def test_extend_needs_json_exporter_uses_configured_or_explicit_value( + monkeypatch: pytest.MonkeyPatch, + exported_project_url: str | None, + expected_project_url: str, +) -> None: + """The export uses either the current config value or its explicit override.""" + config = Config() + config.project_url = "https://example.test/before" + + monkeypatch.setattr(NeedsList, "_finalise", lambda _needs_list: None) + ext_needs.extend_needs_json_exporter( + config, exported_project_url=exported_project_url + ) + if exported_project_url is None: + config.project_url = "https://example.test/after" + needs_list = _finalise(config) + + assert needs_list.needs_list["project_url"] == expected_project_url + assert config.project_url == ( + "https://example.test/after" + if exported_project_url is None + else "https://example.test/before" + ) def test_empty_list():