From a9daf8cbe16638de4f75fa7c37e7caf48d41f31d Mon Sep 17 00:00:00 2001 From: Tom Schraitle Date: Mon, 16 Feb 2026 10:53:04 +0100 Subject: [PATCH 1/7] Adjust commands due to Pydantic validation * As Pydantic takes care of validation the envconfig, we don't have to check that anymore * Remove any checking of if context.envconfig is None * Adjust ManagedGitRepo to allow string or Repo as remote_url This makes it easier * Use `cast(Envconfig, context.envconfig)` to avoid type errors --- .../docbuild/utils/git/ManagedGitRepo.rst | 2 +- src/docbuild/cli/cmd_build/__init__.py | 1 + src/docbuild/cli/cmd_c14n/__init__.py | 1 + src/docbuild/cli/cmd_metadata/metaprocess.py | 8 +- src/docbuild/cli/cmd_repo/cmd_clone.py | 3 - src/docbuild/cli/cmd_repo/cmd_dir.py | 9 +- src/docbuild/cli/cmd_repo/cmd_list.py | 14 +-- src/docbuild/cli/cmd_repo/process.py | 14 ++- src/docbuild/cli/cmd_validate/__init__.py | 14 +-- src/docbuild/utils/git.py | 14 ++- tests/cli/cmd_repo/test_cmd_clone.py | 94 +++++++++++-------- 11 files changed, 95 insertions(+), 79 deletions(-) diff --git a/docs/source/reference/_autoapi/docbuild/utils/git/ManagedGitRepo.rst b/docs/source/reference/_autoapi/docbuild/utils/git/ManagedGitRepo.rst index 27f873a3..e33e12aa 100644 --- a/docs/source/reference/_autoapi/docbuild/utils/git/ManagedGitRepo.rst +++ b/docs/source/reference/_autoapi/docbuild/utils/git/ManagedGitRepo.rst @@ -1,7 +1,7 @@ docbuild.utils.git.ManagedGitRepo ================================= -.. py:class:: docbuild.utils.git.ManagedGitRepo(remote_url: str, rootdir: pathlib.Path, gitconfig: pathlib.Path | None = None) +.. py:class:: docbuild.utils.git.ManagedGitRepo(repo: str | docbuild.models.repo.Repo, rootdir: pathlib.Path, gitconfig: pathlib.Path | None = None) Manages a bare repository and its temporary worktrees. diff --git a/src/docbuild/cli/cmd_build/__init__.py b/src/docbuild/cli/cmd_build/__init__.py index e488c04d..55d72222 100644 --- a/src/docbuild/cli/cmd_build/__init__.py +++ b/src/docbuild/cli/cmd_build/__init__.py @@ -58,6 +58,7 @@ def build(ctx: click.Context, doctypes: tuple[Doctype]) -> None: """ ctx.ensure_object(DocBuildContext) context: DocBuildContext = ctx.obj + # env = cast(EnvConfig, context.envconfig) click.echo(f"[BUILD] Verbosity: {context.verbose}") click.echo(f"{context=}") diff --git a/src/docbuild/cli/cmd_c14n/__init__.py b/src/docbuild/cli/cmd_c14n/__init__.py index b808d5fc..032375fe 100644 --- a/src/docbuild/cli/cmd_c14n/__init__.py +++ b/src/docbuild/cli/cmd_c14n/__init__.py @@ -10,4 +10,5 @@ def c14n(ctx: click.Context) -> None: :param ctx: The Click context object. """ + # env = cast(EnvConfig, context.envconfig) click.echo(f"[C17N] Verbosity: {ctx.obj.verbose}") diff --git a/src/docbuild/cli/cmd_metadata/metaprocess.py b/src/docbuild/cli/cmd_metadata/metaprocess.py index 6300f375..9ff9fd4b 100644 --- a/src/docbuild/cli/cmd_metadata/metaprocess.py +++ b/src/docbuild/cli/cmd_metadata/metaprocess.py @@ -6,7 +6,7 @@ import logging from pathlib import Path import shlex -from typing import Any +from typing import Any, cast from lxml import etree from pydantic import ValidationError @@ -14,6 +14,7 @@ from ...config.xml.stitch import create_stitchfile from ...constants import DEFAULT_DELIVERABLES +from ...models.config.env import EnvConfig from ...models.deliverable import Deliverable from ...models.doctype import Doctype from ...models.manifest import Document, Manifest @@ -407,12 +408,13 @@ async def process( configured correctly. :return: 0 if all files passed validation, 1 if any failures occurred. """ - configdir = Path(context.envconfig.paths.config_dir).expanduser() + env = cast(EnvConfig, context.envconfig) + configdir = Path(env.paths.config_dir).expanduser() stdout.print(f"Config path: {configdir}") xmlconfigs = tuple(configdir.rglob("[a-z]*.xml")) stitchnode: etree._ElementTree = await create_stitchfile(xmlconfigs) - tmp_metadata_dir = context.envconfig.paths.tmp.tmp_metadata_dir + tmp_metadata_dir = env.paths.tmp.tmp_metadata_dir # TODO: Is this necessary here? tmp_metadata_dir.mkdir(parents=True, exist_ok=True) diff --git a/src/docbuild/cli/cmd_repo/cmd_clone.py b/src/docbuild/cli/cmd_repo/cmd_clone.py index 8fc30be7..99ff6958 100644 --- a/src/docbuild/cli/cmd_repo/cmd_clone.py +++ b/src/docbuild/cli/cmd_repo/cmd_clone.py @@ -35,9 +35,6 @@ def clone(ctx: click.Context, repos: tuple[str, ...]) -> None: :param ctx: The Click context object. """ context: DocBuildContext = ctx.obj - if context.envconfig is None: - raise ValueError("No envconfig found in context.") - result = asyncio.run(process(context, repos)) log.info(f"Clone process completed with exit code: {result}") ctx.exit(result) diff --git a/src/docbuild/cli/cmd_repo/cmd_dir.py b/src/docbuild/cli/cmd_repo/cmd_dir.py index f848638c..cd6fd0dd 100644 --- a/src/docbuild/cli/cmd_repo/cmd_dir.py +++ b/src/docbuild/cli/cmd_repo/cmd_dir.py @@ -1,8 +1,11 @@ """Show the directory path for permanent repositories.""" +from typing import cast + import click from ...cli.context import DocBuildContext +from ...models.config.env import EnvConfig @click.command(help=__doc__, name="dir") @@ -16,9 +19,7 @@ def cmd_dir(ctx: click.Context) -> None: :param ctx: The Click context object. """ context: DocBuildContext = ctx.obj - if context.envconfig is None: - raise ValueError("No envconfig found in context.") - - repo_dir = context.envconfig.get("paths", {}).get("repo_dir", None) + env = cast(EnvConfig, context.envconfig) + repo_dir = env.paths.repo_dir print(repo_dir) ctx.exit(0) diff --git a/src/docbuild/cli/cmd_repo/cmd_list.py b/src/docbuild/cli/cmd_repo/cmd_list.py index 2e92f4ac..b5d7e58e 100644 --- a/src/docbuild/cli/cmd_repo/cmd_list.py +++ b/src/docbuild/cli/cmd_repo/cmd_list.py @@ -1,11 +1,13 @@ """List the available permanent repositories.""" from pathlib import Path +from typing import cast import click from rich.console import Console from ...cli.context import DocBuildContext +from ...models.config.env import EnvConfig console = Console() console_err = Console(stderr=True) @@ -24,15 +26,9 @@ def cmd_list(ctx: click.Context) -> None: :param ctx: The Click context object. """ context: DocBuildContext = ctx.obj - if context.envconfig is None: - raise ValueError("No envconfig found in context.") - - repo_dir = context.envconfig.get("paths", {}).get("repo_dir", None) - if repo_dir is None: - raise ValueError( - "No permanent repositories defined, neither with " - "--env-config nor as default." - ) + env = cast(EnvConfig, context.envconfig) + + repo_dir = env.paths.repo_dir repo_dir = Path(repo_dir).resolve() if not repo_dir.exists(): console_err.print( diff --git a/src/docbuild/cli/cmd_repo/process.py b/src/docbuild/cli/cmd_repo/process.py index dc09302d..9048cbe0 100644 --- a/src/docbuild/cli/cmd_repo/process.py +++ b/src/docbuild/cli/cmd_repo/process.py @@ -3,10 +3,12 @@ import asyncio import logging from pathlib import Path +from typing import cast from ...cli.context import DocBuildContext from ...config.xml.stitch import create_stitchfile from ...constants import GITLOGGER_NAME +from ...models.config.env import EnvConfig from ...models.repo import Repo from ...utils.contextmgr import make_timer from ...utils.git import ManagedGitRepo @@ -23,14 +25,9 @@ async def process(context: DocBuildContext, repos: tuple[str, ...]) -> int: :raises ValueError: If configuration paths are missing. """ # The calling command function is expected to have checked context.envconfig. - paths = context.envconfig.get("paths", {}) - config_dir_str = paths.get("config_dir") - repo_dir_str = paths.get("repo_dir") - - if not config_dir_str: - raise ValueError("Could not get a value from envconfig.paths.config_dir") - if not repo_dir_str: - raise ValueError("Could not get a value from envconfig.paths.repo_dir") + envcfg = cast(EnvConfig, context.envconfig) + config_dir_str = envcfg.paths.config_dir + repo_dir_str = envcfg.paths.repo_dir configdir = Path(config_dir_str).expanduser() repo_dir = Path(repo_dir_str).expanduser() @@ -52,6 +49,7 @@ async def process(context: DocBuildContext, repos: tuple[str, ...]) -> int: else: # Create a unique list from user input, preserving order unique_git_repos = list(dict.fromkeys(Repo(r) for r in repos)) + log.debug("User-specified repositories: %s", unique_git_repos) if not unique_git_repos: log.info("No repositories found to clone.") diff --git a/src/docbuild/cli/cmd_validate/__init__.py b/src/docbuild/cli/cmd_validate/__init__.py index 80429ce8..5d9b524e 100644 --- a/src/docbuild/cli/cmd_validate/__init__.py +++ b/src/docbuild/cli/cmd_validate/__init__.py @@ -4,9 +4,11 @@ from collections.abc import Iterator import logging from pathlib import Path +from typing import cast import click +from ...models.config.env import EnvConfig from ..context import DocBuildContext from . import process as process_mod @@ -36,20 +38,12 @@ def validate( :param validation_method: Validation method to use, 'jing' or 'lxml'. """ context: DocBuildContext = ctx.obj + env = cast(EnvConfig, context.envconfig) # Set the chosen validation method in the context for downstream use context.validation_method = validation_method.lower() - if context.envconfig is None: - raise ValueError("No envconfig found in context.") - - if (paths := ctx.obj.envconfig.get("paths")) is None: - raise ValueError("No paths found in envconfig.") - - configdir = paths.get("config_dir", None) - if configdir is None: - raise ValueError("Could not get a value from envconfig.paths.config_dir") - + configdir = env.paths.config_dir configdir_path = Path(configdir).expanduser() if not xmlfiles: diff --git a/src/docbuild/utils/git.py b/src/docbuild/utils/git.py index 3326a2d2..6d60a045 100644 --- a/src/docbuild/utils/git.py +++ b/src/docbuild/utils/git.py @@ -32,16 +32,24 @@ def clear_cache(cls) -> None: cls._is_updated.clear() def __init__( - self: Self, remote_url: str, rootdir: Path, gitconfig: Path | None = None + self: Self, repo: str | Repo, rootdir: Path, gitconfig: Path | None = None ) -> None: """Initialize the managed repository. - :param remote_url: The remote URL of the repository. + :param repo: The remote URL or :class:`~docbuild.models.repo.Repo` instance of the repository to manage. + Repo instance of the repository. :param permanent_root: The root directory for storing permanent bare clones. :param gitconfig: The path to a separate Git configuration file (=None, use the default config from etc/gitconfig) """ - self._repo_model = Repo(remote_url) + if isinstance(repo, str): + self._repo_model = Repo(repo) + elif isinstance(repo, Repo): + self._repo_model = repo + else: + raise TypeError( + f"remote_url must be a string or Repo instance, got {type(repo)}" + ) self._permanent_root = rootdir # The Repo model handles the "sluggification" of the URL self.bare_repo_path = self._permanent_root / self._repo_model.slug diff --git a/tests/cli/cmd_repo/test_cmd_clone.py b/tests/cli/cmd_repo/test_cmd_clone.py index a9fb22cc..e003419e 100644 --- a/tests/cli/cmd_repo/test_cmd_clone.py +++ b/tests/cli/cmd_repo/test_cmd_clone.py @@ -9,9 +9,32 @@ from docbuild.cli.cmd_repo.cmd_clone import clone import docbuild.cli.cmd_repo.process as mod_process from docbuild.cli.context import DocBuildContext +from docbuild.utils import shell as shell_module log = logging.getLogger(__name__) + +class _DummyPaths: + """Lightweight stand-in for EnvPathsConfig used in tests. + + Only the attributes accessed by cmd_repo.process are provided. + """ + + def __init__(self, *, config_dir: str, repo_dir: str) -> None: + self.config_dir = config_dir + self.repo_dir = repo_dir + + +class _DummyEnv: + """Minimal envconfig replacement exposing ``paths`` for tests. + + This avoids constructing a full EnvConfig instance while still matching + the runtime behaviour expected by the cloning logic. + """ + + def __init__(self, *, config_dir: str, repo_dir: str) -> None: + self.paths = _DummyPaths(config_dir=config_dir, repo_dir=repo_dir) + # # @pytest.fixture # def process_mock() -> AsyncMock: @@ -29,8 +52,10 @@ def mock_subprocess(monkeypatch) -> AsyncMock: process_mock.communicate.return_value = (b"stdout", b"stderr") process_mock.returncode = 0 mock_create_subprocess = AsyncMock(return_value=process_mock) + # Git commands go through shell.run_command → asyncio.create_subprocess_exec + # in the shell utility module. monkeypatch.setattr( - mod_process.asyncio, "create_subprocess_exec", mock_create_subprocess + shell_module.asyncio, "create_subprocess_exec", mock_create_subprocess ) return mock_create_subprocess @@ -63,9 +88,10 @@ def test_clone_from_xml_config(runner, tmp_path, mock_subprocess, caplog): (config_dir / "sles.xml").write_text(xml_content) context = DocBuildContext( - envconfig={ - "paths": {"repo_dir": str(repo_dir), "config_dir": str(config_dir)}, - }, + envconfig=_DummyEnv( + config_dir=str(config_dir), + repo_dir=str(repo_dir), + ), ) runner.invoke(clone, [], obj=context) @@ -78,22 +104,6 @@ def test_clone_from_xml_config(runner, tmp_path, mock_subprocess, caplog): assert "https://github.com/test/two.git" in cloned_repos -def test_clone_invalid_envconfig(runner): - """Test that an error is raised if the environment configuration is invalid.""" - context = DocBuildContext(envconfig=None) - - result = runner.invoke( - clone, - ["org/repo"], - obj=context, - # catch_exceptions=True, - ) - - assert result.exit_code != 0 - assert isinstance(result.exception, ValueError) - assert "No envconfig found in context" in str(result.exception) - - # @pytest.mark.asyncio async def test_process_stitchnode_none(monkeypatch, tmp_path): """Test that process raises ValueError if create_stitchfile returns None.""" @@ -101,12 +111,10 @@ async def test_process_stitchnode_none(monkeypatch, tmp_path): monkeypatch.setattr(mod_process, "create_stitchfile", AsyncMock(return_value=None)) context = DocBuildContext( - envconfig={ - "paths": { - "repo_dir": str(tmp_path / "repos"), - "config_dir": str(tmp_path / "config"), - } - } + envconfig=_DummyEnv( + config_dir=str(tmp_path / "config"), + repo_dir=str(tmp_path / "repos"), + ) ) # The config_dir must exist, even if empty @@ -119,18 +127,28 @@ async def test_process_stitchnode_none(monkeypatch, tmp_path): async def test_process_configdir_none(): - context = DocBuildContext(envconfig={"paths": {}}) - with pytest.raises( - ValueError, - match=re.escape("Could not get a value from envconfig.paths.config_dir"), - ): - await mod_process.process(context, repos=()) + """This scenario is no longer reachable with validated EnvConfig. + + The higher-level CLI now ensures ``envconfig`` is a fully validated + environment configuration. Retain this test as a smoke check that + calling ``process`` with a dummy, but structurally valid, envconfig + does not raise and returns an integer exit code. + """ + + context = DocBuildContext( + envconfig=_DummyEnv(config_dir="/non/existent/config", repo_dir="/tmp/repos"), + ) + + result = await mod_process.process(context, repos=()) + assert isinstance(result, int) async def test_process_repodir_none(): - context = DocBuildContext(envconfig={"paths": {"config_dir": "/dummy/config"}}) - with pytest.raises( - ValueError, - match=re.escape("Could not get a value from envconfig.paths.repo_dir"), - ): - await mod_process.process(context, repos=()) + """See docstring of test_process_configdir_none for rationale.""" + + context = DocBuildContext( + envconfig=_DummyEnv(config_dir="/tmp/config", repo_dir="/non/existent/repos"), + ) + + result = await mod_process.process(context, repos=()) + assert isinstance(result, int) From d257f1d4f83d3175a58426857f1723b6f330edc1 Mon Sep 17 00:00:00 2001 From: Tom Schraitle Date: Mon, 16 Feb 2026 17:38:07 +0100 Subject: [PATCH 2/7] Fix tests Most tests had a dict instead of a EnvConfig object. This fix introduces a fake EnvConfig. --- tests/cli/cmd_repo/test_cmd_dir.py | 34 +++---- tests/cli/cmd_repo/test_cmd_list.py | 24 ++--- tests/cli/cmd_repo/test_process.py | 59 +++++++----- .../validate/test_cmd_validate.py | 94 +++++++++---------- 4 files changed, 108 insertions(+), 103 deletions(-) diff --git a/tests/cli/cmd_repo/test_cmd_dir.py b/tests/cli/cmd_repo/test_cmd_dir.py index 60a7e67f..c000ed50 100644 --- a/tests/cli/cmd_repo/test_cmd_dir.py +++ b/tests/cli/cmd_repo/test_cmd_dir.py @@ -1,12 +1,26 @@ from docbuild.cli.cmd_repo.cmd_dir import cmd_dir -def test_cmd_dir_prints_repo_dir(runner, monkeypatch): +class _DummyPaths: + """Minimal paths holder exposing ``repo_dir`` only.""" + + def __init__(self, repo_dir: str) -> None: + self.repo_dir = repo_dir + + +class _DummyEnv: + """Fake EnvConfig-like object with a ``paths`` attribute.""" + + def __init__(self, repo_dir: str) -> None: + self.paths = _DummyPaths(repo_dir) + + +def test_cmd_dir_prints_repo_dir(runner): dummy_repo_dir = "/tmp/myrepo" class DummyContext: - def __init__(self, repo_dir): - self.envconfig = {"paths": {"repo_dir": repo_dir}} + def __init__(self, repo_dir: str) -> None: + self.envconfig = _DummyEnv(repo_dir) ctx_obj = DummyContext(dummy_repo_dir) @@ -14,17 +28,3 @@ def __init__(self, repo_dir): assert result.exit_code == 0 assert dummy_repo_dir in result.output - - -def test_cmd_dir_no_envconfig(runner, capsys): - class DummyContextNoEnv: - envconfig = None - - result = runner.invoke(cmd_dir, obj=DummyContextNoEnv()) - - captured = capsys.readouterr() - - assert captured.out == "" - assert result.exit_code != 0 - assert isinstance(result.exception, ValueError) - assert "No envconfig found in context." in str(result.exception) diff --git a/tests/cli/cmd_repo/test_cmd_list.py b/tests/cli/cmd_repo/test_cmd_list.py index 9eb79f73..98c12b3d 100644 --- a/tests/cli/cmd_repo/test_cmd_list.py +++ b/tests/cli/cmd_repo/test_cmd_list.py @@ -2,23 +2,23 @@ from docbuild.cli.context import DocBuildContext -def test_cmd_list_envconfig_none(runner): - context = DocBuildContext(envconfig=None) - result = runner.invoke(cmd_list, obj=context) - assert result.exit_code != 0 - assert "No envconfig found in context." in str(result.exception) +class _DummyPaths: + """Minimal paths holder exposing ``repo_dir`` only.""" + def __init__(self, repo_dir: str) -> None: + self.repo_dir = repo_dir -def test_cmd_list_repo_dir_none(runner): - context = DocBuildContext(envconfig={"paths": {}}) - result = runner.invoke(cmd_list, obj=context) - assert result.exit_code != 0 - assert "No permanent repositories defined" in str(result.exception) + +class _DummyEnv: + """Fake EnvConfig-like object with a ``paths`` attribute.""" + + def __init__(self, repo_dir: str) -> None: + self.paths = _DummyPaths(repo_dir) def test_cmd_list_repo_dir_not_exists(runner, tmp_path): repo_dir = tmp_path / "repos" - context = DocBuildContext(envconfig={"paths": {"repo_dir": str(repo_dir)}}) + context = DocBuildContext(envconfig=_DummyEnv(str(repo_dir))) result = runner.invoke(cmd_list, obj=context) assert result.exit_code == 1 assert "No permanent repositories found" in result.output @@ -31,7 +31,7 @@ def test_cmd_list_success(runner, tmp_path): (repo_dir / "repo1").mkdir() (repo_dir / "repo2").mkdir() (repo_dir / ".hidden").mkdir() - context = DocBuildContext(envconfig={"paths": {"repo_dir": str(repo_dir)}}) + context = DocBuildContext(envconfig=_DummyEnv(str(repo_dir))) result = runner.invoke(cmd_list, obj=context) assert result.exit_code == 0 assert "Available permanent repositories" in result.output diff --git a/tests/cli/cmd_repo/test_process.py b/tests/cli/cmd_repo/test_process.py index 49144bfa..422ee2c9 100644 --- a/tests/cli/cmd_repo/test_process.py +++ b/tests/cli/cmd_repo/test_process.py @@ -9,6 +9,25 @@ from docbuild.models.repo import Repo +class _DummyPaths: + """Minimal stand-in for EnvPathsConfig used by process tests.""" + + def __init__(self, *, config_dir: str, repo_dir: str) -> None: + self.config_dir = config_dir + self.repo_dir = repo_dir + + +class _DummyEnv: + """Fake EnvConfig-like object exposing ``paths`` only. + + This mirrors the attributes accessed by the repo ``process`` function + without pulling in the full Pydantic EnvConfig model. + """ + + def __init__(self, *, config_dir: str, repo_dir: str) -> None: + self.paths = _DummyPaths(config_dir=config_dir, repo_dir=repo_dir) + + @pytest.fixture def mock_managed_git_repo(monkeypatch) -> AsyncMock: """Fixture to mock the ManagedGitRepo class.""" @@ -46,12 +65,10 @@ async def test_process_with_specific_repos( repo_dir = tmp_path / "repos" context = DocBuildContext( - envconfig={ - "paths": { - "repo_dir": str(repo_dir), - "config_dir": str(tmp_path / "config"), - }, - }, + envconfig=_DummyEnv( + config_dir=str(tmp_path / "config"), + repo_dir=str(repo_dir), + ), ) # The directories must exist for the function to run (tmp_path / "config").mkdir() @@ -77,12 +94,10 @@ async def test_process_with_all_repos_from_xml( """Test `process` when no specific repos are provided, using XML config.""" repo_dir = tmp_path / "repos" context = DocBuildContext( - envconfig={ - "paths": { - "repo_dir": str(repo_dir), - "config_dir": str(tmp_path / "config"), - }, - }, + envconfig=_DummyEnv( + config_dir=str(tmp_path / "config"), + repo_dir=str(repo_dir), + ), ) (tmp_path / "config").mkdir() repo_dir.mkdir() @@ -118,12 +133,10 @@ async def test_process_with_no_repos_found( repo_dir = tmp_path / "repos" context = DocBuildContext( - envconfig={ - "paths": { - "repo_dir": str(repo_dir), - "config_dir": str(tmp_path / "config"), - }, - }, + envconfig=_DummyEnv( + config_dir=str(tmp_path / "config"), + repo_dir=str(repo_dir), + ), ) (tmp_path / "config").mkdir() repo_dir.mkdir() @@ -150,12 +163,10 @@ async def test_process_failure_if_one_clone_fails( repo_dir = tmp_path / "repos" context = DocBuildContext( - envconfig={ - "paths": { - "repo_dir": str(repo_dir), - "config_dir": str(tmp_path / "config"), - }, - }, + envconfig=_DummyEnv( + config_dir=str(tmp_path / "config"), + repo_dir=str(repo_dir), + ), ) (tmp_path / "config").mkdir() repo_dir.mkdir() diff --git a/tests/cli/cmd_validate/validate/test_cmd_validate.py b/tests/cli/cmd_validate/validate/test_cmd_validate.py index cc28242e..e028c2ce 100644 --- a/tests/cli/cmd_validate/validate/test_cmd_validate.py +++ b/tests/cli/cmd_validate/validate/test_cmd_validate.py @@ -84,6 +84,17 @@ def test_display_results_all_success(self, capsys): class TestProcessValidation: """Test cases for the process function.""" + class _DummyEnv(dict): + """Dict-like fake envconfig matching the legacy access pattern. + + The async ``process`` function still expects ``context.envconfig`` to + behave like a plain mapping with a ``paths`` sub-dictionary. This + helper preserves that contract while keeping tests explicit. + """ + + def __init__(self, *, config_dir: str) -> None: + super().__init__({"paths": {"config_dir": config_dir}}) + @pytest.fixture def mock_context(self): """Create a mock DocBuildContext.""" @@ -112,21 +123,33 @@ def invalid_xml_file(self): Path(f.name).unlink(missing_ok=True) async def test_process_no_envconfig(self): - """Test process raises ValueError when no envconfig.""" + """This error path is now handled earlier by the CLI. + + The validate command ensures ``envconfig`` is an EnvConfig instance + before calling the async ``process`` function, so passing ``None`` + here is no longer a realistic scenario. Retain this test as a minimal + smoke check to ensure ``process`` can still be invoked with a + structurally valid envconfig-like object and returns an int. + """ + context = Mock(spec=DocBuildContext) - context.envconfig = None + context.envconfig = {"paths": {"config_dir": "/test/config"}} - with pytest.raises(ValueError, match="No envconfig found in context"): - await process_mod.process(context, []) + result = await process_mod.process(context, []) + assert isinstance(result, int) async def test_process_invalid_paths_config(self): - """Test process raises ValueError when paths is not a dict.""" + """See test_process_no_envconfig for rationale. + + We now exercise ``process`` with a valid envconfig-like object, + asserting only that it returns an integer exit code. + """ + context = Mock(spec=DocBuildContext) - context.envconfig = {"paths": "not_a_dict"} + context.envconfig = self._DummyEnv(config_dir="/test/config") - with pytest.raises(ValueError, - match=re.escape("'paths.config' must be a dictionary")): - await process_mod.process(context, []) + result = await process_mod.process(context, []) + assert isinstance(result, int) async def test_process_with_no_xml_files(self, mock_context, caplog): """Test that process returns 0 when no XML files are provided.""" @@ -276,42 +299,17 @@ async def test_process_stitch_validation_fails_on_duplicates( class TestValidateCommand: """Test cases for the validate CLI command.""" + class _DummyPaths: + """Minimal paths holder for validate CLI tests.""" + + def __init__(self, config_dir: str) -> None: + self.config_dir = config_dir + + class _DummyEnv: + """Fake EnvConfig-like object exposing only ``paths.config_dir``.""" - def test_validate_no_envconfig_in_context(self, runner): - """Test validate command when no envconfig is found.""" - with runner.isolated_filesystem(): - Path("test.xml").write_text('') - # When no context object is passed, a default one is created, - # which has envconfig=None, triggering the error. - result = runner.invoke(validate, ["test.xml"], obj=DocBuildContext()) - - assert result.exit_code != 0 - assert isinstance(result.exception, ValueError) - assert "No envconfig found in context" in str(result.exception) - - def test_validate_no_paths_in_envconfig(self, runner): - """Test validate command when no paths are found in envconfig.""" - with runner.isolated_filesystem(): - Path("test.xml").write_text('') - context = DocBuildContext(envconfig={"some_other_key": "value"}) - result = runner.invoke(validate, ["test.xml"], obj=context) - - assert result.exit_code != 0 - assert isinstance(result.exception, ValueError) - assert "No paths found in envconfig" in str(result.exception) - - def test_validate_no_config_dir_in_paths(self, runner): - """Test validate command when no config_dir is found in paths.""" - with runner.isolated_filesystem(): - Path("test.xml").write_text('') - context = DocBuildContext(envconfig={"paths": {"other_dir": "/path"}}) - result = runner.invoke(validate, ["test.xml"], obj=context) - - assert result.exit_code != 0 - assert isinstance(result.exception, ValueError) - assert "Could not get a value from envconfig.paths.config_dir" in str( - result.exception - ) + def __init__(self, config_dir: str) -> None: + self.paths = TestValidateCommand._DummyPaths(config_dir) def test_validate_uses_provided_files(self, runner): """Test validate uses XML files provided on the command line.""" @@ -324,9 +322,7 @@ def test_validate_uses_provided_files(self, runner): # A config_dir is still needed to pass the initial checks. config_dir = Path(fs) / "config" config_dir.mkdir() - context = DocBuildContext( - envconfig={"paths": {"config_dir": str(config_dir)}} - ) + context = DocBuildContext(envconfig=self._DummyEnv(str(config_dir))) with patch.object( process_mod, "process", new_callable=AsyncMock @@ -355,9 +351,7 @@ def test_validate_finds_files_in_config_dir(self, runner): # This one should not be picked up by rglob('[a-z]*.xml') (config_dir / "Test3.xml").write_text("") - context = DocBuildContext( - envconfig={"paths": {"config_dir": str(config_dir)}} - ) + context = DocBuildContext(envconfig=self._DummyEnv(str(config_dir))) with patch.object( process_mod, "process", new_callable=AsyncMock From 70a098d9427382b5e0dd06ccc25179c58f2bd4f9 Mon Sep 17 00:00:00 2001 From: Tom Schraitle Date: Mon, 16 Feb 2026 17:49:29 +0100 Subject: [PATCH 3/7] Remove obsolete tests for checking None envconfig We have already Pydantic validation, no need to check. --- tests/cli/cmd_validate/test_process.py | 7 ------- 1 file changed, 7 deletions(-) diff --git a/tests/cli/cmd_validate/test_process.py b/tests/cli/cmd_validate/test_process.py index 55f8b0b1..05146a48 100644 --- a/tests/cli/cmd_validate/test_process.py +++ b/tests/cli/cmd_validate/test_process.py @@ -57,13 +57,6 @@ async def test_process_file_with_generic_parsing_error( assert "Generic test error" in captured.err -async def test_process_no_envconfig(mock_context): - mock_context.envconfig = None - with pytest.raises(ValueError, - match=re.escape("No envconfig found in context.")): - await process(mock_context, xmlfiles=(Path("dummy.xml"),)) - - @patch.object(process_module, "process_file", new_callable=AsyncMock, return_value=0) @patch.object(process_module, "create_stitchfile", new_callable=AsyncMock) async def test_process_with_stitchfile_failure( From d7f8a3b0c6f55306db95f54dc5c3027ea5ff40d3 Mon Sep 17 00:00:00 2001 From: Tom Schraitle Date: Mon, 16 Feb 2026 18:01:01 +0100 Subject: [PATCH 4/7] Test ManagedGitRepo with str and Repo --- tests/utils/test_git.py | 21 +++++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/tests/utils/test_git.py b/tests/utils/test_git.py index 236e71c2..cf5b9f51 100644 --- a/tests/utils/test_git.py +++ b/tests/utils/test_git.py @@ -207,10 +207,16 @@ def test_managed_git_repo_remote_url_property(tmp_path: Path): assert repo.remote_url == "https://github.com/my-org/my-repo.git" -def test_managed_git_repo_slug_property(tmp_path: Path): - """Test the slug property of ManagedGitRepo.""" - remote_url = "https://github.com/my-org/my-repo.git" - repo = ManagedGitRepo(remote_url, tmp_path) +@pytest.mark.parametrize( + "repo_input", + [ + "https://github.com/my-org/my-repo.git", + Repo("gh://my-org/my-repo"), + ], +) +def test_managed_git_repo_slug_property(tmp_path: Path, repo_input): + """Test the slug property of ManagedGitRepo for str and Repo inputs.""" + repo = ManagedGitRepo(repo_input, tmp_path) # The slug should be a filesystem-safe version of the canonical URL expected_slug = "https___github_com_my_org_my_repo_git" assert repo.slug == expected_slug @@ -224,6 +230,13 @@ def test_managed_git_repo_permanent_root_property(tmp_path: Path): assert repo.permanent_root == permanent_root +def test_managed_git_repo_invalid_repo_type(tmp_path: Path): + """ManagedGitRepo should reject unsupported repo types with TypeError.""" + + with pytest.raises(TypeError, match="remote_url must be a string or Repo instance"): + ManagedGitRepo(123, tmp_path) # type: ignore[arg-type] + + async def test_fetch_updates_success( tmp_path: Path, mock_execute_git: AsyncMock, monkeypatch ): From 02e2ae83aa0e2ea759ad3f3ab430f9fe148544f3 Mon Sep 17 00:00:00 2001 From: Tom Schraitle Date: Mon, 16 Feb 2026 18:16:40 +0100 Subject: [PATCH 5/7] Fix ruff errors --- tests/cli/cmd_validate/test_process.py | 1 - tests/cli/cmd_validate/validate/test_cmd_validate.py | 1 - 2 files changed, 2 deletions(-) diff --git a/tests/cli/cmd_validate/test_process.py b/tests/cli/cmd_validate/test_process.py index 05146a48..65c05380 100644 --- a/tests/cli/cmd_validate/test_process.py +++ b/tests/cli/cmd_validate/test_process.py @@ -1,7 +1,6 @@ """Tests for the XML validation process module.""" from pathlib import Path -import re from subprocess import CompletedProcess from unittest.mock import AsyncMock, MagicMock, patch diff --git a/tests/cli/cmd_validate/validate/test_cmd_validate.py b/tests/cli/cmd_validate/validate/test_cmd_validate.py index e028c2ce..8252972d 100644 --- a/tests/cli/cmd_validate/validate/test_cmd_validate.py +++ b/tests/cli/cmd_validate/validate/test_cmd_validate.py @@ -3,7 +3,6 @@ from collections.abc import Iterator from os import PathLike from pathlib import Path -import re from subprocess import CompletedProcess import tempfile from unittest.mock import AsyncMock, Mock, patch From c1366ea47d09efb2dbd70443195ead1073ba4dcb Mon Sep 17 00:00:00 2001 From: Tom Schraitle Date: Tue, 17 Feb 2026 08:05:15 +0100 Subject: [PATCH 6/7] Add news fragment --- changelog.d/181.doc.rst | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 changelog.d/181.doc.rst diff --git a/changelog.d/181.doc.rst b/changelog.d/181.doc.rst new file mode 100644 index 00000000..bc742020 --- /dev/null +++ b/changelog.d/181.doc.rst @@ -0,0 +1,4 @@ +Streamlined CLI subcommands by removing redundant environment configuration +checks and implementing strict typing with EnvConfig models. +Updated :class:`~docbuild.utils.git.ManagedGitRepo` to support both string +and :class:`~docbuild.models.repo.Repo` model initialization. From d3e2bcaafe3badbdcd760b7aa68398c2e200e615 Mon Sep 17 00:00:00 2001 From: Tom Schraitle Date: Tue, 17 Feb 2026 08:17:03 +0100 Subject: [PATCH 7/7] Remove cast(EnvConfig, ...) due to DocBuildContext Declare envconfig as EnvConfig type in DocBuildContext --- src/docbuild/cli/cmd_build/__init__.py | 2 +- src/docbuild/cli/cmd_c14n/__init__.py | 2 +- src/docbuild/cli/cmd_check/process.py | 4 +--- src/docbuild/cli/cmd_metadata/metaprocess.py | 5 ++--- src/docbuild/cli/cmd_repo/cmd_dir.py | 4 +--- src/docbuild/cli/cmd_repo/cmd_list.py | 4 +--- src/docbuild/cli/cmd_repo/process.py | 4 +--- src/docbuild/cli/cmd_validate/__init__.py | 4 +--- src/docbuild/cli/context.py | 3 ++- 9 files changed, 11 insertions(+), 21 deletions(-) diff --git a/src/docbuild/cli/cmd_build/__init__.py b/src/docbuild/cli/cmd_build/__init__.py index 55d72222..8eb936b4 100644 --- a/src/docbuild/cli/cmd_build/__init__.py +++ b/src/docbuild/cli/cmd_build/__init__.py @@ -58,7 +58,7 @@ def build(ctx: click.Context, doctypes: tuple[Doctype]) -> None: """ ctx.ensure_object(DocBuildContext) context: DocBuildContext = ctx.obj - # env = cast(EnvConfig, context.envconfig) + # env = context.envconfig click.echo(f"[BUILD] Verbosity: {context.verbose}") click.echo(f"{context=}") diff --git a/src/docbuild/cli/cmd_c14n/__init__.py b/src/docbuild/cli/cmd_c14n/__init__.py index 032375fe..0ba18b10 100644 --- a/src/docbuild/cli/cmd_c14n/__init__.py +++ b/src/docbuild/cli/cmd_c14n/__init__.py @@ -10,5 +10,5 @@ def c14n(ctx: click.Context) -> None: :param ctx: The Click context object. """ - # env = cast(EnvConfig, context.envconfig) + # env = context.envconfig click.echo(f"[C17N] Verbosity: {ctx.obj.verbose}") diff --git a/src/docbuild/cli/cmd_check/process.py b/src/docbuild/cli/cmd_check/process.py index 34d26803..0ceea12d 100644 --- a/src/docbuild/cli/cmd_check/process.py +++ b/src/docbuild/cli/cmd_check/process.py @@ -3,13 +3,11 @@ from collections.abc import Sequence import logging from pathlib import Path -from typing import cast from docbuild.cli.cmd_metadata.metaprocess import get_deliverable_from_doctype from docbuild.cli.context import DocBuildContext from docbuild.config.xml.stitch import create_stitchfile from docbuild.constants import DEFAULT_DELIVERABLES -from docbuild.models.config.env import EnvConfig from docbuild.models.deliverable import Deliverable from docbuild.models.doctype import Doctype from docbuild.utils.git import ManagedGitRepo @@ -56,7 +54,7 @@ async def process_check_files( """Verify DC file existence using official Deliverable models.""" log.info("Starting DC file availability check...") - env_config = cast(EnvConfig, ctx.envconfig) + env_config = ctx.envconfig config_dir = env_config.paths.config_dir.expanduser() repo_root = env_config.paths.repo_dir.expanduser() diff --git a/src/docbuild/cli/cmd_metadata/metaprocess.py b/src/docbuild/cli/cmd_metadata/metaprocess.py index 9ff9fd4b..444fa8c4 100644 --- a/src/docbuild/cli/cmd_metadata/metaprocess.py +++ b/src/docbuild/cli/cmd_metadata/metaprocess.py @@ -6,7 +6,7 @@ import logging from pathlib import Path import shlex -from typing import Any, cast +from typing import Any from lxml import etree from pydantic import ValidationError @@ -14,7 +14,6 @@ from ...config.xml.stitch import create_stitchfile from ...constants import DEFAULT_DELIVERABLES -from ...models.config.env import EnvConfig from ...models.deliverable import Deliverable from ...models.doctype import Doctype from ...models.manifest import Document, Manifest @@ -408,7 +407,7 @@ async def process( configured correctly. :return: 0 if all files passed validation, 1 if any failures occurred. """ - env = cast(EnvConfig, context.envconfig) + env = context.envconfig configdir = Path(env.paths.config_dir).expanduser() stdout.print(f"Config path: {configdir}") xmlconfigs = tuple(configdir.rglob("[a-z]*.xml")) diff --git a/src/docbuild/cli/cmd_repo/cmd_dir.py b/src/docbuild/cli/cmd_repo/cmd_dir.py index cd6fd0dd..5e308e90 100644 --- a/src/docbuild/cli/cmd_repo/cmd_dir.py +++ b/src/docbuild/cli/cmd_repo/cmd_dir.py @@ -1,11 +1,9 @@ """Show the directory path for permanent repositories.""" -from typing import cast import click from ...cli.context import DocBuildContext -from ...models.config.env import EnvConfig @click.command(help=__doc__, name="dir") @@ -19,7 +17,7 @@ def cmd_dir(ctx: click.Context) -> None: :param ctx: The Click context object. """ context: DocBuildContext = ctx.obj - env = cast(EnvConfig, context.envconfig) + env = context.envconfig repo_dir = env.paths.repo_dir print(repo_dir) ctx.exit(0) diff --git a/src/docbuild/cli/cmd_repo/cmd_list.py b/src/docbuild/cli/cmd_repo/cmd_list.py index b5d7e58e..ef0ef53b 100644 --- a/src/docbuild/cli/cmd_repo/cmd_list.py +++ b/src/docbuild/cli/cmd_repo/cmd_list.py @@ -1,13 +1,11 @@ """List the available permanent repositories.""" from pathlib import Path -from typing import cast import click from rich.console import Console from ...cli.context import DocBuildContext -from ...models.config.env import EnvConfig console = Console() console_err = Console(stderr=True) @@ -26,7 +24,7 @@ def cmd_list(ctx: click.Context) -> None: :param ctx: The Click context object. """ context: DocBuildContext = ctx.obj - env = cast(EnvConfig, context.envconfig) + env = context.envconfig repo_dir = env.paths.repo_dir repo_dir = Path(repo_dir).resolve() diff --git a/src/docbuild/cli/cmd_repo/process.py b/src/docbuild/cli/cmd_repo/process.py index 9048cbe0..5c311d7b 100644 --- a/src/docbuild/cli/cmd_repo/process.py +++ b/src/docbuild/cli/cmd_repo/process.py @@ -3,12 +3,10 @@ import asyncio import logging from pathlib import Path -from typing import cast from ...cli.context import DocBuildContext from ...config.xml.stitch import create_stitchfile from ...constants import GITLOGGER_NAME -from ...models.config.env import EnvConfig from ...models.repo import Repo from ...utils.contextmgr import make_timer from ...utils.git import ManagedGitRepo @@ -25,7 +23,7 @@ async def process(context: DocBuildContext, repos: tuple[str, ...]) -> int: :raises ValueError: If configuration paths are missing. """ # The calling command function is expected to have checked context.envconfig. - envcfg = cast(EnvConfig, context.envconfig) + envcfg = context.envconfig config_dir_str = envcfg.paths.config_dir repo_dir_str = envcfg.paths.repo_dir diff --git a/src/docbuild/cli/cmd_validate/__init__.py b/src/docbuild/cli/cmd_validate/__init__.py index 5d9b524e..255bf6fe 100644 --- a/src/docbuild/cli/cmd_validate/__init__.py +++ b/src/docbuild/cli/cmd_validate/__init__.py @@ -4,11 +4,9 @@ from collections.abc import Iterator import logging from pathlib import Path -from typing import cast import click -from ...models.config.env import EnvConfig from ..context import DocBuildContext from . import process as process_mod @@ -38,7 +36,7 @@ def validate( :param validation_method: Validation method to use, 'jing' or 'lxml'. """ context: DocBuildContext = ctx.obj - env = cast(EnvConfig, context.envconfig) + env = context.envconfig # Set the chosen validation method in the context for downstream use context.validation_method = validation_method.lower() diff --git a/src/docbuild/cli/context.py b/src/docbuild/cli/context.py index ea7856dd..4d26eccf 100644 --- a/src/docbuild/cli/context.py +++ b/src/docbuild/cli/context.py @@ -4,6 +4,7 @@ from pathlib import Path from typing import Any +from ..models.config.env import EnvConfig from ..models.doctype import Doctype @@ -32,7 +33,7 @@ class DocBuildContext: envconfig_from_defaults: bool = False """Internal flag to indicate if the env's config was loaded from defaults""" - envconfig: dict[str, Any] | None = None + envconfig: EnvConfig | None = None """The accumulated content of all env config files""" doctypes: list[Doctype] | None = None