test(garm): refactor unit tests to the scenario framework

[cbartz]

What this PR does

Refactors the GARM charm's unit tests in test_charm.py to the Scenario framework, aligning
with garm-configurator. Each test now drives the real charm against a State and asserts on
the resulting State — secrets created, files pushed, Pebble plan, reported status — with only
GARM's HTTP surface stubbed.

The pure config-rendering and secret-generation helpers move to test_garm_toml.py, leaving
test_charm.py purely Scenario-driven.

src/charm.py statement coverage rises from 67% to 90%.

Also adds the arrange/act/assert docstring convention to the root AGENTS.md. It was only
written down in STYLE.md, which is scoped to Go and spells it as // Arrange: comments, so
Python tests had nothing to point at — and it is followed unevenly (planner-operator and
garm do, garm-configurator and webhook-gateway-operator do not), which makes imitating
the nearest neighbour an unreliable guide.

Why we need it

The charm-behaviour tests faked self with a MagicMock and called unbound methods
(GarmCharm.restart(charm)). That could only assert on the arguments the charm passed to its
collaborators — never on what it actually did — and needed scaffolding like a sentinel
exception to stop execution before super().restart(). Scenario removes the scaffolding and
covers the relation-parsing and config-writing paths the mock-based tests never reached.

ops-scenario 8.8.0 expands the go-framework extension itself, so no packing is needed for
the charm's container, peer relation and config options to resolve.

Test plan

tox -c tox.toml -e fmt,lint,unit — 247 unit tests pass. Coverage measured with
tox -c tox.toml -e unit,coverage-report on this branch and on origin/main (67% → 90% on
src/charm.py).

Integration tests (tox -e garm-integration) have not been run — they need a live Juju
model. This PR changes no production code, so the risk is contained to CI.

Test inventory: where each of main's 52 tests went

The diff is large and the test counts are misleading — test_charm.py goes 52 → 33 while
covering more, because 18 tests moved out to test_garm_toml.py, five groups merged, one was
dropped, and six are new. This table is the map. It is a one-time review aid, not a permanent
artifact.

Moved verbatim to test_garm_toml.py (18) — names unchanged, so this part is mechanically
checkable rather than a matter of trust:

# every name in test_garm_toml.py exists verbatim on main, and there are 18 of them
comm -12 <(git show origin/main:charms/garm/tests/unit/test_charm.py | grep '^def test_' | sed 's/^def //;s/(.*//' | sort) \
         <(grep '^def test_' charms/garm/tests/unit/test_garm_toml.py | sed 's/^def //;s/(.*//' | sort) | wc -l
# -> 18, and `comm -13` of the same two files is empty

build_provider_list_password_in_clouds_yaml, build_provider_list_returns_default_when_empty,
generate_admin_password_meets_garm_policy, generate_admin_password_produces_unique_values,
generate_garm_secrets_produces_unique_values, generate_garm_secrets_returns_jwt_and_passphrase,
proxy_environment_edge_cases, proxy_environment_happy_path,
render_clouds_yaml_quotes_special_chars, render_garm_toml_default_provider_applies_proxy_var_names,
render_garm_toml_no_proxy_var_names_omits_key, render_garm_toml_passphrase_in_database_section,
render_garm_toml_postgresql_backend, render_garm_toml_provider_section,
render_garm_toml_section_fields, render_garm_toml_sslmode_propagated,
render_garm_toml_with_configurator_providers, render_garm_toml_with_proxy_var_names.

Seven of these carried single-line summaries rather than AAA docstrings — a gap that predates
this PR on main. Since moving a test is the moment to fix it, they now have AAA docstrings;
both test files are at 33/33 and 18/18.

Converted to Scenario (34 → 27, one dropped):

main	→ test_charm.py
ensure_secrets_creates_secrets_on_first_run	leader_creates_garm_secrets	1:1
ensure_secrets_skips_when_not_leader	non_leader_creates_no_secrets	1:1
ensure_secrets_skips_creation_when_secrets_exist	existing_secrets_are_not_regenerated	1:1
restart_ensures_secrets_before_readiness_gate	secrets_are_created_before_the_workload_is_ready	1:1
maybe_first_run_skips_when_already_initialized	first_run_skipped_when_garm_is_already_initialised	1:1
maybe_first_run_skips_when_not_leader	first_run_skipped_when_not_leader	1:1
maybe_first_run_raises_on_connection_error	first_run_connection_error_errors_out_for_retry	1:1
get_credentials_action_returns_credentials_when_available	get_credentials_action_returns_the_stored_credentials	1:1
get_credentials_action_fails_when_credentials_unavailable	get_credentials_action_fails_before_the_credentials_exist	1:1
reconcile_calls_restart	every_observed_event_reconciles	1:1
reconcile_runners_reconciles_credentials_entities_then_scalesets	reconcilers_run_in_dependency_order	1:1
reconcile_runners_charmed_template_error_sets_waiting_status	charmed_template_error_degrades_to_waiting	1:1
reconcile_runners_success_refreshes_stale_app_status	successful_reconcile_refreshes_stale_app_status	1:1
build_desired_credentials_builds_credential_from_relation	github_credential_is_built_from_relation_data	1:1
build_desired_credentials_dedupes_per_app_installation	units_sharing_a_github_app_yield_one_credential	1:1
restart_proxy_vars_appear_in_pebble_layer	proxy_vars_reach_the_workload	1:1
restart_proxy_value_change_forces_layer_rewrite	proxy_value_change_rewrites_the_workload_layer	1:1
restart_clearing_proxy_removes_it_from_layer	clearing_the_proxy_removes_it_from_the_workload_layer	1:1
restart_unchanged_config_skips_replan	unchanged_config_does_not_restart_the_workload	1:1
restart_without_configurator_relation_still_prunes_orphaned_scalesets	missing_configurator_relation_prunes_orphaned_scalesets	1:1
restart_configurator_relation_present_but_unpopulated_does_not_prune	unpopulated_configurator_relation_does_not_prune	1:1
restart_missing_configurator_relation_refreshes_stale_app_status	missing_configurator_relation_refreshes_stale_app_status	1:1
get_admin_credentials_returns_content_when_secret_exists maybe_first_run_calls_first_run_when_not_initialized	first_run_initialises_garm_with_the_stored_credentials	2→1
get_admin_credentials_returns_none_when_secret_not_found reconcile_runners_skips_when_no_admin_credentials	reconcile_skipped_when_admin_credentials_are_unavailable	2→1
maybe_first_run_skips_on_missing_credential_key maybe_first_run_skips_when_credentials_unavailable	first_run_skipped_when_credentials_are_incomplete	2→1
build_desired_credentials_skips_incomplete_unit build_desired_credentials_skips_non_numeric_ids build_desired_credentials_skips_when_secret_unavailable	incomplete_github_config_yields_no_credential	3→1, parametrized
ensure_controller_urls_sets_urls_from_base_url ensure_controller_urls_strips_trailing_slash_from_base_url	controller_urls_derive_from_the_ingress_url	2→1, parametrized
restart_no_proxy_hash_matches_rendered_config	—	dropped

