Commit Graph
7 Commits
Author SHA1 Message Date
kevinveenbirkenbachandClaude Opus 4.7 478d0679dd refactor(roles): drop compose-nocheck flags + extract include-gated sub-tasks
Refactors four roles that previously used `# nocheck: compose-chdir-in-task` /
`compose-verb-in-task` / `container-exec-resolver` markers to follow the
canonical swarm-aware pattern:

- web-app-bigbluebutton: adds BBB_POSTGRES_CONTAINER_ADDRESS /
  BBB_GREENLIGHT_CONTAINER_ADDRESS vars; 02a_postgres_password_sync.yml +
  03_administrator.yml use `container exec -i {{ ..._CONTAINER_ADDRESS | quote }}`;
  02_compose.yml's compose-build-phase tasks (generate-compose +
  prebuild etherpad) move into the new
  02b_bootstrap_compose_project.yml (>3 entries cannot live in a block)
  under a single `when: DEPLOYMENT_MODE \!= "swarm"` include guard.
- web-app-baserow: BASEROW_APP_CONTAINER_ADDRESS var + 02_manager_ops.yml
  converted to container exec + container restart canonical pattern.
- web-app-matomo: MATOMO_APP_CONTAINER_ADDRESS var; 07_patch_database.yml
  swaps MATOMO_LOCAL_CID for the proper resolver-backed address.
- web-app-matrix: 01_docker.yml's two consecutive `compose pull` /
  `compose up` tasks extract into 03_compose_pull_and_up.yml (with the new
  `# include-gated: when: DEPLOYMENT_MODE \!= "swarm"` header marker) and
  the previous 03_webserver.yml renames to 04_webserver.yml to keep the
  numeric file sequence contiguous.

Supporting lint infrastructure changes so file-level include-gated
sub-task lists survive the swarm-compat lints:

- utils/annotations/task_gate.py: `is_task_compose_only_gated` now walks
  up nested parent blocks (a single `when: DEPLOYMENT_MODE \!= 'swarm'`
  on `- block:` exempts every child task underneath) and a new
  `is_file_compose_only_by_header` recognises files declaring
  `# include-gated: when: DEPLOYMENT_MODE \!= "swarm"` in their first
  five lines as compose-only-by-include.
- tests/lint/ansible/shell/test_no_compose_chdir_in_shell_tasks.py and
  test_no_compose_verb_in_shell_tasks.py call the header helper before
  the per-line scan, so include-only sub-files are exempt.
- tests/unit/utils/annotations/test_task_gate.py covers both new
  behaviours (recursive parent walk plus file header) end-to-end.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
2026-06-14 19:08:46 +02:00
kevinveenbirkenbach fb977e975a test(lint): recognise compose-only task gates + skip comment / YAML metadata lines
The swarm-compat lints (`compose-chdir-in-task`, `compose-verb-in-task`,
`docker-compose-raw-call`) currently flag every offending line and only
accept per-line / file-head `# nocheck:` markers. That is noisy for
two patterns that are objectively safe:

1. Tasks gated by `when: DEPLOYMENT_MODE \!= 'swarm'` (or
   `== 'compose'`) never reach a swarm host - the compose verbs inside
   are legitimate.
2. YAML comment lines, handler-trigger list items, and human-readable
   metadata fields (`name:`, `msg:`, `title:`, `label:`, `description:`)
   contain the word `compose` as documentation, not as a shell call.

The lints now skip both classes automatically.

- utils/annotations/task_gate.py: new helper
  `is_task_compose_only_gated(lines, idx)`. Walks the surrounding
  task block via a line-based heuristic and matches `when:` against
  the compose-only DEPLOYMENT_MODE expressions. Documented as
  deliberately shallow (no YAML parse).
- tests/lint/ansible/shell/test_no_compose_chdir_in_shell_tasks.py and
  tests/lint/ansible/shell/test_no_compose_verb_in_shell_tasks.py:
  add a file-level `is_suppressed_in_head` skip, the
  `is_task_compose_only_gated` skip, and -- for the verb lint --
  three extra regex exclusions for notify-list items, YAML metadata
  fields, and quoted list items.
- tests/lint/ansible/shell/test_no_docker_compose_raw_call.py: skip
  YAML comment lines (bluesky and jenkins each had a `# … docker
  compose build …` doc-comment that was being flagged as a real call).
- tests/unit/utils/annotations/test_task_gate.py: 21 cases covering
  block-bounds + gate detection, including a `TestKnownLimitations`
  class that pins the heuristic's blind spots (folded-scalar `when:`,
  list-form `when:`, `in`/`not in`, unquoted literal, outer-`when:`
  inheritance into `block:` children) so a future YAML-aware refactor
  knows exactly which assertions to flip.
2026-06-14 19:08:45 +02:00
kevinveenbirkenbachandClaude Opus 4.7 a5ba14d5b3 test(lint/cache-read): extend scope to all tests/ + convert callers
Widens the cache-read lint guard from ``tests/lint/`` to every ``.py``
under ``tests/``. Integration tests show the same compounding cost
pattern (one role's compose template / nginx conf / scrape fragment
read across many shape tests) that motivated the original lint for
lint/.

