DEVX-177: fix: fail-open molecule selection and honest fast-path contract

tests/unit/test_fast_molecule.py

@@ -40,9 +40,10 @@ class TestBuildMoleculeCommands:
 
         commands = build_molecule_commands({"role_a", "role_b"}, str(roles_dir))
         assert len(commands) == 2
-        assert all("molecule test -s default" in c for c in commands)
-        assert all("--destroy=never" in c for c in commands)
-        assert all("ubuntu-2604" in c for c in commands)
+        # REQ-3: emitted commands are exactly what run_molecule_scenario
+        # executes — full `molecule test -s <scenario>`, no synthetic flags.
+        assert commands == ["molecule test -s default", "molecule test -s default"]
+        assert all("--destroy" not in c and "--platform-name" not in c for c in commands)
 
     def test_empty_when_no_scenarios(self, tmp_path: Path) -> None:
         commands = build_molecule_commands({"nonexistent"}, str(tmp_path / "ansible" / "roles"))

tests/unit/test_molecule_changed.py

@@ -1,107 +1,186 @@
 """Unit tests for devx.molecule.molecule_changed.
 
-Verifies that the script correctly detects changed roles and maps
-them to make targets.
+Verifies that the script correctly detects changed roles, fails open on
+unmapped ansible paths, and only emits roles that exist on disk.
 """
 
 from __future__ import annotations
 
+from pathlib import Path
 from unittest.mock import patch
 
+import pytest
 from click.testing import CliRunner
 
 from devx.molecule.molecule_changed import (
     detect_changed_roles,
     get_changed_files,
     main,
+    role_to_target,
     roles_to_targets,
 )
 
+TESTABLE_ROLES = ("docker_base", "app_container", "restore", "observability")
 
-def test_detect_role_change():
+
+@pytest.fixture()
+def roles_dir(tmp_path: Path) -> Path:
+    """Fake roles dir: 4 testable roles (molecule/ present) + 1 untestable."""
+    for role in TESTABLE_ROLES:
+        (tmp_path / role / "molecule" / "default").mkdir(parents=True)
+    (tmp_path / "untested_role").mkdir()  # exists but no molecule/ dir
+    return tmp_path
+
+
+def test_detect_role_change(roles_dir: Path):
     """A file in ansible/roles/<role>/ maps to that role."""
-    files = ["ansible/roles/docker_base/tasks/main.yml"]
-    roles = detect_changed_roles(files)
-    assert "docker_base" in roles
+    roles = detect_changed_roles(["ansible/roles/docker_base/tasks/main.yml"], roles_dir)
+    assert roles == {"docker_base"}
 
 
-def test_detect_playbook_change():
-    """A playbook change maps to its included roles."""
+def test_detect_role_not_on_disk_is_dropped(roles_dir: Path):
+    """Changed role absent from roles_dir selects nothing (REQ-2)."""
+    roles = detect_changed_roles(["ansible/roles/sso_config/tasks/main.yml"], roles_dir)
+    assert roles == set()
+
+
+def test_detect_playbook_change(roles_dir: Path):
+    """A mapped playbook maps to its included roles, filtered to disk."""
     files = ["ansible/playbooks/deploy-observability.yml"]
