chore(tests): delete narration comments from twelve lookup suites

74 comments restated the assertion below them, narrated a step the test
name already carries, banner-separated sections, or cross-referenced
another file. All deleted; the only touched code lines are ones that
carried a trailing comment, so the test count is unchanged.

Six of the 74 were invisible to the repo's own comment lint.
find_invalid_comments() exempts any comment block whose next code line is
a def/class (block_def, test_comments_valid.py:385), so section banners
sitting above a test method pass, and real docstrings are never inspected
at all. That let five `# --- section ---` banners through, plus a class
docstring in test_rbac_group_path.py that cited commit 10cb02461 as the
reason the guard exists -- provenance, which belongs in a commit message
and not in the source. A green comment lint is therefore not evidence
that a file complies.

Module docstrings that state what a file covers are kept: describing
behaviour is what the header slot is for.

Verified: checker reports 0 offenders across all twelve; pytest over the
same twelve => 180 passed, 4 subtests.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

tests/unit/library/test_database_query.py

@@ -91,7 +91,6 @@ class BuildCmdPostgresTests(unittest.TestCase):
         self.assertEqual(cmd[cmd.index("-U") + 1], "peertube")
         self.assertEqual(cmd[cmd.index("-d") + 1], "peertube")
         self.assertIn("ON_ERROR_STOP=1", cmd)
-        # Password must NEVER appear in argv (only via env passthrough).
         self.assertNotIn("s3cret", cmd)
 
     def test_postgres_expect_rows_adds_tuples_only_flag(self):
@@ -142,7 +141,6 @@ class ParseRowsTests(unittest.TestCase):
         self.assertEqual(_DB_MOD._parse_rows("", db_type="postgres"), [])
 
     def test_blank_line_is_skipped_no_spurious_row(self):
-        # `SELECT to_regclass('public.nope')` returns NULL → blank line under -tA.
         self.assertEqual(_DB_MOD._parse_rows("\n", db_type="postgres"), [])
 
 
@@ -163,7 +161,6 @@ class IsReadOnlyTests(unittest.TestCase):
         self.assertFalse(_DB_MOD._is_read_only("UPDATE u SET x = 1;"))
 
     def test_with_cte_is_treated_as_read_only(self):
-        # WITH is a CTE prefix — most commonly used in SELECT chains; safe default.
         self.assertTrue(_DB_MOD._is_read_only("WITH x AS (SELECT 1) SELECT * FROM x;"))
 
 
@@ -205,7 +202,6 @@ class EscapeValueTests(unittest.TestCase):
         self.assertEqual(_DB_MOD._escape_value("alice", "postgres"), "'alice'")
 
     def test_postgres_string_doubles_inner_single_quotes(self):
-        # canonical postgres-style escape; no E-prefix, standard_conforming_strings on.
         self.assertEqual(_DB_MOD._escape_value("O'Reilly", "postgres"), "'O''Reilly'")
 
     def test_mariadb_bool_becomes_one_or_zero(self):
@@ -255,12 +251,9 @@ class SubstituteNamedArgsTests(unittest.TestCase):
     def test_quote_inside_value_does_not_break_out_of_string(self):
         sql = "SELECT %(s)s;"
         out = _DB_MOD._substitute_named_args(sql, {"s": "a'b"}, "postgres")
-        # Inner ' must be doubled, breaking the trivial SQL-injection vector.
         self.assertEqual(out, "SELECT 'a''b';")
 
     def test_byte_sequence_not_a_placeholder_passes_through(self):
-        # `%(stuff inside string literal` is not a valid identifier-style
-        # placeholder — the regex requires a Python identifier in parens.
         sql = "SELECT 'a %( percent thing' AS s;"
         self.assertEqual(_DB_MOD._substitute_named_args(sql, {}, "postgres"), sql)
 

tests/unit/plugins/lookup/test_active_alertmanager_channels.py

@@ -12,8 +12,6 @@ def _make_applications(*app_ids: str, channels: tuple = ()) -> dict:
     apps listed in *channels* get services.prometheus.communication.channel: true;
     others do not.
     """
-    # Per the materialised payload moved from
-    # `applications.<app>.compose.services.<X>` to `applications.<app>.services.<X>`.
     return {
         app_id: (
             {"services": {"prometheus": {"communication": {"channel": True}}}}
@@ -71,7 +69,7 @@ class TestActiveAlertmanagerChannelsSelfDeclaration(unittest.TestCase):
     """services.prometheus.communication.channel flag gate — must be true in app config."""
 
     def test_excludes_app_without_channel_flag(self):
-        apps = _make_applications("web-app-mattermost")  # no channel flag
+        apps = _make_applications("web-app-mattermost")
         result = _run(apps, ["web-app-mattermost"])
         self.assertNotIn("web-app-mattermost", result)
 

tests/unit/plugins/lookup/test_cert.py

@@ -5,7 +5,6 @@ from unittest.mock import patch
 
 from ansible.errors import AnsibleError
 
-# Make "ansible.module_utils.tls_common" importable during plain unit tests.
 import utils.tls_common as _tls_common
 from plugins.lookup.cert import LookupModule
 from utils.cache import _reset_cache_for_tests
@@ -36,8 +35,6 @@ class TestCertPlanLookup(unittest.TestCase):
             "web-app-off": {"server": {"tls": {"enabled": False}}},
         }
 
-        # IMPORTANT (new strict behavior):
-        # For TLS_SELFSIGNED_SCOPE=global, CURRENT_PLAY_DOMAINS_ALL MUST be list[str].
         self.vars = {
             "domains": self.domains,
             "applications": self.applications,
@@ -112,8 +109,6 @@ class TestCertPlanLookup(unittest.TestCase):
             out["files"]["key"], "/etc/infinito.nexus/selfsigned/global/privkey.pem"
         )
 
-        # New strict behavior:
-        # SANs come from CURRENT_PLAY_DOMAINS_ALL (list[str]), plus primary_domain first.
         self.assertEqual(
             out["domains"]["san"],
             ["b.example", "a.example", "b-alt.example", "c.example", "api.c.example"],
@@ -126,22 +121,22 @@ class TestCertPlanLookup(unittest.TestCase):
             self.lookup.run(["web-app-b"], variables=v, mode="app")
 
         v2 = dict(self.vars)
-        v2["CURRENT_PLAY_DOMAINS_ALL"] = {"a.example": True}  # wrong type
+        v2["CURRENT_PLAY_DOMAINS_ALL"] = {"a.example": True}
         with self.assertRaises(AnsibleError):
             self.lookup.run(["web-app-b"], variables=v2, mode="app")
 
         v3 = dict(self.vars)
-        v3["CURRENT_PLAY_DOMAINS_ALL"] = []  # empty list not allowed
+        v3["CURRENT_PLAY_DOMAINS_ALL"] = []
         with self.assertRaises(AnsibleError):
             self.lookup.run(["web-app-b"], variables=v3, mode="app")
 
         v4 = dict(self.vars)
-        v4["CURRENT_PLAY_DOMAINS_ALL"] = ["a.example", ""]  # empty string not allowed
+        v4["CURRENT_PLAY_DOMAINS_ALL"] = ["a.example", ""]
         with self.assertRaises(AnsibleError):
             self.lookup.run(["web-app-b"], variables=v4, mode="app")
 
         v5 = dict(self.vars)
-        v5["CURRENT_PLAY_DOMAINS_ALL"] = ["a.example", 123]  # non-string not allowed
+        v5["CURRENT_PLAY_DOMAINS_ALL"] = ["a.example", 123]
         with self.assertRaises(AnsibleError):
             self.lookup.run(["web-app-b"], variables=v5, mode="app")
 
@@ -211,10 +206,6 @@ class TestCertPlanLookup(unittest.TestCase):
         with self.assertRaises(AnsibleError):
             self.lookup.run(["web-app-a"], variables=v, mode="app")
 
-    # -------------------------------------------------------------------------
-    # New tests for "Jinja-in-vars expansion inside lookup" logic
-    # -------------------------------------------------------------------------
-
     def test_selfsigned_base_path_expands_jinja_from_hostvars(self):
         """
         TLS_SELFSIGNED_BASE_PATH may contain Jinja markers and must be expanded
