GRM-53: refactor: enforce script separation and document import rules
This commit is contained in:
@@ -27,9 +27,11 @@ jobs:
|
||||
. .venv/bin/activate
|
||||
python3 scripts/check_test_speed.py --max-seconds 10
|
||||
- name: Documentation coverage check
|
||||
env:
|
||||
PYTHONPATH: src
|
||||
run: |
|
||||
. .venv/bin/activate
|
||||
PYTHONPATH=src python3 scripts/ci/doc_coverage.py
|
||||
python3 scripts/ci/doc_coverage.py
|
||||
|
||||
release-dry-run:
|
||||
needs: [quality, detect-changes]
|
||||
@@ -43,10 +45,12 @@ jobs:
|
||||
- name: Set up environment
|
||||
run: make setup
|
||||
- name: Release dry-run validation
|
||||
env:
|
||||
PYTHONPATH: .
|
||||
run: |
|
||||
. .venv/bin/activate
|
||||
export PATH="$HOME/.local/bin:$PATH"
|
||||
PYTHONPATH=. python3 scripts/ci/release.py --dry-run || true
|
||||
python3 scripts/ci/release.py --dry-run || true
|
||||
|
||||
detect-changes:
|
||||
runs-on: docker
|
||||
|
||||
@@ -237,15 +237,14 @@ types from accidentally skipping releases.
|
||||
- `hooks/**` — Git hooks
|
||||
|
||||
**User-facing paths** (tool changes → release needed) — everything else:
|
||||
- `src/gitea_runner_manager/**` — Python CLI source
|
||||
- `src/gitea_runner_manager/**` — Python CLI source (except `__init__.py` and `api_clients.py`)
|
||||
- `ansible/**` — Ansible role
|
||||
- `pyproject.toml` — Package metadata
|
||||
- `scripts/check_test_speed.py`, `scripts/configure_repo.py`, `scripts/install_checkmake.py` — Dev tools
|
||||
- Any new file type not in the allowlist
|
||||
|
||||
**Script directory structure:**
|
||||
- `scripts/` — Dev tools (run locally by developers): `check_test_speed.py`, `configure_repo.py`, `install_checkmake.py`, `install_tools.py`, `setup.py`, `molecule_all.py`
|
||||
- `scripts/ci/` — CI/CD automation (run by workflows): `release.py`, `publish.py`, `auto_merge.py`, `classify_changes.py`, `detect_release_commit.py`, `push_badges.py`, `doc_coverage.py`, `sync_wiki.py`, etc.
|
||||
- `scripts/` — Dev tools (run locally by developers): `check_test_speed.py`, `configure_repo.py`, `install_checkmake.py`, `install_tools.py`, `setup.py`, `molecule_all.py`, `generate_badges.py`
|
||||
- `scripts/ci/` — CI/CD automation (run by workflows): `release.py`, `publish.py`, `auto_merge.py`, `classify_changes.py`, `detect_release_commit.py`, `push_badges.py`, `doc_coverage.py`, `sync_wiki.py`, `distribute_molecule.py`, `molecule_ci_guard.py`, `discover_runners.py`, `notify_failure.py`, `post_merge.py`, `pr_review.py`, `review_pr.py`, `validate_commit_msg.py`, `platforms.py`
|
||||
|
||||
**CI behavior based on classification:**
|
||||
- **Molecule tests**: Only run when `ansible/` or `.ansible-lint` files change
|
||||
@@ -259,6 +258,52 @@ types from accidentally skipping releases.
|
||||
- The `classify_changes.py` script enforces this automatically — no manual intervention needed
|
||||
- When adding a new CI script, place it in `scripts/ci/`. Dev tools go in `scripts/`.
|
||||
|
||||
## Script Separation and Import Rules
|
||||
|
||||
The codebase enforces strict separation between the GRM tool and CI/dev scripts:
|
||||
|
||||
### Directory Layout
|
||||
|
||||
| Directory | Purpose | Release impact |
|
||||
|-----------|---------|----------------|
|
||||
| `src/gitea_runner_manager/` | User-facing GRM CLI tool | Changes trigger release |
|
||||
| `scripts/` | Dev tools (run locally) | Workflow-only (no release) |
|
||||
| `scripts/ci/` | CI/CD automation (run by workflows) | Workflow-only (no release) |
|
||||
| `ansible/` | Ansible role for runner setup | Changes trigger release |
|
||||
|
||||
### Import Rules
|
||||
|
||||
1. **`src/gitea_runner_manager/` NEVER imports from `scripts/`** — the tool is self-contained
|
||||
2. **Scripts MAY import from `gitea_runner_manager`** — one-way dependency (scripts use the tool's API clients, config, i18n)
|
||||
3. **Cross-script imports** (scripts importing from other scripts) are allowed within `scripts/ci/` but must be documented
|
||||
|
||||
### PYTHONPATH Configuration
|
||||
|
||||
Scripts have different import requirements. Workflows must set `PYTHONPATH` accordingly:
|
||||
|
||||
| PYTHONPATH | When to use | Example scripts |
|
||||
|------------|-------------|-----------------|
|
||||
| `src` | Script imports from `gitea_runner_manager` | `auto_merge.py`, `pr_review.py`, `notify_failure.py`, `sync_wiki.py`, `post_merge.py`, `publish.py`, `classify_changes.py`, `discover_runners.py`, `doc_coverage.py` |
|
||||
| `.` | Script imports from other `scripts.ci.*` modules | `release.py` (imports `classify_changes.has_user_facing_changes`) |
|
||||
| (none) | Script has no GRM or cross-script imports | `detect_release_commit.py`, `distribute_molecule.py`, `molecule_ci_guard.py`, `push_badges.py`, `validate_commit_msg.py` |
|
||||
|
||||
**In workflows**, always use `env:` blocks (not inline `PYTHONPATH=value`):
|
||||
```yaml
|
||||
- name: Run script
|
||||
env:
|
||||
PYTHONPATH: src
|
||||
run: python3 scripts/ci/example.py
|
||||
```
|
||||
|
||||
**Locally**, the current directory is in `sys.path` by default, so `PYTHONPATH` is usually not needed.
|
||||
|
||||
### Shared Constants
|
||||
|
||||
`scripts/ci/platforms.py` is the single source of truth for the molecule
|
||||
platform matrix. Both `scripts/ci/distribute_molecule.py` (CI) and
|
||||
`scripts/molecule_all.py` (dev tool) import `PLATFORMS` from it — this
|
||||
avoids dev tools importing directly from CI scripts.
|
||||
|
||||
2. **Publish workflow** (`.gitea/workflows/publish.yml`):
|
||||
- Triggers on tag push (`v*`)
|
||||
- Validates `PYPI_TOKEN` is set (warns if missing)
|
||||
@@ -319,7 +364,7 @@ main.yml → systemd_check → user_setup → rootless_docker → install_runner
|
||||
|
||||
6 scenarios: `default`, `multi-instance`, `lifecycle`, `template-content`, `deregister`, `update`
|
||||
4 platforms: `ubuntu-2204`, `ubuntu-2404`, `debian-12`, `archlinux`
|
||||
Platform list is defined in `scripts/ci/distribute_molecule.py` (single source of truth)
|
||||
Platform list is defined in `scripts/ci/platforms.py` (single source of truth)
|
||||
|
||||
## Known Issues
|
||||
|
||||
|
||||
@@ -221,6 +221,25 @@ When adding or removing Gitea runners:
|
||||
2. If runners are at the instance level, update the `MOLECULE_RUNNERS` repo variable
|
||||
3. The workflow automatically scales the matrix to match available runners
|
||||
|
||||
### Molecule Test Distribution
|
||||
|
||||
`scripts/ci/distribute_molecule.py` discovers all molecule scenarios
|
||||
under `ansible/roles/*/molecule/` and crosses them with the supported
|
||||
OS platform matrix (defined in `scripts/ci/platforms.py`), then splits
|
||||
the resulting test pairs evenly across the requested number of runners.
|
||||
Each pair is encoded as `scenario|platform_name|platform_image|platform_command`.
|
||||
|
||||
`scripts/ci/molecule_ci_guard.py` runs the actual molecule test for a
|
||||
given test pair, with CI context (Gitea URL, token, run ID) for
|
||||
reporting results back to the commit status API.
|
||||
|
||||
### Commit Message Validation
|
||||
|
||||
`scripts/ci/validate_commit_msg.py` validates that commit messages
|
||||
follow the conventional commit format (`feat:`, `fix:`, `docs:`, etc.).
|
||||
It is used by the pre-commit hook to enforce conventional commits on
|
||||
feature branches.
|
||||
|
||||
### Release Commit Detection
|
||||
|
||||
The `detect-type` job in the post-merge workflow runs
|
||||
|
||||
@@ -154,7 +154,11 @@ def classify_changes(files: list[str]) -> dict[str, list[str]]:
|
||||
|
||||
|
||||
def has_user_facing_changes(base: str, head: str) -> bool:
|
||||
"""Check if any user-facing files changed between base and head."""
|
||||
"""Check if any user-facing files changed between base and head.
|
||||
|
||||
Imported by ``scripts/ci/release.py`` to decide whether a release
|
||||
is needed. This is a cross-CI import that requires ``PYTHONPATH=.``.
|
||||
"""
|
||||
files = get_changed_files(base, head)
|
||||
return any(is_user_facing(f) for f in files)
|
||||
|
||||
|
||||
@@ -25,20 +25,11 @@ from pathlib import Path
|
||||
import click
|
||||
|
||||
from gitea_runner_manager.i18n import _
|
||||
from scripts.ci.platforms import PLATFORMS
|
||||
|
||||
DEFAULT_MAX_RUNNERS = 3
|
||||
MOLECULE_ROOT = Path("ansible/roles/gitea-runner/molecule")
|
||||
|
||||
#: Supported OS platform matrix.
|
||||
#: Each entry maps a short name to (image, command).
|
||||
#: The command must be systemd since rootless Docker requires loginctl/systemctl --user.
|
||||
PLATFORMS: list[dict[str, str]] = [
|
||||
{"name": "ubuntu-2204", "image": "geerlingguy/docker-ubuntu2204-ansible:latest", "command": "/lib/systemd/systemd"},
|
||||
{"name": "ubuntu-2404", "image": "geerlingguy/docker-ubuntu2404-ansible:latest", "command": "/lib/systemd/systemd"},
|
||||
{"name": "debian-12", "image": "geerlingguy/docker-debian12-ansible:latest", "command": "/lib/systemd/systemd"},
|
||||
{"name": "archlinux", "image": "marcstraube/archlinux-ansible:latest", "command": "/usr/lib/systemd/systemd"},
|
||||
]
|
||||
|
||||
|
||||
@dataclass(frozen=True)
|
||||
class TestPair:
|
||||
|
||||
@@ -47,6 +47,9 @@ REQUIRED_SCRIPTS = [
|
||||
"discover_runners.py",
|
||||
"detect_release_commit.py",
|
||||
"push_badges.py",
|
||||
"distribute_molecule.py",
|
||||
"molecule_ci_guard.py",
|
||||
"validate_commit_msg.py",
|
||||
]
|
||||
|
||||
|
||||
|
||||
@@ -0,0 +1,22 @@
|
||||
"""Supported OS platform matrix for molecule tests.
|
||||
|
||||
Single source of truth for the platform list used by both:
|
||||
- ``scripts/ci/distribute_molecule.py`` (CI parallel matrix)
|
||||
- ``scripts/molecule_all.py`` (local sequential runner)
|
||||
|
||||
Keeping this in a dedicated module avoids cross-imports between
|
||||
dev tools and CI scripts.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
#: Supported OS platform matrix.
|
||||
#: Each entry maps a short name to (image, command).
|
||||
#: The command must be systemd since rootless Docker requires
|
||||
#: loginctl/systemctl --user.
|
||||
PLATFORMS: list[dict[str, str]] = [
|
||||
{"name": "ubuntu-2204", "image": "geerlingguy/docker-ubuntu2204-ansible:latest", "command": "/lib/systemd/systemd"},
|
||||
{"name": "ubuntu-2404", "image": "geerlingguy/docker-ubuntu2404-ansible:latest", "command": "/lib/systemd/systemd"},
|
||||
{"name": "debian-12", "image": "geerlingguy/docker-debian12-ansible:latest", "command": "/lib/systemd/systemd"},
|
||||
{"name": "archlinux", "image": "marcstraube/archlinux-ansible:latest", "command": "/usr/lib/systemd/systemd"},
|
||||
]
|
||||
@@ -36,7 +36,7 @@ import click
|
||||
from dotenv import load_dotenv # pyright: ignore[reportMissingImports,reportUnknownVariableType]
|
||||
|
||||
from gitea_runner_manager.i18n import _
|
||||
from scripts.ci.classify_changes import has_user_facing_changes
|
||||
from scripts.ci.classify_changes import has_user_facing_changes # cross-CI import, needs PYTHONPATH=.
|
||||
|
||||
load_dotenv(override=True)
|
||||
|
||||
|
||||
@@ -19,7 +19,7 @@ from pathlib import Path
|
||||
|
||||
import click
|
||||
|
||||
from scripts.ci.distribute_molecule import PLATFORMS
|
||||
from scripts.ci.platforms import PLATFORMS
|
||||
|
||||
ROLE_DIR = Path("ansible/roles/gitea-runner")
|
||||
SCENARIOS = ["default", "multi-instance", "lifecycle", "template-content", "deregister", "update"]
|
||||
|
||||
@@ -78,7 +78,8 @@ class TestMain:
|
||||
(docs / "tech" / "ci-cd-workflow.md").write_text(
|
||||
"auto_merge.py release.py publish.py review_pr.py "
|
||||
"notify_failure.py post_merge.py classify_changes.py discover_runners.py "
|
||||
"detect_release_commit.py push_badges.py"
|
||||
"detect_release_commit.py push_badges.py "
|
||||
"distribute_molecule.py molecule_ci_guard.py validate_commit_msg.py"
|
||||
)
|
||||
runner = CliRunner()
|
||||
result = runner.invoke(main, ["--docs-dir", str(docs)])
|
||||
|
||||
@@ -0,0 +1,25 @@
|
||||
"""Unit tests for scripts/ci/platforms.py."""
|
||||
|
||||
from scripts.ci.platforms import PLATFORMS
|
||||
|
||||
|
||||
class TestPlatforms:
|
||||
def test_platforms_not_empty(self) -> None:
|
||||
assert len(PLATFORMS) >= 4
|
||||
|
||||
def test_each_platform_has_required_keys(self) -> None:
|
||||
for p in PLATFORMS:
|
||||
assert "name" in p
|
||||
assert "image" in p
|
||||
assert "command" in p
|
||||
|
||||
def test_platform_names_unique(self) -> None:
|
||||
names = [p["name"] for p in PLATFORMS]
|
||||
assert len(names) == len(set(names))
|
||||
|
||||
def test_known_platforms_present(self) -> None:
|
||||
names = {p["name"] for p in PLATFORMS}
|
||||
assert "ubuntu-2204" in names
|
||||
assert "ubuntu-2404" in names
|
||||
assert "debian-12" in names
|
||||
assert "archlinux" in names
|
||||
Reference in New Issue
Block a user