DEVX-155: refactor: extract wait_for_checks, consolidate ansible_checks, deprecate ci/discover_runners

2026-08-12 17:37:49 +00:00
parent b6f93cc2a8
commit 48596627d5
27 changed files with 2251 additions and 1197 deletions
@@ -0,0 +1,108 @@
+# ADR-0002: Ansible Check Tool Consolidation and wait_for_checks Extraction
+
+Date: 2026-08-12
+Status: Accepted
+
+## Context
+
+The devx package had two categories of code duplication and inline
+workflow logic that were hard to test and maintain:
+
+### 1. Ansible Check Tools — Duplicated Boilerplate
+
+Five Ansible check tools (`check_ansible_no_log`,
+`check_ansible_patterns`, `check_ansible_set_fact_to_json`,
+`check_ansible_no_state_absent_on_db`, `check_jinja_expr`) each
+implemented their own file discovery, YAML parsing, task iteration, and
+violation reporting logic. While the check logic differed, the
+supporting infrastructure was copy-pasted across all five modules:
+
+- `find_task_files()` — glob YAML files, skip molecule
+- YAML multi-document parsing with error handling
+- Task iteration (bare lists, play dicts with `tasks`/`pre_tasks`/`post_tasks`/`handlers`, nested `block` tasks)
+- Violation formatting (`path:line — message`)
+
+This made it difficult to add new checks (each new tool repeated the
+boilerplate) and risky to change shared behavior (fixes had to be
+applied to all five modules independently).
+
+### 2. Inline Job Polling in Workflow YAML
+
+The `grm` repository's `ci.yml` workflow contained ~25 lines of inline
+shell + Python polling logic to wait for the `molecule-tests` job to
+complete before the auto-merge step. This logic:
+
+- Was not testable (embedded in workflow YAML)
+- Duplicated the Gitea API client pattern already used elsewhere
+- Had no timeout handling, no error reporting, no retry logic
+- Could not be reused by other repositories
+
+### 3. Duplicate discover_runners Modules
+
+`devx.ci.discover_runners` and `devx.molecule.discover_runners` were
+near-identical modules. The `ci/` version had better error logging
+(warnings on non-200 responses, 403 suppression for instance-level
+queries), while the `molecule/` version silently swallowed errors.
+Both were imported by different workflows, making it unclear which was
+canonical.
+
+## Decision
+
+### 1. Composable `ansible_checks/` Subpackage
+
+Consolidate the five Ansible check tools into a
+`devx.tools.ansible_checks/` subpackage with shared utilities:
+
+- `_shared.py` — `AnsibleFileFinder`, `AnsibleYAMLParser`,
+  `ViolationReporter` classes providing composable helpers
+- `no_log.py`, `patterns.py`, `set_fact_to_json.py`,
+  `no_state_absent_on_db.py`, `jinja_expr.py` — canonical check
+  implementations using the shared utilities
+
+The old modules (`check_ansible_*.py`, `check_jinja_expr.py`) remain as
+**thin backward-compat wrappers** that re-export the canonical
+implementation and preserve the CLI entry point. This avoids breaking
+existing Makefile targets and workflow references.
+
+**Composition over inheritance**: each check module picks the helpers it
+needs. Tools that don't parse YAML (for example line-based scanners) can skip
+`AnsibleYAMLParser` entirely.
+
+### 2. Extracted `wait_for_checks` Module
+
+Extract the inline polling logic into `devx.ci.wait_for_checks`:
+
+- Polls the Gitea API for job completion status
+- Configurable job name prefix, timeout, poll interval
+- Exit codes: 0 (success), 1 (failure), 2 (timeout), 3 (API error)
+- `--require-success/--no-require-success` flag for flexibility
+- 100% test coverage with mocked API responses
+
+This replaces the inline shell polling in `grm` `ci.yml` with a
+reusable, testable Python module.
+
+### 3. Deprecated `ci/discover_runners` Wrapper
+
+Merge the `ci/discover_runners` implementation (with its better error
+logging) into `molecule/discover_runners` as the canonical version.
+Make `ci/discover_runners` a deprecated wrapper that:
+
+- Re-exports all public symbols from `molecule.discover_runners`
+- Emits a `DeprecationWarning` when run as `__main__`
+- Preserves backward compatibility for existing workflow references
+
+New code should import from `devx.molecule.discover_runners` directly.
+
+## Consequences
+
+- **New checks are easier to write**: import `_shared` helpers, implement
+  only the check-specific logic
+- **Shared behavior can be fixed in one place**: file discovery, YAML
+  parsing, violation formatting
+- **Workflow polling is testable**: `wait_for_checks` has 26 unit tests
+  covering success, failure, timeout, and API error scenarios
+- **Backward compatibility preserved**: all existing Makefile targets,
+  workflow references, and test imports continue to work via wrappers
+- **Migration path is gradual**: new code uses the subpackage; old code
+  can migrate at its own pace; wrappers can be removed in a future
+  release once all references are updated