-    roles = detect_changed_roles(files)
-    assert "observability" in roles
-    assert "docker_base" in roles
-    assert "zitadel" in roles
+    roles = detect_changed_roles(files, roles_dir)
+    # zitadel + crowdsec are mapped but absent from the fake roles dir.
+    assert roles == {"observability", "docker_base"}
 
 
-def test_detect_shared_infra_triggers_all():
-    """ansible.cfg change triggers all roles."""
-    files = ["ansible/ansible.cfg"]
-    roles = detect_changed_roles(files)
-    assert len(roles) == 10  # all roles
+def test_detect_shared_infra_triggers_all(roles_dir: Path):
+    """ansible.cfg change triggers all testable roles only."""
+    roles = detect_changed_roles(["ansible/ansible.cfg"], roles_dir)
+    assert roles == set(TESTABLE_ROLES)
 
 
-def test_detect_no_ansible_changes():
+def test_detect_molecule_shared_path(roles_dir: Path):
+    """ansible/molecule/ change triggers all testable roles."""
+    roles = detect_changed_roles(["ansible/molecule/Dockerfile"], roles_dir)
+    assert roles == set(TESTABLE_ROLES)
+
+
+def test_detect_requirements_yml_triggers_all(roles_dir: Path):
+    """ansible/requirements.yml change triggers all testable roles."""
+    roles = detect_changed_roles(["ansible/requirements.yml"], roles_dir)
+    assert roles == set(TESTABLE_ROLES)
+
+
+def test_detect_group_vars_triggers_all(roles_dir: Path):
+    """ansible/group_vars/ change triggers all testable roles (REQ-1)."""
+    roles = detect_changed_roles(["ansible/group_vars/all/images.yml"], roles_dir)
+    assert roles == set(TESTABLE_ROLES)
+
+
+def test_unmapped_playbook_fails_open(roles_dir: Path):
+    """An unmapped playbook selects all testable roles (REQ-1)."""
+    roles = detect_changed_roles(["ansible/playbooks/new-deploy.yml"], roles_dir)
+    assert roles == set(TESTABLE_ROLES)
+
+
+def test_playbook_tasks_dir_fails_open(roles_dir: Path):
+    """Shared playbook task files select all testable roles (REQ-1)."""
+    roles = detect_changed_roles(["ansible/playbooks/_tasks/upgrade-postgres-database.yml"], roles_dir)
+    assert roles == set(TESTABLE_ROLES)
+
+
+def test_mapped_playbooks(roles_dir: Path):
+    """Each newly mapped playbook selects its roles (REQ-1)."""
+    expectations = {
+        "ansible/playbooks/restore.yml": {"restore"},
+        "ansible/playbooks/upgrade-postgres.yml": {"app_container"},
+        "ansible/playbooks/rolling-update-gitea.yml": {"app_container"},
+        "ansible/playbooks/update-alertmanager.yml": {"observability"},
+        "ansible/playbooks/build-image.yml": {"docker_base"},
+        "ansible/playbooks/deploy-sso-bridge.yml": set(),
+    }
+    for playbook, expected in expectations.items():
+        assert detect_changed_roles([playbook], roles_dir) == expected, playbook
+
+
+def test_detect_prepare_vms_playbook(roles_dir: Path):
+    """prepare-vms.yml maps to all base roles present on disk."""
+    roles = detect_changed_roles(["ansible/playbooks/prepare-vms.yml"], roles_dir)
+    assert roles == {"docker_base"}
+
+
+def test_detect_deploy_customer_playbook(roles_dir: Path):
+    """deploy-customer.yml maps to its roles present on disk."""
+    roles = detect_changed_roles(["ansible/playbooks/deploy-customer.yml"], roles_dir)
+    assert roles == {"app_container", "docker_base"}
+
+
+def test_detect_no_ansible_changes(roles_dir: Path):
     """Non-Ansible files don't trigger any roles."""
-    files = ["scripts/molecule_changed.py", "Makefile"]
-    roles = detect_changed_roles(files)
-    assert len(roles) == 0
+    roles = detect_changed_roles(["scripts/x.py", "Makefile"], roles_dir)
+    assert roles == set()
+
+
+def test_detect_environments_not_molecule_covered(roles_dir: Path):
+    """ansible/environments/ data is not molecule-covered (documented)."""
+    roles = detect_changed_roles(["ansible/environments/staging/customers.yml"], roles_dir)
+    assert roles == set()
+
+
+def test_detect_missing_roles_dir():
+    """A nonexistent roles_dir yields no roles (honest: nothing testable)."""
+    roles = detect_changed_roles(["ansible/roles/docker_base/tasks/main.yml"], "/nonexistent")
+    assert roles == set()
+
+
+def test_role_to_target():
+    """Role names map to conventional make targets."""
+    assert role_to_target("docker_base") == "molecule-docker-base"
+    assert role_to_target("app_hardening") == "molecule-app-hardening"
 
 
 def test_roles_to_targets():
     """Role names map to make targets."""
     targets = roles_to_targets({"docker_base", "zitadel"})
-    assert "molecule-docker-base" in targets
-    assert "molecule-zitadel" in targets
-
-
-def test_roles_to_targets_unknown_role():
-    """Unknown roles are silently skipped."""
-    targets = roles_to_targets({"docker_base", "unknown_role"})
-    assert targets == ["molecule-docker-base"]
+    assert targets == ["molecule-docker-base", "molecule-zitadel"]
 
 
 def test_main_no_changes():
     """When no files changed, outputs message to stderr."""
     with patch("devx.molecule.molecule_changed.get_changed_files", return_value=[]):
-        runner = CliRunner()
-        result = runner.invoke(main, ["--print-targets"])
+        result = CliRunner().invoke(main, ["--print-targets"])
         assert result.exit_code == 0
         assert "No changed files" in result.output
 
 