The merges collapse tests that a Scenario parametrize expresses as cases of one behaviour;
no assertion was lost in any of them. The one dropped test recomputed the expected hash
using the same if proxy_env: guard as the charm itself, so it could not fail —
unchanged_config_does_not_restart_the_workload covers the real property instead.

The two get_credentials_action_* rows come from main's get-credentials action (#254),
which landed after this branch opened and added its tests in the mock-self style this PR
removes. They are ported rather than dropped or kept as-is: the Scenario versions run the
action through the context and assert on ctx.action_results and on ActionFailed, so they
cover the action's real path to the secret rather than a MagicMock's return value.

New tests with no ancestor on main (6) — coverage the mock-based style could not reach:

new test	covers
workload_is_configured_from_relation_data	the config actually written to the container
workload_is_not_configured_without_postgresql_data	the real status on missing DB data
non_leader_waits_for_the_leader_to_create_secrets	non-leader startup ordering
reconcile_authenticates_against_the_local_garm_listener	login goes to GARM's fixed local port
scaleset_is_built_from_relation_data	scaleset spec parsed from the relation
controller_urls_default_to_the_kubernetes_service_url	the no-ingress default

The whole table is verified against the branch rather than by eye — with the 27 mapped names
in mapped.txt:

comm -23 mapped.txt <(grep '^def test_' charms/garm/tests/unit/test_charm.py | sed 's/^def //;s/(.*//' | sort)
# -> empty: nothing claimed-as-mapped is missing from the branch
comm -13 mapped.txt <(grep '^def test_' charms/garm/tests/unit/test_charm.py | sed 's/^def //;s/(.*//' | sort)
# -> exactly the 6 new tests above.  27 mapped + 6 new = 33 = the file's test count

Review focus

Rewriting the tests surfaced two pre-existing bugs. Neither is fixed here — this PR is test
work only — and both are being addressed in a follow-up that builds on these tests:

1. The charm's Pebble layer is permanently shadowed. restart() adds the garm-command
   layer (service app) before super().restart(). paas_charm's _app_layer() snapshots the
   plan into original-services.json on first call and re-emits those services into the charm
   layer, which is added later and wins. The app service is frozen at its first-run values, so
   proxy changes never reach the running service, and _current_config_hash() reads the frozen
   hash — meaning the unchanged-config short-circuit never fires again after the first config
   change. The old tests missed this by asserting on the dict passed to add_layer rather than
   the resulting plan.
2. A failed GARM reconcile reports Active on the config-changed path, because
   paas_charm.restart() ends with an unconditional ActiveStatus() after _reconcile_runners()
   has set Waiting.

Two tests changed meaning as a result, and are worth a look:

• test_charmed_template_error_degrades_to_waiting now exercises the steady-state
  (unchanged-config) path, the only path where the Waiting status survives (bug 2).
• The two *_workload_layer proxy tests assert on the garm-command layer the charm writes,
  matching what the previous tests checked; asserting the effective plan would fail on bug 1.
  The follow-up renames them once they can assert the plan.

Also: restart()'s "Waiting for postgresql relation" branch is unreachable — the framework's
optional: false gate blocks with missing integrations: postgresql first — so that test now
asserts the real status.

New dependencies

ops-scenario==8.8.0 added to the unit env in charms/garm/tox.toml, matching the pin
already used by garm-configurator. No runtime dependency change; no breaking changes.

Checklist

• Changes comply with the project's coding standards and guidelines (see CONTRIBUTING.md and STYLE.md)
• CONTRIBUTING.md has been updated upon changes to the contribution/development process (e.g. changes to the way tests are run)
• Technical author has been assigned to review the PR in case of documentation changes (usually *.md files)
• I updated docs/changelog.md with user-relevant changes
• I used AI to assist with preparing this PR
• I added or updated tests as needed (unit and integration)
• If integration test modules are used: I updated the workflow configuration
• If this PR involves a Grafana dashboard: I added a screenshot of the dashboard
• If this PR involves Terraform: terraform fmt passes and tflint reports no errors
• If this PR involves Rockcraft: I updated the version
• If this PR adds/removes a charm, or changes a charm's base class, conventions, tooling, or repo structure: I updated the relevant AGENTS.md
• If this PR changes .copilot-collections.yaml or .github/instructions/: I re-checked whether the AGENTS.md "12-factor divergences" guidance still matches the upstream copilot-collections guidance

[florentianayuwono]

thanks chris!