@@ -223,7 +214,6 @@ class TestCertPlanLookup(unittest.TestCase):
         v = dict(self.vars)
         v["TLS_SELFSIGNED_BASE_PATH"] = "/etc/{{ SOFTWARE_DOMAIN }}/selfsigned"
 
-        # Simulate real ansible lookup context
         v["inventory_hostname"] = "localhost"
         v["hostvars"] = {
             "localhost": {
@@ -273,7 +263,6 @@ class TestCertPlanLookup(unittest.TestCase):
         v = dict(self.vars)
         v["TLS_SELFSIGNED_BASE_PATH"] = "/etc/{{ SOFTWARE_DOMAIN }}/selfsigned"
 
-        # No SOFTWARE_DOMAIN present in either variables or hostvars context
         v["inventory_hostname"] = "localhost"
         v["hostvars"] = {"localhost": {}}
 
@@ -288,10 +277,8 @@ class TestCertPlanLookup(unittest.TestCase):
         v = dict(self.vars)
         v["TLS_SELFSIGNED_BASE_PATH"] = "/etc/{{ SOFTWARE_DOMAIN }}/selfsigned"
 
-        # variables has one value
         v["SOFTWARE_DOMAIN"] = "Wrong.Name"
 
-        # hostvars has another, should win
         v["inventory_hostname"] = "localhost"
         v["hostvars"] = {
             "localhost": {
@@ -311,7 +298,6 @@ class TestCertPlanLookup(unittest.TestCase):
         after rendering by injecting literal braces.
         """
         v = dict(self.vars)
-        # This will render to a string that still contains "{{ STILL_JINJA }}"
         v["TLS_SELFSIGNED_BASE_PATH"] = "/etc/{{ '{{ STILL_JINJA }}' }}/selfsigned"
         v["inventory_hostname"] = "localhost"
         v["hostvars"] = {"localhost": {"SOFTWARE_DOMAIN": "infinito.nexus"}}

tests/unit/plugins/lookup/test_container_address.py

@@ -123,12 +123,6 @@ class TestContainerExecAddrLookup(unittest.TestCase):
         )
 
     def test_swarm_mode_uses_compose_yaml_service_key(self):
-        # The resolver script composes the docker-swarm service name as
-        # `<stack>_<service_key>` because the compose-yaml service key
-        # is what `docker stack deploy` names the service after.
-        # `services.<key>.name` is the compose-side `container_name`
-        # override and is ignored by swarm, so the lookup MUST pass
-        # the service_key (not the bare name) to the resolver.
         apps = _apps(service_name="custom-mm-name")
         out = _run(
             "web-app-mattermost",

tests/unit/plugins/lookup/test_container_service.py

@@ -89,11 +89,6 @@ class TestContainerServiceLookup(unittest.TestCase):
         self.assertEqual(out, ["mattermost_mattermost"])
 
     def test_swarm_mode_ignores_services_name_field(self):
-        # docker stack deploy names services <stack>_<compose-key>. The
-        # compose `name:` field maps to `container_name` (compose-only)
-        # and is silently ignored by swarm, so the lookup must derive the
-        # swarm-addressable name from the service KEY, not from
-        # `services.<key>.name`.
         apps = _apps(service_name="custom-mm-name")
         out = _run(
             "web-app-mattermost",

tests/unit/plugins/lookup/test_docker_cards.py

@@ -8,7 +8,6 @@ from pathlib import Path
 from ansible.errors import AnsibleError
 from jinja2 import Environment, StrictUndefined, select_autoescape
 
-# Adjust the PYTHONPATH to include the lookup_plugins folder from the web-app-dashboard role.
 sys.path.insert(
     0,
     str(
@@ -65,11 +64,9 @@ class DummyTemplar:
         if value is None:
             return value
 
-        # Keep non-strings untouched
         if not isinstance(value, str):
             return value
 
-        # Only render if it looks like a Jinja template
         if "{{" in value and "}}" in value:
             tmpl = self._env.from_string(value)
             return tmpl.render(**self._vars)
@@ -89,7 +86,6 @@ class DummyTlsResolveLookup:
     def run(self, terms, variables=None, **kwargs):
         variables = variables or {}
 
-        # NEW API: want-path is positional (2nd term)
         if not terms or len(terms) not in (1, 2):
             raise AnsibleError(
                 "dummy tls: terms must be [app_id] or [app_id, want_path]"
@@ -107,7 +103,6 @@ class DummyTlsResolveLookup:
         if app_id not in domains:
             raise AnsibleError(f"dummy tls: app_id '{app_id}' not in domains")
 
-        # normalize domain mapping value (string / dict / list)
         domain_val = domains[app_id]
         if isinstance(domain_val, list):
             domain = domain_val[0] if domain_val else ""
@@ -132,10 +127,8 @@ class DummyTlsResolveLookup:
 
 class TestDockerCardsLookup(unittest.TestCase):
     def setUp(self):
-        # Create a temporary directory to simulate the roles directory.
         self.test_roles_dir = tempfile.mkdtemp(prefix="test_roles_")
 
-        # Create a sample role "web-app-dashboard" under that directory.
         self.role_name = "web-app-dashboard"
         self.role_dir = str(Path(self.test_roles_dir) / self.role_name)
         Path(str(Path(self.role_dir) / "meta")).mkdir(parents=True)
@@ -145,14 +138,10 @@ class TestDockerCardsLookup(unittest.TestCase):
         with Path(vars_main).open("w", encoding="utf-8") as f:
             f.write("application_id: portfolio\n")
 
-        # Create a sample README.md with a H1 line for the title.
         readme_path = str(Path(self.role_dir) / "README.md")
         with Path(readme_path).open("w", encoding="utf-8") as f:
             f.write("# Portfolio Application\nThis is a sample portfolio role.")
 
-        # Create a sample meta/main.yml in the meta folder. Per
-        # the icon class lives in meta/info.yml (file-root convention),
-        # not under galaxy_info.
         meta_main_path = str(Path(self.role_dir) / ROLE_FILE_META_MAIN)
         meta_yaml = """
 galaxy_info:
@@ -190,18 +179,14 @@ logo:
         docker_cards_module.lookup_loader.get = _patched_get
 
     def tearDown(self):
-        # Restore patched lookup_loader
         docker_cards_module.lookup_loader.get = self._orig_lookup_get
 
-        # Remove the temporary roles directory after the test.
         shutil.rmtree(self.test_roles_dir)
 
     def _base_fake_variables(self):
-        # include TLS vars (even if our stub doesn't need all of them)
         return {
             "domains": {"portfolio": "myportfolio.com"},
             "applications": {
-                # Per the materialised payload moved to services.<X>.
                 "portfolio": {"services": {"dashboard": {"enabled": True}}}
             },
             "group_names": ["portfolio"],
@@ -212,7 +197,6 @@ logo:
         }
 
     def _run_lookup(self, lookup_module, fake_variables):
-        # Provide deterministic templating behavior for unit tests.
         lookup_module._templar = DummyTemplar(fake_variables)
         lookup_module._loader = mock.MagicMock()
         return lookup_module.run([self.test_roles_dir], variables=fake_variables)
@@ -281,7 +265,7 @@ logo:
     def test_lookup_when_group_excludes_application_id(self):
         lookup_module = LookupModule()
         fake_variables = self._base_fake_variables()
-        fake_variables["group_names"] = []  # Not including "portfolio"
+        fake_variables["group_names"] = []
 
         result = self._run_lookup(lookup_module, fake_variables)
 

tests/unit/plugins/lookup/test_nginx.py

@@ -20,7 +20,6 @@ class _FakeTlsResolveLookup:
         if len(terms) != 2 or terms[1] != "protocols.web":
             raise AssertionError(f"Unexpected terms passed to tls: {terms}")
 
-        # Legacy kwarg want must be ignored; if it appears, fail (we don't expect it anymore)
         if kwargs.get("want"):
             raise AssertionError(f"Unexpected want kwarg passed to tls: {kwargs}")
 
@@ -117,7 +116,6 @@ class TestNginxPathsLookup(unittest.TestCase):
         self.assertIsInstance(ensure_paths, list)
         self.assertEqual(ensure_paths, [d["path"] for d in ensure])
 
-        # well_known is container path → must NOT be part of host dir creation
         self.assertNotIn("/usr/share/nginx/well-known/", ensure_paths)
 
     def test_canonical_dict_supplies_volume_paths(self):

tests/unit/plugins/lookup/test_prometheus_integration_active.py

@@ -84,7 +84,6 @@ class TestPrometheusIntegrationActivePrometheusVhost(unittest.TestCase):
         self.assertTrue(result)
 
     def test_true_for_prometheus_vhost_even_without_service_dep(self):
-        # web-app-prometheus doesn't need its own service dep — it IS the host.
         apps = {"web-app-prometheus": {}}
         result = _run(apps, "web-app-prometheus", ["web-app-prometheus"])
         self.assertTrue(result)

tests/unit/plugins/lookup/test_rbac_group_path.py

@@ -59,7 +59,6 @@ class TestRbacGroupPathLookup(unittest.TestCase):
             }
         }
 
-    # --- non-tenant apps -----------------------------------------------------
 
     def test_non_tenant_app_declared_role(self):
         result = self.lookup.run(
@@ -80,9 +79,6 @@ class TestRbacGroupPathLookup(unittest.TestCase):
         self.assertEqual(result, ["roles/web-app-gitea/editor"])
 
     def test_non_tenant_app_implicit_administrator_always_valid(self):
-        # Even if an app does not explicitly declare administrator in its
-        # rbac.roles, the auto-add rule provides it; the lookup MUST accept
-        # that without failing.
         apps = {
             "web-app-gitea": {
                 "rbac": {
@@ -111,7 +107,6 @@ class TestRbacGroupPathLookup(unittest.TestCase):
             )
         self.assertIn("global", str(cm.exception))
 
-    # --- tenant-aware apps ---------------------------------------------------
 
     def test_tenant_aware_per_tenant_role_requires_tenant(self):
         with self.assertRaises(AnsibleError) as cm:
@@ -163,7 +158,6 @@ class TestRbacGroupPathLookup(unittest.TestCase):
         )
         self.assertEqual(result, ["roles/web-app-wordpress/blog.example/editor"])
 
-    # --- failure modes -------------------------------------------------------
 
     def test_unknown_role_fails(self):
         with self.assertRaises(AnsibleError) as cm:
@@ -213,9 +207,6 @@ class TestRbacGroupPathLookup(unittest.TestCase):
 
 
 class TestRbacGroupPathReadsMergedApplications(unittest.TestCase):
-    """Regression guard: meta/rbac.yml roles must remain visible after
-    10cb02461 switched payload.py to get_variant_overrides_only."""
-
     def test_falls_back_to_merged_when_variables_lack_rbac(self):
         lookup = LookupModule()
         lookup._loader = mock.MagicMock()

tests/unit/plugins/lookup/test_sso.py

@@ -19,8 +19,6 @@ from . import PROJECT_ROOT
 
 
 def _load_module(rel_path: str, name: str):
-    # Mirror test_sso_oidc_plugin.py's defensive sys.modules eviction so
-    # sibling tests cannot leak a stubbed config module into this load.
     for key in (
         "utils.roles.applications.config",
         "utils.roles.applications",
@@ -88,8 +86,6 @@ class SsoLookupTests(unittest.TestCase):
             )
             return lk.run(terms, variables={"applications": applications})
 
-    # --- term parsing ----------------------------------------------------
-
     def test_zero_terms_raises(self):
         with self.assertRaises(AnsibleError):
             self._run(_apps(), [])
@@ -107,10 +103,7 @@ class SsoLookupTests(unittest.TestCase):
             self._run(_apps(enabled=True), ["web-app-x", "bogus_key"])
         self.assertIn("unknown want_path", str(ctx.exception))
 
-    # --- want-path resolution -------------------------------------------
-
     def test_default_returns_full_dict(self):
-        # No want-path → entire resolver dict.
         result = self._run(_apps(enabled=True, flavor="oauth2"), ["web-app-x"])
         self.assertEqual(len(result), 1)
         payload = result[0]
@@ -182,12 +175,10 @@ class SsoLookupTests(unittest.TestCase):
         self.assertEqual(result, [groups])
 
     def test_missing_app_returns_defaults(self):
-        # No app entry → resolver gives defaults; want='is_enabled' → False.
         result = self._run({}, ["web-app-x", "is_enabled"])
         self.assertEqual(result, [False])
 
     def test_empty_want_path_treated_as_all(self):
-        # Whitespace-only want collapses to 'all' per the docstring.
         result = self._run(_apps(enabled=True), ["web-app-x", "  "])
         self.assertEqual(len(result), 1)
         self.assertIsInstance(result[0], dict)

tests/unit/plugins/lookup/test_sso_proxy_consumers.py

@@ -72,13 +72,11 @@ class SsoProxyConsumersLookupTests(unittest.TestCase):
         lk._loader = mock.MagicMock()
         return lk.run(terms or [], variables={"applications": applications})
 
-    # --- term-shape contract --------------------------------------------
 
     def test_positional_terms_raise(self):
         with self.assertRaises(AnsibleError):
             self._run({}, ["extra-arg"])
 
-    # --- enumeration ----------------------------------------------------
 
     def test_empty_applications_returns_empty_list(self):
         result = self._run({})
@@ -103,7 +101,7 @@ class SsoProxyConsumersLookupTests(unittest.TestCase):
             "web-app-b": _role(enabled=True, flavor="oidc"),
             "web-app-c": _role(enabled=False, flavor="oauth2"),
             "web-app-d": _role(enabled=True, flavor="oauth2"),
-            "web-app-e": {"services": {}},  # no sso block at all
+            "web-app-e": {"services": {}},
         }
         result = self._run(apps)
         self.assertEqual(result, [["web-app-a", "web-app-d"]])
@@ -118,9 +116,6 @@ class SsoProxyConsumersLookupTests(unittest.TestCase):
         self.assertEqual(result, [["web-app-alpha", "web-app-mu", "web-app-zeta"]])
 
     def test_jinja_enabled_string_is_treated_as_potentially_truthy(self):
-        # An unrendered Jinja string (the dynamic-flag form) is the merged
-        # payload's shape for SSO-gated roles; the resolver renders it to
-        # a bool via Python's bool(...) → True (non-empty string truthy).
         apps = {
             "web-app-a": {
                 "services": {

tests/unit/plugins/lookup/test_tls.py

@@ -5,7 +5,6 @@ from unittest.mock import patch
 
 from ansible.errors import AnsibleError
 
-# Make "ansible.module_utils.tls_common" importable during plain unit tests.
 import utils.tls_common as _tls_common
 from plugins.lookup.tls import LookupModule