From d545bc5c51de2791e19634f4b7be302e5618f8b7 Mon Sep 17 00:00:00 2001 From: scaf Date: Thu, 4 Jun 2026 14:09:34 +0700 Subject: [PATCH 1/2] feat: improve dependency preflight error messages MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Report all missing dependencies at once instead of exiting on first - Map executables to human-readable package names (dot → Graphviz) - Add OS-specific installation instructions (brew, apt, Windows links) - Link to official installation docs and README - Add unit tests for check_dependencies and OS detection --- modules/helpers.py | 133 +++++++++++++++++++++++++++++++++---- tests/test_dependencies.py | 115 ++++++++++++++++++++++++++++++++ 2 files changed, 236 insertions(+), 12 deletions(-) create mode 100644 tests/test_dependencies.py diff --git a/modules/helpers.py b/modules/helpers.py index 3605c61..a21e50f 100644 --- a/modules/helpers.py +++ b/modules/helpers.py @@ -7,6 +7,7 @@ import json import os +import platform import re import shutil import subprocess @@ -23,7 +24,6 @@ from modules.config_loader import load_config from modules.provider_detector import get_provider_for_resource - # When True, pretty_name() returns the raw Terraform resource name unchanged. # Set by the CLI --use-tf-names flag. USE_TF_NAMES: bool = False @@ -1759,26 +1759,135 @@ def safe_remove_connection( return False +def _get_os_family() -> str: + """Detect the operating system family for install instructions.""" + system = platform.system() + if system == "Darwin": + return "macos" + elif system == "Linux": + if is_wsl(): + return "wsl" + try: + with open("/etc/os-release") as f: + content = f.read().lower() + if "ubuntu" in content or "debian" in content: + return "debian" + except OSError: + pass + return "linux" + elif system == "Windows": + return "windows" + return "unknown" + + def check_dependencies() -> None: - """Check if required command-line tools are available.""" - dependencies = ["dot", "gvpr", "git", "terraform"] - bundle_dir = Path(__file__).parent + """Check if required command-line tools are available. + + Reports all missing dependencies together with OS-specific + installation instructions and a link to the documentation. + """ import sys + dependency_info = { + "dot": ("Graphviz", "graphviz"), + "gvpr": ("Graphviz", "graphviz"), + "git": ("Git", "git"), + "terraform": ("Terraform", "terraform"), + } + + bundle_dir = Path(__file__).parent sys.path.append(str(bundle_dir)) - for exe in dependencies: + + missing = [] + for exe, (name, pkg) in dependency_info.items(): location = shutil.which(exe) or os.path.isfile(exe) if location: click.echo(f" {exe} command detected: {location}") else: - click.echo( - click.style( - f"\n ERROR: {exe} command executable not detected in path. Please ensure you have installed all required dependencies first", - fg="red", - bold=True, - ) + missing.append((exe, name, pkg)) + + if not missing: + return + + packages_to_install: Dict[str, Dict[str, Any]] = {} + for exe, name, pkg in missing: + if pkg not in packages_to_install: + packages_to_install[pkg] = {"name": name, "exes": []} + packages_to_install[pkg]["exes"].append(exe) + + click.echo("\n") + for pkg, info in packages_to_install.items(): + exes_str = ", ".join(f"'{e}'" for e in info["exes"]) + click.echo( + click.style( + f" ERROR: {exes_str} not found in PATH.", + fg="red", + bold=True, ) - exit() + ) + click.echo( + click.style( + f" {info['name']} is required but not installed.", + fg="red", + ) + ) + + os_family = _get_os_family() + + click.echo("\n Install the missing dependencies:\n") + + if os_family == "macos": + for pkg in packages_to_install: + if pkg == "graphviz": + click.echo(" brew install graphviz") + elif pkg == "git": + click.echo(" brew install git") + elif pkg == "terraform": + click.echo(" brew tap hashicorp/tap") + click.echo(" brew install hashicorp/terraform") + elif os_family in ("debian", "wsl"): + apt_packages = [] + for pkg in packages_to_install: + if pkg == "graphviz": + apt_packages.append("graphviz") + apt_packages.append("libgvplugin-neato-layout8") + elif pkg == "git": + apt_packages.append("git") + elif pkg == "terraform": + click.echo(" # Terraform install steps:") + click.echo(" # https://developer.hashicorp.com/terraform/install") + if apt_packages: + click.echo(" sudo apt update") + click.echo(f" sudo apt install {' '.join(apt_packages)}") + elif os_family == "windows": + click.echo(" # Download and install from the official websites:") + for pkg in packages_to_install: + if pkg == "graphviz": + click.echo(" # Graphviz: https://graphviz.org/download/") + elif pkg == "git": + click.echo(" # Git: https://git-scm.com/download/win") + elif pkg == "terraform": + click.echo( + " # Terraform: https://developer.hashicorp.com/terraform/install" + ) + else: + for pkg in packages_to_install: + if pkg == "graphviz": + click.echo(" # Install Graphviz using your package manager") + click.echo(" # e.g. sudo apt install graphviz (Debian/Ubuntu)") + click.echo(" # sudo yum install graphviz (RHEL/CentOS)") + elif pkg == "git": + click.echo(" # Install Git using your package manager") + elif pkg == "terraform": + click.echo( + " # Install Terraform: https://developer.hashicorp.com/terraform/install" + ) + + click.echo("\n For detailed installation instructions:") + click.echo(" https://patrickchugh.github.io/terravision/installation/") + click.echo(" https://github.com/patrickchugh/terravision#install\n") + + exit(1) def check_terraform_version() -> None: diff --git a/tests/test_dependencies.py b/tests/test_dependencies.py new file mode 100644 index 0000000..c64a572 --- /dev/null +++ b/tests/test_dependencies.py @@ -0,0 +1,115 @@ +"""Tests for dependency preflight checks in helpers module.""" + +import pytest +from unittest.mock import patch + +from modules.helpers import check_dependencies, _get_os_family + + +class TestCheckDependencies: + """Tests for check_dependencies() function.""" + + @patch("modules.helpers.shutil.which") + @patch("modules.helpers.os.path.isfile") + def test_all_dependencies_present(self, mock_isfile, mock_which): + """When all dependencies are present, function should echo and return.""" + mock_which.return_value = "/usr/bin/fake" + mock_isfile.return_value = False + + check_dependencies() + + @patch("modules.helpers.shutil.which") + @patch("modules.helpers.os.path.isfile") + def test_missing_single_dependency(self, mock_isfile, mock_which): + """Missing a single dependency should report it and exit.""" + + def fake_which(exe): + return None if exe == "dot" else "/usr/bin/fake" + + mock_which.side_effect = fake_which + mock_isfile.return_value = False + + with pytest.raises(SystemExit) as exc_info: + check_dependencies() + + assert exc_info.value.code == 1 + + @patch("modules.helpers.shutil.which") + @patch("modules.helpers.os.path.isfile") + def test_missing_multiple_dependencies(self, mock_isfile, mock_which): + """Missing multiple dependencies should report all of them.""" + + def fake_which(exe): + return None if exe in ("dot", "terraform") else "/usr/bin/fake" + + mock_which.side_effect = fake_which + mock_isfile.return_value = False + + with pytest.raises(SystemExit) as exc_info: + check_dependencies() + + assert exc_info.value.code == 1 + + @patch("modules.helpers.shutil.which") + @patch("modules.helpers.os.path.isfile") + def test_missing_git_dependency(self, mock_isfile, mock_which): + """Missing git should report Git specifically.""" + + def fake_which(exe): + return None if exe == "git" else "/usr/bin/fake" + + mock_which.side_effect = fake_which + mock_isfile.return_value = False + + with pytest.raises(SystemExit) as exc_info: + check_dependencies() + + assert exc_info.value.code == 1 + + +class TestGetOsFamily: + """Tests for _get_os_family() helper.""" + + @patch("modules.helpers.platform.system") + def test_detects_macos(self, mock_system): + mock_system.return_value = "Darwin" + assert _get_os_family() == "macos" + + @patch("modules.helpers.platform.system") + @patch("modules.helpers.is_wsl") + def test_detects_wsl(self, mock_is_wsl, mock_system): + mock_system.return_value = "Linux" + mock_is_wsl.return_value = True + assert _get_os_family() == "wsl" + + @patch("modules.helpers.platform.system") + @patch("modules.helpers.is_wsl") + @patch("builtins.open") + def test_detects_debian(self, mock_open, mock_is_wsl, mock_system): + mock_system.return_value = "Linux" + mock_is_wsl.return_value = False + mock_open.return_value.__enter__.return_value.read.return_value = ( + 'ID="ubuntu"\nID_LIKE="debian"\n' + ) + assert _get_os_family() == "debian" + + @patch("modules.helpers.platform.system") + @patch("modules.helpers.is_wsl") + @patch("builtins.open") + def test_detects_generic_linux(self, mock_open, mock_is_wsl, mock_system): + mock_system.return_value = "Linux" + mock_is_wsl.return_value = False + mock_open.return_value.__enter__.return_value.read.return_value = ( + 'ID="fedora"\n' + ) + assert _get_os_family() == "linux" + + @patch("modules.helpers.platform.system") + def test_detects_windows(self, mock_system): + mock_system.return_value = "Windows" + assert _get_os_family() == "windows" + + @patch("modules.helpers.platform.system") + def test_detects_unknown(self, mock_system): + mock_system.return_value = "FreeBSD" + assert _get_os_family() == "unknown" From 39cf7e1dd36f2580477d839655984ae6502a5628 Mon Sep 17 00:00:00 2001 From: scaf Date: Sat, 13 Jun 2026 13:08:13 +0700 Subject: [PATCH 2/2] fix: refactor check_dependencies into a structured deps instead of long if else --- modules/helpers.py | 143 +++++++++++++++++++++------------------------ 1 file changed, 66 insertions(+), 77 deletions(-) diff --git a/modules/helpers.py b/modules/helpers.py index a21e50f..48ee612 100644 --- a/modules/helpers.py +++ b/modules/helpers.py @@ -1780,6 +1780,52 @@ def _get_os_family() -> str: return "unknown" +# install commands are looked up by OS family, falling back to "default". +# Lines starting with "#" are printed verbatim as guidance/doc links. +DEPENDENCIES: Dict[str, Dict[str, Any]] = { + "graphviz": { + "name": "Graphviz", + "executables": ["dot", "gvpr"], + "install": { + "macos": ["brew install graphviz"], + "debian": ["sudo apt update", "sudo apt install graphviz"], + "wsl": ["sudo apt update", "sudo apt install graphviz"], + "windows": ["# Graphviz: https://graphviz.org/download/"], + "default": [ + "# Install Graphviz using your package manager:", + "# sudo apt install graphviz (Debian/Ubuntu)", + "# sudo dnf install graphviz (Fedora/RHEL)", + "# Or download from: https://graphviz.org/download/", + ], + }, + }, + "git": { + "name": "Git", + "executables": ["git"], + "install": { + "macos": ["brew install git"], + "debian": ["sudo apt update", "sudo apt install git"], + "wsl": ["sudo apt update", "sudo apt install git"], + "windows": ["# Git: https://git-scm.com/download/win"], + "default": ["# Install Git: https://git-scm.com/download"], + }, + }, + "terraform": { + "name": "Terraform", + "executables": ["terraform"], + "install": { + "macos": [ + "brew tap hashicorp/tap", + "brew install hashicorp/terraform", + ], + "default": [ + "# Install Terraform: https://developer.hashicorp.com/terraform/install" + ], + }, + }, +} + + def check_dependencies() -> None: """Check if required command-line tools are available. @@ -1788,100 +1834,43 @@ def check_dependencies() -> None: """ import sys - dependency_info = { - "dot": ("Graphviz", "graphviz"), - "gvpr": ("Graphviz", "graphviz"), - "git": ("Git", "git"), - "terraform": ("Terraform", "terraform"), - } - bundle_dir = Path(__file__).parent sys.path.append(str(bundle_dir)) - missing = [] - for exe, (name, pkg) in dependency_info.items(): - location = shutil.which(exe) or os.path.isfile(exe) - if location: - click.echo(f" {exe} command detected: {location}") - else: - missing.append((exe, name, pkg)) + missing: List[Tuple[Dict[str, Any], List[str]]] = [] + for info in DEPENDENCIES.values(): + not_found = [] + for exe in info["executables"]: + location = shutil.which(exe) or os.path.isfile(exe) + if location: + click.echo(f" {exe} command detected: {location}") + else: + not_found.append(exe) + if not_found: + missing.append((info, not_found)) if not missing: return - packages_to_install: Dict[str, Dict[str, Any]] = {} - for exe, name, pkg in missing: - if pkg not in packages_to_install: - packages_to_install[pkg] = {"name": name, "exes": []} - packages_to_install[pkg]["exes"].append(exe) + os_family = _get_os_family() click.echo("\n") - for pkg, info in packages_to_install.items(): - exes_str = ", ".join(f"'{e}'" for e in info["exes"]) + for info, not_found in missing: + exes_str = ", ".join(f"'{e}'" for e in not_found) click.echo( - click.style( - f" ERROR: {exes_str} not found in PATH.", - fg="red", - bold=True, - ) + click.style(f" ERROR: {exes_str} not found in PATH.", fg="red", bold=True) ) click.echo( click.style( - f" {info['name']} is required but not installed.", - fg="red", + f" {info['name']} is required but not installed.", fg="red" ) ) - os_family = _get_os_family() - click.echo("\n Install the missing dependencies:\n") - - if os_family == "macos": - for pkg in packages_to_install: - if pkg == "graphviz": - click.echo(" brew install graphviz") - elif pkg == "git": - click.echo(" brew install git") - elif pkg == "terraform": - click.echo(" brew tap hashicorp/tap") - click.echo(" brew install hashicorp/terraform") - elif os_family in ("debian", "wsl"): - apt_packages = [] - for pkg in packages_to_install: - if pkg == "graphviz": - apt_packages.append("graphviz") - apt_packages.append("libgvplugin-neato-layout8") - elif pkg == "git": - apt_packages.append("git") - elif pkg == "terraform": - click.echo(" # Terraform install steps:") - click.echo(" # https://developer.hashicorp.com/terraform/install") - if apt_packages: - click.echo(" sudo apt update") - click.echo(f" sudo apt install {' '.join(apt_packages)}") - elif os_family == "windows": - click.echo(" # Download and install from the official websites:") - for pkg in packages_to_install: - if pkg == "graphviz": - click.echo(" # Graphviz: https://graphviz.org/download/") - elif pkg == "git": - click.echo(" # Git: https://git-scm.com/download/win") - elif pkg == "terraform": - click.echo( - " # Terraform: https://developer.hashicorp.com/terraform/install" - ) - else: - for pkg in packages_to_install: - if pkg == "graphviz": - click.echo(" # Install Graphviz using your package manager") - click.echo(" # e.g. sudo apt install graphviz (Debian/Ubuntu)") - click.echo(" # sudo yum install graphviz (RHEL/CentOS)") - elif pkg == "git": - click.echo(" # Install Git using your package manager") - elif pkg == "terraform": - click.echo( - " # Install Terraform: https://developer.hashicorp.com/terraform/install" - ) + for info, _ in missing: + commands = info["install"].get(os_family, info["install"]["default"]) + for line in commands: + click.echo(f" {line}") click.echo("\n For detailed installation instructions:") click.echo(" https://patrickchugh.github.io/terravision/installation/")