Reword the Molecule scenario comments

They were hard-wrapped at 80 characters, broke mid-parenthesis, and spent lines
restating what the code below them does.

Rewrapped at natural boundaries instead, with the narration dropped and only the
reasons, gotchas and surprises kept. Section dividers stay - they delineate long
plays rather than narrate them.

Comments only; no scenario behaviour changes.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SEH3vxYSQ5SV4N5z61eyGT

molecule-shared/homeserver-stub.py

@@ -4,21 +4,18 @@
 
 """A stand-in homeserver for Molecule scenarios.
 
-Most components in this playbook talk to a homeserver while starting up and
-exit if it is unreachable, so a scenario cannot get them running without one.
-Standing up a real Synapse for every role would dominate the run time and drag
-in Postgres, and the scenarios are not testing Synapse - they are testing that
-the role's configuration reaches the component and that it starts.
+Most components talk to a homeserver while starting up and exit if it is unreachable,
+so a scenario cannot get them running without one. A real Synapse for every role would
+dominate the run time and drag in Postgres, and the scenarios are not testing Synapse.
 
-So this answers the handful of endpoints components touch during startup, with
-the blandest plausible response in each case. It is deliberately permissive: an
-unknown path returns `{}` with a 200 rather than a 404, because the goal is to
-get the component past its startup checks, not to model the Matrix spec.
+This answers the handful of endpoints components touch during startup, with the blandest
+plausible response in each case. Deliberately permissive: an unknown path returns `{}` with
+a 200 rather than a 404, because the goal is to get the component past its startup checks.
 
-What it is NOT: an authentication check, a room state machine, or anything a
-scenario should assert *about*. Assert on what the role rendered and on what the
-component reports about itself. If a scenario starts needing this stub to behave
-like a real homeserver, that scenario has outgrown what these tests are for.
+What it is NOT: an authentication check, a room state machine, or anything a scenario should
+assert *about*. Assert on what the role rendered and what the component reports about itself.
+A scenario that needs this stub to behave like a real homeserver has outgrown what these
+tests are for.
 """
 
 import json
@@ -32,18 +29,17 @@ from urllib.parse import parse_qs, urlparse
 SERVER_NAME = os.environ.get("STUB_SERVER_NAME", "molecule.local")
 PORT = int(os.environ.get("STUB_PORT", "8008"))
 
-# Rooms reported as already joined. Components that resolve a room mapping at
-# startup (matrix-alertmanager-receiver, for one) fail if the rooms they were
-# configured with are missing, so a scenario passes its own room IDs in.
+# Rooms reported as already joined. Components that resolve a room mapping at startup
+# (matrix-alertmanager-receiver, for one) fail if the rooms they were configured with
+# are missing, so a scenario passes its own room IDs in.
 JOINED_ROOMS = [r for r in os.environ.get("STUB_JOINED_ROOMS", "").split(",") if r]
 
 USER_ID = os.environ.get("STUB_USER_ID", f"@stub:{SERVER_NAME}")
 
-# Longest a /sync call is held open. Long-polling clients (anything on
-# matrix-sdk: baibot and the other bots) ask for a 30s timeout and immediately
-# ask again when the call returns, so answering instantly would spin them into a
-# hot loop that eats the test machine. Honouring the requested timeout, capped
-# here, keeps an idle bot idle.
+# Longest a /sync call is held open. Long-polling clients ask for a 30s timeout and
+# immediately ask again when the call returns, so answering instantly spins them into a hot
+# loop that eats the test machine. Honouring the requested timeout, capped here, keeps an
+# idle bot idle.
 SYNC_MAX_HOLD_SECONDS = 30
 
 
@@ -109,12 +105,10 @@ class Handler(BaseHTTPRequestHandler):
         if path.endswith("/capabilities"):
             return {"capabilities": {}}
 