-def test_main_print_targets():
-    """--print-targets outputs make targets."""
+def test_main_print_targets(roles_dir: Path):
+    """--print-targets outputs make targets for existing roles."""
     with patch(
         "devx.molecule.molecule_changed.get_changed_files",
         return_value=["ansible/roles/docker_base/tasks/main.yml"],
     ):
-        runner = CliRunner()
-        result = runner.invoke(main, ["--print-targets"])
+        result = CliRunner().invoke(main, ["--print-targets", "--roles-dir", str(roles_dir)])
         assert result.exit_code == 0
         assert "molecule-docker-base" in result.output
 
 
-def test_main_print_roles():
+def test_main_print_roles(roles_dir: Path):
     """--print-roles outputs role names."""
     with patch(
         "devx.molecule.molecule_changed.get_changed_files",
-        return_value=["ansible/roles/zitadel/tasks/main.yml"],
+        return_value=["ansible/roles/restore/tasks/main.yml"],
     ):
-        runner = CliRunner()
-        result = runner.invoke(main, ["--print-roles"])
+        result = CliRunner().invoke(main, ["--print-roles", "--roles-dir", str(roles_dir)])
         assert result.exit_code == 0
-        assert "zitadel" in result.output
+        assert "restore" in result.output
 
 
-def test_main_no_ansible_changes():
+def test_main_no_ansible_changes(roles_dir: Path):
     """When only non-Ansible files changed, outputs no scenarios message."""
     with patch(
         "devx.molecule.molecule_changed.get_changed_files",
         return_value=["scripts/molecule_changed.py"],
     ):
-        runner = CliRunner()
-        result = runner.invoke(main, ["--print-targets"])
+        result = CliRunner().invoke(main, ["--print-targets", "--roles-dir", str(roles_dir)])
         assert result.exit_code == 0
         assert "No molecule scenarios" in result.output
 
@@ -119,7 +198,6 @@ def test_get_changed_files_falls_back_to_master():
 
     def mock_git(args):
         calls.append(args)
-        # First call (origin/master) returns empty, second (master) returns files
         if "origin/master...HEAD" in args[2]:
             return ""
         return "ansible/roles/docker_base/tasks/main.yml\n"
@@ -137,56 +215,12 @@ def test_get_changed_files_empty():
         assert files == []
 
 
-def test_detect_molecule_shared_path():
-    """ansible/molecule/ change triggers all roles."""
-    files = ["ansible/molecule/Dockerfile"]
-    roles = detect_changed_roles(files)
-    assert len(roles) == 10
-
-
-def test_detect_requirements_yml_triggers_all():
-    """ansible/requirements.yml change triggers all roles."""
-    files = ["ansible/requirements.yml"]
-    roles = detect_changed_roles(files)
-    assert len(roles) == 10
-
-
-def test_detect_configure_oidc_playbook():
-    """configure-oidc.yml maps to sso_config and app_container."""
-    files = ["ansible/playbooks/configure-oidc.yml"]
-    roles = detect_changed_roles(files)
-    assert "sso_config" in roles
-    assert "app_container" in roles
-
-
-def test_detect_prepare_vms_playbook():
-    """prepare-vms.yml maps to all base roles."""
-    files = ["ansible/playbooks/prepare-vms.yml"]
-    roles = detect_changed_roles(files)
-    assert "docker_base" in roles
-    assert "app_hardening" in roles
-    assert "storage" in roles
-    assert "disk_cleanup" in roles
-    assert "crowdsec" in roles
-
-
-def test_detect_deploy_customer_playbook():
-    """deploy-customer.yml maps to its roles."""
-    files = ["ansible/playbooks/deploy-customer.yml"]
-    roles = detect_changed_roles(files)
-    assert "app_container" in roles
-    assert "docker_base" in roles
-    assert "app_hardening" in roles
-    assert "sso_config" in roles
-
-
-def test_main_default_base():
+def test_main_default_base(roles_dir: Path):
     """main() with no --base uses origin/master."""
     with patch(
         "devx.molecule.molecule_changed.get_changed_files",
-        return_value=["ansible/roles/zitadel/tasks/main.yml"],
+        return_value=["ansible/roles/restore/tasks/main.yml"],
     ) as mock:
-        runner = CliRunner()
-        result = runner.invoke(main, ["--print-roles"])
+        result = CliRunner().invoke(main, ["--print-roles", "--roles-dir", str(roles_dir)])
         assert result.exit_code == 0
         mock.assert_called_once_with("origin/master")