Converts 91 violations across 28 test files:

- **Cache (utils.cache.files.read_text)** for project-tracked reads
  and single-read tempdir fixtures, e.g. test_prometheus.py (32x),
  test_keycloak_import_flow.py (11x), test_dashboard_oidc.py (3x),
  test_changelog_md.py (8x), test_package_mirror.py (8x), plus
  test_logging.py / test_summarize.py / test_version_updater.py /
  test_get_url_retry.py / test_pull_specific_host.py.

- **Cache + UnicodeDecodeError handler** for project-walk scanners
  that previously used ``errors="replace"`` / ``errors="ignore"`` to
  defend against non-UTF-8 files (compose volumes, password quoting,
  template image, mailu dependency, URL reachability). Wraps each
  read in ``try/except (OSError, UnicodeDecodeError): continue``,
  which has the same observable effect (skip undecodable files).

- **Refactor away the binary heuristic** in test_no_raw_docker.py:
  ``read_bytes() + b"\x00" not in raw`` becomes
  ``read_text() / except UnicodeDecodeError``. Same files skipped,
  with cache benefit.

- **Variable extraction** in test_changelog_md.py / test_package_mirror.py
  for cases where ``self.assertEqual(path.read_text(...), expected)``
  exceeded ruff's 88-char limit. Ruff was wrapping the marker onto a
  line outside the AST span of the read_text call.

- **Compute-from-setup** for two test methods that previously read
  ``self.changelog`` twice (before/after) to verify a no-op:
  ``before`` is now derived from the known input, dropping the first
  read.

Five ``# nocheck: cache-read`` markers remain, each annotated inline
with the reason: idempotency tests where the second read must hit
disk to catch non-idempotent regressions, and tests with intermediate
``write_text`` whose subsequent read would return stale cache data.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-15 14:23:32 +02:00
kevinveenbirkenbachandClaude Opus 4.7 16e292e805 chore(ruff): enable RUF/SIM/C4/PERF/RET/PIE rule families — 364 → 0
Extends ``[tool.ruff.lint].select`` with six additional rule families:
``RUF`` (ruff-specific), ``SIM`` (flake8-simplify), ``C4``
(flake8-comprehensions), ``PERF`` (perflint), ``RET`` (flake8-return),
``PIE`` (flake8-pie). Adds ``RUF001/002/003`` to ``ignore`` because
those exist for homoglyph-attack detection, not English typography —
the project uses em/en-dashes and ``ℹ`` as a UI-hint glyph in CLI help
output deliberately.

Auto-fixed (~225, mechanical & behavior-preserving)
====================================================
- RUF005:  ``a + [b]``     → ``[*a, b]``
- C408:    ``dict(k=v)``   → ``{"k": v}``
- C420:    ``dict.fromkeys(seq)`` shape simplifications
- SIM103/SIM110/SIM118:    truthy-return / dict-key / dict-method
                           idiom simplifications
- RET504/RET505:           drop unneeded final assigns / ``else`` after
                           ``return``
- PIE810:                  combine ``str.startswith(a) or .startswith(b)``
                           into one call with a tuple
- RUF010:                  ``f"{str(x)}"`` → ``f"{x\!s}"``
- RUF100:                  removed unused ``# noqa`` directives that
                           became stale (mostly old ``# noqa: E402``
                           markers that ruff's smart E402 implementation
                           no longer flags after ``sys.path`` shims)
- RUF015 / RUF013 / RUF022 / RUF046 / RUF059: misc small wins
- I001:                    re-sorted import blocks where the new
                           imports above shifted them

Hand-fixed (~140, scripted where mechanical)
============================================
PERF401 (42): scripted AST rewrite of
  ``for x in iter: target.append(expr)`` (with optional ``if`` guard)
  →  ``target.extend(expr for x in iter [if cond])``.
  ``list.extend(genexp)`` consumes the generator immediately, so the
  rewrite is semantically equivalent to the original eager loop.

SIM117 (41): scripted AST combiner for nested ``with`` blocks where
  the inner is the only statement of the outer (38 sites). The remaining
  3 multi-line ``patch.object(...)`` chains were combined by hand using
  the parenthesised ``with (\\n A,\\n B,\\n):`` form.

SIM102 (14): scripted AST combiner for nested ``if`` (10 sites). The
  remaining 4 carry an inline ``# noqa: SIM102`` with rationale —
  combining would either change semantics (the outer body has siblings
  beyond the inner ``if``, e.g. ``cli/encrypt/inventory/__main__.py``
  where ``encrypted_value = …`` runs regardless of ``ask_confirmation``)
  or hurt readability (long literal whitelists).

RUF012 (9): annotated mutable class attributes with ``ClassVar[…]``
  and added ``from typing import ClassVar``.

SIM105 (6): ``try/except: pass`` → ``with contextlib.suppress(…):``
  with the necessary ``import contextlib`` added. Verified on
  ``utils/templating.py`` (3 occurrences in a ``finally`` block) —
  cleanup behaviour is identical.

SIM115 (4): wrapped ``tempfile.NamedTemporaryFile(..., delete=False)``
  call patterns in a ``with`` block; ``delete=False`` keeps the file
  on disk after the block, so callers' ``return f.name`` still works.

PERF402, PERF403, B007, SIM108, SIM103: 5 misc fixes (loop→list-copy,
  loop→dict-comprehension, unused-loop-var rename, ternary, return
  expression).

Verification
============
- ``ruff check .`` → All checks passed
- ``ruff format --check .`` → 894 files already formatted
- 1621 unit tests → all pass
- Integration suite: 44 baseline failures, 1 skipped (unchanged)
- Sample-audited PERF401 rewrites in ``resolver.py``, ``inventory.py``,
  ``cache/files.py``, ``ressources/__main__.py``; SIM117 in
  ``test_main.py``; SIM105 in ``utils/templating.py``; SIM102 noqa
  rationales in ``cli/encrypt/inventory/__main__.py``.

182 files changed, +993 / −960. With this and the prior strict-rule
commit the branch satisfies 11 ruff rule families end-to-end.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-07 17:23:32 +02:00
kevinveenbirkenbach 837d0c5335 test(suppression): unify # noqa / # nocheck marker grammar
Introduce a single grammar for per-line, per-block, and per-file
suppression markers across the test suite, replacing seven ad-hoc
syntaxes that had drifted apart.

Helper
  utils/annotations/suppress.py exposes is_suppressed_at,
  is_suppressed_in_head, is_suppressed_anywhere, line_has_rule, and
  suppressed_line_numbers. Markers may use either `noqa` or `nocheck`
  (case-insensitive synonyms), live inside `#`, `//`, `{# ... #}`, or
  `<\!-- ... -->` comments, and combine multiple kebab-case rules
  comma-separated. 17 unit tests cover the grammar in isolation.

Refactored consumers
  - tests/external/repository/test_urls_reachable.py (rule: url)
  - tests/external/docker/test_image_versions.py via
    utils/docker/version_updater.py (rule: docker-version)
  - tests/lint/repository/test_no_direct_yaml_calls.py (rule:
    direct-yaml)
  - tests/lint/ansible/test_service_shared_consistency.py (rule:
    shared)
  - tests/lint/ansible/roles/test_web_app_email_integration.py (rule:
    email)
  - tests/lint/repository/test_python_file_size.py (rule: file-size)
  - tests/lint/ansible/test_run_once_tags.py +
    tests/integration/roles/run_once/test_schema.py (rules: run-once,
    run-once-suffix)

Marker migration
  12 role tasks/main.yml files: `# run_once_<key>: deactivated`
  -> `# nocheck: run-once`. roles/web-app-suitecrm/meta/services.yml
  drops a malformed inline `noqa: intentional legacy pin` from a TODO
  comment so the new parser does not pick it up as a stray rule.

Caching wins
  Test files that previously loaded YAML / read files via raw
  yaml.safe_load + Path.read_text now route through utils.cache.yaml
  (load_yaml_any / load_yaml_str) and utils.cache.files.read_text:
    - test_service_shared_consistency.py
    - test_web_app_email_integration.py
    - tests/integration/roles/run_once/test_schema.py
    - utils/docker/version_updater.py
  This both speeds up repeated reads and stays consistent with the
  direct-yaml lint policy.

Documentation
  New SPOT page at docs/contributing/actions/testing/suppression.md
  with grammar, position semantics, the catalog of rules, examples,
  and an "adding a new rule" checklist. lint.md and external.md drop
  their per-rule tables and link to the new page. All touched docs
  conform to docs/contributing/documentation.md (no em-dash thought
  breaks, no SPOT self-labels, RFC-2119 keywords used consistently).

Settings sort tooling
  scripts/lint/sort_claude_settings.sh wraps utils/claude/settings_sort.py
  and is wired into scripts/lint/autoformat.sh. The tooling fixes the
  ASCII-ascending requirement enforced by
  tests/lint/repository/test_claude_settings_sorted.py; the test's
  failure message now points operators at the script. .claude/settings.json
  is committed in sorted form.
2026-04-30 11:54:46 +02:00
Environment TestandClaude Sonnet 4.6 ec98dfbe46 fix(test): resolve CodeQL import-and-import-from alerts #263 #264
Move sys, unittest.mock and main to top-level imports. Replace inline
module imports and mod.main() calls with the already-imported main().

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-04-13 12:08:27 +02:00
kevinveenbirkenbachandClaude Sonnet 4.6 0ce6b1347c refactor(annotations): move to utils/annotations, add step summary support
- Move utils/gha/annotations.py → utils/annotations/message.py
- Move scripts/gha/summarize_annotations.py → utils/annotations/summarize.py
- Keep utils/gha/annotations.py as compatibility shim
- Update all imports across codebase to utils.annotations.message
- Wire tee + summary step into test-code-lint, unit, and integration workflows
- Add unit tests for utils/annotations/summarize (20 tests)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-04-08 14:05:24 +02:00