diff --git a/bzl/needs_rules.bzl b/bzl/needs_rules.bzl index 758bb1549..3495d4a6e 100644 --- a/bzl/needs_rules.bzl +++ b/bzl/needs_rules.bzl @@ -36,21 +36,21 @@ def _sphinx_docs_impl(ctx): if source_dir.startswith("../"): source_dir = "external/" + source_dir[3:] - # Expand file labels at analysis time, then encode the argument list as - # JSON so spaces, quotes and '=' in Sphinx options survive the environment - # transport unchanged. The launcher adds these after its default options. - # ``config`` is transported separately because the launcher derives - # Sphinx's ``-c`` directory from its path; it is not just another data file. + # Expand file labels at analysis time, then add the paths that only exist + # once this action's output and execution inputs have been declared. The + # resulting object is the complete versioned contract consumed by cli.py. + # Keeping this as one JSON environment value avoids shell quoting problems + # for paths and labels containing spaces, quotes or equals signs. + config_payload = json.decode( + ctx.expand_location(ctx.attr.config_payload, targets = ctx.attr.tools), + ) + config_payload["action"] = "build_needs_json" + config_payload["source_directory"] = source_dir or "." + config_payload["output_directory"] = output.path + config_payload["config_file"] = ctx.file.config.path + env = { - "ACTION": "build_needs_json", - "SOURCE_DIRECTORY": source_dir or ".", - "OUTPUT_DIRECTORY": output.path, - "SPHINX_CONFIG_FILE": ctx.file.config.path, - "DATA": "[]", - "SPHINX_EXTRA_OPTS": json.encode([ - ctx.expand_location(option, targets = ctx.attr.tools) - for option in ctx.attr.extra_opts - ]), + "SCORE_DOCS_CONFIG": json.encode(config_payload), } # Data and mounted sources must be present at their execution-root paths. @@ -77,7 +77,7 @@ sphinx_docs = rule( "bundle": attr.label(providers = [DocsBundleInfo], mandatory = True), "data": attr.label_list(allow_files = True), "tools": attr.label_list(allow_files = True), - "extra_opts": attr.string_list(), + "config_payload": attr.string(mandatory = True), # The launcher runs on the build host and carries extension runfiles. "sphinx": attr.label(cfg = "exec", executable = True, mandatory = True), }, diff --git a/docs.bzl b/docs.bzl index c4e16e459..a556d7305 100644 --- a/docs.bzl +++ b/docs.bzl @@ -71,37 +71,73 @@ load( # Sphinx policy while ``needs_rules.bzl`` owns Bazel's input/output plumbing. load("@score_docs_as_code//:bzl/needs_rules.bzl", "sphinx_docs") -def _sphinx_define(name, value): - """Return a Sphinx ``--define`` option when ``value`` is configured.""" - if value == None: - return [] - return ["--define=" + name + "=" + value] - -def _needs_sphinx_extra_opts( +def _docs_config_payload( + action, + package_directory, + source_directory, + output_directory, + config_file, + external_needs_sources, + testcase_source_dirs, + mounts_manifest, + source_links, + metamodel, + known_good, master_doc, - external_needs_source, - score_bundle_needs_export, - score_sourcelinks_json, - score_source_code_linker_plain_links, + bundle_needs_export, + plain_links): + """Return the versioned configuration shared by all documentation launchers. + + Paths containing Bazel ``$(location ...)`` or ``$(rlocationpath ...)`` + expressions are intentionally left for the owning rule to expand. This + keeps label expansion in Bazel and path interpretation in the Python CLI. + """ + return json.encode({ + "version": 1, + "action": action, + "package_directory": package_directory, + "source_directory": source_directory, + "output_directory": output_directory, + "config_file": config_file, + "external_needs_sources": [str(source) for source in external_needs_sources], + "testcase_source_dirs": testcase_source_dirs, + "mounts_manifest": mounts_manifest, + "source_links": source_links, + "metamodel": metamodel, + "known_good": known_good, + "master_doc": master_doc, + "bundle_needs_export": bundle_needs_export, + "plain_links": plain_links, + }) + +def _interactive_docs_config( + action, + package_directory, + source_directory, + config_file, + external_needs_sources, + testcase_source_dirs, mounts_manifest, - score_metamodel_yaml): - """Return per-target Sphinx configuration defines for a Needs build.""" - # The launcher supplies diagnostics shared by every builder. Keep only - # target-specific defines here so the action does not receive duplicate - # ``-W``, ``--keep-going``, and ``-T`` options after JSON transport. - return [ - option - for name, value in [ - ("master_doc", master_doc), - ("external_needs_source", external_needs_source), - ("score_bundle_needs_export", score_bundle_needs_export), - ("score_sourcelinks_json", score_sourcelinks_json), - ("score_source_code_linker_plain_links", score_source_code_linker_plain_links), - ("mounts_manifest", mounts_manifest), - ("score_metamodel_yaml", score_metamodel_yaml), - ] - for option in _sphinx_define(name, value) - ] + source_links, + metamodel, + known_good): + """Build the payload for a ``bazel run`` documentation binary.""" + return _docs_config_payload( + action = action, + package_directory = package_directory, + source_directory = source_directory, + output_directory = "_build", + config_file = config_file, + external_needs_sources = external_needs_sources, + testcase_source_dirs = testcase_source_dirs, + mounts_manifest = mounts_manifest, + source_links = source_links, + metamodel = metamodel, + known_good = known_good, + master_doc = None, + bundle_needs_export = None, + plain_links = None, + ) def _declare_sphinx_build_binary(name, data, deps): """Declare the private Sphinx executable used by one Needs target.""" @@ -125,12 +161,13 @@ def _needs_sphinx_docs( sphinx_build_deps, bundle, master_doc = None, - external_needs_source = None, - score_bundle_needs_export = None, - score_sourcelinks_json = None, - score_source_code_linker_plain_links = None, + external_needs_sources = [], + bundle_needs_export = None, + source_links = None, + plain_links = None, mounts_manifest = None, - score_metamodel_yaml = None, + metamodel = None, + testcase_source_dirs = [], tools = [], sphinx_build_data = [], visibility = None): @@ -148,14 +185,21 @@ def _needs_sphinx_docs( # made available as ordinary runtime input. config = config, data = sphinx_build_data, - extra_opts = _needs_sphinx_extra_opts( - master_doc, - external_needs_source, - score_bundle_needs_export, - score_sourcelinks_json, - score_source_code_linker_plain_links, - mounts_manifest, - score_metamodel_yaml, + config_payload = _docs_config_payload( + action = "build_needs_json", + package_directory = "", + source_directory = None, + output_directory = None, + config_file = None, + external_needs_sources = external_needs_sources, + testcase_source_dirs = testcase_source_dirs, + mounts_manifest = mounts_manifest, + source_links = source_links, + metamodel = metamodel, + known_good = None, + master_doc = master_doc, + bundle_needs_export = bundle_needs_export, + plain_links = plain_links, ), sphinx = sphinx_build, tools = tools, @@ -369,10 +413,10 @@ def _declare_bundle_local_needs( sphinx_build_deps = sphinx_build_deps, sphinx_build_data = data, master_doc = entry_doc, - external_needs_source = "[]", - score_bundle_needs_export = "1", - score_sourcelinks_json = "$(location " + str(sourcelinks_json) + ")" if sourcelinks_json else None, - score_source_code_linker_plain_links = "1", + external_needs_sources = [], + bundle_needs_export = True, + source_links = "$(location " + str(sourcelinks_json) + ")" if sourcelinks_json else None, + plain_links = True, tools = [sourcelinks_json] if sourcelinks_json else [], visibility = visibility, ) @@ -465,16 +509,15 @@ def _sphinx_runtime_deps(deps): result.append(fixed_dep) return result -def _declare_docs_binary(name, data, deps, env, action): +def _declare_docs_binary(name, data, deps, config_payload): """Declare one of the interactive documentation command targets.""" docs_cli_src = Label("//src/docs_cli:cli.py") - command_env = env | {"ACTION": action} py_binary( name = name, srcs = [docs_cli_src], data = data, deps = deps, - env = command_env, + env = {"SCORE_DOCS_CONFIG": config_payload}, tags = ["manual"], ) @@ -626,28 +669,15 @@ def docs( # generated configuration must be present in the runfiles tree. docs_data += [sphinx_config] - docs_env = { - "SOURCE_DIRECTORY": source_dir, - "PACKAGE_DIR": native.package_name(), - "TEST_SOURCES": str(test_sources), - "DATA": str(data), - "EXTERNAL_NEEDS_FILES": str(external_needs), - # `bazel run` starts from a runfiles tree, so this logical path is - # resolved by score_mounts through ``RUNFILES_DIR``. - "MOUNTS_MANIFEST": "$(rlocationpath :_mounts_manifest)" if bundles else "", - "SCORE_SOURCELINKS": "$(location :sourcelinks_json)", - } - if config_is_generated: - # The generated file is named conf.py. Run targets pass its containing - # directory to Sphinx via -c. - docs_env["SPHINX_CONFIG_FILE"] = "$(rlocationpath " + sphinx_config + ")" - if metamodel: - # The interactive ``py_binary`` targets run from a runfiles tree. - # docs_cli resolves this logical path through ``RUNFILES_DIR``. - docs_env["SCORE_METAMODEL_YAML"] = "$(rlocationpath " + str(metamodel) + ")" + # The same target settings are copied into each command's payload; only + # the action changes between incremental, linkcheck, check and preview. + config_file = "$(rlocationpath " + sphinx_config + ")" if config_is_generated else None + mounts_manifest = "$(rlocationpath :_mounts_manifest)" if bundles else None + source_links = "$(rlocationpath :sourcelinks_json)" + metamodel_path = "$(rlocationpath " + str(metamodel) + ")" if metamodel else None + known_good_path = None if known_good_label: - known_good_str = str(known_good_label[0]) - docs_env["KNOWN_GOOD_JSON"] = "$(location " + known_good_str + ")" + known_good_path = "$(rlocationpath " + str(known_good_label[0]) + ")" docs_data += known_good_label # Generated documentation artifacts may live below ``docs/``. A @@ -657,8 +687,18 @@ def docs( name = "_score_docs_cli", data = docs_data, deps = deps, - env = docs_env, - action = "incremental", + config_payload = _interactive_docs_config( + "incremental", + native.package_name(), + source_dir, + config_file, + data + external_needs, + test_sources, + mounts_manifest, + source_links, + metamodel_path, + known_good_path, + ), ) native.alias( @@ -671,22 +711,52 @@ def docs( name = "docs_link_check", data = docs_data, deps = deps, - env = docs_env, - action = "linkcheck", + config_payload = _interactive_docs_config( + "linkcheck", + native.package_name(), + source_dir, + config_file, + data + external_needs, + test_sources, + mounts_manifest, + source_links, + metamodel_path, + known_good_path, + ), ) _declare_docs_binary( name = "docs_check", data = docs_data, deps = deps, - env = docs_env, - action = "check", + config_payload = _interactive_docs_config( + "check", + native.package_name(), + source_dir, + config_file, + data + external_needs, + test_sources, + mounts_manifest, + source_links, + metamodel_path, + known_good_path, + ), ) _declare_docs_binary( name = "live_preview", data = docs_data, deps = deps, - env = docs_env, - action = "live_preview", + config_payload = _interactive_docs_config( + "live_preview", + native.package_name(), + source_dir, + config_file, + data + external_needs, + test_sources, + mounts_manifest, + source_links, + metamodel_path, + known_good_path, + ), ) py_venv( @@ -704,13 +774,14 @@ def docs( config = sphinx_config, sphinx_build_deps = deps, sphinx_build_data = data + external_needs + metamodel_label + [":docs_bundle"], - external_needs_source = str(data + external_needs), - score_sourcelinks_json = "$(location :sourcelinks_json)", - score_source_code_linker_plain_links = "1", + external_needs_sources = data + external_needs, + source_links = "$(location :sourcelinks_json)", + plain_links = True, # The build action runs in a sandbox, so it needs the action-input path # rather than the runfiles-relative spelling. mounts_manifest = "$(location :_mounts_manifest)" if bundles else None, - score_metamodel_yaml = "$(location " + str(metamodel) + ")" if metamodel else None, + metamodel = "$(location " + str(metamodel) + ")" if metamodel else None, + testcase_source_dirs = test_sources, tools = external_needs + metamodel_label + [":sourcelinks_json", ":docs_bundle"] + mounts_manifest_label, visibility = ["//visibility:public"], ) diff --git a/src/docs_cli/README.md b/src/docs_cli/README.md index 4d83b8ccd..f026a690a 100644 --- a/src/docs_cli/README.md +++ b/src/docs_cli/README.md @@ -39,19 +39,43 @@ workspace root; for a nested package, use a label such as `//component:docs`. `_declare_docs_binary()` in `docs.bzl` creates a separate `py_binary` for each command, using the exported `cli.py` directly as its source. Each binary receives -its own `ACTION`, documentation environment and dependencies from `docs()`. +one `SCORE_DOCS_CONFIG` payload and its dependencies from `docs()`. The `all_sources` filegroup is included by `//src:all_sources` so source-code linking can traverse this Bazel package boundary. ## Configuration and build state -`docs.bzl` provides `SOURCE_DIRECTORY`, `PACKAGE_DIR`, `DATA`, and optional -configuration such as `SPHINX_CONFIG_FILE`, `SCORE_METAMODEL_YAML`, -`MOUNTS_MANIFEST`, `EXTERNAL_NEEDS_FILES`, `TEST_SOURCES` and `KNOWN_GOOD_JSON`. -Bazel provides the workspace and runfiles locations. The CLI resolves source -and output paths relative to the package containing the `docs()` call; generated -configuration is resolved through runfiles. +`SCORE_DOCS_CONFIG` is a versioned JSON object. Version `1` contains the action, +package/source/output/config paths, merged external Needs labels, testcase source +directories, and optional mounts, source-links, metamodel, known-good, and Needs +settings. A typical interactive payload looks like this: + +```json +{ + "version": 1, + "action": "incremental", + "package_directory": "component", + "source_directory": "docs", + "output_directory": "_build", + "config_file": "_main/component/docs/conf.py", + "external_needs_sources": ["//other:needs_json"], + "testcase_source_dirs": ["src/tests"], + "mounts_manifest": "_main/component/_mounts_manifest.json", + "source_links": "_main/component/sourcelinks_json.json", + "metamodel": null, + "known_good": null, + "master_doc": null, + "bundle_needs_export": null, + "plain_links": null +} +``` + +Interactive paths are runfiles-relative and sandboxed Needs paths are +execution-root-relative; the CLI resolves both forms before invoking Sphinx. +`BUILD_WORKSPACE_DIRECTORY`, `RUNFILES_DIR`, `JAVA_RUNFILES`, and +`GITHUB_REPOSITORY` remain process environment because they describe runtime or +CI context rather than documentation-target configuration. All actions share the package's `_build` directory. Before starting, the CLI removes stale output if the previous build recorded warnings, the stored hash diff --git a/src/docs_cli/cli.py b/src/docs_cli/cli.py index 1e9fb18d1..531ac9d35 100644 --- a/src/docs_cli/cli.py +++ b/src/docs_cli/cli.py @@ -22,6 +22,7 @@ import sys import time from pathlib import Path +from typing import Any, cast import debugpy from sphinx.cmd.build import main as sphinx_main @@ -36,25 +37,115 @@ _MODULE_HASH_FILE = ".module_bazel_hash" +_CONFIG_ENVIRONMENT_VARIABLE = "SCORE_DOCS_CONFIG" +_CONFIG_VERSION = 1 -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 +def _config_string(config: dict[str, Any], name: str, default: str = "") -> str: + """Read a string field from the versioned configuration payload.""" + value = config.get(name, default) + if not isinstance(value, str): + raise ValueError( + f"{_CONFIG_ENVIRONMENT_VARIABLE} field '{name}' must be a string" + ) + return value + + +def _config_string_list( + config: dict[str, Any], name: str, default: list[str] | None = None +) -> list[str]: + """Read and validate a list-of-strings field from the payload.""" + value = config.get(name, [] if default is None else default) + if not isinstance(value, list): + raise ValueError( + f"{_CONFIG_ENVIRONMENT_VARIABLE} field '{name}' must be a list of strings" + ) + items = cast(list[object], value) + if not all(isinstance(item, str) for item in items): + raise ValueError( + f"{_CONFIG_ENVIRONMENT_VARIABLE} field '{name}' must be a list of strings" + ) + return cast(list[str], items) + +def _config_optional_string(config: dict[str, Any], name: str) -> str | None: + """Read an optional string field, preserving JSON ``null`` as ``None``.""" + value = config.get(name) + if value is not None and not isinstance(value, str): + raise ValueError( + f"{_CONFIG_ENVIRONMENT_VARIABLE} field '{name}' must be a string or null" + ) + return cast(str | None, value) + + +def _config_optional_bool(config: dict[str, Any], name: str) -> bool | None: + """Read an optional boolean field, preserving JSON ``null`` as ``None``.""" + value = config.get(name) + if value is not None and not isinstance(value, bool): + raise ValueError( + f"{_CONFIG_ENVIRONMENT_VARIABLE} field '{name}' must be a boolean or null" + ) + return cast(bool | None, value) -def _merged_external_needs() -> str: - """Combine DATA and EXTERNAL_NEEDS_FILES into one JSON label list. - Both env vars hold JSON lists of Bazel labels; the extension parses the - resulting `external_needs_source` define uniformly. +def parse_docs_config(raw: str | None = None) -> dict[str, Any]: + """Parse and validate the single Bazel-to-CLI configuration contract. + + The process environment deliberately remains responsible only for runtime + context supplied by Bazel or CI (workspace/runfiles locations and GitHub + metadata). Documentation-target settings all travel in this payload so a + command cannot accidentally combine values from different configuration + transports. """ - data = json.loads(get_env("DATA") or "[]") - external = json.loads(os.environ.get("EXTERNAL_NEEDS_FILES", "[]") or "[]") - return json.dumps(data + external) + if raw is None: + raw = os.environ.get(_CONFIG_ENVIRONMENT_VARIABLE) + if raw is None: + raise ValueError( + f"Environment variable {_CONFIG_ENVIRONMENT_VARIABLE} is not set" + ) + + try: + parsed = json.loads(raw) + except json.JSONDecodeError as error: + raise ValueError( + f"{_CONFIG_ENVIRONMENT_VARIABLE} must contain valid JSON: {error.msg}" + ) from error + if not isinstance(parsed, dict): + raise ValueError( + f"{_CONFIG_ENVIRONMENT_VARIABLE} must contain a JSON object, " + f"got {type(parsed).__name__}" + ) + + config = cast(dict[str, Any], parsed) + if "version" not in config: + raise ValueError( + f"{_CONFIG_ENVIRONMENT_VARIABLE} is missing required field 'version'" + ) + if config["version"] != _CONFIG_VERSION: + raise ValueError( + f"Unsupported {_CONFIG_ENVIRONMENT_VARIABLE} version " + f"{config['version']!r}; supported version is {_CONFIG_VERSION}" + ) + + action = config.get("action") + if not isinstance(action, str) or not action: + raise ValueError( + f"{_CONFIG_ENVIRONMENT_VARIABLE} field 'action' must be a non-empty string" + ) + _config_string(config, "source_directory") + _config_string(config, "package_directory", "") + _config_string(config, "output_directory", "_build") + _config_string(config, "config_file", "") + _config_string_list(config, "external_needs_sources") + _config_string_list(config, "testcase_source_dirs") + _config_optional_string(config, "mounts_manifest") + _config_optional_string(config, "source_links") + _config_optional_string(config, "metamodel") + _config_optional_string(config, "known_good") + _config_optional_string(config, "master_doc") + _config_optional_bool(config, "bundle_needs_export") + _config_optional_bool(config, "plain_links") + return config def _compute_hash(files: list[Path]) -> str: @@ -144,10 +235,49 @@ def add_watch_dir(path: Path) -> None: return watch_dirs -def sphinx_arguments(ws_root: Path, package_dir: Path, build_dir: Path) -> 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") +def _resolve_context_path( + raw_path: str, + *, + is_bazel_build: bool, + ws_root: Path | None, + runfiles_dir: Path | None, +) -> Path: + """Resolve a payload path in either a runfiles tree or an action sandbox.""" + path = Path(raw_path) + if path.is_absolute(): + return path + if is_bazel_build: + # Bazel action paths are relative to the execution root. The launcher + # is invoked with that directory as its working directory. + return Path.cwd() / path + if runfiles_dir is not None: + # Interactive binaries receive rlocation-relative paths in the payload. + return runfiles_dir / path + if ws_root is not None: + return ws_root / path + return Path.cwd() / path + + +def _sphinx_define(name: str, value: str | None) -> list[str]: + """Return one Sphinx configuration define when a field is configured.""" + if value is None: + return [] + return [f"--define={name}={value}"] + + +def sphinx_arguments( + config: dict[str, Any], + ws_root: Path | None, + package_dir: Path, + build_dir: Path, + runfiles_dir: Path | None = None, +) -> list[str]: + """Generate the complete Sphinx command from one validated payload.""" + action = _config_string(config, "action") + is_bazel_build = action == "build_needs_json" + source_directory = _config_string(config, "source_directory") + external_needs_sources = _config_string_list(config, "external_needs_sources") + testcase_source_dirs = _config_string_list(config, "testcase_source_dirs") base_arguments = [ str(package_dir / source_directory), str(build_dir), @@ -156,15 +286,65 @@ def sphinx_arguments(ws_root: Path, package_dir: Path, build_dir: Path) -> list[ "-T", # show details in case of errors in extensions "--jobs", "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', '[]')}", - # 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', '')}", + # These are the only diagnostics shared by all builders. Keeping them + # here ensures interactive and sandboxed invocations have one source + # for warning behavior and never receive duplicate options. ] + base_arguments.extend( + _sphinx_define("external_needs_source", json.dumps(external_needs_sources)) + ) + base_arguments.extend( + _sphinx_define("testcase_source_dirs", json.dumps(testcase_source_dirs)) + ) + + mounts_manifest = _config_optional_string(config, "mounts_manifest") + if mounts_manifest: + base_arguments.extend( + _sphinx_define( + "mounts_manifest", + str( + _resolve_context_path( + mounts_manifest, + is_bazel_build=is_bazel_build, + ws_root=ws_root, + runfiles_dir=runfiles_dir, + ) + ), + ) + ) + else: + base_arguments.extend(_sphinx_define("mounts_manifest", "")) + + for config_name, payload_name in ( + ("master_doc", "master_doc"), + ("score_bundle_needs_export", "bundle_needs_export"), + ("score_source_code_linker_plain_links", "plain_links"), + ): + value = config.get(payload_name) + if value is not None: + if isinstance(value, bool): + base_arguments.extend( + _sphinx_define(config_name, "1" if value else "0") + ) + else: + base_arguments.extend(_sphinx_define(config_name, str(value))) + + for config_name, payload_name in ( + ("score_sourcelinks_json", "source_links"), + ("score_metamodel_yaml", "metamodel"), + ("KNOWN_GOOD_JSON", "known_good"), + ): + raw_path = _config_optional_string(config, payload_name) + if raw_path: + resolved_path = _resolve_context_path( + raw_path, + is_bazel_build=is_bazel_build, + ws_root=ws_root, + runfiles_dir=runfiles_dir, + ) + base_arguments.extend(_sphinx_define(config_name, str(resolved_path))) + if 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 @@ -172,10 +352,6 @@ def sphinx_arguments(ws_root: Path, package_dir: Path, build_dir: Path) -> list[ # mixing action state into the declared output. base_arguments.extend(["-d", str(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", "[]"))) else: # Interactive builds keep warnings in the workspace so developers can # inspect them after a failed build. A Bazel action reports failure @@ -183,37 +359,16 @@ def sphinx_arguments(ws_root: Path, package_dir: Path, build_dir: Path) -> list[ # this diagnostic side file. base_arguments.extend(["--warning-file", str(build_dir / "warnings.txt")]) - generated_config = os.environ.get("SPHINX_CONFIG_FILE", "") + generated_config = _config_string(config, "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 + config_file = _resolve_context_path( + generated_config, + is_bazel_build=is_bazel_build, + ws_root=ws_root, + runfiles_dir=runfiles_dir, + ) base_arguments.extend(["-c", str(config_file.parent)]) - metamodel_yaml = os.environ.get("SCORE_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) - base_arguments.append(f"--define=score_metamodel_yaml={metamodel_yaml}") - if github_repository := os.getenv("GITHUB_REPOSITORY"): # GITHUB_REPOSITORY is expected as "owner/repo"; partition("/") splits # once into (owner, separator, repo), so we can ignore the separator. @@ -224,32 +379,31 @@ def sphinx_arguments(ws_root: Path, package_dir: Path, build_dir: Path) -> list[ 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 = ( + Path(_config_string(config, "package_directory", "")) / source_directory + ) 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')}") - return base_arguments -def watch_arguments() -> list[str]: - """Build autobuild options using the same runfiles resolution as Sphinx.""" - mounts_manifest = os.environ.get("MOUNTS_MANIFEST", "") +def watch_arguments( + config: dict[str, Any], ws_root: Path | None, runfiles_dir: Path | None +) -> list[str]: + """Build autobuild options using the payload's resolved mount manifest.""" + mounts_manifest = _config_optional_string(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) + manifest_path = _resolve_context_path( + mounts_manifest, + is_bazel_build=False, + ws_root=ws_root, + runfiles_dir=runfiles_dir, ) for watch_dir in mounted_watch_dirs( manifest_path, ws_root, - get_runfiles_dir() if ws_root is not None else None, + runfiles_dir, ): watch_arguments.extend(["--watch", watch_dir]) return watch_arguments @@ -278,34 +432,70 @@ def parse_args(argv: list[str] | None = None) -> argparse.Namespace: def main(argv: list[str] | None = None) -> int: """Run the requested builder and record whether its output can be reused.""" + config = parse_docs_config() args = parse_args(argv) if args.debug: debugpy.listen(("0.0.0.0", args.debug_port)) logger.info("Waiting for client to connect on port: " + str(args.debug_port)) debugpy.wait_for_client() - action = get_env("ACTION") + action = _config_string(config, "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" + ws_root = find_ws_root() + runtime_paths = [ + _config_string(config, "config_file", ""), + _config_optional_string(config, "mounts_manifest") or "", + _config_optional_string(config, "source_links") or "", + _config_optional_string(config, "metamodel") or "", + _config_optional_string(config, "known_good") or "", + ] + # Avoid requiring a complete runfiles tree for ordinary local builds that + # only use workspace paths. Bazel-run targets with generated inputs have at + # least one relative runtime path and obtain the real runfiles directory. + needs_runfiles = any( + path and not Path(path).is_absolute() for path in runtime_paths + ) + runfiles_dir = ( + get_runfiles_dir() if ws_root is not None and needs_runfiles else None + ) + 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() + build_dir = _resolve_context_path( + _config_string(config, "output_directory"), + is_bazel_build=True, + ws_root=None, + runfiles_dir=None, + ) + else: + # Interactive output is relative to the package containing docs(). An + # absent workspace marker is supported for direct CLI experimentation. + effective_ws_root = ws_root or Path.cwd() + package_dir = effective_ws_root / _config_string( + config, "package_directory", "" + ) + output_directory = _config_string(config, "output_directory", "_build") + build_dir = ( + Path(output_directory) + if Path(output_directory).is_absolute() + else package_dir / output_directory + ) + + effective_ws_root = ws_root or Path.cwd() sentinel_files = [ - ws_root / "MODULE.bazel", - ws_root / "MODULE.bazel.lock", + effective_ws_root / "MODULE.bazel", + effective_ws_root / "MODULE.bazel.lock", package_dir / "BUILD", ] if not is_bazel_build: clean_builddir_if_stale(build_dir, sentinel_files) warning_file = build_dir / "warnings.txt" - base_arguments = sphinx_arguments(ws_root, package_dir, build_dir) + base_arguments = sphinx_arguments( + config, ws_root, package_dir, build_dir, runfiles_dir + ) if action == "live_preview": sphinx_autobuild_main( @@ -315,7 +505,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, ws_root, runfiles_dir) ) return 0 diff --git a/src/docs_cli/dirty_build_test.py b/src/docs_cli/dirty_build_test.py index f8e82281c..4d2d2f2c8 100644 --- a/src/docs_cli/dirty_build_test.py +++ b/src/docs_cli/dirty_build_test.py @@ -12,6 +12,7 @@ # ******************************************************************************* import json +import os from pathlib import Path import pytest @@ -34,9 +35,28 @@ def docs_workspace(fs: FFS, monkeypatch: pytest.MonkeyPatch) -> Path: """Create the minimal workspace environment used by ``cli.main``.""" monkeypatch.setenv("BUILD_WORKSPACE_DIRECTORY", str(_WORKSPACE)) - monkeypatch.setenv("PACKAGE_DIR", "component") - monkeypatch.setenv("SOURCE_DIRECTORY", "docs") - monkeypatch.setenv("DATA", "[]") + monkeypatch.setenv( + "SCORE_DOCS_CONFIG", + json.dumps( + { + "version": 1, + "action": "incremental", + "package_directory": "component", + "source_directory": "docs", + "output_directory": "_build", + "config_file": "", + "external_needs_sources": [], + "testcase_source_dirs": [], + "mounts_manifest": None, + "source_links": None, + "metamodel": None, + "known_good": None, + "master_doc": None, + "bundle_needs_export": None, + "plain_links": None, + } + ), + ) fs.create_dir(_WORKSPACE / "component") for name in ("MODULE.bazel", "MODULE.bazel.lock", "component/BUILD"): fs.create_file(_WORKSPACE / name, contents="stable") @@ -148,7 +168,9 @@ def test_successful_build_reuses_output_until_module_changes( builder: str, ) -> None: """A successful CLI run records a reusable cache and invalidates it on changes.""" - monkeypatch.setenv("ACTION", action) + config = json.loads(os.environ["SCORE_DOCS_CONFIG"]) + config["action"] = action + monkeypatch.setenv("SCORE_DOCS_CONFIG", json.dumps(config)) build_dir = docs_workspace / "component/_build" reused: list[bool] = [] diff --git a/src/docs_cli/main_test.py b/src/docs_cli/main_test.py index 04ca0b6fd..fb1a95764 100644 --- a/src/docs_cli/main_test.py +++ b/src/docs_cli/main_test.py @@ -5,12 +5,13 @@ # information regarding copyright ownership. # # This program and the accompanying materials are made available under the -# terms of the Apache License Version 2.0 which is available at +# terms of the Apache License 2.0 which is available at # https://www.apache.org/licenses/LICENSE-2.0 # # SPDX-License-Identifier: Apache-2.0 # ******************************************************************************* +import json from pathlib import Path from unittest.mock import Mock @@ -21,32 +22,50 @@ from src.docs_cli.cli import sphinx_arguments +def _docs_config(**overrides: object) -> dict[str, object]: + """Return a complete version-1 payload suitable for CLI unit tests.""" + config: dict[str, object] = { + "version": 1, + "action": "incremental", + "package_directory": "component", + "source_directory": "docs", + "output_directory": "_build", + "config_file": "", + "external_needs_sources": [], + "testcase_source_dirs": [], + "mounts_manifest": None, + "source_links": None, + "metamodel": None, + "known_good": None, + "master_doc": None, + "bundle_needs_export": None, + "plain_links": None, + } + config.update(overrides) + return config + + +def _set_docs_config( + monkeypatch: pytest.MonkeyPatch, **overrides: object +) -> dict[str, object]: + """Install one structured payload and return the decoded fixture value.""" + config = _docs_config(**overrides) + monkeypatch.setenv("SCORE_DOCS_CONFIG", json.dumps(config)) + return config + + @pytest.fixture def workspace(fs: FFS, monkeypatch: pytest.MonkeyPatch) -> Path: """Create the minimum Bazel workspace needed by the CLI tests.""" - # The CLI reads its configuration from the process environment, so remove - # optional values left behind by the test runner before setting the basics. - ENVIRONMENT_OVERRIDES = ( - "EXTERNAL_NEEDS_FILES", - "TEST_SOURCES", - "MOUNTS_MANIFEST", - "SPHINX_CONFIG_FILE", - "SCORE_METAMODEL_YAML", - "GITHUB_REPOSITORY", - "KNOWN_GOOD_JSON", - "RUNFILES_DIR", - "RUNFILES_MANIFEST_FILE", - ) - for name in ENVIRONMENT_OVERRIDES: + # Runtime context is intentionally separate from the documentation payload. + for name in ("GITHUB_REPOSITORY", "RUNFILES_DIR", "RUNFILES_MANIFEST_FILE"): monkeypatch.delenv(name, raising=False) workspace = Path("/workspace") monkeypatch.setenv("BUILD_WORKSPACE_DIRECTORY", str(workspace)) - monkeypatch.setenv("PACKAGE_DIR", "component") - monkeypatch.setenv("SOURCE_DIRECTORY", "docs") - monkeypatch.setenv("DATA", "[]") monkeypatch.setenv("RUNFILES_DIR", str(workspace / "runfiles")) + _set_docs_config(monkeypatch) fs.create_dir(workspace / "component") fs.create_dir(workspace / "runfiles") @@ -72,34 +91,33 @@ def test_build_action_selects_sphinx_builder( ) -> None: """Each public action invokes Sphinx with its corresponding builder.""" - # Arrange - monkeypatch.setenv("ACTION", action) + _set_docs_config(monkeypatch, action=action) build_dir = workspace / "component/_build" if action == "build_needs_json": # The sandboxed action uses its declared output, not the package cache. monkeypatch.chdir(workspace) - monkeypatch.setenv("OUTPUT_DIRECTORY", "outputs/needs") + _set_docs_config( + monkeypatch, + action=action, + package_directory="", + output_directory="outputs/needs", + ) build_dir = workspace / "outputs/needs" noop_sphinx = Mock(return_value=0) update_hash = Mock() monkeypatch.setattr(docs_cli, "sphinx_main", noop_sphinx) monkeypatch.setattr(docs_cli, "update_module_hash", update_hash) - # Act exit_code = docs_cli.main([]) - # Assert - # The CLI propagates Sphinx's successful result. 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. if action == "build_needs_json": assert arguments[:2] == [str(workspace / "docs"), str(build_dir)] update_hash.assert_not_called() else: assert arguments[:2] == [str(workspace / "component/docs"), str(build_dir)] - # The action selects the builder exposed by its public Bazel target. assert arguments[-2:] == ["-b", builder] @@ -109,109 +127,90 @@ def test_failed_build_returns_exit_code_and_forces_next_build_clean( ) -> None: """A failure without Sphinx warnings must still invalidate partial output.""" - # Arrange - monkeypatch.setenv("ACTION", "incremental") + _set_docs_config(monkeypatch, action="incremental") build_dir = workspace / "component/_build" def failing_sphinx_mock(arguments: list[str]) -> int: build_dir.mkdir() - # Simulate a partial build by creating a dummy output file. - # That file must not survive a failed build. (build_dir / "partial-output").touch() return 2 monkeypatch.setattr(docs_cli, "sphinx_main", failing_sphinx_mock) - # Act exit_code = docs_cli.main([]) - # Assert - # The original Sphinx failure is returned to Bazel. assert exit_code == 2 - # The failure marker explains why the partial output must not be reused. assert "Build failed with exit code 2" in (build_dir / "warnings.txt").read_text() - # Failed builds must not record a successful input hash. assert not (build_dir / ".module_bazel_hash").exists() - # Arrange - # Use a second replacement to observe whether the failed output was removed. def rebuild(arguments: list[str]) -> int: assert not build_dir.exists() build_dir.mkdir() return 0 monkeypatch.setattr(docs_cli, "sphinx_main", rebuild) - - # Act - rebuild_exit_code = docs_cli.main([]) - - # Assert - # The marker from the failed build causes the next invocation to start clean. - assert rebuild_exit_code == 0 + assert docs_cli.main([]) == 0 def test_live_preview_uses_port_and_bundle_watches( workspace: Path, monkeypatch: pytest.MonkeyPatch, ) -> None: - # Arrange - monkeypatch.setenv("ACTION", "live_preview") manifest = workspace / "mounts.json" manifest.write_text( '{"mounts": [{"src_root": "extra/docs", "runtime_path": "extra/docs", "mount_at": "extra"}]}' ) - monkeypatch.setenv("MOUNTS_MANIFEST", str(manifest)) + _set_docs_config(monkeypatch, action="live_preview", mounts_manifest=str(manifest)) autobuild = Mock() monkeypatch.setattr(docs_cli, "sphinx_autobuild_main", autobuild) - # Act exit_code = docs_cli.main(["--port", "42424242424"]) - # Assert - # Live preview exits after handing control to sphinx-autobuild. assert exit_code == 0 autobuild.assert_called_once() arguments = autobuild.call_args.args[0] - # The requested port and source-linker setting are forwarded unchanged. assert "--port=42424242424" in arguments assert "--define=skip_rescanning_via_source_code_linker=1" in arguments - # Mounted bundle sources are watched in addition to the main docs tree. assert arguments[-2:] == ["--watch", str(workspace / "extra/docs")] - # Live preview does not write the successful-build hash. assert not (workspace / "component/_build/.module_bazel_hash").exists() -def test_bazel_configuration_resolves_runfiles_and_preserves_repo_relative_edit_path( +def test_interactive_configuration_resolves_runfiles_and_preserves_repo_relative_edit_path( workspace: Path, monkeypatch: pytest.MonkeyPatch, ) -> None: - # Arrange - monkeypatch.setenv("SPHINX_CONFIG_FILE", "config/conf.py") - monkeypatch.setenv("SCORE_METAMODEL_YAML", "config/metamodel.yaml") - monkeypatch.setenv("DATA", '[":bundle"]') - monkeypatch.setenv("EXTERNAL_NEEDS_FILES", '["@vendor//:needs"]') + config = _set_docs_config( + monkeypatch, + config_file="config/conf.py", + metamodel="config/metamodel.yaml", + source_links="links.json", + external_needs_sources=[":bundle", "@vendor//:needs"], + testcase_source_dirs=["src/tests"], + known_good="baseline.json", + ) monkeypatch.setenv("GITHUB_REPOSITORY", "owner/repo") - monkeypatch.setenv("KNOWN_GOOD_JSON", "baseline.json") package = workspace / "component" - # Act - arguments = sphinx_arguments(workspace, package, package / "_build") + arguments = sphinx_arguments( + config, + workspace, + package, + package / "_build", + workspace / "runfiles", + ) - # Assert expected_arguments = { - # Generated configuration and metamodel paths use the runfiles tree. "-c", str(workspace / "runfiles/config"), f"--define=score_metamodel_yaml={workspace}/runfiles/config/metamodel.yaml", - # DATA and EXTERNAL_NEEDS_FILES are passed as one Sphinx define. + f"--define=score_sourcelinks_json={workspace}/runfiles/links.json", '--define=external_needs_source=[":bundle", "@vendor//:needs"]', - # GitHub metadata must keep edit links repository-relative. + '--define=testcase_source_dirs=["src/tests"]', "-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}/runfiles/baseline.json", } - # Every expected option is present; their relative order is irrelevant here. assert expected_arguments <= set(arguments) @@ -219,16 +218,104 @@ def test_direct_invocation_resolves_metamodel_relative_to_workspace( workspace: Path, monkeypatch: pytest.MonkeyPatch, ) -> None: - # Arrange - # This test covers the non-Bazel fallback, so no runfiles directory exists. monkeypatch.delenv("RUNFILES_DIR", raising=False) - monkeypatch.setenv("SCORE_METAMODEL_YAML", "metamodel.yaml") + config = _set_docs_config(monkeypatch, metamodel="metamodel.yaml") - # Act - arguments = sphinx_arguments(workspace, workspace, workspace / "_build") + arguments = sphinx_arguments(config, workspace, workspace, workspace / "_build") - # 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_sandbox_configuration_resolves_execution_root_paths( + workspace: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + monkeypatch.chdir(workspace) + config = _set_docs_config( + monkeypatch, + action="build_needs_json", + package_directory="", + source_directory="external/vendor/docs", + output_directory="bazel-out/k8-fastbuild/bin/needs_json/_build/needs", + config_file="bazel-out/k8-fastbuild/bin/docs/conf.py", + source_links="bazel-out/k8-fastbuild/bin/sourcelinks.json", + metamodel="bazel-out/k8-fastbuild/bin/metamodel.yaml", + known_good="bazel-out/k8-fastbuild/bin/known_good.json", + ) + + arguments = sphinx_arguments( + config, + None, + workspace, + workspace / "bazel-out/k8-fastbuild/bin/needs_json/_build/needs", + ) + + assert arguments[:2] == [ + str(workspace / "external/vendor/docs"), + str(workspace / "bazel-out/k8-fastbuild/bin/needs_json/_build/needs"), + ] + assert ( + f"--define=score_sourcelinks_json={workspace}/bazel-out/k8-fastbuild/bin/sourcelinks.json" + in arguments + ) + assert ( + f"--define=score_metamodel_yaml={workspace}/bazel-out/k8-fastbuild/bin/metamodel.yaml" + in arguments + ) + assert "-c" in arguments + assert str(workspace / "bazel-out/k8-fastbuild/bin/docs") in arguments + + +def test_missing_configuration_payload_is_rejected( + workspace: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + monkeypatch.delenv("SCORE_DOCS_CONFIG", raising=False) + + with pytest.raises(ValueError, match="SCORE_DOCS_CONFIG is not set"): + docs_cli.main([]) + + +def test_missing_configuration_version_is_rejected( + workspace: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + monkeypatch.setenv("SCORE_DOCS_CONFIG", json.dumps({"action": "incremental"})) + + with pytest.raises(ValueError, match="missing required field 'version'"): + docs_cli.main([]) + + +def test_malformed_configuration_json_is_rejected( + workspace: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + monkeypatch.setenv("SCORE_DOCS_CONFIG", "not-json") + + with pytest.raises(ValueError, match="SCORE_DOCS_CONFIG must contain valid JSON"): + docs_cli.main([]) + + +@pytest.mark.parametrize("payload", [{"version": 2}, {"version": 0}]) +def test_unsupported_configuration_payload_version_is_rejected( + workspace: Path, monkeypatch: pytest.MonkeyPatch, payload: dict[str, int] +) -> None: + monkeypatch.setenv("SCORE_DOCS_CONFIG", json.dumps(payload)) + + with pytest.raises(ValueError, match="Unsupported SCORE_DOCS_CONFIG version"): + docs_cli.main([]) + + +def test_shared_sphinx_diagnostics_are_each_emitted_once( + workspace: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + config = _set_docs_config(monkeypatch) + + arguments = sphinx_arguments( + config, + workspace, + workspace / "component", + workspace / "component/_build", + workspace / "runfiles", + ) + + assert arguments.count("-W") == 1 + assert arguments.count("--keep-going") == 1 + assert arguments.count("-T") == 1 diff --git a/src/extensions/score_mounts/__init__.py b/src/extensions/score_mounts/__init__.py index f00ec2f04..d1a9ce85a 100644 --- a/src/extensions/score_mounts/__init__.py +++ b/src/extensions/score_mounts/__init__.py @@ -68,9 +68,12 @@ def _read_manifest(config: Config): if not raw or not raw.strip() or not isinstance(raw, str): 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) + # The CLI resolves both runfiles-relative and execution-root payload paths + # before passing them as a Sphinx define. Keep the relative fallback for + # extension callers that configure this value without the CLI. + manifest_path = Path(raw) + if not manifest_path.is_absolute() and find_ws_root(): + manifest_path = get_runfiles_dir() / manifest_path return load_mounts_manifest(manifest_path) diff --git a/src/extensions/score_source_code_linker/xml_parser.py b/src/extensions/score_source_code_linker/xml_parser.py index 97a5bc044..8393d7ab8 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: str | None = None +) -> MetaData: """ Will parse out the metadata from the testpath. If test is local then the metadata will be: @@ -144,7 +146,10 @@ 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") + # Sphinx configuration defines are the normal transport for Bazel builds; + # retain the environment fallback for direct extension callers and older + # integrations that invoke this helper without a Sphinx app. + known_good_json = known_good_json or 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) @@ -206,7 +211,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: str | None = None, ) -> tuple[list[DataOfTestCase], list[str], list[str]]: """ Reading & parsing the test.xml files into TestCaseNeeds @@ -222,7 +229,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") @@ -406,7 +413,11 @@ def build_test_needs_from_files( 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, + str(getattr(app.config, "KNOWN_GOOD_JSON", "") or ""), + ) ) non_prop_tests = ", ".join(n for n in tests_missing_all_props) if non_prop_tests: