From 6650aa91b9ab5ecf91af181e0d4313a4c0d6a95b Mon Sep 17 00:00:00 2001 From: Alexander Lanin Date: Wed, 9 Sep 2026 23:47:15 +0200 Subject: [PATCH] refactor: DocsCliConfig in cli.py --- src/docs_cli/cli.py | 359 +++++++++++++----- src/docs_cli/dirty_build_test.py | 1 + src/docs_cli/main_test.py | 153 +++++++- .../__init__.py | 23 +- src/extensions/score_layout/__init__.py | 8 +- src/extensions/score_metamodel/__init__.py | 7 +- src/extensions/score_metamodel/log.py | 8 +- src/extensions/score_mounts/__init__.py | 30 +- .../score_source_code_linker/__init__.py | 10 +- .../score_source_code_linker/needlinks.py | 12 - .../tests/test_codelink.py | 50 ++- .../test_repo_source_link_integration.py | 7 +- .../test_source_code_link_integration.py | 11 +- .../tests/test_xml_parser.py | 28 +- .../score_source_code_linker/xml_parser.py | 25 +- 15 files changed, 533 insertions(+), 199 deletions(-) diff --git a/src/docs_cli/cli.py b/src/docs_cli/cli.py index 1e9fb18d1..07139cc67 100644 --- a/src/docs_cli/cli.py +++ b/src/docs_cli/cli.py @@ -21,7 +21,9 @@ import shutil import sys import time +from collections.abc import Mapping from pathlib import Path +from typing import cast import debugpy from sphinx.cmd.build import main as sphinx_main @@ -30,7 +32,7 @@ ) from src.extensions.score_mounts._resolver import load_mounts_manifest, resolve_walk_dir -from src.helper_lib import find_ws_root, get_runfiles_dir +from src.helper_lib import get_runfiles_dir logger = logging.getLogger(__name__) @@ -38,23 +40,221 @@ _MODULE_HASH_FILE = ".module_bazel_hash" -def get_env(name: str) -> str: - val = os.environ.get(name) - logger.debug("Env: %s = %s", name, val) - if val is None: - raise ValueError(f"Environment variable {name} is not set") - return val +class Environment: + """Typed access to the process environment used by the CLI config loader.""" + + def __init__(self, values: Mapping[str, str] | None = None) -> None: + self._values = os.environ if values is None else values + + def get(self, name: str, default: str | None = None) -> str: + """Read a value, raising when it is missing and no default is supplied.""" + value = self._values.get(name) + logger.debug("Env: %s = %s", name, value) + if value is not None: + # Preserve an explicitly configured value, including an empty string. + return value + elif default is not None: + # A caller-provided default makes this environment variable optional. + return default + else: + # A missing value without a default is required configuration. + raise ValueError(f"Environment variable {name} is not set") + + def optional_path(self, name: str) -> Path | None: + """Read an optional path from the environment.""" + value = self.get(name, "") + return Path(value) if value else None + + def required_path(self, name: str) -> Path: + """Read a required path from the environment.""" + value = self.get(name, "") + if not value: + raise ValueError(f"Environment variable {name} is not set") + return Path(value) + + def json(self, name: str, default: str | None = None) -> object: + """Read and decode a JSON value from the environment.""" + return json.loads(self.get(name, default)) + + def string_list(self, name: str, default: str | None = None) -> list[str]: + """Read a JSON list and validate that every item is a string.""" + raw_value = self.get(name, default) + # DATA was historically allowed to be present but empty. Treat that as + # an empty list while still requiring the environment variable itself. + if not raw_value: + return [] + value = json.loads(raw_value) + if not isinstance(value, list) or not all( + isinstance(item, str) for item in cast(list[object], value) + ): + raise ValueError( + f"Environment variable {name} must contain a list of strings" + ) + return cast(list[str], value) -def _merged_external_needs() -> str: - """Combine DATA and EXTERNAL_NEEDS_FILES into one JSON label list. +def _merged_external_needs(env: Environment) -> list[str]: + """Combine DATA and EXTERNAL_NEEDS_FILES into one label list. Both env vars hold JSON lists of Bazel labels; the extension parses the resulting `external_needs_source` define uniformly. """ - data = json.loads(get_env("DATA") or "[]") - external = json.loads(os.environ.get("EXTERNAL_NEEDS_FILES", "[]") or "[]") - return json.dumps(data + external) + return env.string_list("DATA") + env.string_list( + "EXTERNAL_NEEDS_FILES", default="[]" + ) + + +class DocsCliConfig: + """Configuration consumed by the documentation launcher. + + Keeping environment parsing in one place lets the launcher operate on a + stable configuration object. Paths stored on this object are resolved to + the filesystem visible to the current process. The logical package and + source paths remain available for repository metadata such as GitHub edit + links. + """ + + action: str + ws_root: Path | None + package_directory: Path + package_dir: Path + source_directory_relative: Path + source_directory: Path + output_directory: Path + build_dir: Path + external_needs_sources: list[str] + testcase_source_dirs: list[str] + mounts_manifest: Path | None + sphinx_config_file: Path | None + metamodel_yaml: Path | None + score_sourcelinks_json: Path | None + known_good_json: Path | None + sphinx_extra_opts: list[str] + github_repository: str | None + + @property + def is_bazel_build(self) -> bool: + """Whether this configuration belongs to the sandboxed Needs action.""" + return self.action == "build_needs_json" + + @property + def is_bazel_run(self) -> bool: + """Whether this configuration belongs to a ``bazel run`` target.""" + return self.ws_root is not None and not self.is_bazel_build + + @property + def is_direct(self) -> bool: + """Whether the launcher was started outside Bazel.""" + return not self.is_bazel_build and not self.is_bazel_run + + @classmethod + def from_environment(cls, env: Environment | None = None) -> "DocsCliConfig": + """Load configuration from the process environment or a test mapping.""" + return cls(env if env is not None else Environment()) + + def __init__(self, env: Environment): + """ + Load launcher configuration from the current Bazel environment. + + Specifically, this method handles bazel build and run differences. + """ + self.action = env.get("ACTION") + self.ws_root = env.optional_path("BUILD_WORKSPACE_DIRECTORY") + if self.ws_root is not None: + self._require_directory(self.ws_root, "BUILD_WORKSPACE_DIRECTORY") + # An empty PACKAGE_DIR intentionally denotes the workspace root. + self.package_directory = Path(env.get("PACKAGE_DIR", "")) + self.source_directory_relative = Path(env.get("SOURCE_DIRECTORY")) + + if self.is_bazel_build: + # Build actions run from the execution root and do not expose the + # caller's workspace directory. Their source and output paths must + # therefore be resolved from the action's current working directory. + self.package_dir = Path.cwd() + self.source_directory = ( + self.package_dir / self.source_directory_relative + ).absolute() + self.output_directory = env.required_path("OUTPUT_DIRECTORY").absolute() + else: + workspace_root = self.ws_root or Path.cwd() + self.package_dir = workspace_root / self.package_directory + self.source_directory = self.package_dir / self.source_directory_relative + self.output_directory = self.package_dir / "_build" + self.build_dir = self.output_directory + self._require_directory(self.source_directory, "SOURCE_DIRECTORY") + + self.external_needs_sources = _merged_external_needs(env) + self.testcase_source_dirs = env.string_list("TEST_SOURCES", "[]") + self.mounts_manifest = self._resolve_input_path( + env.optional_path("MOUNTS_MANIFEST") + ) + self._require_file(self.mounts_manifest, "MOUNTS_MANIFEST") + self.sphinx_config_file = self._resolve_input_path( + env.optional_path("SPHINX_CONFIG_FILE") + ) + self._require_file(self.sphinx_config_file, "SPHINX_CONFIG_FILE") + self.metamodel_yaml = self._resolve_input_path( + env.optional_path("SCORE_METAMODEL_YAML") + ) + self._require_file(self.metamodel_yaml, "SCORE_METAMODEL_YAML") + self.score_sourcelinks_json = self._resolve_execution_path( + env.optional_path("SCORE_SOURCELINKS") + ) + self._require_file(self.score_sourcelinks_json, "SCORE_SOURCELINKS") + self.known_good_json = self._resolve_execution_path( + env.optional_path("KNOWN_GOOD_JSON") + ) + self._require_file(self.known_good_json, "KNOWN_GOOD_JSON") + self.sphinx_extra_opts = ( + env.string_list("SPHINX_EXTRA_OPTS", "[]") if self.is_bazel_build else [] + ) + self.github_repository = env.get("GITHUB_REPOSITORY", "") or None + + def _resolve_input_path(self, path: Path | None) -> Path | None: + """Resolve an optional config input in its current execution context.""" + if path is None or path.is_absolute(): + return path + elif self.is_bazel_build: + return (Path.cwd() / path).absolute() + elif self.is_bazel_run: + # Interactive Bazel targets receive runfiles-relative paths from + # ``rlocationpath``. The runfiles tree is the only stable location + # for generated files and external repository inputs. + return get_runfiles_dir() / path + else: + # Direct invocations resolve relative inputs from the workspace or + # current working directory. + return ((self.ws_root or Path.cwd()) / path).absolute() + + def _resolve_execution_path(self, path: Path | None) -> Path | None: + """Resolve a path consumed directly from the process working directory.""" + if path is None or path.is_absolute(): + return path + elif self.is_bazel_run: + # ``KNOWN_GOOD_JSON`` comes from Bazel's ``$(location)`` expansion, + # not ``$(rlocationpath)``. Bazel-run processes use the workspace as + # their working directory, so preserve that consumer-facing path. + return ((self.ws_root or Path.cwd()) / path).absolute() + else: + return (Path.cwd() / path).absolute() + + @staticmethod + def _require_directory(path: Path, environment_name: str) -> None: + """Fail while loading config when a required directory is unavailable.""" + if not path.is_dir(): + raise ValueError( + f"Environment variable {environment_name} must name an existing " + f"directory: {path}" + ) + + @staticmethod + def _require_file(path: Path | None, environment_name: str) -> None: + """Fail while loading config when an optional file is configured badly.""" + if path is not None and not path.is_file(): + raise ValueError( + f"Environment variable {environment_name} must name an existing " + f"file: {path}" + ) def _compute_hash(files: list[Path]) -> str: @@ -144,13 +344,11 @@ def add_watch_dir(path: Path) -> None: return watch_dirs -def sphinx_arguments(ws_root: Path, package_dir: Path, build_dir: Path) -> list[str]: +def sphinx_arguments(config: DocsCliConfig) -> list[str]: """Resolve package sources and Bazel-provided configuration for every builder.""" - is_bazel_build = os.environ.get("ACTION") == "build_needs_json" - source_directory = get_env("SOURCE_DIRECTORY") base_arguments = [ - str(package_dir / source_directory), - str(build_dir), + str(config.source_directory), + str(config.output_directory), "-W", # treat warning as errors "--keep-going", # do not abort after one error "-T", # show details in case of errors in extensions @@ -158,98 +356,85 @@ def sphinx_arguments(ws_root: Path, package_dir: Path, build_dir: Path) -> list[ "auto", # Merge DATA (:needs_json / :docs_sources) with EXTERNAL_NEEDS_FILES # (:needs_json_file) into one define consumed by the Sphinx extensions. - f"--define=external_needs_source={_merged_external_needs()}", - f"--define=testcase_source_dirs={os.environ.get('TEST_SOURCES', '[]')}", + f"--define=external_needs_source={json.dumps(config.external_needs_sources)}", + f"--define=testcase_source_dirs={json.dumps(config.testcase_source_dirs)}", # Path to the Bazel-emitted mounts manifest (empty when no mounts are # configured); consumed by the score_mounts extension. - f"--define=mounts_manifest={os.environ.get('MOUNTS_MANIFEST', '')}", + f"--define=mounts_manifest={config.mounts_manifest or ''}", ] - if is_bazel_build: + if config.is_bazel_build: # The Bazel action declares ``build_dir`` as its output tree, and that # tree must contain only the Needs inventory consumed by downstream # actions. Keep Sphinx's internal doctree cache beside it instead of # mixing action state into the declared output. - base_arguments.extend(["-d", str(build_dir) + "_doctrees"]) + base_arguments.extend(["-d", str(config.build_dir) + "_doctrees"]) # The sandboxed Needs rule transports options as JSON so spaces, quotes and # equals signs survive the environment boundary. Append them last so an # action-specific value can override one of the shared defaults above. - base_arguments.extend(json.loads(os.environ.get("SPHINX_EXTRA_OPTS", "[]"))) + base_arguments.extend(config.sphinx_extra_opts) else: # Interactive builds keep warnings in the workspace so developers can # inspect them after a failed build. A Bazel action reports failure # through its exit code and must leave its declared output tree free of # this diagnostic side file. - base_arguments.extend(["--warning-file", str(build_dir / "warnings.txt")]) - - generated_config = os.environ.get("SPHINX_CONFIG_FILE", "") - if generated_config: - # The action receives ctx.file.config.path, which is interpreted from - # the action's execution-root working directory. Resolve it locally - # instead of using runfiles lookup; interactive targets receive a - # runfiles-relative path and need that lookup before Sphinx gets the - # containing directory. - config_file = Path(generated_config) - if is_bazel_build: - config_file = config_file.absolute() - elif not config_file.is_absolute(): - config_file = get_runfiles_dir() / config_file - base_arguments.extend(["-c", str(config_file.parent)]) - - metamodel_yaml = os.environ.get("SCORE_METAMODEL_YAML", "") + base_arguments.extend( + ["--warning-file", str(config.build_dir / "warnings.txt")] + ) + + if config.sphinx_config_file: + # ``DocsCliConfig`` has already resolved the action path from the + # execution root or the interactive runfiles tree. Sphinx needs the + # containing directory rather than the ``conf.py`` path itself. + base_arguments.extend(["-c", str(config.sphinx_config_file.parent)]) + + metamodel_yaml = config.metamodel_yaml if metamodel_yaml: - # Under ``bazel run``, this environment variable is runfiles-relative - # and must be resolved through RUNFILES_DIR. A sandboxed Needs action - # instead expands the metamodel label to an execution-root path in - # SPHINX_EXTRA_OPTS; applying runfiles lookup there would escape the - # action's declared inputs. - if not is_bazel_build and not os.path.isabs(metamodel_yaml): - runfiles_dir = os.environ.get("RUNFILES_DIR", "") - metamodel_yaml = str( - (Path(runfiles_dir) / metamodel_yaml) - if runfiles_dir - else (ws_root / metamodel_yaml) - ) - metamodel_yaml = os.path.abspath(metamodel_yaml) + # ``DocsCliConfig`` resolves runfiles-relative paths for interactive + # targets and execution-root paths for the sandboxed action. The + # sandbox must not perform a second runfiles lookup because that would + # escape the action's declared inputs. base_arguments.append(f"--define=score_metamodel_yaml={metamodel_yaml}") - if github_repository := os.getenv("GITHUB_REPOSITORY"): + if config.score_sourcelinks_json: + base_arguments.append( + f"--define=score_sourcelinks_json={config.score_sourcelinks_json}" + ) + + if config.github_repository: # GITHUB_REPOSITORY is expected as "owner/repo"; partition("/") splits # once into (owner, separator, repo), so we can ignore the separator. - github_user, _, github_repo = github_repository.partition("/") + github_user, _, github_repo = config.github_repository.partition("/") base_arguments.append(f"-A=github_user={github_user}") base_arguments.append(f"-A=github_repo={github_repo}") base_arguments.append("-A=github_version=main") # doc_path must be repo-relative so the edit URL does not contain the # absolute runner filesystem path (e.g. /home/runner/work/…/docs). - relative_doc_path = Path(os.environ.get("PACKAGE_DIR", "")) / source_directory + relative_doc_path = config.package_directory / config.source_directory_relative base_arguments.append(f"-A=doc_path={relative_doc_path}") - if os.getenv("KNOWN_GOOD_JSON"): - base_arguments.append(f"--define=KNOWN_GOOD_JSON={get_env('KNOWN_GOOD_JSON')}") + if config.known_good_json: + base_arguments.append(f"--define=KNOWN_GOOD_JSON={config.known_good_json}") return base_arguments -def watch_arguments() -> list[str]: +def watch_arguments(config: DocsCliConfig) -> list[str]: """Build autobuild options using the same runfiles resolution as Sphinx.""" - mounts_manifest = os.environ.get("MOUNTS_MANIFEST", "") + mounts_manifest = config.mounts_manifest watch_arguments: list[str] = [] if mounts_manifest: - # ``MOUNTS_MANIFEST`` is runfiles-relative under ``bazel run`` and - # an ordinary path for direct invocations, matching score_mounts. - ws_root = find_ws_root() - manifest_path = ( - get_runfiles_dir() / mounts_manifest - if ws_root is not None - else Path(mounts_manifest) - ) + # ``DocsCliConfig`` has already resolved the manifest to the filesystem + # path consumed by score_mounts. Keep the runfiles root separately for + # resolving mounted data directories. + ws_root = config.ws_root + runfiles_dir = get_runfiles_dir() if config.is_bazel_run else None for watch_dir in mounted_watch_dirs( - manifest_path, + mounts_manifest, ws_root, - get_runfiles_dir() if ws_root is not None else None, + runfiles_dir, ): watch_arguments.extend(["--watch", watch_dir]) return watch_arguments @@ -284,28 +469,24 @@ def main(argv: list[str] | None = None) -> int: logger.info("Waiting for client to connect on port: " + str(args.debug_port)) debugpy.wait_for_client() - action = get_env("ACTION") - is_bazel_build = action == "build_needs_json" - ws_root = Path(os.getenv("BUILD_WORKSPACE_DIRECTORY", "")) - # Docs source and output are resolved relative to the package where docs() - # was called; an empty PACKAGE_DIR denotes the workspace root. - package_dir = ws_root / os.environ.get("PACKAGE_DIR", "") - build_dir = package_dir / "_build" - if is_bazel_build: - # Bazel owns the action's paths; never use the caller's workspace cache. - package_dir = Path.cwd() - build_dir = Path(get_env("OUTPUT_DIRECTORY")).absolute() + config = DocsCliConfig.from_environment() + action = config.action + is_bazel_build = config.is_bazel_build + # Interactive Bazel targets reuse the package cache. A direct invocation + # uses the current directory as its workspace fallback. Build actions do + # not inspect sentinels because their declared output is always isolated. + workspace_root = config.ws_root or Path.cwd() sentinel_files = [ - ws_root / "MODULE.bazel", - ws_root / "MODULE.bazel.lock", - package_dir / "BUILD", + workspace_root / "MODULE.bazel", + workspace_root / "MODULE.bazel.lock", + config.package_dir / "BUILD", ] if not is_bazel_build: - clean_builddir_if_stale(build_dir, sentinel_files) + clean_builddir_if_stale(config.build_dir, sentinel_files) - warning_file = build_dir / "warnings.txt" - base_arguments = sphinx_arguments(ws_root, package_dir, build_dir) + warning_file = config.build_dir / "warnings.txt" + base_arguments = sphinx_arguments(config) if action == "live_preview": sphinx_autobuild_main( @@ -315,7 +496,7 @@ def main(argv: list[str] | None = None) -> int: "--define=skip_rescanning_via_source_code_linker=1", f"--port={args.port}", ] - + watch_arguments() + + watch_arguments(config) ) return 0 @@ -341,7 +522,7 @@ def main(argv: list[str] | None = None) -> int: return exit_code if exit_code == 0: - update_module_hash(build_dir, sentinel_files) + update_module_hash(config.build_dir, sentinel_files) else: with warning_file.open("a", encoding="utf-8") as f: f.write("-" * 80 + "\n") diff --git a/src/docs_cli/dirty_build_test.py b/src/docs_cli/dirty_build_test.py index f8e82281c..fa74f48b1 100644 --- a/src/docs_cli/dirty_build_test.py +++ b/src/docs_cli/dirty_build_test.py @@ -38,6 +38,7 @@ def docs_workspace(fs: FFS, monkeypatch: pytest.MonkeyPatch) -> Path: monkeypatch.setenv("SOURCE_DIRECTORY", "docs") monkeypatch.setenv("DATA", "[]") fs.create_dir(_WORKSPACE / "component") + fs.create_dir(_WORKSPACE / "component/docs") for name in ("MODULE.bazel", "MODULE.bazel.lock", "component/BUILD"): fs.create_file(_WORKSPACE / name, contents="stable") return _WORKSPACE diff --git a/src/docs_cli/main_test.py b/src/docs_cli/main_test.py index 04ca0b6fd..de87379ce 100644 --- a/src/docs_cli/main_test.py +++ b/src/docs_cli/main_test.py @@ -18,7 +18,7 @@ from pyfakefs.fake_filesystem import FakeFilesystem as FFS from src.docs_cli import cli as docs_cli -from src.docs_cli.cli import sphinx_arguments +from src.docs_cli.cli import DocsCliConfig, sphinx_arguments @pytest.fixture @@ -33,8 +33,10 @@ def workspace(fs: FFS, monkeypatch: pytest.MonkeyPatch) -> Path: "MOUNTS_MANIFEST", "SPHINX_CONFIG_FILE", "SCORE_METAMODEL_YAML", + "SCORE_SOURCELINKS", "GITHUB_REPOSITORY", "KNOWN_GOOD_JSON", + "SPHINX_EXTRA_OPTS", "RUNFILES_DIR", "RUNFILES_MANIFEST_FILE", ) @@ -46,12 +48,28 @@ def workspace(fs: FFS, monkeypatch: pytest.MonkeyPatch) -> Path: monkeypatch.setenv("PACKAGE_DIR", "component") monkeypatch.setenv("SOURCE_DIRECTORY", "docs") monkeypatch.setenv("DATA", "[]") + monkeypatch.setenv("SPHINX_EXTRA_OPTS", "[]") monkeypatch.setenv("RUNFILES_DIR", str(workspace / "runfiles")) fs.create_dir(workspace / "component") + fs.create_dir(workspace / "component/docs") + fs.create_dir(workspace / "docs") + fs.create_dir(workspace / "bundle/docs") fs.create_dir(workspace / "runfiles") + fs.create_dir(workspace / "runfiles/config") for name in ("MODULE.bazel", "MODULE.bazel.lock", "component/BUILD"): fs.create_file(workspace / name, contents="stable") + for name in ( + "runfiles/config/conf.py", + "runfiles/config/metamodel.yaml", + "source_links.json", + "baseline.json", + "metamodel.yaml", + "bundle/conf.py", + "bundle/metamodel.yaml", + ): + fs.create_file(workspace / name, contents="{}") + fs.create_file(workspace / "runfiles/config/mounts.json", contents='{"mounts": []}') return workspace @@ -93,12 +111,19 @@ def test_build_action_selects_sphinx_builder( assert exit_code == 0 noop_sphinx.assert_called_once() arguments = noop_sphinx.call_args.args[0] - # The source and output paths are derived from the Bazel package directory. + # The source and output paths are derived from the correct execution + # context. The build action uses its execution-root source and declared + # output; interactive actions use the workspace package cache. if action == "build_needs_json": assert arguments[:2] == [str(workspace / "docs"), str(build_dir)] + # The declared output contains only the Needs inventory; Sphinx's + # doctrees and warning diagnostics stay outside that output tree. + assert ["-d", str(build_dir) + "_doctrees"] == arguments[10:12] + assert "--warning-file" not in arguments update_hash.assert_not_called() else: assert arguments[:2] == [str(workspace / "component/docs"), str(build_dir)] + assert "--warning-file" in arguments # The action selects the builder exposed by its public Bazel target. assert arguments[-2:] == ["-b", builder] @@ -186,30 +211,41 @@ def test_bazel_configuration_resolves_runfiles_and_preserves_repo_relative_edit_ monkeypatch: pytest.MonkeyPatch, ) -> None: # Arrange + monkeypatch.setenv("ACTION", "incremental") monkeypatch.setenv("SPHINX_CONFIG_FILE", "config/conf.py") monkeypatch.setenv("SCORE_METAMODEL_YAML", "config/metamodel.yaml") + monkeypatch.setenv("MOUNTS_MANIFEST", "config/mounts.json") + monkeypatch.setenv("SCORE_SOURCELINKS", "source_links.json") monkeypatch.setenv("DATA", '[":bundle"]') monkeypatch.setenv("EXTERNAL_NEEDS_FILES", '["@vendor//:needs"]') monkeypatch.setenv("GITHUB_REPOSITORY", "owner/repo") monkeypatch.setenv("KNOWN_GOOD_JSON", "baseline.json") - package = workspace / "component" # Act - arguments = sphinx_arguments(workspace, package, package / "_build") + config = DocsCliConfig.from_environment() + assert config.source_directory == workspace / "component/docs" + assert config.sphinx_config_file == workspace / "runfiles/config/conf.py" + assert config.metamodel_yaml == workspace / "runfiles/config/metamodel.yaml" + assert config.mounts_manifest == workspace / "runfiles/config/mounts.json" + assert config.score_sourcelinks_json == workspace / "source_links.json" + assert config.known_good_json == workspace / "baseline.json" + arguments = sphinx_arguments(config) # Assert expected_arguments = { - # Generated configuration and metamodel paths use the runfiles tree. + # Runfiles-backed configuration inputs use the runfiles tree. "-c", str(workspace / "runfiles/config"), f"--define=score_metamodel_yaml={workspace}/runfiles/config/metamodel.yaml", + f"--define=mounts_manifest={workspace}/runfiles/config/mounts.json", + f"--define=score_sourcelinks_json={workspace}/source_links.json", # DATA and EXTERNAL_NEEDS_FILES are passed as one Sphinx define. '--define=external_needs_source=[":bundle", "@vendor//:needs"]', # GitHub metadata must keep edit links repository-relative. "-A=github_user=owner", "-A=github_repo=repo", "-A=doc_path=component/docs", - "--define=KNOWN_GOOD_JSON=baseline.json", + f"--define=KNOWN_GOOD_JSON={workspace}/baseline.json", } # Every expected option is present; their relative order is irrelevant here. assert expected_arguments <= set(arguments) @@ -221,14 +257,117 @@ def test_direct_invocation_resolves_metamodel_relative_to_workspace( ) -> None: # Arrange # This test covers the non-Bazel fallback, so no runfiles directory exists. + monkeypatch.setenv("ACTION", "incremental") + monkeypatch.delenv("BUILD_WORKSPACE_DIRECTORY", raising=False) monkeypatch.delenv("RUNFILES_DIR", raising=False) monkeypatch.setenv("SCORE_METAMODEL_YAML", "metamodel.yaml") + monkeypatch.chdir(workspace) # Act - arguments = sphinx_arguments(workspace, workspace, workspace / "_build") + arguments = sphinx_arguments(DocsCliConfig.from_environment()) # Assert # Without Bazel runfiles, the metamodel falls back to the workspace root. assert f"--define=score_metamodel_yaml={workspace}/metamodel.yaml" in arguments # A direct invocation has no generated Sphinx config to resolve. assert "-c" not in arguments + + +def test_bazel_run_allows_workspace_root_package( + workspace: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """The root package is represented by an intentionally empty PACKAGE_DIR.""" + # Arrange + monkeypatch.setenv("ACTION", "incremental") + monkeypatch.setenv("PACKAGE_DIR", "") + + # Act + config = DocsCliConfig.from_environment() + + # Assert + assert config.is_bazel_run + assert config.package_directory == Path() + assert config.package_dir == workspace + assert config.source_directory == workspace / "docs" + assert config.build_dir == workspace / "_build" + + +def test_bazel_build_configuration_uses_execution_root_paths_and_extra_options( + workspace: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Build actions use declared paths and preserve action-specific options.""" + # Arrange + monkeypatch.setenv("ACTION", "build_needs_json") + monkeypatch.setenv("SOURCE_DIRECTORY", "bundle/docs") + monkeypatch.setenv("OUTPUT_DIRECTORY", "outputs/needs") + monkeypatch.setenv("SPHINX_CONFIG_FILE", "bundle/conf.py") + monkeypatch.setenv("SCORE_METAMODEL_YAML", "bundle/metamodel.yaml") + monkeypatch.setenv("SPHINX_EXTRA_OPTS", '["--define=custom=value with spaces"]') + monkeypatch.chdir(workspace) + + # Act + config = DocsCliConfig.from_environment() + arguments = sphinx_arguments(config) + + # Assert + assert config.is_bazel_build + assert not config.is_bazel_run + assert config.source_directory == workspace / "bundle/docs" + assert config.output_directory == workspace / "outputs/needs" + assert config.sphinx_config_file == workspace / "bundle/conf.py" + assert config.metamodel_yaml == workspace / "bundle/metamodel.yaml" + assert arguments[:2] == [ + str(workspace / "bundle/docs"), + str(workspace / "outputs/needs"), + ] + assert ["-d", str(workspace / "outputs/needs_doctrees")] == arguments[10:12] + assert "--warning-file" not in arguments + assert "--define=custom=value with spaces" in arguments + + +@pytest.mark.parametrize( + "environment_name,value", + [ + ("BUILD_WORKSPACE_DIRECTORY", "/workspace/missing"), + ("SOURCE_DIRECTORY", "missing/docs"), + ("SPHINX_CONFIG_FILE", "config/missing.py"), + ("SCORE_METAMODEL_YAML", "config/missing.yaml"), + ("KNOWN_GOOD_JSON", "missing.json"), + ("MOUNTS_MANIFEST", "/workspace/missing-mounts.json"), + ("SCORE_SOURCELINKS", "missing-source-links.json"), + ], +) +def test_configuration_rejects_missing_input_paths_early( + workspace: Path, + monkeypatch: pytest.MonkeyPatch, + environment_name: str, + value: str, +) -> None: + """Configured workspace and input paths fail before Sphinx is invoked.""" + # Arrange + monkeypatch.setenv("ACTION", "incremental") + monkeypatch.setenv(environment_name, value) + + # Act and assert + with pytest.raises(ValueError, match=environment_name): + DocsCliConfig.from_environment() + + +def test_bazel_build_allows_declared_output_to_be_created_by_sphinx( + workspace: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """A sandboxed action validates inputs but leaves its output creation to Sphinx.""" + # Arrange + monkeypatch.setenv("ACTION", "build_needs_json") + monkeypatch.setenv("OUTPUT_DIRECTORY", "outputs/not-created-yet") + monkeypatch.chdir(workspace) + + # Act + config = DocsCliConfig.from_environment() + + # Assert + assert config.output_directory == workspace / "outputs/not-created-yet" + assert not config.output_directory.exists() diff --git a/src/extensions/score_cross_module_compatibility/__init__.py b/src/extensions/score_cross_module_compatibility/__init__.py index 3c408ee51..b60a2d295 100644 --- a/src/extensions/score_cross_module_compatibility/__init__.py +++ b/src/extensions/score_cross_module_compatibility/__init__.py @@ -13,7 +13,6 @@ import html import json -import os import re from dataclasses import asdict, dataclass, replace from pathlib import Path @@ -220,21 +219,21 @@ def write(self, outdir: str | Path) -> None: def _manifest_path(app: Sphinx) -> Path | None: - raw = getattr(app.config, "mounts_manifest", "") or os.environ.get( - "MOUNTS_MANIFEST", "" - ) + raw = getattr(app.config, "mounts_manifest", "") if not isinstance(raw, str) or not raw.strip(): return None direct = Path(raw) - runfiles = get_runfiles_dir() / raw - # ``mounts_manifest`` may be an execroot path, while the environment value - # passed to ``bazel run`` is runfiles-relative. Prefer an existing path so - # the policy does not depend on the current working directory. - if direct.is_file(): + if direct.is_absolute(): + return direct + elif direct.is_file(): + # A direct path is already usable from the current Sphinx process. + return direct + elif find_ws_root(): + # ``bazel run`` may provide a runfiles-relative path when this + # extension is used without the documentation launcher. + return get_runfiles_dir() / direct + else: return direct - if runfiles.is_file(): - return runfiles - return runfiles if find_ws_root() else direct def get_reporter(app: Sphinx) -> CompatibilityReporter: diff --git a/src/extensions/score_layout/__init__.py b/src/extensions/score_layout/__init__.py index b51f0c02a..9ea8d7dee 100644 --- a/src/extensions/score_layout/__init__.py +++ b/src/extensions/score_layout/__init__.py @@ -20,7 +20,7 @@ import sphinx_options from sphinx.application import Sphinx -from src.helper_lib import config_setdefault +from src.helper_lib import config_setdefault, find_ws_root logger = logging.getLogger(__name__) @@ -118,9 +118,9 @@ def configure_mounted_source_controls( return source_path = source_path.resolve() - workspace_directory = os.environ.get("BUILD_WORKSPACE_DIRECTORY") - if workspace_directory: - workspace_root = Path(workspace_directory).resolve() + workspace_root = find_ws_root() + if workspace_root is not None: + workspace_root = workspace_root.resolve() if ( source_path.is_relative_to(workspace_root) and not {"bazel-bin", "bazel-out"}.intersection(source_path.parts) diff --git a/src/extensions/score_metamodel/__init__.py b/src/extensions/score_metamodel/__init__.py index 98c8d19fd..a14f300c9 100644 --- a/src/extensions/score_metamodel/__init__.py +++ b/src/extensions/score_metamodel/__init__.py @@ -11,7 +11,6 @@ # SPDX-License-Identifier: Apache-2.0 # ******************************************************************************* import importlib -import os import pkgutil from collections.abc import Callable from pathlib import Path @@ -35,7 +34,7 @@ load_metamodel_data as load_metamodel_data, validate_mandatory_regexes as validate_mandatory_regexes, ) -from src.helper_lib import config_setdefault +from src.helper_lib import config_setdefault, find_ws_root logger = logging.get_logger(__name__) @@ -110,8 +109,8 @@ def _run_checks(app: Sphinx) -> None: logger.debug(f"Running checks for {len(needs_all_needs)} needs") - ws_root = os.environ.get("BUILD_WORKSPACE_DIRECTORY", None) - cwd_or_ws_root = Path(ws_root) if ws_root else Path.cwd() + ws_root = find_ws_root() + cwd_or_ws_root = ws_root or Path.cwd() prefix = str(Path(app.srcdir).relative_to(cwd_or_ws_root)) log = CheckLogger(logger, prefix, get_reporter(app)) diff --git a/src/extensions/score_metamodel/log.py b/src/extensions/score_metamodel/log.py index ae0088ac2..8db688f34 100644 --- a/src/extensions/score_metamodel/log.py +++ b/src/extensions/score_metamodel/log.py @@ -10,7 +10,6 @@ # # SPDX-License-Identifier: Apache-2.0 # ******************************************************************************* -import os from typing import Any from docutils.nodes import Node @@ -19,6 +18,8 @@ from sphinx_needs.logging import SphinxLoggerAdapter from sphinx_needs.need_item import NeedItem +from src.helper_lib import ExecutionEnvironment, identify_environment + Location = str | tuple[str | None, int | None] | Node | None NewCheck = tuple[str, Location] logger = logging.get_logger(__name__) @@ -47,7 +48,10 @@ def get(key: str) -> Any: # Note: passing the location as a string allows us to use # readable relative paths, passing as a tuple results # in absolute paths to ~/.cache/.../bazel-out/.. - if "RUNFILES_DIR" in os.environ or "RUNFILES_MANIFEST_FILE" in os.environ: + if identify_environment() in ( + ExecutionEnvironment.BAZEL_RUN, + ExecutionEnvironment.BAZEL_BUILD, + ): matching_file = f"{need['docname']}{need['doctype']}" else: matching_file = f"{prefix}/{need['docname']}{need['doctype']}" diff --git a/src/extensions/score_mounts/__init__.py b/src/extensions/score_mounts/__init__.py index f00ec2f04..8bfbd7892 100644 --- a/src/extensions/score_mounts/__init__.py +++ b/src/extensions/score_mounts/__init__.py @@ -33,7 +33,6 @@ from __future__ import annotations -import os from pathlib import Path from sphinx.application import Sphinx @@ -55,24 +54,29 @@ def _read_manifest(config: Config): """Locate and load the mounts manifest, or return ``None`` when unset. - The manifest path is passed by Bazel either via the ``mounts_manifest`` config - value or the ``MOUNTS`` env var. Its interpretation depends on the build - context: under ``bazel run`` it is a runfiles-relative path + The manifest path is passed through the ``mounts_manifest`` Sphinx config + value. Its interpretation depends on the build context: under ``bazel run`` + it is a runfiles-relative path (``$(rlocationpath)``) resolved against the runfiles dir; in a sandbox build it is relative to the exec root (``$(location)``). Resolving the path here keeps that context branch out of the pure ``_resolver`` module. """ - raw = getattr(config, "mounts_manifest", None) or os.environ.get( - "MOUNTS_MANIFEST", None - ) - if not raw or not raw.strip() or not isinstance(raw, str): + raw = getattr(config, "mounts_manifest", None) + if not isinstance(raw, str) or not raw.strip(): return None - # ``bazel run`` passes an rlocation-relative path; ``sphinx_docs`` in a - # sandbox passes its execroot-relative ``$(location)`` path directly. - manifest_path = get_runfiles_dir() / raw if find_ws_root() else Path(raw) - - return load_mounts_manifest(manifest_path) + manifest_path = Path(raw) + if manifest_path.is_absolute(): + # The documentation launcher resolves configured paths before passing + # them to Sphinx. Absolute paths are already in the consumer's view. + return load_mounts_manifest(manifest_path) + elif find_ws_root(): + # Preserve support for direct extension users that still provide the + # runfiles-relative value from ``$(rlocationpath)``. + manifest_path = get_runfiles_dir() / manifest_path + return load_mounts_manifest(manifest_path) + else: + return load_mounts_manifest(manifest_path) def _resolve_data_mounts( diff --git a/src/extensions/score_source_code_linker/__init__.py b/src/extensions/score_source_code_linker/__init__.py index 2de69f1f4..8d97bade3 100644 --- a/src/extensions/score_source_code_linker/__init__.py +++ b/src/extensions/score_source_code_linker/__init__.py @@ -20,7 +20,6 @@ # req-Id: tool_req__docs_dd_link_source_code_link # This whole directory implements the above mentioned tool requirements -import os from copy import deepcopy from pathlib import Path from typing import Any, cast @@ -86,8 +85,8 @@ def build_and_save_combined_file(outdir: Path, app: Sphinx | None = None): Reads the saved partial caches of codelink & testlink Builds the combined JSON cache & saves it """ - source_code_links_path = os.environ.get("SCORE_SOURCELINKS") - if not source_code_links_path and app is not None: + source_code_links_path = "" + if app is not None: source_code_links_path = str( getattr(app.config, "score_sourcelinks_json", "") or "" ).strip() @@ -98,8 +97,7 @@ def build_and_save_combined_file(outdir: Path, app: Sphinx | None = None): except FileNotFoundError as exc: raise FileNotFoundError( "Pre-generated source-code links file does not exist: " - f"{source_code_links_json}. Check SCORE_SOURCELINKS or " - "score_sourcelinks_json." + f"{source_code_links_json}. Check score_sourcelinks_json." ) from exc except AssertionError: source_code_links = load_source_code_links_with_metadata_json( @@ -308,7 +306,7 @@ def setup(app: Sphinx) -> dict[str, str | bool]: default="", rebuild="env", types=str, - description="Path to pre-generated source code links JSON from Bazel via SCORE_SOURCELINKS env var", + description="Path to pre-generated source code links JSON provided by the docs CLI", ) app.add_config_value( "score_source_code_linker_plain_links", diff --git a/src/extensions/score_source_code_linker/needlinks.py b/src/extensions/score_source_code_linker/needlinks.py index 2998240fc..ead0a41d5 100644 --- a/src/extensions/score_source_code_linker/needlinks.py +++ b/src/extensions/score_source_code_linker/needlinks.py @@ -13,7 +13,6 @@ # req-Id: tool_req__docs_dd_link_source_code_link import json -import os from dataclasses import asdict, dataclass from pathlib import Path from typing import Any, TypedDict, TypeGuard @@ -179,11 +178,6 @@ def load_source_code_links_with_metadata_json(file: Path) -> list[NeedLink]: This normally should be the one called 'locally' => :docs target """ - if not file.is_absolute(): - ws_root = os.environ.get("BUILD_WORKSPACE_DIRECTORY") - if ws_root: - file = Path(ws_root) / file - data: list[object] = json.loads( file.read_text(encoding="utf-8"), object_hook=needlink_decoder, @@ -219,12 +213,6 @@ def load_source_code_links_json(file: Path) -> list[NeedLink]: This is used when mounted external documentation contributes source links. """ - if not file.is_absolute(): - # use env variable set by Bazel - ws_root = os.environ.get("BUILD_WORKSPACE_DIRECTORY") - if ws_root: - file = Path(ws_root) / file - links: list[NeedLink] = json.loads( file.read_text(encoding="utf-8"), object_hook=needlink_decoder, diff --git a/src/extensions/score_source_code_linker/tests/test_codelink.py b/src/extensions/score_source_code_linker/tests/test_codelink.py index 8b3d9bf61..4c76f9381 100644 --- a/src/extensions/score_source_code_linker/tests/test_codelink.py +++ b/src/extensions/score_source_code_linker/tests/test_codelink.py @@ -21,9 +21,11 @@ from collections.abc import Generator from dataclasses import asdict from pathlib import Path -from typing import Any +from types import SimpleNamespace +from typing import Any, cast import pytest +from sphinx.application import Sphinx # S-CORE plugin to allow for properties/attributes in xml # Enables Testlinking @@ -362,27 +364,40 @@ def test_combining_without_source_links_continues_with_empty_code_links( temp_dir: Path, monkeypatch: pytest.MonkeyPatch ) -> None: """A build without a pre-generated source-link input must not scan or fail.""" - monkeypatch.delenv("SCORE_SOURCELINKS", raising=False) - - build_and_save_combined_file(temp_dir) + # The extension consumes Sphinx config, not the legacy process variable. + monkeypatch.setenv("SCORE_SOURCELINKS", str(temp_dir / "ignored.json")) + build_and_save_combined_file( + temp_dir, + cast( + Sphinx, + SimpleNamespace(config=SimpleNamespace(score_sourcelinks_json="")), + ), + ) grouped_cache = temp_dir / "score_scl_grouped_cache.json" assert json.loads(grouped_cache.read_text(encoding="utf-8")) == [] def test_combining_with_missing_source_links_reports_configured_path( - temp_dir: Path, monkeypatch: pytest.MonkeyPatch + temp_dir: Path, ) -> None: """Report the configured source-link file when it cannot be found.""" missing_file = temp_dir / "missing_source_links.json" - monkeypatch.setenv("SCORE_SOURCELINKS", str(missing_file)) with pytest.raises(FileNotFoundError) as exc_info: - build_and_save_combined_file(temp_dir) + build_and_save_combined_file( + temp_dir, + cast( + Sphinx, + SimpleNamespace( + config=SimpleNamespace(score_sourcelinks_json=str(missing_file)) + ), + ), + ) assert str(exc_info.value) == ( "Pre-generated source-code links file does not exist: " - f"{missing_file}. Check SCORE_SOURCELINKS or score_sourcelinks_json." + f"{missing_file}. Check score_sourcelinks_json." ) @@ -788,10 +803,8 @@ def test_load_with_metadata_invalid_items_after_metadata(tmp_path: Path): # ────────────────[ File Path Resolution Tests ]──────────────── -def test_load_resolves_relative_path_with_env_var( - tmp_path: Path, monkeypatch: pytest.MonkeyPatch -): - """Test if relative path is resolved using BUILD_WORKSPACE_DIRECTORY""" +def test_load_accepts_absolute_source_links_path(tmp_path: Path): + """Source-link consumers receive paths already resolved by the CLI.""" workspace = tmp_path / "workspace" workspace.mkdir() @@ -809,18 +822,14 @@ def test_load_resolves_relative_path_with_env_var( cache_file = workspace / "cache.json" store_source_code_links_json(cache_file, needlinks) - # Set env var and load with relative path - monkeypatch.setenv("BUILD_WORKSPACE_DIRECTORY", str(workspace)) - loaded = load_source_code_links_json(Path("cache.json")) + loaded = load_source_code_links_json(cache_file) assert len(loaded) == 1 assert loaded[0].need == "REQ_1" -def test_load_with_metadata_resolves_relative_path( - tmp_path: Path, monkeypatch: pytest.MonkeyPatch -): - """Edge case: load_with_metadata resolves relative paths using env var""" +def test_load_with_metadata_accepts_absolute_source_links_path(tmp_path: Path): + """Metadata source-link consumers receive paths already resolved by the CLI.""" workspace = tmp_path / "workspace" workspace.mkdir() @@ -842,8 +851,7 @@ def test_load_with_metadata_resolves_relative_path( cache_file = workspace / "metadata_cache.json" store_source_code_links_with_metadata_json(cache_file, metadata, needlinks) - monkeypatch.setenv("BUILD_WORKSPACE_DIRECTORY", str(workspace)) - loaded = load_source_code_links_with_metadata_json(Path("metadata_cache.json")) + loaded = load_source_code_links_with_metadata_json(cache_file) assert len(loaded) == 1 assert loaded[0].repo_name == "mod" diff --git a/src/extensions/score_source_code_linker/tests/test_repo_source_link_integration.py b/src/extensions/score_source_code_linker/tests/test_repo_source_link_integration.py index 80f2e9818..81181495e 100644 --- a/src/extensions/score_source_code_linker/tests/test_repo_source_link_integration.py +++ b/src/extensions/score_source_code_linker/tests/test_repo_source_link_integration.py @@ -265,10 +265,6 @@ def sphinx_app_setup( git_repo_setup: Path, monkeypatch: pytest.MonkeyPatch, ) -> Callable[[], SphinxTestApp]: - # Source links are generated before Sphinx starts, matching the Bazel build - # contract used by the extension in production. - monkeypatch.setenv("SCORE_SOURCELINKS", str(sphinx_base_dir / "source_links.json")) - def _create_app(): base_dir = sphinx_base_dir docs_dir = base_dir / "docs" @@ -283,6 +279,9 @@ def _create_app(): outdir=sphinx_base_dir / "out", buildername="html", warningiserror=True, + confoverrides={ + "score_sourcelinks_json": str(sphinx_base_dir / "source_links.json") + }, ) return _create_app diff --git a/src/extensions/score_source_code_linker/tests/test_source_code_link_integration.py b/src/extensions/score_source_code_linker/tests/test_source_code_link_integration.py index d2f000fc3..c13774a31 100644 --- a/src/extensions/score_source_code_linker/tests/test_source_code_link_integration.py +++ b/src/extensions/score_source_code_linker/tests/test_source_code_link_integration.py @@ -213,12 +213,6 @@ def sphinx_app_setup( git_repo_setup: Path, monkeypatch: pytest.MonkeyPatch, ) -> Callable[[], SphinxTestApp]: - # Source links are generated before Sphinx starts, matching the Bazel build - # contract used by the extension in production. - monkeypatch.setenv( - "SCORE_SOURCELINKS", str(sphinx_base_dir / ".expected_codelink.json") - ) - def _create_app(): base_dir = sphinx_base_dir docs_dir = base_dir / "docs" @@ -233,6 +227,11 @@ def _create_app(): outdir=sphinx_base_dir / "out", buildername="html", warningiserror=True, + confoverrides={ + "score_sourcelinks_json": str( + sphinx_base_dir / ".expected_codelink.json" + ) + }, ) return _create_app diff --git a/src/extensions/score_source_code_linker/tests/test_xml_parser.py b/src/extensions/score_source_code_linker/tests/test_xml_parser.py index bd1d1e5e5..edd83f784 100644 --- a/src/extensions/score_source_code_linker/tests/test_xml_parser.py +++ b/src/extensions/score_source_code_linker/tests/test_xml_parser.py @@ -571,27 +571,35 @@ def test_get_metadata_from_test_path_local(): assert md["url"] == "" -def test_get_metadata_from_test_path_combo_with_hash( - tmp_path: Path, monkeypatch: pytest.MonkeyPatch -): +def test_get_metadata_from_test_path_combo_with_hash(tmp_path: Path): """Combo builds with 'hash' in known_good.json populate metadata correctly.""" json_file = tmp_path / "known_good.json" json_file.write_text(json.dumps(_KNOWN_GOOD_WITH_HASH)) - monkeypatch.setenv("KNOWN_GOOD_JSON", str(json_file)) - md = xml_parser.get_metadata_from_test_path(_COMBO_TEST_PATH) + md = xml_parser.get_metadata_from_test_path(_COMBO_TEST_PATH, json_file) assert md["repo_name"] == "score_docs_as_code" assert md["hash"] == "abc123hashvalue" assert md["url"] == "https://github.com/eclipse-score/docs-as-code" -def test_get_metadata_from_test_path_combo_with_version( - tmp_path: Path, monkeypatch: pytest.MonkeyPatch -): +def test_get_metadata_from_test_path_combo_with_version(tmp_path: Path): """Combo builds with 'version' in known_good.json populate metadata correctly.""" json_file = tmp_path / "known_good.json" json_file.write_text(json.dumps(_KNOWN_GOOD_WITH_VERSION)) - monkeypatch.setenv("KNOWN_GOOD_JSON", str(json_file)) - md = xml_parser.get_metadata_from_test_path(_COMBO_TEST_PATH) + md = xml_parser.get_metadata_from_test_path(_COMBO_TEST_PATH, json_file) assert md["repo_name"] == "score_docs_as_code" assert md["hash"] == "v2.1.0" assert md["url"] == "https://github.com/eclipse-score/docs-as-code" + + +def test_get_metadata_from_test_path_uses_explicit_known_good_path(tmp_path: Path): + """Test metadata uses the path supplied by the Sphinx configuration.""" + json_file = tmp_path / "known_good.json" + json_file.write_text(json.dumps(_KNOWN_GOOD_WITH_HASH)) + + md = xml_parser.get_metadata_from_test_path( + _COMBO_TEST_PATH, + known_good_json=json_file, + ) + + assert md["hash"] == "abc123hashvalue" + assert md["url"] == "https://github.com/eclipse-score/docs-as-code" diff --git a/src/extensions/score_source_code_linker/xml_parser.py b/src/extensions/score_source_code_linker/xml_parser.py index 97a5bc044..bdeace01a 100644 --- a/src/extensions/score_source_code_linker/xml_parser.py +++ b/src/extensions/score_source_code_linker/xml_parser.py @@ -114,7 +114,9 @@ def clean_test_file_name(raw_filepath: Path) -> Path: ) -def get_metadata_from_test_path(raw_filepath: Path) -> MetaData: +def get_metadata_from_test_path( + raw_filepath: Path, known_good_json: Path | None = None +) -> MetaData: """ Will parse out the metadata from the testpath. If test is local then the metadata will be: @@ -144,16 +146,13 @@ def get_metadata_from_test_path(raw_filepath: Path) -> MetaData: Removing everything up to and including 'bazel-testlogs' or 'tests-report' """ # print("THIs IS FILEPATH IN GET MD FROm TestPATH: ", raw_filepath) - known_good_json = os.environ.get("KNOWN_GOOD_JSON") clean_filepath = clean_test_file_name(raw_filepath) # print(f"This is the cleaned filepath: {clean_filepath}") repo_name = parse_repo_name_from_path(clean_filepath) md = DefaultMetaData() md["repo_name"] = repo_name - if repo_name != "local_repo" and known_good_json: - md["hash"], md["url"] = parse_info_from_known_good( - Path(known_good_json), repo_name - ) + if repo_name != "local_repo" and known_good_json is not None: + md["hash"], md["url"] = parse_info_from_known_good(known_good_json, repo_name) return md @@ -206,7 +205,9 @@ def parse_properties(case_properties: dict[str, Any], properties: Element): def read_test_xml_file( - file: Path, allowed_dirs: list[str] | None = None + file: Path, + allowed_dirs: list[str] | None = None, + known_good_json: Path | None = None, ) -> tuple[list[DataOfTestCase], list[str], list[str]]: """ Reading & parsing the test.xml files into TestCaseNeeds @@ -222,7 +223,7 @@ def read_test_xml_file( missing_prop_tests: list[str] = [] tree = ET.parse(file) root = tree.getroot() - md = get_metadata_from_test_path(file) + md = get_metadata_from_test_path(file, known_good_json) for testsuite in root.findall("testsuite"): for testcase in testsuite.findall("testcase"): test_file = testcase.get("file") @@ -402,11 +403,17 @@ def build_test_needs_from_files( Returns: - list[TestCaseNeed] """ + raw_known_good_json = getattr(app.config, "KNOWN_GOOD_JSON", "") + known_good_json = ( + Path(raw_known_good_json) + if isinstance(raw_known_good_json, str) and raw_known_good_json + else None + ) tcns: list[DataOfTestCase] = [] for file in xml_paths: # Last value can be ignored. The 'is_valid' function already prints infos test_cases, tests_missing_all_props, tests_missing_some_props = ( - read_test_xml_file(file, allowed_dirs) + read_test_xml_file(file, allowed_dirs, known_good_json) ) non_prop_tests = ", ".join(n for n in tests_missing_all_props) if non_prop_tests: