From 9d39119867e074cd05079badc1ab1ad9b8d90b6c Mon Sep 17 00:00:00 2001 From: thekevinbot <248217931+thekevinbot@users.noreply.github.com> Date: Fri, 4 Sep 2026 19:38:01 -0400 Subject: [PATCH] refactor(distcheck): give select_tarball its own module node_flow/gate.py held two functions over the 12-line one-function-per-file threshold, so the gate flagged `run` (89 lines) as sharing a file with its holder `select_tarball` (13). Move the holder, not `run`: gate.py is the orchestration entry point the flow's CLI imports from. select_tarball raises DistcheckError, which gate.py defined, so the class moves to errors.py to keep the new dependency one-way. gate.py re-exports it, leaving node_flow/cli.py's import untouched. Its five test cases move to tarball_test.py rather than being duplicated. --- .../src/distcheck/node_flow/errors.py | 10 +++++ .../src/distcheck/node_flow/errors_test.py | 14 ++++++ .../distcheck/src/distcheck/node_flow/gate.py | 23 +--------- .../src/distcheck/node_flow/gate_test.py | 27 ------------ .../src/distcheck/node_flow/tarball.py | 23 ++++++++++ .../src/distcheck/node_flow/tarball_test.py | 44 +++++++++++++++++++ 6 files changed, 93 insertions(+), 48 deletions(-) create mode 100644 internals/distcheck/src/distcheck/node_flow/errors.py create mode 100644 internals/distcheck/src/distcheck/node_flow/errors_test.py create mode 100644 internals/distcheck/src/distcheck/node_flow/tarball.py create mode 100644 internals/distcheck/src/distcheck/node_flow/tarball_test.py diff --git a/internals/distcheck/src/distcheck/node_flow/errors.py b/internals/distcheck/src/distcheck/node_flow/errors.py new file mode 100644 index 000000000..3e27dbf52 --- /dev/null +++ b/internals/distcheck/src/distcheck/node_flow/errors.py @@ -0,0 +1,10 @@ +"""The failure type every node distcheck stage raises. + +Its own module so `tarball.py` can raise it without importing `gate.py`, which +imports `tarball.py`. +""" +from __future__ import annotations + + +class DistcheckError(RuntimeError): + """A distcheck stage failed -- carries a human-readable diagnostic.""" diff --git a/internals/distcheck/src/distcheck/node_flow/errors_test.py b/internals/distcheck/src/distcheck/node_flow/errors_test.py new file mode 100644 index 000000000..f650756a6 --- /dev/null +++ b/internals/distcheck/src/distcheck/node_flow/errors_test.py @@ -0,0 +1,14 @@ +"""Colocated unit tests for the node distcheck error type (no collaborators).""" +import pytest + +from distcheck.node_flow.errors import DistcheckError + + +def test_distcheck_error_is_a_runtime_error(): + assert issubclass(DistcheckError, RuntimeError) + + +def test_distcheck_error_carries_its_diagnostic(): + with pytest.raises(DistcheckError, match="boom") as raised: + raise DistcheckError("boom") + assert str(raised.value) == "boom" diff --git a/internals/distcheck/src/distcheck/node_flow/gate.py b/internals/distcheck/src/distcheck/node_flow/gate.py index 6605d3dd9..ba81a3002 100644 --- a/internals/distcheck/src/distcheck/node_flow/gate.py +++ b/internals/distcheck/src/distcheck/node_flow/gate.py @@ -24,14 +24,12 @@ import json import os import subprocess -from typing import Optional from distcheck.filesystem import FileSystem from distcheck.node_flow.platforms import Platform - -class DistcheckError(RuntimeError): - """A distcheck stage failed -- carries a human-readable diagnostic.""" +from .errors import DistcheckError +from .tarball import select_tarball def _require_zero(result, message: str) -> None: @@ -40,23 +38,6 @@ def _require_zero(result, message: str) -> None: raise DistcheckError(message) -def select_tarball(names, prefix: str, exclude: Optional[str] = None) -> str: - """The single `.tgz` in `names` matching `prefix` (and not `exclude`).""" - matches = [ - name - for name in names - if name.startswith(prefix) - and name.endswith(".tgz") - and (exclude is None or not name.startswith(exclude)) - ] - if len(matches) != 1: - raise DistcheckError( - f"expected exactly one {prefix!r} tarball, saw {matches} in {list(names)}" - ) - (only,) = matches - return only - - def staged_addon_path(ts_pkg: str, host: Platform) -> str: """Where `pnpm build` stages the host addon -- the flow's one prerequisite. diff --git a/internals/distcheck/src/distcheck/node_flow/gate_test.py b/internals/distcheck/src/distcheck/node_flow/gate_test.py index 51e3d30e0..a73f1ccf1 100644 --- a/internals/distcheck/src/distcheck/node_flow/gate_test.py +++ b/internals/distcheck/src/distcheck/node_flow/gate_test.py @@ -13,7 +13,6 @@ DistcheckError, _require_zero, run, - select_tarball, staged_addon_path, ) @@ -60,32 +59,6 @@ def _ok_sequence(): return [_res(), _res(), _res(), _res(stdout="dirsql 1.0")] -def test_select_tarball_finds_the_cli_tarball(): - assert select_tarball([_CLI_TGZ, _MAIN_TGZ], "dirsql-lib-") == _CLI_TGZ - - -def test_select_tarball_excludes_the_cli_tarball_for_the_main_prefix(): - assert ( - select_tarball([_CLI_TGZ, _MAIN_TGZ], "dirsql-", exclude="dirsql-lib-") - == _MAIN_TGZ - ) - - -def test_select_tarball_rejects_none(): - with pytest.raises(DistcheckError, match="exactly one"): - select_tarball(["other.tgz"], "dirsql-lib-") - - -def test_select_tarball_rejects_many(): - with pytest.raises(DistcheckError, match="exactly one"): - select_tarball(["dirsql-lib-a.tgz", "dirsql-lib-b.tgz"], "dirsql-lib-") - - -def test_select_tarball_ignores_non_tgz(): - with pytest.raises(DistcheckError, match="exactly one"): - select_tarball(["dirsql-lib-a.txt"], "dirsql-lib-") - - def test_run_success_executes_the_full_sequence(): fs = _fs() runner = mock.Mock(side_effect=_ok_sequence()) diff --git a/internals/distcheck/src/distcheck/node_flow/tarball.py b/internals/distcheck/src/distcheck/node_flow/tarball.py new file mode 100644 index 000000000..870c73590 --- /dev/null +++ b/internals/distcheck/src/distcheck/node_flow/tarball.py @@ -0,0 +1,23 @@ +"""Pick the one packed tarball a distcheck stage means to install.""" +from __future__ import annotations + +from typing import Optional + +from .errors import DistcheckError + + +def select_tarball(names, prefix: str, exclude: Optional[str] = None) -> str: + """The single `.tgz` in `names` matching `prefix` (and not `exclude`).""" + matches = [ + name + for name in names + if name.startswith(prefix) + and name.endswith(".tgz") + and (exclude is None or not name.startswith(exclude)) + ] + if len(matches) != 1: + raise DistcheckError( + f"expected exactly one {prefix!r} tarball, saw {matches} in {list(names)}" + ) + (only,) = matches + return only diff --git a/internals/distcheck/src/distcheck/node_flow/tarball_test.py b/internals/distcheck/src/distcheck/node_flow/tarball_test.py new file mode 100644 index 000000000..4dec90463 --- /dev/null +++ b/internals/distcheck/src/distcheck/node_flow/tarball_test.py @@ -0,0 +1,44 @@ +"""Colocated unit tests for tarball selection (a pure function, nothing to mock).""" +import pytest + +from distcheck.node_flow.tarball import DistcheckError, select_tarball + +_CLI_TGZ = "dirsql-lib-linux-x64-gnu-0.0.0-e2e.tgz" +_MAIN_TGZ = "dirsql-0.0.1.tgz" + + +def test_select_tarball_finds_the_cli_tarball(): + assert select_tarball([_CLI_TGZ, _MAIN_TGZ], "dirsql-lib-") == _CLI_TGZ + + +def test_select_tarball_excludes_the_cli_tarball_for_the_main_prefix(): + assert ( + select_tarball([_CLI_TGZ, _MAIN_TGZ], "dirsql-", exclude="dirsql-lib-") + == _MAIN_TGZ + ) + + +def test_select_tarball_rejects_none(): + with pytest.raises(DistcheckError, match="exactly one"): + select_tarball(["other.tgz"], "dirsql-lib-") + + +def test_select_tarball_rejects_many(): + with pytest.raises(DistcheckError, match="exactly one"): + select_tarball(["dirsql-lib-a.tgz", "dirsql-lib-b.tgz"], "dirsql-lib-") + + +def test_select_tarball_ignores_non_tgz(): + with pytest.raises(DistcheckError, match="exactly one"): + select_tarball(["dirsql-lib-a.txt"], "dirsql-lib-") + + +def test_select_tarball_reports_the_matches_and_the_candidates(): + with pytest.raises(DistcheckError) as raised: + select_tarball( + ["a.tgz", "dirsql-lib-a.tgz", "dirsql-lib-b.tgz"], "dirsql-lib-" + ) + message = str(raised.value) + assert "'dirsql-lib-'" in message + assert "['dirsql-lib-a.tgz', 'dirsql-lib-b.tgz']" in message + assert "['a.tgz', 'dirsql-lib-a.tgz', 'dirsql-lib-b.tgz']" in message