-        # Bots that authenticate with a username and password rather than as an
-        # appservice with a token log in here. Matched loosely on purpose:
-        # clients differ on the API version prefix (matrix-nio has shipped both
-        # /_matrix/client/r0/login and /_matrix/client/v3/login over time), and a
-        # login that falls through to the catch-all `{}` below looks to the
-        # client like bad credentials.
+        # Where bots authenticating with a username and password log in, rather than as an
+        # appservice with a token. Matched loosely on purpose, because clients differ on the
+        # API version prefix, and a login falling through to the catch-all `{}` below looks
+        # to the client like bad credentials.
         if path.endswith("/login"):
             return {
                 "user_id": USER_ID,
@@ -155,8 +149,8 @@ class Handler(BaseHTTPRequestHandler):
         if path.endswith("/health") or path.endswith("/_matrix/federation/v1/version"):
             return {"server": {"name": "molecule-stub", "version": "0"}}
 
-        # Anything unrecognised: an empty object, so a component doing a startup
-        # probe of an endpoint not listed here still gets past it.
+        # Anything unrecognised: an empty object, so a component probing an endpoint
+        # not listed here still gets past it.
         return {}
 
     def do_GET(self):
@@ -175,8 +169,8 @@ class Handler(BaseHTTPRequestHandler):
         self._send({})
 
     def log_message(self, fmt, *args):
-        # Quiet by default; STUB_VERBOSE=1 when a scenario will not start and you
-        # need to see what the component is actually asking for.
+        # Quiet by default. STUB_VERBOSE=1 when a scenario will not start and you need
+        # to see what the component is actually asking for.
         if os.environ.get("STUB_VERBOSE"):
             sys.stderr.write("stub: " + (fmt % args) + "\n")
 

molecule-shared/playbook-context.yml

@@ -3,21 +3,18 @@
 # SPDX-License-Identifier: AGPL-3.0-or-later
 
 ---
-# The variables a role here reads from its surroundings rather than from its own
-# defaults. In a real run `matrix-base` and `group_vars/matrix_servers` provide
-# them; in a scenario they have to come from somewhere, and including
-# `matrix-base` itself does far more than a role scenario needs.
+# The variables a role reads from its surroundings rather than from its own defaults.
+# In a real run `matrix-base` and `group_vars/matrix_servers` provide them.
 #
 # Include from a scenario's prepare.yml, converge.yml and verify.yml:
 #
 #   vars_files:
 #     - "{{ lookup('env', 'MOLECULE_PROJECT_DIRECTORY') }}/../../../molecule-shared/playbook-context.yml"
 #
-# A scenario can override any of these in its own group_vars - that is the point
-# of testing a role with values it would not have chosen for itself.
+# Gotcha: `vars_files` outranks inventory `group_vars`, so a scenario cannot override these there.
 #
-# Keep this to variables that come from OUTSIDE the role under test. Anything the
-# role defines belongs in the scenario, not here.
+# Keep this to variables that come from OUTSIDE the role under test.
+# Anything the role defines itself belongs in the scenario.
 
 # --- Identity and paths (matrix-base) --------------------------------------
 
@@ -27,32 +24,26 @@ matrix_domain: molecule.local
 matrix_user_name: matrix
 matrix_group_name: matrix
 
-# Deliberately not 1000: the base images already have a user there, so a distinct
-# id is what proves a role used the one it was given rather than coinciding with
-# the image's own.
+# Deliberately not 1000: the base images already have a user there, so a distinct id
+# is what proves a role used the one it was given rather than coinciding with the image's own.
 matrix_user_uid: 1234
 matrix_user_gid: 1234
 
-# Empty in the playbook's own defaults too. Components that would invite an
-# administrator into a room skip doing so when it is empty, which is what a
-# scenario wants.
+# Empty in the playbook's own defaults too. Components that would invite an administrator
+# into a room skip doing so when it is empty.
 matrix_admin: ''
 
 # --- Host commands (matrix-base) -------------------------------------------
-#
-# Some roles shell out to a host binary through this indirection instead of
-# naming it directly (matrix-bridge-hookshot and matrix-bridge-appservice-irc
-# both generate a key with it). matrix-base's defaults are what supplies the
-# value in a real run; those roles install the binary themselves, by including
-# matrix-base's `ensure_openssl_installed` tasks.
 
+# Some roles shell out through this indirection instead of naming the binary directly
+# (matrix-bridge-hookshot and matrix-bridge-appservice-irc both generate a key with it).
+# They install it themselves by including matrix-base's `ensure_openssl_installed` tasks.
 matrix_host_command_openssl: "/usr/bin/env openssl"
 
 # --- Bridge-wide switches (matrix-base) ------------------------------------
-#
-# Every bridge role reads these, so they live here rather than in each bridge's
-# scenario. The values match the playbook's own defaults: encryption off, no
-# relay, no MSC4190. A bridge scenario that wants to prove one of these reaches
+
+# Every bridge role reads these, so they live here rather than in each bridge's scenario.
+# The values match the playbook's own defaults. A scenario proving one of these reaches
 # the rendered configuration should override it in its own group_vars.
 
 matrix_bridges_encryption_enabled: false
@@ -66,10 +57,9 @@ matrix_bridges_exposure_hostname: molecule.local
 matrix_bridges_exposure_path_prefix: /bridges
 
 # --- Public hostnames (matrix-base) ----------------------------------------
-#
-# 18 of the roles here read one of these. Rendered against the scenario's
-# matrix_domain rather than left as Jinja, so a scenario can read them in
-# verify.yml without the role's defaults being in scope.
+
+# 18 of the roles here read one of these. Rendered against the scenario's matrix_domain
+# rather than left as Jinja, so verify.yml can read them without the role's defaults in scope.
 
 matrix_server_fqn_matrix: matrix.molecule.local
 matrix_server_fqn_matrix_federation: matrix.molecule.local

molecule-shared/requirements.yml

@@ -1,7 +1,5 @@
 ---
-# Shared by every role scenario under roles/custom/*/molecule/, referenced from
-# each scenario's molecule.yml. Kept in one place so the pins cannot drift
-# apart across roles.
+# Shared by every role scenario, so the pins cannot drift apart across roles.
 roles:
   - name: ansible-role-docker
     src: https://github.com/geerlingguy/ansible-role-docker

molecule-shared/tasks/homeserver-stub.yml

@@ -3,8 +3,8 @@
 # SPDX-License-Identifier: AGPL-3.0-or-later
 
 ---
-# Stands up a stand-in homeserver on a container network, for scenarios whose
-# component contacts a homeserver while starting up.
+# Stands up a stand-in homeserver on a container network, for scenarios whose component
+# contacts a homeserver while starting up.
 #
 # Include from a scenario's prepare.yml:
 #
@@ -17,8 +17,7 @@
 #
 # The component should then be pointed at http://matrix.molecule.local:8008.
 #
-# See molecule-shared/homeserver-stub.py for what it answers and, more
-# importantly, for what it is not.
+# See molecule-shared/homeserver-stub.py for what it answers and, more importantly, what it is not.
 
 - name: Ensure the homeserver stub script is present
   ansible.builtin.copy:
@@ -37,8 +36,8 @@
   changed_when: molecule_shared_stub_removal.rc == 0
   failed_when: false
 
-# The alias is what the component resolves, so its configuration can name a
-# hostname rather than a container name.
+# The alias is what the component resolves, so its configuration can name a hostname
+# rather than a container name.
 - name: Ensure the homeserver stub is running
   ansible.builtin.command:
     argv:
@@ -50,15 +49,13 @@
       - --network-alias={{ molecule_shared_stub_hostname | default('matrix.molecule.local') }}
       - --env=STUB_SERVER_NAME={{ molecule_shared_stub_server_name | default('molecule.local') }}
       - --env=STUB_JOINED_ROOMS={{ (molecule_shared_stub_joined_rooms | default([])) | join(',') }}
-      # Appservices call /whoami on startup and refuse to run if the id
-      # returned is not the bot user they were configured as, so a scenario
-      # bridging anything has to tell the stub who it should claim to be.
+      # Appservices call /whoami on startup and refuse to run if the id returned is not
+      # the bot user they were configured as, so a scenario bridging anything has to tell
+      # the stub who it should claim to be.
       - --env=STUB_USER_ID={{ molecule_shared_stub_user_id | default('@stub:' + (molecule_shared_stub_server_name | default('molecule.local'))) }}
-      # Off by default. Set molecule_shared_stub_verbose to "1" to have the stub
-      # log every request it is asked for, which is both how you find out why a
-      # component will not start and - for a component that exposes no port of
-      # its own - the only place a scenario can observe it acting on what the
-      # role configured.
+      # Set molecule_shared_stub_verbose to "1" to log every request the stub is asked for.
+      # How you find out why a component will not start, and for a component with no port of
+      # its own, the only place to observe it acting on what the role configured.
       - --env=STUB_VERBOSE={{ molecule_shared_stub_verbose | default('') }}
       - --volume=/root/molecule-homeserver-stub.py:/stub.py:ro
       - "{{ molecule_shared_image_python }}"

molecule-shared/vars.yml

@@ -1,13 +1,11 @@
 ---
-# Helper container images the scenarios use for probing. They live here rather
-# than inline in each verify.yml so that there is one pin per image instead of
-# one per role, and so Renovate can see them (see the customManager in
-# .github/renovate.json).
+# Helper container images the scenarios use for probing. Here rather than inline in each
+# verify.yml, so there is one pin per image instead of one per role, and so Renovate can see
+# them. See the customManager in .github/renovate.json.
 
-# Used to reach a role's container over its own container network. A helper is
-# needed because the role publishes no host port - exactly as in a real
-# deployment - and publishing one for the test would collide between scenarios
-# running in parallel.
+# Used to reach a role's container over its own container network, because the role publishes
+# no host port - exactly as in a real deployment. Publishing one for the test would collide
+# between scenarios running in parallel.
 # renovate: datasource=docker depName=docker.io/curlimages/curl
 molecule_shared_image_curl: "docker.io/curlimages/curl:8.11.1"